Bug 155291

Summary: [Fetch API] Use DeferredWrapper directly in FetchBody promise handling
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, cdumez, commit-queue, darin, esprehn+autocc, kondapallykalyan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 151937    
Attachments:
Description Flags
Patch
none
Patch for landing
none
Patch for landing none

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
DetailsFormatted DiffDiff
Patch for landing (23.20 KB, patch)
2016-03-11 00:13 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
Patch for landing (23.32 KB, patch)
2016-03-11 02:33 PST, youenn fablet 		no flags
DetailsFormatted DiffDiff
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.