Bug 147487 – [Streams API] ReadableStreamReader closed promise should use CachedAttribute

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

[youenn fablet]

This would allow removing the current specific promise attribute binding.
This would allow using a JSC::WriteBarrier in lieu of JSC::Strong as well.

[youenn fablet]

Created attachment 257918 [details]
Patch

[youenn fablet]

(In reply to comment #1)
> Created attachment 257918 [details]
> Patch

This patch does not remove the specific binding generator code related to promise attribute. This would be done as a follow-up patch.

[Darin Adler]

Comment on attachment 257918 [details]
Patch

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

Looks OK. I don’t really know what CachedAttribute does but this looks sensible.

> Source/WebCore/bindings/js/JSReadableStreamReaderCustom.cpp:51
> +        const_cast<JSReadableStreamReader*>(this)->m_closed.set(exec->vm(), this, closedPromise->promise());

Is this preferred over marking m_closed mutable?

[youenn fablet]

(In reply to comment #3)
> Comment on attachment 257918 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=257918&action=review
> 
> Looks OK. I don’t really know what CachedAttribute does but this looks
> sensible.

CachedAttribute generates some code like:
- a class member field in the JS wrapper class (as would be done in JSC)
- marking the member field when the JS wrapper object is being visited.


In our case, this removes one JSC::Strong.
This does not solve entirely the issue since the promise is strongified when being wrapped as a DeferredWrapper.

> > Source/WebCore/bindings/js/JSReadableStreamReaderCustom.cpp:51
> > +        const_cast<JSReadableStreamReader*>(this)->m_closed.set(exec->vm(), this, closedPromise->promise());
> 
> Is this preferred over marking m_closed mutable?

Right, marking cached attributes as mutable would be an improvement.

[WebKit Commit Bot]

Comment on attachment 257918 [details]
Patch

Clearing flags on attachment: 257918

Committed r188209: <http://trac.webkit.org/changeset/188209>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.