WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
Trying to fix Win build
(18.99 KB, patch)
2015-06-10 02:25 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Fixing log
(18.94 KB, patch)
2015-06-11 11:36 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Cleaning NativeStdFunction definition
(18.92 KB, patch)
2015-06-12 10:28 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Fixing JSStdFunction
(18.94 KB, patch)
2015-06-12 12:44 PDT
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
youenn fablet
Comment 1
2015-06-10 01:32:04 PDT
Created
attachment 254636
[details]
Patch
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
This broke at least the 32-bit mac build.
https://build.webkit.org/builders/Apple%20Yosemite%20Release%20%2832-bit%20Build%29/builds/3693
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.
Top of Page
Format For Printing
XML
Clone This Bug