ResourceHandleCurlDelegate was created for migration to the CurlRequest classes. Every features required by NetworkLoadTask are moved to CurlRequest class and remainings are ResourceHandle related logic. It's time to merge back to ResourceHandle.
Created attachment 333397 [details] patch
Created attachment 333400 [details] Another try
Created attachment 333410 [details] Fix break of other port
Created attachment 333537 [details] WIP redesign classes With Alex's advice, I restructured classes and made no need to implement interface directy on ResourceHandle.
Attachment 333537 [details] did not pass style-queue: ERROR: Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.cpp:47: Wrong number of spaces before statement. (expected: 4) [whitespace/indent] [4] ERROR: Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.cpp:47: Weird number of spaces at line-start. Are you using a 4-space indent? [whitespace/indent] [3] ERROR: Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.h:1: One or more unexpected \r (^M) found; better to use only a \n [whitespace/carriage_return] [1] ERROR: Suppressing further [whitespace/carriage_return] reports for this file. ERROR: Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.h:45: The parameter name "handle" adds no information, so it should be removed. [readability/parameter_name] [5] ERROR: Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.h:51: The parameter name "response" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 80 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
One of the goal with this patch is to make WinCairo port looks same with other ports. We use Mac port as a reference for task separation. WebCoreResourceHandleAsOperationQueueDelegate => CurlResourceHandleDelegate We have our dedicated scheduler which handle thread handling, so CurlResourceHandleDelegate is much simpler than Mac's one.
Created attachment 333601 [details] Patch
Created attachment 333709 [details] PATCH
Comment on attachment 333709 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=333709&action=review > Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.cpp:93 > + for (auto header : response.headers) { const auto& header > Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:342 > + return std::pair<String, String>("", ""); If you really need empty String which is non-null internally, use WTF::emptyString() > Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:519 > + mediaType = "text/plain"; It should be ASCIILiteral("text/plain") > Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:525 > + charset = "US-ASCII"; Ditto
(In reply to Konstantin Tokarev from comment #9) > Comment on attachment 333709 [details] > PATCH > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333709&action=review > > > Source/WebCore/platform/network/curl/CurlResourceHandleDelegate.cpp:93 > > + for (auto header : response.headers) { > > const auto& header right. > > Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:342 > > + return std::pair<String, String>("", ""); > > If you really need empty String which is non-null internally, use > WTF::emptyString() Good point. These empty strings are not required to be set, so I will make them std::optional. > > Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:519 > > + mediaType = "text/plain"; > > It should be ASCIILiteral("text/plain") > > > Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:525 > > + charset = "US-ASCII"; > > Ditto Got them.
Created attachment 333926 [details] Fix reviewed points
Attachment 333926 [details] did not pass style-queue: ERROR: Source/WebCore/platform/network/curl/CurlRequest.h:97: The parameter name "client" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 333927 [details] Fix style
Comment on attachment 333927 [details] Fix style Clearing flags on attachment: 333927 Committed r228577: <https://trac.webkit.org/changeset/228577>
All reviewed patches have been landed. Closing bug.
<rdar://problem/37616884>