Summary: | Video errors should be instances of Error | ||
---|---|---|---|
Product: | WebKit | Reporter: | Andrew Morris <andrew> |
Component: | Media | Assignee: | Chris Dumez <cdumez> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | buildbot, cdumez, commit-queue, eric.carlson, jer.noble, jonlee, rniwa, webkit-bug-importer, youennf |
Priority: | P2 | Keywords: | InRadar |
Version: | Safari Technology Preview | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
URL: | http://output.jsbin.com/jewolar | ||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=177201 | ||
Bug Depends on: | |||
Bug Blocks: | 177207 | ||
Attachments: |
Description
Andrew Morris
2017-07-16 18:14:43 PDT
It reports two different errors. On first load on Safari 11 on High Sierra the result is false, then true when I reload. The error on load is "AbortError". Subsequently it is a "NotAllowedError". Repro steps: 1. Set a break point on line 35 on the webpage source ahead of time. 2. Open a new tab. 3. Go to any website (google.com) 4. Open the web inspector. 5. Load the webpage and notice the AbortError. 6. Continue. Reload the page. 7. Notice the NotAllowedError. DOMException correctly inherits from EcmaScript's Error. So it could be that media code is not using DOMException in some places (maybe its own kind of exception)? In particular, the following interface looks interesting: interface MediaError; Because it is an IDL interface and not an IDL exception, it likely does not inherit from Error. (In reply to Chris Dumez from comment #4) > In particular, the following interface looks interesting: > interface MediaError; > > Because it is an IDL interface and not an IDL exception, it likely does not > inherit from Error. We match the spec though: https://html.spec.whatwg.org/multipage/media.html#error-codes So maybe it is not supposed to inherit Error? Displaying more information about the error, we see that it is a: DOMError {name: "AbortError", message: "The operation was aborted."} not a DOMException, nor a MediaError. DOMError does not exist in the specification anymore and we are supposed to use Error nowadays: https://heycam.github.io/webidl/#es-Error I think the right fix is to drop DOMError and use JS's Error directly instead. Media code relies on DOMError here in HTMLMediaElement.h: void rejectPendingPlayPromises(DOMError&); Killing DOMError aside, the media code needs to be fixed to reject play promises with DOMException rather than DOMError. As per the spec, this is what we should do: - https://html.spec.whatwg.org/multipage/media.html#reject-pending-play-promises - https://html.spec.whatwg.org/multipage/media.html#loading-the-media-resource:reject-pending-play-promises-2 - https://html.spec.whatwg.org/multipage/media.html#loading-the-media-resource:reject-pending-play-promises-3 - https://html.spec.whatwg.org/multipage/media.html#playing-the-media-resource:reject-pending-play-promises-3 - https://html.spec.whatwg.org/multipage/media.html#playing-the-media-resource:reject-pending-play-promises Eric / Jer, please take a look. Created attachment 321266 [details]
WIP Patch
(In reply to Chris Dumez from comment #10) > Created attachment 321266 [details] > WIP Patch Patch should work but needs layout test coverage. Comment on attachment 321266 [details] WIP Patch Attachment 321266 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/4598896 New failing tests: imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/paused_true_during_pause.html imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/event_play_noautoplay.html imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/event_pause_noautoplay.html http/tests/security/video-cross-origin-caching.html Created attachment 321270 [details]
Archive of layout-test-results from ews102 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Comment on attachment 321266 [details] WIP Patch Attachment 321266 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/4598770 New failing tests: http/tests/security/video-cross-origin-caching.html Created attachment 321271 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.12.5
Comment on attachment 321266 [details] WIP Patch Attachment 321266 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/4599114 New failing tests: http/tests/security/video-cross-origin-caching.html imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/paused_true_during_pause.html imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/event_play_noautoplay.html imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/event_pause_noautoplay.html fast/mediastream/MediaStream-MediaElement-setObject-null.html Created attachment 321272 [details]
Archive of layout-test-results from ews106 for mac-elcapitan-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-elcapitan-wk2 Platform: Mac OS X 10.11.6
Comment on attachment 321266 [details] WIP Patch Attachment 321266 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/4599109 New failing tests: imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/paused_true_during_pause.html imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/event_play_noautoplay.html imported/w3c/web-platform-tests/html/semantics/embedded-content/media-elements/event_pause_noautoplay.html http/tests/security/video-cross-origin-caching.html Created attachment 321275 [details]
Archive of layout-test-results from ews114 for mac-elcapitan
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-elcapitan Platform: Mac OS X 10.11.6
Created attachment 321322 [details]
Patch
Comment on attachment 321322 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=321322&action=review > Source/WebCore/html/HTMLMediaElement.cpp:1045 > + promise.rejectType<IDLInterface<DOMException>>(error); If that is a pattern that will happen often, maybe we should be able to write it as promise.reject(error) or promise.rejectAsException(error)? Created attachment 321327 [details]
Patch
Comment on attachment 321327 [details] Patch Clearing flags on attachment: 321327 Committed r222270: <http://trac.webkit.org/changeset/222270> All reviewed patches have been landed. Closing bug. |