Bug 143752 – [Streams API] Implement ReadableStreamController constructor

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

ReadableStreamController should not be visible but its constructor can be retrieved from ReadableStreamController object prototype.
This constructor should throw exception.

[youenn fablet]

Created attachment 250783 [details]
Patch

[youenn fablet]

Created attachment 251024 [details]
Improving test coverage

[Benjamin Poulain]

Comment on attachment 251024 [details]
Improving test coverage

View in context: https://bugs.webkit.org/attachment.cgi?id=251024&action=review

Any way to change the bindings to support Custom constructor function instead?

> Source/WebCore/bindings/js/JSReadableStreamControllerCustom.cpp:127
> +        constructor = JSReadableStreamControllerConstructor::create(exec->vm(), JSReadableStreamControllerConstructor::createStructure(exec->vm(), globalObject, globalObject->objectPrototype()), globalObject);

Couldn't you just use JSFunction::create()?

Why subclass DOMConstructorObject here?

[youenn fablet]

(In reply to comment #3)
> Comment on attachment 251024 [details]
> Improving test coverage
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=251024&action=review
> 
> Any way to change the bindings to support Custom constructor function
> instead?

That is certainly doable, but:
- WebIDL spec is not allowing to put Constructor and NoInterfaceObject .
Since CustomConstructor is semantically equivalent to Constructor, that would 
- It might be better to agree on how the code looks like and then generate it automatically
- Is it a case that happens often enough to add it into the binding generator?
- I am no expert in the bindings (althouhgh I'd like to have some time to study it).

> 
> > Source/WebCore/bindings/js/JSReadableStreamControllerCustom.cpp:127
> > +        constructor = JSReadableStreamControllerConstructor::create(exec->vm(), JSReadableStreamControllerConstructor::createStructure(exec->vm(), globalObject, globalObject->objectPrototype()), globalObject);
> 
> Couldn't you just use JSFunction::create()?

I wrote the code the way it is so that the class name would be settable.
A call to toString would get "[object ReadableStreamControllerConstructor]" which seems to align with ReadableStreamController objects.

Going the function way, we would get "function() {}".

I guess that in most case, this does not change anything, so I am fine either way. 

> Why subclass DOMConstructorObject here?

This is just closer to currently binding generated code.
We could go with DOMConstructorObject parent class or above I guess.

[youenn fablet]

Created attachment 253860 [details]
Using CustomConstructor

[youenn fablet]

Comment on attachment 253860 [details]
Using CustomConstructor

View in context: https://bugs.webkit.org/attachment.cgi?id=253860&action=review

> LayoutTests/streams/readable-stream.html:42
> +            assert_equals(propDesc.writable, false, 'method should be writable');

Some typos I forgot after copy/pasting the code here: rename 'method' to 'property', 'writable' to 'not writable' and 'enumerable' to 'not enumerable'
I plan to fix that when landing patch.

[youenn fablet]

Created attachment 253976 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 253976 [details]
Patch for landing

Clearing flags on attachment: 253976

Committed r185039: <http://trac.webkit.org/changeset/185039>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.