Bug 181506 – [Curl] Extract multipart handling from ResourceHandle to CurlRequest.

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Basuke Suzuki]

Move multipart handling tasks into newly created class CurlMultiHandler and be prepared for NetworkLoadTask.

[Basuke Suzuki]

Created attachment 330988 [details]
Patch

[Basuke Suzuki]

Created attachment 330991 [details]
patch

[Alex Christensen]

Comment on attachment 330991 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330991&action=review

> Source/WebCore/platform/network/curl/CurlMultipartHandle.cppSource/WebCore/platform/network/curl/MultipartHandle.cpp:303
> +bool CurlMultipartHandle::matchForBoundary(const char* data, size_t position, size_t& matchedLength)

Could this return a std::optional<size_t> instead?

> Source/WebCore/platform/network/curl/CurlMultipartHandle.cppSource/WebCore/platform/network/curl/MultipartHandle.cpp:339
> +    char* p = const_cast<char*>(content);

Do we need this to be a non-const char*?  I don't see us writing to it.

[Don Olmstead]

Comment on attachment 330991 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330991&action=review

> Source/WebCore/platform/network/curl/CurlMultipartHandle.cppSource/WebCore/platform/network/curl/MultipartHandle.cpp:3
> + * Copyright (C) 2017 Sony Interactive Entertainment Inc.

Copyright (C) 2018

[Basuke Suzuki]

Comment on attachment 330991 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=330991&action=review

>> Source/WebCore/platform/network/curl/CurlMultipartHandle.cppSource/WebCore/platform/network/curl/MultipartHandle.cpp:3
>> + * Copyright (C) 2017 Sony Interactive Entertainment Inc.
> 
> Copyright (C) 2018

Yeah. Happy new year.

>> Source/WebCore/platform/network/curl/CurlMultipartHandle.cppSource/WebCore/platform/network/curl/MultipartHandle.cpp:303
>> +bool CurlMultipartHandle::matchForBoundary(const char* data, size_t position, size_t& matchedLength)
> 
> Could this return a std::optional<size_t> instead?

No, when return true, first argument is meaningful. On the other case, second argument is used. We'll refactor later, but not this time.

>> Source/WebCore/platform/network/curl/CurlMultipartHandle.cppSource/WebCore/platform/network/curl/MultipartHandle.cpp:339
>> +    char* p = const_cast<char*>(content);
> 
> Do we need this to be a non-const char*?  I don't see us writing to it.

Right.

[Basuke Suzuki]

Created attachment 331095 [details]
Patch

Fixed reviewed points

[WebKit Commit Bot]

Comment on attachment 331095 [details]
Patch

Rejecting attachment 331095 [details] from commit-queue.

New failing tests:
webgl/1.0.2/conformance/uniforms/uniform-default-values.html
Full output: http://webkit-queues.webkit.org/results/6039153

[WebKit Commit Bot]

Created attachment 331108 [details]
Archive of layout-test-results from webkit-cq-01 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: webkit-cq-01  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Don Olmstead]

I don't see how a change to curl would have anything to do with a mac failure and I've also seen this test fail in another bug.

Just going to manually commit it..

[Don Olmstead]

Comment on attachment 331095 [details]
Patch

Clearing flags on attachment: 331095

Committed r226800: <https://trac.webkit.org/changeset/226800>

[Don Olmstead]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/36448712>