Summary: | WebRTC: The RTCTrackEventInit dictionary needs required members | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Bergkvist <adam.bergkvist> | ||||||||
Component: | WebCore Misc. | Assignee: | Nael Ouedraogo <nael.ouedp> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | buildbot, cdumez, commit-queue, nael.ouedp, rniwa, youennf | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Bug Depends on: | |||||||||||
Bug Blocks: | 143211 | ||||||||||
Attachments: |
|
Description
Adam Bergkvist
2016-06-08 12:20:05 PDT
(In reply to comment #0) > An attribute marked with [InitializedByEventConstructor] seem to be nullable > and non-required [1] by default. It is fixed now so we can update RTCTrackEvent IDL. [InitializedByEventConstructor] is no longer supported. You should use a dictionary, as in the specification. Also, as far as I know, we support required dictionary members already. Marking as Invalid, feel free to reopen if you find issues remain. (In reply to comment #2) > [InitializedByEventConstructor] is no longer supported. You should use a > dictionary, as in the specification. Also, as far as I know, we support > required dictionary members already. > > Marking as Invalid, feel free to reopen if you find issues remain. Ok, I modified the bug title to update RTCTrackEvent IDL as described in comment #0. Is it better to file a new bug? (In reply to comment #3) > (In reply to comment #2) > > [InitializedByEventConstructor] is no longer supported. You should use a > > dictionary, as in the specification. Also, as far as I know, we support > > required dictionary members already. > > > > Marking as Invalid, feel free to reopen if you find issues remain. > Ok, I modified the bug title to update RTCTrackEvent IDL as described in > comment #0. Is it better to file a new bug? I do not mind either way. Reopened to update RTCTrackEvent IDL. Created attachment 292218 [details]
Patch
Comment on attachment 292218 [details] Patch Attachment 292218 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/2332277 New failing tests: compositing/iframes/page-cache-layer-tree.html Created attachment 292225 [details]
Archive of layout-test-results from ews101 for mac-yosemite
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5
(In reply to comment #8) > Created attachment 292225 [details] > Archive of layout-test-results from ews101 for mac-yosemite > > The attached test failures were seen while running run-webkit-tests on the > mac-ews. > Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5 Failure observed in compositing/iframes/page-cache-layer-tree.html seems not related to the patch. Same test on mac-elcapitan successfully passed locally. Comment on attachment 292218 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=292218&action=review r=me with comments > Source/WebCore/Modules/mediastream/RTCTrackEvent.idl:37 > + readonly attribute sequence<MediaStream> streams; Looks like this should be a FrozenArray while you're at it (We support it). > LayoutTests/fast/mediastream/RTCTrackEvent-constructor.html:55 > shouldThrow("new RTCTrackEvent('eventType', { receiver: null, track: track, transceiver: transceiver})"); Could you please update the test so that it only passes if we throw a TypeError (and not other exception types). e.g. shouldThrowErrorName("new RTCTrackEvent('eventType', { receiver: null, track: track, transceiver: transceiver})", "TypeError"); (In reply to comment #10) > > Source/WebCore/Modules/mediastream/RTCTrackEvent.idl:37 > > + readonly attribute sequence<MediaStream> streams; > > Looks like this should be a FrozenArray while you're at it (We support it). Great. That would match the spec. https://w3c.github.io/webrtc-pc/archives/20160913/webrtc.html#rtctrackevent (In reply to comment #11) > (In reply to comment #10) > > > Source/WebCore/Modules/mediastream/RTCTrackEvent.idl:37 > > > + readonly attribute sequence<MediaStream> streams; > > > > Looks like this should be a FrozenArray while you're at it (We support it). > > Great. That would match the spec. > > https://w3c.github.io/webrtc-pc/archives/20160913/webrtc.html#rtctrackevent This is why I suggested it :) Created attachment 292912 [details]
Patch
> > Source/WebCore/Modules/mediastream/RTCTrackEvent.idl:37 > > + readonly attribute sequence<MediaStream> streams; > > Looks like this should be a FrozenArray while you're at it (We support it). Ok done in uploaded patch, I was not sure that it was supported. > > LayoutTests/fast/mediastream/RTCTrackEvent-constructor.html:55 > > shouldThrow("new RTCTrackEvent('eventType', { receiver: null, track: track, transceiver: transceiver})"); > > Could you please update the test so that it only passes if we throw a > TypeError (and not other exception types). > e.g. > > shouldThrowErrorName("new RTCTrackEvent('eventType', { receiver: null, > track: track, transceiver: transceiver})", "TypeError"); Done in uploaded patch. Thanks for the review. Comment on attachment 292912 [details] Patch Clearing flags on attachment: 292912 Committed r207895: <http://trac.webkit.org/changeset/207895> All reviewed patches have been landed. Closing bug. |