Bug 50848 – Rename Loader

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Nate Chapin]

...I've thrown around the name CachedResourceRequest in a couple of places and haven't heard any objections, so that's what I'll call it in my first attempt :)

[Nate Chapin]

Created attachment 76265 [details]
patch

[Adam Barth]

Comment on attachment 76265 [details]
patch

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

Interesting how the variable of this type is often called a request.

> WebCore/loader/cache/CachedResourceRequest.cpp:105
> -PassRefPtr<Loader> Loader::load(CachedResourceLoader* cachedResourceLoader, CachedResource* resource, bool incremental, SecurityCheckPolicy securityCheck, bool sendResourceLoadCallbacks)
> +PassRefPtr<CachedResourceRequest> CachedResourceRequest::load(CachedResourceLoader* cachedResourceLoader, CachedResource* resource, bool incremental, SecurityCheckPolicy securityCheck, bool sendResourceLoadCallbacks)

Should this just be called "create" ?  Does this method actually kick off the load?

[Nate Chapin]

(In reply to comment #2)
> (From update of attachment 76265 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=76265&action=review
> 
> Interesting how the variable of this type is often called a request.

In http://trac.webkit.org/changeset/73749, I changed those functions from taking a Request to a Loader but didn't rename everything because this patch would soon follow it.
> 
> > WebCore/loader/cache/CachedResourceRequest.cpp:105
> > -PassRefPtr<Loader> Loader::load(CachedResourceLoader* cachedResourceLoader, CachedResource* resource, bool incremental, SecurityCheckPolicy securityCheck, bool sendResourceLoadCallbacks)
> > +PassRefPtr<CachedResourceRequest> CachedResourceRequest::load(CachedResourceLoader* cachedResourceLoader, CachedResource* resource, bool incremental, SecurityCheckPolicy securityCheck, bool sendResourceLoadCallbacks)
> 
> Should this just be called "create" ?  Does this method actually kick off the load?

CachedResourceRequest::load() schedules the load with ResourceLoadScheduler, which from the perspective of its user (CachedResourceLoader) is kicking off the load.

I'm open the idea of renaming it if that's what people want.

[Adam Barth]

Comment on attachment 76265 [details]
patch

Let's move forward with this patch.  In the next patch, it might be good to split up load into a create and a start method (or something) to separate the notion of the a request (which should be mostly data, like noun) from a loader (which shown do something, like a verb).

[WebKit Commit Bot]

Comment on attachment 76265 [details]
patch

Rejecting attachment 76265 [details] from commit-queue.

Failed to run "['./WebKitTools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=eseidel-cq-sl', 'apply-attachment', '--non-interactive', 76265]" exit_code: 2
Last 500 characters of output:
ceRequest.h
|===================================================================
|--- WebCore/loader/cache/CachedResourceRequest.h	(revision 73747)
|+++ WebCore/loader/cache/CachedResourceRequest.h	(working copy)
--------------------------
No file to patch.  Skipping patch.
2 out of 2 hunks ignored
patching file WebCore/loader/loader.cpp
rm 'WebCore/loader/loader.cpp'

Failed to run "[u'/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Adam Barth', u'--force']" exit_code: 1

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

[WebKit Review Bot]

http://trac.webkit.org/changeset/74049 might have broken Qt Linux ARMv5 Release, Qt Linux ARMv7 Release, Qt Windows 32-bit Release, and Qt Windows 32-bit Debug

[Nate Chapin]

Landed manually in http://trac.webkit.org/changeset/74049