Bug 143564

Created attachment 250440 [details] Patch

Comment on attachment 250440 [details] Patch Attachment 250440 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5129403331772416 New failing tests: js/dom/global-constructors-attributes.html

Created attachment 250441 [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 250440 [details] Patch Attachment 250440 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/6013501948559360 New failing tests: js/dom/global-constructors-attributes.html

Created attachment 250442 [details] Archive of layout-test-results from ews107 for mac-mavericks-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews107 Port: mac-mavericks-wk2 Platform: Mac OS X 10.9.5

Created attachment 250443 [details] Fixing expectations

Comment on attachment 250443 [details] Fixing expectations View in context: https://bugs.webkit.org/attachment.cgi?id=250443&action=review > Source/WebCore/ChangeLog:9 > + Made ReadableStreamReader not exposed to JS. > + Removed ReadableStreamReader constructor. What am I missing? https://streams.spec.whatwg.org/#reader-constructor

(In reply to comment #7) > Comment on attachment 250443 [details] > Fixing expectations > > View in context: > https://bugs.webkit.org/attachment.cgi?id=250443&action=review > > > Source/WebCore/ChangeLog:9 > > + Made ReadableStreamReader not exposed to JS. > > + Removed ReadableStreamReader constructor. > > What am I missing? https://streams.spec.whatwg.org/#reader-constructor Well, the WebIDL spec says that the Constructor keyword should not be used with NoInterfaceObject. The streams spec says that users should not use the constructor, since there is getReader(). There seems to be little value in keeping it. But the implementation will also work with the constructor I guess.

Created attachment 250604 [details] Rebasing according new tests

Comment on attachment 250604 [details] Rebasing according new tests Attachment 250604 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/6053874439815168 New failing tests: js/dom/global-constructors-attributes.html

Created attachment 250606 [details] Archive of layout-test-results from ews102 for mac-mavericks The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews102 Port: mac-mavericks Platform: Mac OS X 10.9.5

Comment on attachment 250604 [details] Rebasing according new tests Attachment 250604 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4545507896590336 New failing tests: js/dom/global-constructors-attributes.html

Created attachment 250609 [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 250672 [details] Fixing expectations

(In reply to comment #8) > (In reply to comment #7) > > Comment on attachment 250443 [details] > > Fixing expectations > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=250443&action=review > > > > > Source/WebCore/ChangeLog:9 > > > + Made ReadableStreamReader not exposed to JS. > > > + Removed ReadableStreamReader constructor. > > > > What am I missing? https://streams.spec.whatwg.org/#reader-constructor > > Well, the WebIDL spec says that the Constructor keyword should not be used > with NoInterfaceObject. > The streams spec says that users should not use the constructor, since there > is getReader(). > There seems to be little value in keeping it. > But the implementation will also work with the constructor I guess. I was wrong. The binding generator is not generating the constructor property if NoInterfaceObject is set, even if CustomConstructor is set.

Comment on attachment 250672 [details] Fixing expectations View in context: https://bugs.webkit.org/attachment.cgi?id=250672&action=review Ok. > Source/WebCore/ChangeLog:9 > + Made ReadableStreamReader not exposed to JS. > + Removed ReadableStreamReader constructor as the binding generator does not use it when interface is not exposed. Can you please reference https://streams.spec.whatwg.org/#globals here? > LayoutTests/js/dom/global-constructors-attributes-expected.txt:-1005 > -PASS Object.getOwnPropertyDescriptor(global, 'ReadableStreamReader').value is ReadableStreamReader > -PASS Object.getOwnPropertyDescriptor(global, 'ReadableStreamReader').hasOwnProperty('get') is false > -PASS Object.getOwnPropertyDescriptor(global, 'ReadableStreamReader').hasOwnProperty('set') is false > -PASS Object.getOwnPropertyDescriptor(global, 'ReadableStreamReader').enumerable is false > -PASS Object.getOwnPropertyDescriptor(global, 'ReadableStreamReader').configurable is true You could cover this by getting the constructor property of the object you get from getReader(). I think getting enumerable is valuable. > LayoutTests/streams/reference-implementation/readable-stream-reader-expected.txt:-3 > -PASS ReadableStreamReader constructor should get a ReadableStream object as argument You need to adapt those tests. You could create a new reader after getting the constructor of an existing reader object. > LayoutTests/streams/reference-implementation/readable-stream-reader-expected.txt:9 > +FAIL Constructing a ReadableStreamReader directly should fail if the stream is already locked (via direct construction) assert_throws: constructing directly should fail function "function () { new ReadableStreamReader(rs); }" did not throw > +FAIL Getting a ReadableStreamReader via getReader should fail if the stream is already locked (via direct construction) assert_throws: getReader() should fail function "function () { rs.getReader(); }" did not throw > +FAIL Constructing a ReadableStreamReader directly should fail if the stream is already locked (via getReader) assert_throws: constructing directly should fail function "function () { new ReadableStreamReader(rs); }" did not throw What about those?

(In reply to comment #16) > Comment on attachment 250672 [details] > Fixing expectations > > View in context: > https://bugs.webkit.org/attachment.cgi?id=250672&action=review > > Ok. > > > Source/WebCore/ChangeLog:9 > > + Made ReadableStreamReader not exposed to JS. > > + Removed ReadableStreamReader constructor as the binding generator does not use it when interface is not exposed. > > Can you please reference https://streams.spec.whatwg.org/#globals here? > > > LayoutTests/js/dom/global-constructors-attributes-expected.txt:-1005 > > -PASS Object.getOwnPropertyDescriptor(global, 'ReadableStreamReader').value is ReadableStreamReader > > -PASS Object.getOwnPropertyDescriptor(global, 'ReadableStreamReader').hasOwnProperty('get') is false > > -PASS Object.getOwnPropertyDescriptor(global, 'ReadableStreamReader').hasOwnProperty('set') is false > > -PASS Object.getOwnPropertyDescriptor(global, 'ReadableStreamReader').enumerable is false > > -PASS Object.getOwnPropertyDescriptor(global, 'ReadableStreamReader').configurable is true > > You could cover this by getting the constructor property of the object you > get from getReader(). > > I think getting enumerable is valuable. > > > LayoutTests/streams/reference-implementation/readable-stream-reader-expected.txt:-3 > > -PASS ReadableStreamReader constructor should get a ReadableStream object as argument > > You need to adapt those tests. > > You could create a new reader after getting the constructor of an existing > reader object. > > > LayoutTests/streams/reference-implementation/readable-stream-reader-expected.txt:9 > > +FAIL Constructing a ReadableStreamReader directly should fail if the stream is already locked (via direct construction) assert_throws: constructing directly should fail function "function () { new ReadableStreamReader(rs); }" did not throw > > +FAIL Getting a ReadableStreamReader via getReader should fail if the stream is already locked (via direct construction) assert_throws: getReader() should fail function "function () { rs.getReader(); }" did not throw > > +FAIL Constructing a ReadableStreamReader directly should fail if the stream is already locked (via getReader) assert_throws: constructing directly should fail function "function () { new ReadableStreamReader(rs); }" did not throw > > What about those? A similar strategy to implement reader constructor could be done as https://bugs.webkit.org/show_bug.cgi?id=143752 That would solve those issues properly. Can we make that as another patch?

Created attachment 253738 [details] Adding NoInterfaceObject to IDL

Comment on attachment 253738 [details] Adding NoInterfaceObject to IDL Clearing flags on attachment: 253738 Committed r184955: <http://trac.webkit.org/changeset/184955>

All reviewed patches have been landed. Closing bug.