WebKit current implementation of AudioContext resume, close and suspend may throw exceptions in case of error. It would make more sense to reject the promise with the exception as value. This would be more consistent with other promise based APIs.
That will require a spec change. I encourage you to file a spec bug at https://github.com/WebAudio/web-audio-api.
(In reply to comment #1) > That will require a spec change. I encourage you to file a spec bug at > https://github.com/WebAudio/web-audio-api. I just checked and the issue is already filed and apparently on its way to be landed in the spec: https://github.com/WebAudio/web-audio-api/issues/441
Ok then!
Created attachment 253274 [details] Patch
Comment on attachment 253274 [details] Patch Attachment 253274 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/4657628286287872 New failing tests: fast/dom/document-contentType-DOMParser.html fast/dom/xmlserializer-serialize-to-string-exception.html media/track/regions-webvtt/vtt-region-constructor.html fast/dom/DOMURL/set-href-attribute-pathname.html fast/canvas/canvas-imageData.html fast/canvas/canvas-2d-imageData-create-nonfinite.html fast/dom/DOMURL/set-href-attribute-hostname.html fast/canvas/canvas-putImageData.html fast/dom/DOMURL/set-href-attribute-host.html fast/dom/DOMURL/url-constructor.html fast/canvas/canvas-getImageData-invalid.html fast/canvas/radialGradient-infinite-values.html fast/dom/DOMURL/set-href-attribute-hash.html fast/canvas/linearGradient-infinite-values.html
Created attachment 253277 [details] Archive of layout-test-results from ews100 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews100 Port: mac-mavericks Platform: Mac OS X 10.9.5
Comment on attachment 253274 [details] Patch Attachment 253274 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/5251477308178432 New failing tests: fast/dom/document-contentType-DOMParser.html fast/canvas/radialGradient-infinite-values.html fast/dom/xmlserializer-serialize-to-string-exception.html fast/canvas/linearGradient-infinite-values.html media/track/regions-webvtt/vtt-region-constructor.html storage/indexeddb/objectstore-cursor.html fast/canvas/canvas-imageData.html fast/canvas/canvas-2d-imageData-create-nonfinite.html fast/dom/DOMURL/set-href-attribute-hostname.html fast/canvas/canvas-putImageData.html fast/dom/DOMURL/set-href-attribute-host.html fast/canvas/canvas-getImageData-invalid.html fast/dom/DOMURL/set-href-attribute-pathname.html fast/dom/DOMURL/url-constructor.html storage/indexeddb/intversion-bad-parameters.html fast/dom/DOMURL/set-href-attribute-hash.html
Created attachment 253278 [details] Archive of layout-test-results from ews104 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews104 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5
Created attachment 253284 [details] Fixing TypeError message
Comment on attachment 253284 [details] Fixing TypeError message View in context: https://bugs.webkit.org/attachment.cgi?id=253284&action=review > Source/WebCore/Modules/webaudio/AudioContext.cpp:-1115 > - scriptExecutionContext()->postTask(successCallback); Calling postTask here is unneeded as the promise will call the JS resolve callback through postTask anyway > Source/WebCore/Modules/webaudio/AudioContext.cpp:1120 > + failureCallback(0); This is a way to reject with no value, using the same mechanism as other error promise rejection. It seems better than to keep using a ExceptionCode that would be used to reject the promise in the custom binding code. In the future, successCallback may also take a std::nullptr_t to make promise rejection/resolution consistent and potentially automated by code generator.
Comment on attachment 253284 [details] Fixing TypeError message View in context: https://bugs.webkit.org/attachment.cgi?id=253284&action=review >> Source/WebCore/Modules/webaudio/AudioContext.cpp:-1115 >> - scriptExecutionContext()->postTask(successCallback); > > Calling postTask here is unneeded as the promise will call the JS resolve callback through postTask anyway I don't believe that is true, unless the Promise code has drastically changed lately.
(In reply to comment #11) > Comment on attachment 253284 [details] > Fixing TypeError message > > View in context: > https://bugs.webkit.org/attachment.cgi?id=253284&action=review > > >> Source/WebCore/Modules/webaudio/AudioContext.cpp:-1115 > >> - scriptExecutionContext()->postTask(successCallback); > > > > Calling postTask here is unneeded as the promise will call the JS resolve callback through postTask anyway > > I don't believe that is true, unless the Promise code has drastically > changed lately. My comment was not very clear, here is a longer rephrasing. The spec says that the promise should be immediately resolved if the audio context state is already suspended. The current implementation does not do that immediately but after a micro-task. The JS function assigned to the promise through Promise.then() will be called asynchronously by the JS promise anyway. I believe that this is implemented by postTask but maybe I am wrong here. Let's split that as a separate bug then.
(In reply to comment #12) > (In reply to comment #11) > > Comment on attachment 253284 [details] > > Fixing TypeError message > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=253284&action=review > > > > >> Source/WebCore/Modules/webaudio/AudioContext.cpp:-1115 > > >> - scriptExecutionContext()->postTask(successCallback); > > > > > > Calling postTask here is unneeded as the promise will call the JS resolve callback through postTask anyway > > > > I don't believe that is true, unless the Promise code has drastically > > changed lately. > > My comment was not very clear, here is a longer rephrasing. > > The spec says that the promise should be immediately resolved if the audio > context state is already suspended. The current implementation does not do > that immediately but after a micro-task. > > The JS function assigned to the promise through Promise.then() will be > called asynchronously by the JS promise anyway. I believe that this is > implemented by postTask but maybe I am wrong here. > > Let's split that as a separate bug then. Filed bug 145164, a 100 above this bug ;)
Comment on attachment 253284 [details] Fixing TypeError message View in context: https://bugs.webkit.org/attachment.cgi?id=253284&action=review > Source/JavaScriptCore/runtime/Error.h:59 > -JS_EXPORT_PRIVATE JSObject* createTypeError(ExecState*, const String&); > +JS_EXPORT_PRIVATE JSObject* createTypeError(ExecState*, const String& message = ASCIILiteral("Type error")); This creates unnecessary code bloat as each call site generates the literal. It’s better to overload instead of using a default argument value. > Source/WebCore/bindings/js/JSDOMBinding.h:253 > // Convert a DOM implementation exception code into a JavaScript exception in the execution state. This comment describes the second function below; it’s not good to have it immediately before a function that does something subtly different!
Created attachment 253455 [details] Patch
Comment on attachment 253455 [details] Patch Clearing flags on attachment: 253455 Committed r184651: <http://trac.webkit.org/changeset/184651>
All reviewed patches have been landed. Closing bug.