Clean up response handling in ResourceLoader/ResourceFetcher

Clean up response handling in ResourceLoader/ResourceFetcher

This moves all of ResourceLoader's non-trivial response handling logic to ResourceFetcher, and changes the ordering of some steps to better match other callbacks.

Old order:
- Access control checks in ResourceLoader::didReceiveResponse()
- Resource::responseReceived()
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()

New order:
- Access control checks in ResourceLoader::didReceiveResponse() (now in ResourceFetcher)
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()
- Resource::responseReceived()

BUG=

Committed: https://crrev.com/17f873489bd87f24bff5f0b93aeeedb19754694f
Cr-Commit-Position: refs/heads/master@{#408808}

[Nate Chapin, 2016-05-13 21:13:16]

The CQ bit was checked by japhet@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-13 21:13:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1975373002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1975373002/20001

[commit-bot: I haz the power, 2016-05-14 01:07:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-14 01:07:37]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Nate Chapin, 2016-05-19 21:23:53]

The CQ bit was checked by japhet@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-19 21:24:40]

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

[commit-bot: I haz the power, 2016-05-19 23:58:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-19 23:58:02]

Dry run: This issue passed the CQ dry run.

[Nate Chapin, 2016-06-14 21:40:40]

The CQ bit was checked by japhet@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-06-14 21:41:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1975373002/80001

[Nate Chapin, 2016-06-16 17:41:00]

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

[Nate Chapin, 2016-06-16 17:41:01]

hiroshige, does this seem like a reasonable cleanup?

I'm trying to make ResourceLoader as simple as possible, and I'm considering the
possibility of trying to merge it entirely in to ResourceFetcher. This is a step
towards either of those.

[Nate Chapin, 2016-07-25 23:25:06]

The CQ bit was checked by japhet@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 23:25:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 00:35:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-26 00:35:44]

Dry run: This issue passed the CQ dry run.

[Nate Chapin, 2016-07-26 16:44:03]

On 2016/06/16 17:41:01, Nate Chapin wrote:
> hiroshige, does this seem like a reasonable cleanup?
> 
> I'm trying to make ResourceLoader as simple as possible, and I'm considering
the
> possibility of trying to merge it entirely in to ResourceFetcher. This is a
step
> towards either of those.

Ping :)

[hiroshige, 2016-07-28 09:56:42]

This CL reorders
- Access control checks in ResourceLoader::didReceiveResponse()
- Resource::responseReceived() (which contains access control checks in
ThreadableLoader)
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()
to:
- FetchContext::dispatchDidReceiveResponse()
- Access control checks in ResourceLoader::didReceiveResponse() (now merged into
ResourceFetcher::didReceiveResponse())
- Access control checks in ResourceFetcher::didReceiveResponse()
- Resource::responseReceived()

Could you mention this reordering in the CL description?

