Bug 155291 – [Fetch API] Use DeferredWrapper directly in FetchBody promise handling

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[youenn fablet]

The DOMPromise data typing is not helpful here, we should switch back to DeferredWrapper.

[youenn fablet]

Created attachment 273552 [details]
Patch

[WebKit Commit Bot]

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 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]

Created attachment 273694 [details]
Patch for landing

[WebKit Commit Bot]

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]

Created attachment 273714 [details]
Patch for landing

[youenn fablet]

> > 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 on attachment 273714 [details]
Patch for landing

Clearing flags on attachment: 273714

Committed r198005: <http://trac.webkit.org/changeset/198005>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.