[dart:html] Fix decodeAudioData to use both syntaxes
Closes https://github.com/dart-lang/sdk/issues/47520
decodeAudioData has an older callback-based syntax and newer
Promise-based syntax. In order to be consistent with the method
signature as well as be able to use both syntaxes, this CL
provides an API that can handle both.
Change-Id: I875defcfec9e429496a1ac9866f1b53d204eff69
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/221744
Reviewed-by: Riley Porter <rileyporter@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Commit-Queue: Srujan Gaddam <srujzs@google.com>
diff --git a/sdk/lib/web_audio/dart2js/web_audio_dart2js.dart b/sdk/lib/web_audio/dart2js/web_audio_dart2js.dart
index a7db2d5..29544d8 100644
--- a/sdk/lib/web_audio/dart2js/web_audio_dart2js.dart
+++ b/sdk/lib/web_audio/dart2js/web_audio_dart2js.dart
@@ -225,29 +225,89 @@
}
}
- @JSName('decodeAudioData')
- Future<AudioBuffer> _decodeAudioData(ByteBuffer audioData,
- [DecodeSuccessCallback? successCallback,
- DecodeErrorCallback? errorCallback]) native;
-
Future<AudioBuffer> decodeAudioData(ByteBuffer audioData,
[DecodeSuccessCallback? successCallback,
DecodeErrorCallback? errorCallback]) {
- if (successCallback != null && errorCallback != null) {
- return _decodeAudioData(audioData, successCallback, errorCallback);
+ // Both callbacks need to be provided if they're being used.
+ assert((successCallback == null) == (errorCallback == null));
+ // `decodeAudioData` can exist either in the older callback syntax or the
+ // newer `Promise`-based syntax that also accepts callbacks. In the former,
+ // we synthesize a `Future` to be consistent.
+ // For more details:
+ // https://developer.mozilla.org/en-US/docs/Web/API/BaseAudioContext/decodeAudioData
+ // https://www.w3.org/TR/webaudio/#dom-baseaudiocontext-decodeaudiodata
+ final completer = Completer<Object>();
+ var errorInCallbackIsNull = false;
+
+ void success(AudioBuffer decodedData) {
+ completer.complete(decodedData);
+ successCallback!.call(decodedData);
}
- var completer = new Completer<AudioBuffer>();
- _decodeAudioData(audioData, (value) {
- completer.complete(value);
- }, (error) {
- if (error == null) {
- completer.completeError('');
+ final nullErrorString =
+ '[AudioContext.decodeAudioData] completed with a null error.';
+
+ void error(DomException? error) {
+ // Safari has a bug where it may return null for the error callback. In
+ // the case where the Safari version still returns a `Promise` and the
+ // error is not null after the `Promise` is finished, the error callback
+ // is called instead in the `Promise`'s `catch` block. Otherwise, and in
+ // the case where a `Promise` is not returned by the API at all, the
+ // callback never gets called (for backwards compatibility, it can not
+ // accept null). Instead, the `Future` completes with a custom string,
+ // indicating that null was given.
+ // https://github.com/mdn/webaudio-examples/issues/5
+ if (error != null) {
+ // Note that we `complete` and not `completeError`. This is to make sure
+ // that errors in the `Completer` are not thrown if the call gets back
+ // a `Promise`.
+ completer.complete(error);
+ errorCallback!.call(error);
} else {
- completer.completeError(error);
+ completer.complete(nullErrorString);
+ errorInCallbackIsNull = true;
}
+ }
+
+ var decodeResult;
+ if (successCallback == null) {
+ decodeResult =
+ JS("creates:AudioBuffer;", "#.decodeAudioData(#)", this, audioData);
+ } else {
+ decodeResult = JS(
+ "creates:AudioBuffer;",
+ "#.decodeAudioData(#, #, #)",
+ this,
+ audioData,
+ convertDartClosureToJS(success, 1),
+ convertDartClosureToJS(error, 1));
+ }
+
+ if (decodeResult != null) {
+ // Promise-based syntax.
+ return promiseToFuture<AudioBuffer>(decodeResult).catchError((error) {
+ // If the error was null in the callback, but no longer is now that the
+ // `Promise` is finished, call the error callback. If it's still null,
+ // throw the error string. This is to handle the aforementioned bug in
+ // Safari.
+ if (errorInCallbackIsNull) {
+ if (error != null) {
+ errorCallback?.call(error);
+ } else {
+ throw nullErrorString;
+ }
+ }
+ throw error;
+ });
+ }
+
+ // Callback-based syntax. We use the above completer to synthesize a
+ // `Future` from the callback values. Since we don't use `completeError`
+ // above, `then` is used to simulate an error.
+ return completer.future.then((value) {
+ if (value is AudioBuffer) return value;
+ throw value;
});
- return completer.future;
}
}
// Copyright (c) 2012, the Dart project authors. Please see the AUTHORS file
diff --git a/tests/lib/html/audiocontext_test.dart b/tests/lib/html/audiocontext_test.dart
index 8316130..a091790 100644
--- a/tests/lib/html/audiocontext_test.dart
+++ b/tests/lib/html/audiocontext_test.dart
@@ -7,6 +7,7 @@
import 'dart:typed_data';
import 'dart:web_audio';
+import 'package:async_helper/async_helper.dart';
import 'package:expect/minitest.dart';
main() {
@@ -95,5 +96,74 @@
expect(oscillator.type, equals('triangle'));
}
});
+
+ asyncTest(() async {
+ if (AudioContext.supported) {
+ final audioSourceUrl = "/root_dart/tests/lib/html/small.mp3";
+
+ Future<void> requestAudioDecode(
+ {bool triggerDecodeError: false,
+ DecodeSuccessCallback? successCallback,
+ DecodeErrorCallback? errorCallback}) async {
+ HttpRequest audioRequest = HttpRequest();
+ audioRequest.open("GET", audioSourceUrl, async: true);
+ audioRequest.responseType = "arraybuffer";
+ var completer = new Completer<void>();
+ audioRequest.onLoad.listen((_) {
+ ByteBuffer audioData = audioRequest.response;
+ if (triggerDecodeError) audioData = Uint8List.fromList([]).buffer;
+ context
+ .decodeAudioData(audioData, successCallback, errorCallback)
+ .then((_) {
+ completer.complete();
+ }).catchError((e) {
+ completer.completeError(e);
+ });
+ });
+ audioRequest.send();
+ return completer.future;
+ }
+
+ // Decode successfully without callback.
+ await requestAudioDecode();
+
+ // Decode successfully with callback. Use counter to make sure it's only
+ // called once.
+ var successCallbackCalled = 0;
+ await requestAudioDecode(
+ successCallback: (_) {
+ successCallbackCalled += 1;
+ },
+ errorCallback: (_) {});
+ expect(successCallbackCalled, 1);
+
+ // Fail decode without callback.
+ try {
+ await requestAudioDecode(triggerDecodeError: true);
+ fail('Expected decode failure.');
+ } catch (_) {}
+
+ // Fail decode with callback.
+ var errorCallbackCalled = 0;
+ try {
+ await requestAudioDecode(
+ triggerDecodeError: true,
+ successCallback: (_) {},
+ errorCallback: (_) {
+ errorCallbackCalled += 1;
+ });
+ fail('Expected decode failure.');
+ } catch (e) {
+ // Safari may return a null error. Assuming Safari is version >= 14.1,
+ // the Future should complete with a string error if the error
+ // callback never gets called.
+ if (errorCallbackCalled == 0) {
+ expect(e is String, true);
+ } else {
+ expect(errorCallbackCalled, 1);
+ }
+ }
+ }
+ });
});
}
diff --git a/tests/lib/html/small.mp3 b/tests/lib/html/small.mp3
new file mode 100644
index 0000000..3fcc88b
--- /dev/null
+++ b/tests/lib/html/small.mp3
Binary files differ
diff --git a/tests/lib_2/html/audiocontext_test.dart b/tests/lib_2/html/audiocontext_test.dart
index b66e54b..396cf39 100644
--- a/tests/lib_2/html/audiocontext_test.dart
+++ b/tests/lib_2/html/audiocontext_test.dart
@@ -1,3 +1,6 @@
+// Copyright (c) 2022, the Dart project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
// @dart = 2.9
import 'dart:async';
@@ -5,6 +8,7 @@
import 'dart:typed_data';
import 'dart:web_audio';
+import 'package:async_helper/async_helper.dart';
import 'package:expect/minitest.dart';
main() {
@@ -93,5 +97,74 @@
expect(oscillator.type, equals('triangle'));
}
});
+
+ asyncTest(() async {
+ if (AudioContext.supported) {
+ final audioSourceUrl = "/root_dart/tests/lib_2/html/small.mp3";
+
+ Future<void> requestAudioDecode(
+ {bool triggerDecodeError: false,
+ DecodeSuccessCallback successCallback,
+ DecodeErrorCallback errorCallback}) async {
+ HttpRequest audioRequest = HttpRequest();
+ audioRequest.open("GET", audioSourceUrl, async: true);
+ audioRequest.responseType = "arraybuffer";
+ var completer = new Completer<void>();
+ audioRequest.onLoad.listen((_) {
+ ByteBuffer audioData = audioRequest.response;
+ if (triggerDecodeError) audioData = Uint8List.fromList([]).buffer;
+ context
+ .decodeAudioData(audioData, successCallback, errorCallback)
+ .then((_) {
+ completer.complete();
+ }).catchError((e) {
+ completer.completeError(e);
+ });
+ });
+ audioRequest.send();
+ return completer.future;
+ }
+
+ // Decode successfully without callback.
+ await requestAudioDecode();
+
+ // Decode successfully with callback. Use counter to make sure it's only
+ // called once.
+ var successCallbackCalled = 0;
+ await requestAudioDecode(
+ successCallback: (_) {
+ successCallbackCalled += 1;
+ },
+ errorCallback: (_) {});
+ expect(successCallbackCalled, 1);
+
+ // Fail decode without callback.
+ try {
+ await requestAudioDecode(triggerDecodeError: true);
+ fail('Expected decode failure.');
+ } catch (_) {}
+
+ // Fail decode with callback.
+ var errorCallbackCalled = 0;
+ try {
+ await requestAudioDecode(
+ triggerDecodeError: true,
+ successCallback: (_) {},
+ errorCallback: (_) {
+ errorCallbackCalled += 1;
+ });
+ fail('Expected decode failure.');
+ } catch (e) {
+ // Safari may return a null error. Assuming Safari is version >= 14.1,
+ // the Future should complete with a string error if the error
+ // callback never gets called.
+ if (errorCallbackCalled == 0) {
+ expect(e is String, true);
+ } else {
+ expect(errorCallbackCalled, 1);
+ }
+ }
+ }
+ });
});
}
diff --git a/tests/lib_2/html/small.mp3 b/tests/lib_2/html/small.mp3
new file mode 100644
index 0000000..3fcc88b
--- /dev/null
+++ b/tests/lib_2/html/small.mp3
Binary files differ
diff --git a/tools/dom/templates/html/impl/impl_AudioContext.darttemplate b/tools/dom/templates/html/impl/impl_AudioContext.darttemplate
index d716afd..77f860d 100644
--- a/tools/dom/templates/html/impl/impl_AudioContext.darttemplate
+++ b/tools/dom/templates/html/impl/impl_AudioContext.darttemplate
@@ -41,28 +41,88 @@
}
}
- @JSName('decodeAudioData')
- Future$#NULLSAFECAST(<AudioBuffer>) _decodeAudioData(ByteBuffer audioData,
- [DecodeSuccessCallback$NULLABLE successCallback,
- DecodeErrorCallback$NULLABLE errorCallback]) native;
-
Future<AudioBuffer> decodeAudioData(ByteBuffer audioData,
[DecodeSuccessCallback$NULLABLE successCallback,
DecodeErrorCallback$NULLABLE errorCallback]) {
- if (successCallback != null && errorCallback != null) {
- return _decodeAudioData(audioData, successCallback, errorCallback);
+ // Both callbacks need to be provided if they're being used.
+ assert((successCallback == null) == (errorCallback == null));
+ // `decodeAudioData` can exist either in the older callback syntax or the
+ // newer `Promise`-based syntax that also accepts callbacks. In the former,
+ // we synthesize a `Future` to be consistent.
+ // For more details:
+ // https://developer.mozilla.org/en-US/docs/Web/API/BaseAudioContext/decodeAudioData
+ // https://www.w3.org/TR/webaudio/#dom-baseaudiocontext-decodeaudiodata
+ final completer = Completer<Object>();
+ var errorInCallbackIsNull = false;
+
+ void success(AudioBuffer decodedData) {
+ completer.complete(decodedData);
+ successCallback$NULLASSERT.call(decodedData);
}
- var completer = new Completer<AudioBuffer>();
- _decodeAudioData(audioData, (value) {
- completer.complete(value);
- }, (error) {
- if (error == null) {
- completer.completeError('');
+ final nullErrorString =
+ '[AudioContext.decodeAudioData] completed with a null error.';
+
+ void error(DomException$NULLABLE error) {
+ // Safari has a bug where it may return null for the error callback. In
+ // the case where the Safari version still returns a `Promise` and the
+ // error is not null after the `Promise` is finished, the error callback
+ // is called instead in the `Promise`'s `catch` block. Otherwise, and in
+ // the case where a `Promise` is not returned by the API at all, the
+ // callback never gets called (for backwards compatibility, it can not
+ // accept null). Instead, the `Future` completes with a custom string,
+ // indicating that null was given.
+ // https://github.com/mdn/webaudio-examples/issues/5
+ if (error != null) {
+ // Note that we `complete` and not `completeError`. This is to make sure
+ // that errors in the `Completer` are not thrown if the call gets back
+ // a `Promise`.
+ completer.complete(error);
+ errorCallback$NULLASSERT.call(error);
} else {
- completer.completeError(error);
+ completer.complete(nullErrorString);
+ errorInCallbackIsNull = true;
}
+ }
+
+ var decodeResult;
+ if (successCallback == null) {
+ decodeResult =
+ JS("creates:AudioBuffer;", "#.decodeAudioData(#)", this, audioData);
+ } else {
+ decodeResult = JS(
+ "creates:AudioBuffer;",
+ "#.decodeAudioData(#, #, #)",
+ this,
+ audioData,
+ convertDartClosureToJS(success, 1),
+ convertDartClosureToJS(error, 1));
+ }
+
+ if (decodeResult != null) {
+ // Promise-based syntax.
+ return promiseToFuture<AudioBuffer>(decodeResult).catchError((error) {
+ // If the error was null in the callback, but no longer is now that the
+ // `Promise` is finished, call the error callback. If it's still null,
+ // throw the error string. This is to handle the aforementioned bug in
+ // Safari.
+ if (errorInCallbackIsNull) {
+ if (error != null) {
+ errorCallback?.call(error);
+ } else {
+ throw nullErrorString;
+ }
+ }
+ throw error;
+ });
+ }
+
+ // Callback-based syntax. We use the above completer to synthesize a
+ // `Future` from the callback values. Since we don't use `completeError`
+ // above, `then` is used to simulate an error.
+ return completer.future.then((value) {
+ if (value is AudioBuffer) return value;
+ throw value;
});
- return completer.future;
}
}
