WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
155291
[Fetch API] Use DeferredWrapper directly in FetchBody promise handling
https://bugs.webkit.org/show_bug.cgi?id=155291
Summary
[Fetch API] Use DeferredWrapper directly in FetchBody promise handling
youenn fablet
Reported
2016-03-10 00:25:10 PST
The DOMPromise data typing is not helpful here, we should switch back to DeferredWrapper.
Attachments
Patch
(23.31 KB, patch)
2016-03-10 02:17 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch for landing
(23.20 KB, patch)
2016-03-11 00:13 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Patch for landing
(23.32 KB, patch)
2016-03-11 02:33 PST
,
youenn fablet
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
youenn fablet
Comment 1
2016-03-10 02:17:42 PST
Created
attachment 273552
[details]
Patch
WebKit Commit Bot
Comment 2
2016-03-10 02:19:36 PST
Attachment 273552
[details]
did not pass style-queue: ERROR: Source/WebCore/Modules/fetch/FetchBody.h:77: enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums. [readability/enum_casing] [4] Total errors found: 1 in 10 files If any of these errors are false positives, please file a bug against check-webkit-style.
Darin Adler
Comment 3
2016-03-10 16:05:32 PST
Comment on
attachment 273552
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=273552&action=review
> Source/WebCore/Modules/fetch/FetchBody.cpp:178 > + Vector<char> data = extractFromText(); > + promise.resolve<RefPtr<ArrayBuffer>>(ArrayBuffer::create(data.data(), data.size()));
This extra copying of the data is really unfortunate.
> Source/WebCore/Modules/fetch/FetchBody.cpp:205 > + m_consumer = Consumer(type, WTFMove(promise));
I think this can be: m_consumer = { type, WTFMove(promise) }; And if so, I think it’s nicer.
> Source/WebCore/Modules/fetch/FetchBody.h:80 > + Consumer(Type t, DeferredWrapper&& p) > + : type(t) > + , promise(WTFMove(p)) { }
Shouldn’t need this constructor. Should just be able to use initializer lists to initialize a struct.
youenn fablet
Comment 4
2016-03-11 00:13:12 PST
Created
attachment 273694
[details]
Patch for landing
WebKit Commit Bot
Comment 5
2016-03-11 01:05:38 PST
Comment on
attachment 273694
[details]
Patch for landing Rejecting
attachment 273694
[details]
from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Last 500 characters of output: cts-normal/x86_64/FileReaderSync.dia -c /Volumes/Data/EWS/WebKit/Source/WebCore/fileapi/FileReaderSync.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/FileReaderSync.o ** BUILD FAILED ** The following build commands failed: CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/FetchBody.o Modules/fetch/FetchBody.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler (1 failure) Full output:
http://webkit-queues.webkit.org/results/959271
youenn fablet
Comment 6
2016-03-11 02:33:15 PST
Created
attachment 273714
[details]
Patch for landing
youenn fablet
Comment 7
2016-03-11 02:36:08 PST
> > Source/WebCore/Modules/fetch/FetchBody.cpp:205 > > + m_consumer = Consumer(type, WTFMove(promise)); > > I think this can be: > > m_consumer = { type, WTFMove(promise) }; > > And if so, I think it’s nicer.
Changed to m_consumer = Consumer({...});
> > Source/WebCore/Modules/fetch/FetchBody.h:80 > > + Consumer(Type t, DeferredWrapper&& p) > > + : type(t) > > + , promise(WTFMove(p)) { } > > Shouldn’t need this constructor. Should just be able to use initializer > lists to initialize a struct.
OK
WebKit Commit Bot
Comment 8
2016-03-11 04:08:11 PST
Comment on
attachment 273714
[details]
Patch for landing Clearing flags on attachment: 273714 Committed
r198005
: <
http://trac.webkit.org/changeset/198005
>
WebKit Commit Bot
Comment 9
2016-03-11 04:08:17 PST
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