Refactor the API for setting dynamic resource load priorities

Refactor the API for setting dynamic resource load priorities

Currently, only images can change priority after blink sends
the resource load out to chromium. After each layout, we
iterate through a registry of LayoutObjects with associated
images and update those images' priority based on whether
the LayoutObject is in the viewport and how much of the
viewport it fills.

Change this API to be more general, so that it could more
easily support other resource types and other factors for
updating priority.

Move the current logic to ResourceFetcher, which has a list
of all in-progress resource loads. For each, ask the
associated ResourceClients for updated priority information.
This should not entail any behavior change at this time.

BUG=536147
Committed: https://crrev.com/837d108907e02d5c267720fc41ecebf28ce36d2a
Cr-Commit-Position: refs/heads/master@{#353188}

[Nate Chapin, 2015-10-07 19:40:39]

japhet@chromium.org changed reviewers:
+ esprehn@chromium.org

[Nate Chapin, 2015-10-07 19:40:40]

This is the reason for the cleanup in
https://codereview.chromium.org/1362693004.

esprehn, is this something you're comfortable reviewing?

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/Resource.cpp:949: ResourcePriority
Resource::priorityFromClients()
I could go either way on whether this belongs here or on ImageResource.

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:1000: for (const auto&
loader : m_loaders->hashSet()) {
This feels a little hacky, but it's the cleanest way to get the Resources loaded
by this ResourceFetcher that are currently loading. It omits mjpeg images (which
are in m_nonBlockingLoaders after the first payload http response), which should
probably be handled separately for prioritization, because of their long-running
nature.

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1354: ResourcePriority
LayoutBox::updatePriority(Resource*)
Merge the basically identical logic in LayoutImage and LayoutBlock here.

[esprehn, 2015-10-08 05:39:11]

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/Resource.cpp:952:
ResourceClientWalker<ResourceClient> w(m_clients);
walker

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/Resource.cpp:953: while (ResourceClient* c
= w.next()) {
client = walker.next()

we should also really make this walker just a plain iterator and use for each
loops.

for (auto& client : walker)

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ResourceLoaderSet.h (right):

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ResourceLoaderSet.h:53: const
HeapHashSet<Member<ResourceLoader>>& hashSet() const { return m_set; }
SetType&

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1354: ResourcePriority
LayoutBox::updatePriority(Resource*)
You ignore the argument, why pass it at all?

This also doesn't actually update anything, it's a const computation.

updatePriority -> computeResourcePriority() const

[Nate Chapin, 2015-10-08 21:26:38]

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/Resource.cpp (right):

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/Resource.cpp:952:
ResourceClientWalker<ResourceClient> w(m_clients);
On 2015/10/08 05:39:11, esprehn wrote:
> walker

Done.

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/Resource.cpp:953: while (ResourceClient* c
= w.next()) {
On 2015/10/08 05:39:11, esprehn wrote:
> client = walker.next()
> 
> we should also really make this walker just a plain iterator and use for each
> loops.
> 
> for (auto& client : walker)

I'll add that to my list of things to do eventually :)

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/fetch/ResourceLoaderSet.h (right):

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/fetch/ResourceLoaderSet.h:53: const
HeapHashSet<Member<ResourceLoader>>& hashSet() const { return m_set; }
On 2015/10/08 05:39:11, esprehn wrote:
> SetType&

Done.

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right):

https://codereview.chromium.org/1378743002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/layout/LayoutBox.cpp:1354: ResourcePriority
LayoutBox::updatePriority(Resource*)
On 2015/10/08 05:39:11, esprehn wrote:
> You ignore the argument, why pass it at all?

I was thinking that the return value might depend on the Resource, but it
doesn't, and I forgot to remove it. Done.

> 
> This also doesn't actually update anything, it's a const computation.
> 
> updatePriority -> computeResourcePriority() const

Good point. Done.

[esprehn, 2015-10-08 22:01:05]

lgtm

[Nate Chapin, 2015-10-08 22:38:11]

The CQ bit was checked by japhet@chromium.org

[commit-bot: I haz the power, 2015-10-08 22:40:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1378743002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1378743002/60001

[commit-bot: I haz the power, 2015-10-09 00:40:44]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-09 00:41:34]

Patchset 4 (id:??) landed as
https://crrev.com/837d108907e02d5c267720fc41ecebf28ce36d2a
Cr-Commit-Position: refs/heads/master@{#353188}