I'm not sure this reordering is safe, particularly calling
FetchContext::dispatchDidReceiveResponse() before access control checks and
ResourceLoader::restart().
For example, FetchContext::dispatchDidReceiveResponse() contains inspector
calls, so in the case where ResourceLoader::restart() is called, the inspector
is notified of response receiving twice (before and after restart()).

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (left):

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:242: bool
ResourceFetcher::canAccessResource(Resource* resource, SecurityOrigin*
sourceOrigin, const KURL& url, AccessControlLoggingDecision logErrorsDecision)
const
This CL removes AccessControlLoggingDecision because it is always
ShouldLogAccessControlErrors.
Can we split changes related to this to a separate CL?

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:257: const
ResourceResponse& responseForAccessControl = response.httpStatusCode() == 304 ?
resource->response() : response;
We need to check m_resource->isCacheValidator() here, because a server can
return 304 for non-revalidating request (such server is not well behaving
though).

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:259: if
(!passesAccessControlCheck(responseForAccessControl,
resource->options().allowCredentials, sourceOrigin, errorDescription,
resource->lastResourceRequest().requestContext())) {
This replaces ResourceRequest::allowStoredCredentials() with
ResourceLoaderOptions::allowCredentials. I think it's good because they are
duplicated. Are we going to merge them further? (If so, let's create a tracking
crbug)

Placing DCHECK_EQ(resource->options().allowCredentials,
resource->lastResourceRequest().allowStoredCredentials() ?
AllowStoredCredentials : DoNotAllowStoredCredentials) is good for catching
potential bugs.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:259: if
(!passesAccessControlCheck(responseForAccessControl,
resource->options().allowCredentials, sourceOrigin, errorDescription,
resource->lastResourceRequest().requestContext())) {
Memo: This inlines Resource::passesAccessControlCheck().

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:956: if
(resource->options().corsEnabled == IsCORSEnabled &&
response.wasFallbackRequiredByServiceWorker()) {
memo for myself: this block is moved from the callee:
ResourceLoader::didReceiveResponse().

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.h (right):

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.h:117: bool
didReceiveResponse(Resource*, const ResourceResponse&);
It is important to check the return value of this function, but the function
name doesn't look like so.
How about adding comments and adding WARN_UNUSED_RETURN?
Changing the function name is also preferable -- e.g. checkAccessControl().

Also, if we keep the function name, the meaning of return type of |bool| is not
clear and thus comments are needed.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceLoader.cpp (left):

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:181:
m_resource->setResponse(resourceResponse);
This CL removes this setResponse(). It's probably OK because
Resource::responseReceived() sets response.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceLoader.cpp (right):

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:96: void
ResourceLoader::restart(const ResourceRequest& request)
I think the name of |restart| is too general, because this function cannot
restart loading of any Resource, and thus need renaming and/or comments.
(In this case, restart() is called before Resource::responseReceived() (and
before/without Resource::willFollowRedirect()?) and thus we can restart loading
by simply restarting WebURLLoader, not touching Resource's state.)

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:164: if
(response.httpStatusCode() >= 400 &&
!m_resource->shouldIgnoreHTTPStatusCodeErrors())
Do we need |if (!m_loader) return;| before this?

[Nate Chapin, 2016-07-28 22:28:24]

Description was changed from

==========
Clean up response handling in ResourceLoader/ResourceFetcher

BUG=
==========

to

==========
Clean up response handling in ResourceLoader/ResourceFetcher

This moves all of ResourceLoader's non-trivial response handling logic to
ResourceFetcher, and changes the ordering of some steps to better match other
callbacks.

Old order:
- Access control checks in ResourceLoader::didReceiveResponse()
- Resource::responseReceived()
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()

New order:
- Access control checks in ResourceLoader::didReceiveResponse() (now in
ResourceFetcher)
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()
- Resource::responseReceived()

BUG=
==========

[Nate Chapin, 2016-07-28 22:29:03]

The CQ bit was checked by japhet@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-28 22:29:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Nate Chapin, 2016-07-28 22:29:36]

Update CL description.

Also moved the remaining bits of ResourceLoader::didReceiveResponse to
ResourceFetcher, since there's no reason to leave that bit in ResourceLoader and
removes the need for the didReceiveResponse return value.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (left):

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:242: bool
ResourceFetcher::canAccessResource(Resource* resource, SecurityOrigin*
sourceOrigin, const KURL& url, AccessControlLoggingDecision logErrorsDecision)
const
On 2016/07/28 09:56:42, hiroshige wrote:
> This CL removes AccessControlLoggingDecision because it is always
> ShouldLogAccessControlErrors.
> Can we split changes related to this to a separate CL?

Ok, will do.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:257: const
ResourceResponse& responseForAccessControl = response.httpStatusCode() == 304 ?
resource->response() : response;
On 2016/07/28 09:56:42, hiroshige wrote:
> We need to check m_resource->isCacheValidator() here, because a server can
> return 304 for non-revalidating request (such server is not well behaving
> though).

Oops, I don't remember dropping that. Done.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:259: if
(!passesAccessControlCheck(responseForAccessControl,
resource->options().allowCredentials, sourceOrigin, errorDescription,
resource->lastResourceRequest().requestContext())) {
On 2016/07/28 09:56:42, hiroshige wrote:
> Memo: This inlines Resource::passesAccessControlCheck().

Also, see comment about setReponse() call in
ResourceLoader::didReceiveResponse(). The old variant implicitly uses
Resource::m_response, which is not yet set in the non-304 case.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:259: if
(!passesAccessControlCheck(responseForAccessControl,
resource->options().allowCredentials, sourceOrigin, errorDescription,
resource->lastResourceRequest().requestContext())) {
On 2016/07/28 09:56:42, hiroshige wrote:
> This replaces ResourceRequest::allowStoredCredentials() with
> ResourceLoaderOptions::allowCredentials. I think it's good because they are
> duplicated. Are we going to merge them further? (If so, let's create a
tracking
> crbug)
> 
> Placing DCHECK_EQ(resource->options().allowCredentials,
> resource->lastResourceRequest().allowStoredCredentials() ?
> AllowStoredCredentials : DoNotAllowStoredCredentials) is good for catching
> potential bugs.

That may make sense, but some places don't use the StoredCredentials enum at
all, and it's not exposed outside of blink. ResourceFetcher always holds them to
be equivalent, so it seemed correct in this specific scenario.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.h (right):

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.h:117: bool
didReceiveResponse(Resource*, const ResourceResponse&);
On 2016/07/28 09:56:42, hiroshige wrote:
> It is important to check the return value of this function, but the function
> name doesn't look like so.
> How about adding comments and adding WARN_UNUSED_RETURN?
> Changing the function name is also preferable -- e.g. checkAccessControl().
> 
> Also, if we keep the function name, the meaning of return type of |bool| is
not
> clear and thus comments are needed.

Refactored so that this returns void again.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceLoader.cpp (left):

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:181:
m_resource->setResponse(resourceResponse);
On 2016/07/28 09:56:42, hiroshige wrote:
> This CL removes this setResponse(). It's probably OK because
> Resource::responseReceived() sets response.

Mostly correct. The only reason it needed to be here was because
canAccessResource() expected Resource::response() to be valid. I refactored
canAccessResource() and renamed it to canAccessResponse(), and removed this
expectation.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceLoader.cpp (right):

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:96: void
ResourceLoader::restart(const ResourceRequest& request)
On 2016/07/28 09:56:42, hiroshige wrote:
> I think the name of |restart| is too general, because this function cannot
> restart loading of any Resource, and thus need renaming and/or comments.
> (In this case, restart() is called before Resource::responseReceived() (and
> before/without Resource::willFollowRedirect()?) and thus we can restart
loading
> by simply restarting WebURLLoader, not touching Resource's state.)

Renamed to restartForServiceWorkerFallback.

https://codereview.chromium.org/1975373002/diff/140001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceLoader.cpp:164: if
(response.httpStatusCode() >= 400 &&
!m_resource->shouldIgnoreHTTPStatusCodeErrors())
On 2016/07/28 09:56:42, hiroshige wrote:
> Do we need |if (!m_loader) return;| before this?

This moved to ResourceFetcher, but add a resource->loader() nullcheck there.

[commit-bot: I haz the power, 2016-07-28 23:49:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-28 23:49:49]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2016-07-29 08:00:26]

hiroshige@chromium.org changed reviewers:
+ horo@chromium.org

[hiroshige, 2016-07-29 08:00:27]

lgtm.

https://codereview.chromium.org/1975373002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (left):

https://codereview.chromium.org/1975373002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:958:
context().dispatchDidFail(resource->identifier(),
ResourceError(errorDomainBlinkInternal, 0, originalURL.getString(), "Unsafe
attempt to load URL " + originalURL.elidedString() + " fetched by a
ServiceWorker."), isInternalRequest);
FYI +horo@, this changes an error message related to ServiceWorker.

[Nate Chapin, 2016-07-29 19:04:52]

https://codereview.chromium.org/1975373002/diff/160001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (left):

https://codereview.chromium.org/1975373002/diff/160001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:958:
context().dispatchDidFail(resource->identifier(),
ResourceError(errorDomainBlinkInternal, 0, originalURL.getString(), "Unsafe
attempt to load URL " + originalURL.elidedString() + " fetched by a
ServiceWorker."), isInternalRequest);
On 2016/07/29 08:00:27, hiroshige wrote:
> FYI +horo@, this changes an error message related to ServiceWorker.

I probably shouldn't have done that. I'll revert it before landing.

[Nate Chapin, 2016-07-29 19:05:00]

The CQ bit was checked by japhet@chromium.org

[Nate Chapin, 2016-07-29 19:05:01]

The patchset sent to the CQ was uploaded after l-g-t-m from
hiroshige@chromium.org
Link to the patchset: https://codereview.chromium.org/1975373002/#ps180001
(title: "Revert changes to service worker failure message")

[commit-bot: I haz the power, 2016-07-29 19:05:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 20:16:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-29 20:16:36]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Nate Chapin, 2016-07-29 20:41:04]

The CQ bit was checked by japhet@chromium.org

[Nate Chapin, 2016-07-29 20:41:04]

The patchset sent to the CQ was uploaded after l-g-t-m from
hiroshige@chromium.org
Link to the patchset: https://codereview.chromium.org/1975373002/#ps200001
(title: "Back to ResourceError:cancelledDueToAccessCheckError")

[Nate Chapin, 2016-07-29 20:41:29]

https://codereview.chromium.org/1975373002/diff/180001/third_party/WebKit/Sou...
File third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp (right):

https://codereview.chromium.org/1975373002/diff/180001/third_party/WebKit/Sou...
third_party/WebKit/Source/core/fetch/ResourceFetcher.cpp:977:
resource->loader()->didFail(nullptr, ResourceError(errorDomainBlinkInternal, 0,
originalURL.getString(), "Unsafe attempt to load URL " +
originalURL.elidedString() + " fetched by a ServiceWorker."));
Ah...that's what it is. didFail() implicitly converts the ResourceError to a
WebURLError, and it gets converted back to a ResourceError when
ResourceFetcher::didFailLoading() is called. That conversion back to
ResourceError results in a null ResourceError if the WebURLError::reason is 0,
and blink doesn't have a way to get a valid error reason.

So I've returned to using a ResourceError::cancelledDueToAccessCheckError() 
here, which changes the error message. If it's important to keep this error
message, let me know and I'll do the requisite plumbing.

[commit-bot: I haz the power, 2016-07-29 20:41:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-29 23:55:48]

Description was changed from

==========
Clean up response handling in ResourceLoader/ResourceFetcher

This moves all of ResourceLoader's non-trivial response handling logic to
ResourceFetcher, and changes the ordering of some steps to better match other
callbacks.

Old order:
- Access control checks in ResourceLoader::didReceiveResponse()
- Resource::responseReceived()
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()

New order:
- Access control checks in ResourceLoader::didReceiveResponse() (now in
ResourceFetcher)
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()
- Resource::responseReceived()

BUG=
==========

to

==========
Clean up response handling in ResourceLoader/ResourceFetcher

This moves all of ResourceLoader's non-trivial response handling logic to
ResourceFetcher, and changes the ordering of some steps to better match other
callbacks.

Old order:
- Access control checks in ResourceLoader::didReceiveResponse()
- Resource::responseReceived()
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()

New order:
- Access control checks in ResourceLoader::didReceiveResponse() (now in
ResourceFetcher)
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()
- Resource::responseReceived()

BUG=
==========

[commit-bot: I haz the power, 2016-07-29 23:55:49]

Committed patchset #11 (id:200001)

[commit-bot: I haz the power, 2016-07-29 23:57:14]

Description was changed from

==========
Clean up response handling in ResourceLoader/ResourceFetcher

This moves all of ResourceLoader's non-trivial response handling logic to
ResourceFetcher, and changes the ordering of some steps to better match other
callbacks.

Old order:
- Access control checks in ResourceLoader::didReceiveResponse()
- Resource::responseReceived()
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()

New order:
- Access control checks in ResourceLoader::didReceiveResponse() (now in
ResourceFetcher)
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()
- Resource::responseReceived()

BUG=
==========

to

==========
Clean up response handling in ResourceLoader/ResourceFetcher

This moves all of ResourceLoader's non-trivial response handling logic to
ResourceFetcher, and changes the ordering of some steps to better match other
callbacks.

Old order:
- Access control checks in ResourceLoader::didReceiveResponse()
- Resource::responseReceived()
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()

New order:
- Access control checks in ResourceLoader::didReceiveResponse() (now in
ResourceFetcher)
- Access control checks in ResourceFetcher::didReceiveResponse()
- FetchContext::dispatchDidReceiveResponse()
- Resource::responseReceived()

BUG=

Committed: https://crrev.com/17f873489bd87f24bff5f0b93aeeedb19754694f
Cr-Commit-Position: refs/heads/master@{#408808}
==========

[commit-bot: I haz the power, 2016-07-29 23:57:16]

Patchset 11 (id:??) landed as
https://crrev.com/17f873489bd87f24bff5f0b93aeeedb19754694f
Cr-Commit-Position: refs/heads/master@{#408808}