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
youenn fablet
2016-03-10 00:25:10 PST
Created attachment 273552 [details]
Patch
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.
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. Created attachment 273694 [details]
Patch for landing
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 Created attachment 273714 [details]
Patch for landing
> > 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 Comment on attachment 273714 [details] Patch for landing Clearing flags on attachment: 273714 Committed r198005: <http://trac.webkit.org/changeset/198005> All reviewed patches have been landed. Closing bug. |