Limit Link header based SW installations for fetches made by SW.

Limit Link header based SW installations for fetches made by SW.

To prevent a service worker from spawning new service workers in the
background, only process link headers on requests made by service
workers if the service worker is controlling at least one client.

BUG=662443
Committed: https://crrev.com/e8313bb499c439b0a5091225faf97b06f4cf1182
Cr-Commit-Position: refs/heads/master@{#433489}

[Marijn Kruisselbrink, 2016-11-19 00:22:56]

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

[commit-bot: I haz the power, 2016-11-19 00:24:08]

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-11-19 01:54:35]

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

[commit-bot: I haz the power, 2016-11-19 01:54:36]

Dry run: This issue passed the CQ dry run.

[Marijn Kruisselbrink, 2016-11-20 03:39:05]

Description was changed from

==========
Limit Link header based SW installataions for fetches made by SW.

BUG=662443
==========

to

==========
Limit Link header based SW installations for fetches made by SW.

To prevent a service worker from spawning new service workers in the
background, only process link headers on requests made by service
workers if the service worker is controlling at least one client.

BUG=662443
==========

[Marijn Kruisselbrink, 2016-11-20 03:39:17]

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

[falken, 2016-11-21 04:13:42]

lgtm to land this but I think the discussion is still ongoing. In particular I'm
wondering about the predictability impact: would developers expect link
registration to succeed/fail depending on whether the requesting worker has a
client? Anyway since this is an origin trial, it should be OK to experiment and
see. I'll comment on the bug too.

https://codereview.chromium.org/2512103003/diff/1/content/browser/service_wor...
File content/browser/service_worker/link_header_support.cc (right):

https://codereview.chromium.org/2512103003/diff/1/content/browser/service_wor...
content/browser/service_worker/link_header_support.cc:72: // controlling at
least one client.
nit: maybe it'd help the reader to add "To prevent a service worker from
spawning new service workers in the background" as in the CL description.

https://codereview.chromium.org/2512103003/diff/1/content/browser/service_wor...
File content/browser/service_worker/link_header_support_unittest.cc (right):

https://codereview.chromium.org/2512103003/diff/1/content/browser/service_wor...
content/browser/service_worker/link_header_support_unittest.cc:86: int
render_process_id() const { return helper_->mock_render_process_id(); }
nit: If we introduce this, there's several helper_->mock_render_process_id()
calls that can be replaced with it.

[Marijn Kruisselbrink, 2016-11-21 04:54:33]

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

[commit-bot: I haz the power, 2016-11-21 04:54:47]

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

[Marijn Kruisselbrink, 2016-11-21 05:10:57]

On 2016/11/21 at 04:13:42, falken wrote:
> lgtm to land this but I think the discussion is still ongoing. In particular
I'm wondering about the predictability impact: would developers expect link
registration to succeed/fail depending on whether the requesting worker has a
client? Anyway since this is an origin trial, it should be OK to experiment and
see. I'll comment on the bug too.

I agree that maybe this won't be the final state of this feature. But I don't
think it's that surprising either. I'll definitely have to make sure that the
origin trial questionnaire includes questions about expected behaviour, and what
functionality (if any) people actually care about here.

https://codereview.chromium.org/2512103003/diff/1/content/browser/service_wor...
File content/browser/service_worker/link_header_support.cc (right):

https://codereview.chromium.org/2512103003/diff/1/content/browser/service_wor...
content/browser/service_worker/link_header_support.cc:72: // controlling at
least one client.
On 2016/11/21 at 04:13:42, falken (slow until next week) wrote:
> nit: maybe it'd help the reader to add "To prevent a service worker from
spawning new service workers in the background" as in the CL description.

Done

https://codereview.chromium.org/2512103003/diff/1/content/browser/service_wor...
File content/browser/service_worker/link_header_support_unittest.cc (right):

https://codereview.chromium.org/2512103003/diff/1/content/browser/service_wor...
content/browser/service_worker/link_header_support_unittest.cc:86: int
render_process_id() const { return helper_->mock_render_process_id(); }
On 2016/11/21 at 04:13:42, falken (slow until next week) wrote:
> nit: If we introduce this, there's several helper_->mock_render_process_id()
calls that can be replaced with it.

Done

[Marijn Kruisselbrink, 2016-11-21 05:11:43]

The CQ bit was unchecked by mek@chromium.org

[Marijn Kruisselbrink, 2016-11-21 05:11:47]

The CQ bit was checked by mek@chromium.org

[Marijn Kruisselbrink, 2016-11-21 05:11:48]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org
Link to the patchset: https://codereview.chromium.org/2512103003/#ps20001
(title: "address comments")

[commit-bot: I haz the power, 2016-11-21 05:12:00]

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-11-21 06:44:20]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1479705107865990,
"parent_rev": ["802feb1c54da97d2a4c478c8b8670c60149197ba", null], "commit_rev":
["6a43c5707dbf3493029410b3b337fd1797f99b8c", null]}

[commit-bot: I haz the power, 2016-11-21 06:44:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-21 06:47:11]

Description was changed from

==========
Limit Link header based SW installations for fetches made by SW.

To prevent a service worker from spawning new service workers in the
background, only process link headers on requests made by service
workers if the service worker is controlling at least one client.

BUG=662443
==========

to

==========
Limit Link header based SW installations for fetches made by SW.

To prevent a service worker from spawning new service workers in the
background, only process link headers on requests made by service
workers if the service worker is controlling at least one client.

BUG=662443

Committed: https://crrev.com/e8313bb499c439b0a5091225faf97b06f4cf1182
Cr-Commit-Position: refs/heads/master@{#433489}
==========

[commit-bot: I haz the power, 2016-11-21 06:47:12]

Patchset 2 (id:??) landed as
https://crrev.com/e8313bb499c439b0a5091225faf97b06f4cf1182
Cr-Commit-Position: refs/heads/master@{#433489}