Set service worker response info data for foreign fetch intercepted fetches.

Set service worker response info data for foreign fetch intercepted fetches.

Without this response info data for foreign fetch requests blink doesn't
know to skip CORS checks for foreign fetch requests (any CORS related checks
have already been done at the point of the foreign fetch handler).

This refactors the way service worker specific response data is associated with
a response by moving it out of ServiceWorkerRequestHandler, and instead storing
it in a separate ServiceWorkerResponseInfo class. The data there is updated
directly by ServiceWorkerURLRequestJob.

BUG=540509
Committed: https://crrev.com/32db1b33e904f50b57d57612ba47b7f4b43f680d
Cr-Commit-Position: refs/heads/master@{#388914}

[Marijn Kruisselbrink, 2016-04-16 00:18:28]

Patchset #1 (id:1) has been deleted

[Marijn Kruisselbrink, 2016-04-16 00:20:30]

Patchset #1 (id:20001) has been deleted

[Marijn Kruisselbrink, 2016-04-18 23:53:55]

Description was changed from

==========
Set service worker response info data for foreign fetch intercepted fetches.

This refactors the way service worker specific response data is associated with
a response by moving it out of ServiceWorkerRequestHandler, and instead storing
it in a separate ServiceWorkerResponseInfo class. The data there is updated
directly by ServiceWorkerURLRequestJob.

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

to

==========
Set service worker response info data for foreign fetch intercepted fetches.

This refactors the way service worker specific response data is associated with
a response by moving it out of ServiceWorkerRequestHandler, and instead storing
it in a separate ServiceWorkerResponseInfo class. The data there is updated
directly by ServiceWorkerURLRequestJob.

BUG=540509
==========

[Marijn Kruisselbrink, 2016-04-18 23:55:29]

Description was changed from

==========
Set service worker response info data for foreign fetch intercepted fetches.

This refactors the way service worker specific response data is associated with
a response by moving it out of ServiceWorkerRequestHandler, and instead storing
it in a separate ServiceWorkerResponseInfo class. The data there is updated
directly by ServiceWorkerURLRequestJob.

BUG=540509
==========

to

==========
Set service worker response info data for foreign fetch intercepted fetches.

Without this response info data for foreign fetch requests blink doesn't
know to skip CORS checks for foreign fetch requests (any CORS related checks
have already been done at the point of the foreign fetch handler).

This refactors the way service worker specific response data is associated with
a response by moving it out of ServiceWorkerRequestHandler, and instead storing
it in a separate ServiceWorkerResponseInfo class. The data there is updated
directly by ServiceWorkerURLRequestJob.

BUG=540509
==========

[Marijn Kruisselbrink, 2016-04-19 00:00:27]

mek@chromium.org changed reviewers:
+ falken@chromium.org

[Marijn Kruisselbrink, 2016-04-19 00:00:28]

Not sure if abstracting the SW specific response info data like this is really
the
best approach; maybe just keeping the existing code as is and separately have 
foreign_fetch_request_handler also keep track of it would have made more sense,
but
not duplicating code seemed like it might be nicer.

Also more in general not sure what to do about CORS preflights and falling back
to
the network with foreign fetch. See the comment in
service_worker_url_request_job.cc
and bug 604084. Any idea who the best people to talk to about that would be?

[falken, 2016-04-19 01:38:15]

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

[falken, 2016-04-19 01:38:16]

Factoring out common code into a new class makes sense to me. I'd like horo@ to
take a look at this too.

[horo, 2016-04-19 11:07:05]

lgtm

I think this refactoring sounds reasonable.

[Marijn Kruisselbrink, 2016-04-19 17:46:55]

mek@chromium.org changed reviewers:
+ mmenke@chromium.org

[Marijn Kruisselbrink, 2016-04-19 17:46:55]

+mmenke for content/browser/loader/resource_loader.cc OWNERS

[Marijn Kruisselbrink, 2016-04-21 18:21:57]

On 2016/04/19 at 17:46:55, Marijn Kruisselbrink wrote:
> +mmenke for content/browser/loader/resource_loader.cc OWNERS

mmenke: ping?

[mmenke, 2016-04-21 18:28:46]

On 2016/04/21 18:21:57, Marijn Kruisselbrink wrote:
> On 2016/04/19 at 17:46:55, Marijn Kruisselbrink wrote:
> > +mmenke for content/browser/loader/resource_loader.cc OWNERS
> 
> mmenke: ping?

Sorry, took two days off...Tried to publish a response earlier today, but
rietveld ate it (?)

[mmenke, 2016-04-21 18:30:31]

loader/ LGTM

https://codereview.chromium.org/1867293004/diff/60001/content/browser/loader/...
File content/browser/loader/resource_loader.cc (right):

https://codereview.chromium.org/1867293004/diff/60001/content/browser/loader/...
content/browser/loader/resource_loader.cc:107: if (ServiceWorkerResponseInfo*
service_worker_info =
Please move declaration of ServiceWorkerResponseInfo outside of the if, while
you're here.  Declaring variables in if's is just not common in chrome.  I'd
suggest moving the initialization outside the if as well, just because that's
how things are usually done.

[Marijn Kruisselbrink, 2016-04-21 19:20:45]

https://codereview.chromium.org/1867293004/diff/60001/content/browser/loader/...
File content/browser/loader/resource_loader.cc (right):

https://codereview.chromium.org/1867293004/diff/60001/content/browser/loader/...
content/browser/loader/resource_loader.cc:107: if (ServiceWorkerResponseInfo*
service_worker_info =
On 2016/04/21 at 18:30:31, mmenke wrote:
> Please move declaration of ServiceWorkerResponseInfo outside of the if, while
you're here.  Declaring variables in if's is just not common in chrome.  I'd
suggest moving the initialization outside the if as well, just because that's
how things are usually done.

Done

[Marijn Kruisselbrink, 2016-04-21 19:20:48]

The CQ bit was checked by mek@chromium.org

[Marijn Kruisselbrink, 2016-04-21 19:20:49]

The patchset sent to the CQ was uploaded after l-g-t-m from mmenke@chromium.org,
horo@chromium.org
Link to the patchset: https://codereview.chromium.org/1867293004/#ps80001
(title: "address comment")

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

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

[commit-bot: I haz the power, 2016-04-21 20:36:38]

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

[commit-bot: I haz the power, 2016-04-21 20:36:39]

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_...)

[Marijn Kruisselbrink, 2016-04-21 20:39:02]

The CQ bit was checked by mek@chromium.org

[commit-bot: I haz the power, 2016-04-21 20:39:26]

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

[commit-bot: I haz the power, 2016-04-21 21:58:19]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-04-22 19:40:08]

Description was changed from

==========
Set service worker response info data for foreign fetch intercepted fetches.

Without this response info data for foreign fetch requests blink doesn't
know to skip CORS checks for foreign fetch requests (any CORS related checks
have already been done at the point of the foreign fetch handler).

This refactors the way service worker specific response data is associated with
a response by moving it out of ServiceWorkerRequestHandler, and instead storing
it in a separate ServiceWorkerResponseInfo class. The data there is updated
directly by ServiceWorkerURLRequestJob.

BUG=540509
==========

to

==========
Set service worker response info data for foreign fetch intercepted fetches.

Without this response info data for foreign fetch requests blink doesn't
know to skip CORS checks for foreign fetch requests (any CORS related checks
have already been done at the point of the foreign fetch handler).

This refactors the way service worker specific response data is associated with
a response by moving it out of ServiceWorkerRequestHandler, and instead storing
it in a separate ServiceWorkerResponseInfo class. The data there is updated
directly by ServiceWorkerURLRequestJob.

BUG=540509

Committed: https://crrev.com/32db1b33e904f50b57d57612ba47b7f4b43f680d
Cr-Commit-Position: refs/heads/master@{#388914}
==========

[commit-bot: I haz the power, 2016-04-22 19:40:09]

Patchset 3 (id:??) landed as
https://crrev.com/32db1b33e904f50b57d57612ba47b7f4b43f680d
Cr-Commit-Position: refs/heads/master@{#388914}