Bug 49837 – Loader cleanup: Make Loader per-request, attach to CachedResourceLoader

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Nate Chapin]

This changes will give each request its own Loader, effectively merging Loader and Request.

After this change, Loader should be renamed to something more descriptive (maybe CachedResourceRequest?)

[Nate Chapin]

Created attachment 74431 [details]
patch

Note that I have not thoroughly tested this patch yet (it passes layout tests, but I haven't done extensive browsing with it yet).  I figured I should get some eyes more on it to check whether the general design seems sane.

[Antti Koivisto]

I like the design direction. Eliminating Request is very nice. Loader seriously needs renaming after this.

Could you explain the rationale for this change bit more (in the ChangeLog too)?

Looks good but I'll have try it before r+'ing.

[Nate Chapin]

(In reply to comment #2)
> I like the design direction. Eliminating Request is very nice. Loader seriously needs renaming after this.
> 
> Could you explain the rationale for this change bit more (in the ChangeLog too)?
> 
> Looks good but I'll have try it before r+'ing.

The rationale is something like:
(1) Loader shouldn't be a member of MemoryCache (especially since we're trying to make MemoryCache storage-only, and not involved in the actual loading process).  It should be a member of CachedResourceLoader.
(2) Loader no longer needs to be a singleton if it's not tied to MemoryCache.  Request's only purpose is to store per-request data that Loader can't store because it's a singleton.  Ergo, merge the two.

I'll upload a new patch with a more descriptive ChangeLog in a bit once I've had a chance to test the patch a little bit more.

[Nate Chapin]

Created attachment 75181 [details]
More descriptive ChangeLog and merged to tip of tree

[Eric Seidel (no email)]

Attachment 75181 [details] did not build on mac:
Build output: http://queues.webkit.org/results/6447048

[Antti Koivisto]

Comment on attachment 75181 [details]
More descriptive ChangeLog and merged to tip of tree

r=me but please rename Loader to something more sensible in a follow-up patch and fix naming problems like:

void setRequest(Loader*)

at that point.

[WebKit Commit Bot]

Comment on attachment 75181 [details]
More descriptive ChangeLog and merged to tip of tree

Rejecting patch 75181 from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'build', '--no-clean', '--no-update', '--build-style=both']" exit_code: 2
Last 500 characters of output:
ERSION_MAJOR 0300
    setenv XCODE_VERSION_MINOR 0320
    setenv YACC /Developer/usr/bin/yacc
    /bin/sh -c /Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Script-5DF50887116F3077005202AB.sh

** BUILD FAILED **


The following build commands failed:
WebCore:
	CompileC /Projects/CommitQueue/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/loader.o /Projects/CommitQueue/WebCore/loader/loader.cpp normal x86_64 c++ com.apple.compilers.gcc.4_2
(1 failure)


Full output: http://queues.webkit.org/results/6965014

[Nate Chapin]

Looks like I forgot to close this: http://trac.webkit.org/changeset/73749.