RESOLVED FIXED 145792
[Streams API] ReadableJSStream should handle promises returned by JS source start callback
https://bugs.webkit.org/show_bug.cgi?id=145792
Summary [Streams API] ReadableJSStream should handle promises returned by JS source s...
youenn fablet
Reported 2015-06-09 01:29:56 PDT
"start", "pull" and "cancel" may finish asynchronously by returning promises. ReadableJSStream should handle these promises. To start with, this should be done for the "start" callback.
Attachments
Patch (18.97 KB, patch)
2015-06-10 01:32 PDT, youenn fablet
no flags
Trying to fix Win build (18.99 KB, patch)
2015-06-10 02:25 PDT, youenn fablet
no flags
Fixing log (18.94 KB, patch)
2015-06-11 11:36 PDT, youenn fablet
no flags
Cleaning NativeStdFunction definition (18.92 KB, patch)
2015-06-12 10:28 PDT, youenn fablet
no flags
Fixing JSStdFunction (18.94 KB, patch)
2015-06-12 12:44 PDT, youenn fablet
no flags
youenn fablet
Comment 1 2015-06-10 01:32:04 PDT
youenn fablet
Comment 2 2015-06-10 02:25:21 PDT
Created attachment 254641 [details] Trying to fix Win build
youenn fablet
Comment 3 2015-06-10 03:34:21 PDT
Comment on attachment 254641 [details] Trying to fix Win build View in context: https://bugs.webkit.org/attachment.cgi?id=254641&action=review > Source/WebCore/bindings/js/ReadableJSStream.cpp:78 > + JSPromise* promise = jsDynamicCast<JSPromise*>(callFunction(state, function, m_source.get(), arguments)); By doing the cast here, we ensure that thenPromise is only called if JS callback returns a promise. But the code inside thenPromise is nothing special to promise. We could call thenPromise whenever callback returned value has a then property which is callable.
WebKit Commit Bot
Comment 4 2015-06-11 10:36:36 PDT
Comment on attachment 254641 [details] Trying to fix Win build Rejecting attachment 254641 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-02', 'validate-changelog', '--check-oops', '--non-interactive', 254641, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: http://webkit-queues.appspot.com/results/6265508483563520
youenn fablet
Comment 5 2015-06-11 11:36:12 PDT
Created attachment 254734 [details] Fixing log
WebKit Commit Bot
Comment 6 2015-06-11 12:31:00 PDT
Comment on attachment 254734 [details] Fixing log Clearing flags on attachment: 254734 Committed r185465: <http://trac.webkit.org/changeset/185465>
WebKit Commit Bot
Comment 7 2015-06-11 12:31:04 PDT
All reviewed patches have been landed. Closing bug.
Alex Christensen
Comment 8 2015-06-11 13:05:22 PDT
WebKit Commit Bot
Comment 9 2015-06-11 13:07:29 PDT
Re-opened since this is blocked by bug 145893
youenn fablet
Comment 10 2015-06-12 10:28:40 PDT
Created attachment 254807 [details] Cleaning NativeStdFunction definition
youenn fablet
Comment 11 2015-06-12 10:29:29 PDT
Comment on attachment 254807 [details] Cleaning NativeStdFunction definition Let's try it again
Darin Adler
Comment 12 2015-06-12 10:30:54 PDT
Comment on attachment 254807 [details] Cleaning NativeStdFunction definition View in context: https://bugs.webkit.org/attachment.cgi?id=254807&action=review > Source/JavaScriptCore/runtime/JSFunction.cpp:78 > } else WebKit coding style normally omits else after a return.
Darin Adler
Comment 13 2015-06-12 10:31:56 PDT
Comment on attachment 254807 [details] Cleaning NativeStdFunction definition View in context: https://bugs.webkit.org/attachment.cgi?id=254807&action=review > Source/JavaScriptCore/runtime/JSFunction.cpp:92 > +struct JSStdFunction: public JSFunction { In a struct we normally omit public since it’s the default. We also put a space before the colon here. I’m not sure it’s right to make this a struct, though, since it’s derived from a a class. I think this should probably be a class instead.
youenn fablet
Comment 14 2015-06-12 10:33:50 PDT
(In reply to comment #13) > Comment on attachment 254807 [details] > Cleaning NativeStdFunction definition > > View in context: > https://bugs.webkit.org/attachment.cgi?id=254807&action=review > > > Source/JavaScriptCore/runtime/JSFunction.cpp:92 > > +struct JSStdFunction: public JSFunction { > > In a struct we normally omit public since it’s the default. We also put a > space before the colon here. > > I’m not sure it’s right to make this a struct, though, since it’s derived > from a a class. I think this should probably be a class instead. Thanks. I am postponing cq until processing further feedback
youenn fablet
Comment 15 2015-06-12 12:44:43 PDT
Created attachment 254815 [details] Fixing JSStdFunction
WebKit Commit Bot
Comment 16 2015-06-13 02:10:30 PDT
Comment on attachment 254815 [details] Fixing JSStdFunction Clearing flags on attachment: 254815 Committed r185537: <http://trac.webkit.org/changeset/185537>
WebKit Commit Bot
Comment 17 2015-06-13 02:10:35 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.