Move suborigin/serviceworker processing to ResourceFetcher::prepareRequest()

Move suborigin/serviceworker processing to prepareRequest()

To remove modifications to ResourceRequest in
ResourceFetcher::startLoad(), this CL moves setServiceWorkerMode() call
to ResourceFetcher::prepareRequest() from startLoad().

BUG=632580

[hiroshige, 2017-03-10 18:56:52]

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

[commit-bot: I haz the power, 2017-03-10 18:57:44]

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

[hiroshige, 2017-03-10 20:16:33]

Description was changed from

==========
Move suborigin/serviceworker processing to ResourceFetcher::prepareRequest()

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

to

==========
Move suborigin/serviceworker processing to prepareRequest()

To remove modifications to ResourceRequest in
ResourceFetcher::startLoad(), this CL moves setServiceWorkerMode() call
to ResourceFetcher::prepareRequest() from startLoad().

BUG=632580
==========

[hiroshige, 2017-03-10 20:16:33]

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

[hiroshige, 2017-03-10 20:17:16]

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

[hiroshige, 2017-03-10 20:20:59]

PTAL,

japhet@ as loading owner,

jww@ horo@ service worker: jww@ introduced this code block in
https://codereview.chromium.org/2343053002/; is it safe to move the check to
prepareRequest()?
(I think it is safe if requests from different origins that are controlled by
serviceworker are not shared via MemoryCache, but I'm not sure whether it is
enforced)

[hiroshige, 2017-03-10 20:20:59]

PTAL,

japhet@ as loading owner,

jww@ horo@ service worker: jww@ introduced this code block in
https://codereview.chromium.org/2343053002/; is it safe to move the check to
prepareRequest()?
(I think it is safe if requests from different origins that are controlled by
serviceworker are not shared via MemoryCache, but I'm not sure whether it is
enforced)

[commit-bot: I haz the power, 2017-03-10 20:24:57]

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

[commit-bot: I haz the power, 2017-03-10 20:24:58]

Dry run: This issue passed the CQ dry run.

[hiroshige, 2017-03-10 23:30:38]

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

[commit-bot: I haz the power, 2017-03-10 23:31:01]

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

[Nate Chapin, 2017-03-11 00:51:38]

LGTM if serviceworker folks are happy.

[commit-bot: I haz the power, 2017-03-11 01:53:58]

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

[commit-bot: I haz the power, 2017-03-11 01:53:58]

Dry run: 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_...)

[horo, 2017-03-13 08:07:52]

lgtm

If two pages are controlled by different service workers, they use different
MemoryCache.

But if the request from a non-suborigin page is handled by ForingnFetch event
handler, the response could be re-used for the suborigin pages.
jww@: Is this the expected behavior? Or a bug?

[Mike West, 2017-03-22 18:05:03]

mkwst@chromium.org changed reviewers:
+ jochen@chromium.org, mkwst@chromium.org
- jww@chromium.org

[Mike West, 2017-03-22 18:05:03]

LGTM. We haven't thought through the impact of suborigins on Foreign Fetch; we
ought to. This patch is fine for the moment, but it does point to an outstanding
piece of work that fell off our radar.

Would you mind filing a bug against
https://github.com/w3c/webappsec-suborigins/issues so we don't forget it again?
:)

+jochen: FYI

[horo, 2017-03-23 05:48:29]

On 2017/03/22 18:05:03, Mike West wrote:
> LGTM. We haven't thought through the impact of suborigins on Foreign Fetch; we
> ought to. This patch is fine for the moment, but it does point to an
outstanding
> piece of work that fell off our radar.
> 
> Would you mind filing a bug against
> https://github.com/w3c/webappsec-suborigins/issues so we don't forget it
again?
> :)
> 
> +jochen: FYI

I think this is not a spec bug, because there is no spec of MemoryCache.
MemoryCache is a kind of browser specific optimization.
So I think this is an implementation issue.

[jochen (gone - plz use gerrit), 2017-03-23 15:52:12]

is it possible to add a test for this?

[horo, 2017-03-24 04:26:29]

On 2017/03/23 15:52:12, jochen wrote:
> is it possible to add a test for this?

Created: https://codereview.chromium.org/2770213002