Bug 173195

Summary: [Curl] Use SynchronousLoaderClient for platformLoadResourceSynchronously
Product: WebKit Reporter: Basuke Suzuki <basuke>
Component: WebCore Misc.Assignee: Basuke Suzuki <basuke>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, basuke, bfulgham, buildbot, commit-queue, don.olmstead, pvollan
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 117300    
Attachments:
Description Flags
Patch
none
Patch none

Basuke Suzuki
Reported 2017-06-09 17:18:21 PDT
Curl implementation uses adhoc loader client to load resource synchronously, but there's no reason not using SynchronousLoaderClient as other port do.
Attachments
Patch (5.87 KB, patch)
2017-06-12 11:54 PDT, Basuke Suzuki 		no flags
DetailsFormatted DiffDiff
Patch (6.81 KB, patch)
2017-06-12 12:41 PDT, Basuke Suzuki 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Basuke Suzuki
Comment 1 2017-06-12 11:54:46 PDT
Created attachment 312681 [details] Patch
Build Bot
Comment 2 2017-06-12 11:57:07 PDT
Attachment 312681 [details] did not pass style-queue: ERROR: Source/WebCore/platform/network/curl/SynchronousLoaderClientCurl.cpp:30: You should not add a blank line before implementation file's own header. [build/include_order] [4] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Alex Christensen
Comment 3 2017-06-12 12:05:24 PDT
Comment on attachment 312681 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=312681&action=review Seems like a step in the right direction to match other ports. Could you use Tools/Scripts/prepare-ChangeLog to add a ChangeLog entry? > Source/WebCore/platform/network/curl/ResourceHandleCurl.cpp:130 > + SynchronousLoaderClient client; > + RefPtr<ResourceHandle> handle = adoptRef(new ResourceHandle(context, request, &client, false, false)); > + if (!handle) > + return; null check isn't needed after new in WebKit. In fact, you could do Ref<ResourceHandle> handle = adoptRef(*new ResourceHandle(...))
Basuke Suzuki
Comment 4 2017-06-12 12:11:54 PDT
Yeah, I'm working on it. Still figuring out which tool is right for what I want to.
Basuke Suzuki
Comment 5 2017-06-12 12:41:52 PDT
Created attachment 312685 [details] Patch
WebKit Commit Bot
Comment 6 2017-06-12 15:09:59 PDT
Comment on attachment 312685 [details] Patch Clearing flags on attachment: 312685 Committed r218142: <http://trac.webkit.org/changeset/218142>
WebKit Commit Bot
Comment 7 2017-06-12 15:10:01 PDT
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.