PlzNavigate: Make ServiceWorker work with PlzNavigate.

PlzNavigate: Make ServiceWorker work with PlzNavigate.

This CL makes the ServiceWorker architecture work with PlzNavigate enabled.
This is done by allowing the browser to pre-create a ServiceWorkerProviderHost
during navigations if needed. If the navigation is successful, the renderer
chosen to commit the navigation will create a matching
ServiceWorkerNetworkProvider. If no ServiceWorkerProviderHost was pre-created,
the renderer creates a ServiceWorkerNetworkProvider in the traditional way.

This is a continuation of https://codereview.chromium.org/1294243004/ by
fdegans@ following his departure from the team.

BUG=440463
Committed: https://crrev.com/2a7a25b565fd4b702bb12ce40f16284d965a11ad
Cr-Commit-Position: refs/heads/master@{#356310}

[clamy, 2015-10-14 13:48:36]

clamy@chromium.org changed reviewers:
+ kinuko@chromium.org, michaeln@chromium.org, nasko@chromium.org

[clamy, 2015-10-14 14:08:08]

@nasko, michaeln, kinuko: PTAL

This is a continuation of fdegans@ patch from
https://codereview.chromium.org/1294243004/.

I tried the latest patchset from that issue, and it had a design flaw that
caused ~400 content_browsertests to fail with PlzNavigate enabled. The issue is
that the UI thread think we always create a SWProviderHost for navigations, but
this is not always the case in the IO thread (eg we don't for file urls). When
the renderer later tries to create a ServiceWorkerNetworkProvider with matching
id, it gets killed because no SWProviderHost is found.

So I changed the design to take that into account:
1) We now have 2 parameters in RequestNavigationParams, whether the renderer
should create a ServiceWorkerNetworkProvider, and an eventual id if a
SWProviderHost was created by the browser. The renderer always create a
ServiceWorkerNetworkProvider if the first parameter is true. Its id is the one
created by the browser, if a SWProviderHost was created. Otherwise it is a
renderer created id as usual.
2) The ownership of the pre-created host has changed. We now have a UI thread
class and its Io class to handle it (SWNavigationHandle and
SWNavigationHandleCore). SWNavigationHandle is also used to get the
SWProviderHost id from the ui thread. This value is only set after a
SWProviderHost was actually created.
3) We still have a map of SWNavigationHandleCores in the SWContextCore. It does
not own them, but keeps a reference to them so that we can find the right one
when a renderer has created a matching SWNetworkProvider.

Note: when we agree on the design, I will try to break this patch in smaller
chunks and see about adding more unit tests.

[clamy, 2015-10-19 12:11:35]

@michealn, kinuko: friendly ping :)

[kinuko, 2015-10-20 14:46:07]

Sorry for very slow review, it took some time for me to take a look.

I don't feel I could digest all of these well but this looks really good to me
overall.  Made some comments but mostly nits.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:767: }
Since this logic has bloated quite a bit I feel it'd be probably better to
factor out the SW-specific part into a method in SW-specific file/class.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_navigation_handle.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle.cc:31:
base::Unretained(core_)));
nit: could this use BrowserThread::DeleteSoon() and some of the code in Destroy
be in the dtor?

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_navigation_handle_core.cc
(right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle_core.cc:43: if
(!context_wrapper_->context())
could this happen? looks like this could be DCHECK

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle_core.cc:55:
&ServiceWorkerNavigationHandle::DidCreateServiceWorkerProviderHost,
This part made me feel a bit unsure but looks like this should always reach
before OnResponseStarted

[michaeln, 2015-10-20 22:12:53]

lgtm, thnx for picking it up!

i have a question about potential race between the id generataion and commit
message being sent? assuming that race can't be lost, lgtm... if you do need to
change it i'll take another look. also, we should add some tests

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_request.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_request.cc:277:
(frame_tree_node_->current_replication_state().sandbox_flags &
i'm wondering about potential errors stemming from the sandbox_flags in the
renderer and browser not matching or the flag changing between when we're
sampleing it (here vs in the ctor)

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:767: }
On 2015/10/20 14:46:07, kinuko wrote:
> Since this logic has bloated quite a bit I feel it'd be probably better to
> factor out the SW-specific part into a method in SW-specific file/class.

we're in a sw specific class, do you mean to move it to a different class?

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_navigation_handle.h (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle.h:18: // on the
IO thread, the ServiceWorkerNavigationHandleCore.
thnx for the nice doc comments!

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle.h:36: //   id
was updated.
Q: Is it possible for the navigation to commit on the UI thread prior to receipt
of the precreated host id? Is there a race there or does something gaurantee
this id will make it to the UI thread prior to commit msg being sent?

If that is a potential problem, we could coin the id on the UI thread.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_navigation_handle_core.cc
(right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle_core.cc:55:
&ServiceWorkerNavigationHandle::DidCreateServiceWorkerProviderHost,
On 2015/10/20 14:46:07, kinuko wrote:
> This part made me feel a bit unsure but looks like this should always reach
> before OnResponseStarted

What guarantees that it does?

[nasko, 2015-10-20 23:07:42]

Reviewed some of the CL, but not the full thing.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_handle_impl.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_handle_impl.cc:65: }
Why put this code here instead of NavigationRequest? Logically it seems more
related to have the navigation request manage the SW related objects, not the
handle.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_request.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_request.cc:277:
(frame_tree_node_->current_replication_state().sandbox_flags &
On 2015/10/20 22:12:53, michaeln wrote:
> i'm wondering about potential errors stemming from the sandbox_flags in the
> renderer and browser not matching or the flag changing between when we're
> sampleing it (here vs in the ctor)

There is important behavior here. Sandbox flags updating works by updating
sandbox_flags, but keeping effective_sandbox_flags around. Security checks
usually need to be performed against the effective_sandbox_flags. That said, I
can't find where any spec talks about the interaction of the allow-same-origin
flag and ServiceWorker. Can you point me to that?

[clamy, 2015-10-21 16:33:48]

Thanks! I'll be adding tests in the next patch set.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_handle_impl.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_handle_impl.cc:65: }
On 2015/10/20 23:07:42, nasko (slow to review) wrote:
> Why put this code here instead of NavigationRequest? Logically it seems more
> related to have the navigation request manage the SW related objects, not the
> handle.

We need the SWHandle to persist until navigation commit. The
NavigationHandleImpl has that property, unlike the NavigationRequest which is
destroyed when the navigation is ready to commit.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_request.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_request.cc:277:
(frame_tree_node_->current_replication_state().sandbox_flags &
On 2015/10/20 23:07:42, nasko (slow to review) wrote:
> On 2015/10/20 22:12:53, michaeln wrote:
> > i'm wondering about potential errors stemming from the sandbox_flags in the
> > renderer and browser not matching or the flag changing between when we're
> > sampleing it (here vs in the ctor)
> 
> There is important behavior here. Sandbox flags updating works by updating
> sandbox_flags, but keeping effective_sandbox_flags around. Security checks
> usually need to be performed against the effective_sandbox_flags. That said, I
> can't find where any spec talks about the interaction of the allow-same-origin
> flag and ServiceWorker. Can you point me to that?

So should I change to checking against effective_sandbox_flags?

For the effects of it changing between when we're sampleing it:
- allowed to not allowed: we pre-created a SWProviderHost, but will not create
the corresponding renderer object. The SWProviderHost will be destroyed when the
navigation commits.
- not to allowed to allowed: we did not pre-create a SWProviderHost for the
navigation. However, we will create a SWNetworkProvider in the renderer when the
navigation commits.

Is that acceptable behavior for ServiceWorker?

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_navigation_handle.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle.cc:31:
base::Unretained(core_)));
On 2015/10/20 14:46:07, kinuko wrote:
> nit: could this use BrowserThread::DeleteSoon() and some of the code in
Destroy
> be in the dtor?

Done.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_navigation_handle.h (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle.h:36: //   id
was updated.
On 2015/10/20 22:12:53, michaeln wrote:
> Q: Is it possible for the navigation to commit on the UI thread prior to
receipt
> of the precreated host id? Is there a race there or does something gaurantee
> this id will make it to the UI thread prior to commit msg being sent?
> 
> If that is a potential problem, we could coin the id on the UI thread.

No. The task will be posted when we create the ServiceWorkerResourceHandler, so
before the URLRequest was started. Therefore it cannot have committed yet.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_navigation_handle_core.cc
(right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle_core.cc:43: if
(!context_wrapper_->context())
On 2015/10/20 14:46:07, kinuko wrote:
> could this happen? looks like this could be DCHECK

Done.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle_core.cc:55:
&ServiceWorkerNavigationHandle::DidCreateServiceWorkerProviderHost,
On 2015/10/20 22:12:53, michaeln wrote:
> On 2015/10/20 14:46:07, kinuko wrote:
> > This part made me feel a bit unsure but looks like this should always reach
> > before OnResponseStarted
> 
> What guarantees that it does?

We call that before we start the URLRequest, so the response cannot have started
yet.

[michaeln, 2015-10-21 20:07:29]

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_handle_impl.h (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_handle_impl.h:51: // transferred to the
RenderFrameHost in which the navigation will commit.
Also extending this comment to describe when it's deleted in the expected case
might be nice.

Is there a test to see is this lifetime guarantee is provided?

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_handle_impl.h:179: // corresponding
ServiceWorkerNetworkProvider is created in the renderer.
Maybe include in the doc comment that this needs to survive until the renderer
commits the navigation.

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_navigation_handle_core.cc
(right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_navigation_handle_core.cc:55:
&ServiceWorkerNavigationHandle::DidCreateServiceWorkerProviderHost,
On 2015/10/21 16:33:48, clamy wrote:
> On 2015/10/20 22:12:53, michaeln wrote:
> > On 2015/10/20 14:46:07, kinuko wrote:
> > > This part made me feel a bit unsure but looks like this should always
reach
> > > before OnResponseStarted
> > 
> > What guarantees that it does?
> 
> We call that before we start the URLRequest, so the response cannot have
started
> yet.

thnx, got it, NavigationResourceHandler::OnResponseStarted can't possibly have
been called yet, and that io thread method is what posts the notification to the
ui thread

[nasko, 2015-10-21 22:28:08]

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_request.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_request.cc:277:
(frame_tree_node_->current_replication_state().sandbox_flags &
On 2015/10/21 16:33:48, clamy wrote:
> On 2015/10/20 23:07:42, nasko (slow to review) wrote:
> > On 2015/10/20 22:12:53, michaeln wrote:
> > > i'm wondering about potential errors stemming from the sandbox_flags in
the
> > > renderer and browser not matching or the flag changing between when we're
> > > sampleing it (here vs in the ctor)
> > 
> > There is important behavior here. Sandbox flags updating works by updating
> > sandbox_flags, but keeping effective_sandbox_flags around. Security checks
> > usually need to be performed against the effective_sandbox_flags. That said,
I
> > can't find where any spec talks about the interaction of the
allow-same-origin
> > flag and ServiceWorker. Can you point me to that?
> 
> So should I change to checking against effective_sandbox_flags?
> 
> For the effects of it changing between when we're sampleing it:
> - allowed to not allowed: we pre-created a SWProviderHost, but will not create
> the corresponding renderer object. The SWProviderHost will be destroyed when
the
> navigation commits.
> - not to allowed to allowed: we did not pre-create a SWProviderHost for the
> navigation. However, we will create a SWNetworkProvider in the renderer when
the
> navigation commits.
> 
> Is that acceptable behavior for ServiceWorker?

I can't say for certain which one should be used, since I don't know what the
spec requires. michaeln@/kinuko@, can you help point us to the spec that
explains how sandbox flags and SW interact?

[kinuko, 2015-10-22 12:16:02]

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
File content/browser/frame_host/navigation_request.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
content/browser/frame_host/navigation_request.cc:277:
(frame_tree_node_->current_replication_state().sandbox_flags &
On 2015/10/21 16:33:48, clamy wrote:
> On 2015/10/20 23:07:42, nasko (slow to review) wrote:
> > On 2015/10/20 22:12:53, michaeln wrote:
> > > i'm wondering about potential errors stemming from the sandbox_flags in
the
> > > renderer and browser not matching or the flag changing between when we're
> > > sampleing it (here vs in the ctor)
> > 
> > There is important behavior here. Sandbox flags updating works by updating
> > sandbox_flags, but keeping effective_sandbox_flags around. Security checks
> > usually need to be performed against the effective_sandbox_flags. That said,
I
> > can't find where any spec talks about the interaction of the
allow-same-origin
> > flag and ServiceWorker. Can you point me to that?
> 
> So should I change to checking against effective_sandbox_flags?
> 
> For the effects of it changing between when we're sampleing it:
> - allowed to not allowed: we pre-created a SWProviderHost, but will not create
> the corresponding renderer object. The SWProviderHost will be destroyed when
the
> navigation commits.
> - not to allowed to allowed: we did not pre-create a SWProviderHost for the
> navigation. However, we will create a SWNetworkProvider in the renderer when
the
> navigation commits.
> 
> Is that acceptable behavior for ServiceWorker?

In the latter case the first main resource load may not be controlled but
subsequent resource loads could be controlled?  That might feel slightly weird,
though I don't strongly feel we should make it behave differently. (The spec
text for dynamically changing sandbox flag seems to be a little unclear too)

It'd be still probably worth noting this behavior in a comment?

[nasko, 2015-10-22 22:40:25]

On 2015/10/22 12:16:02, kinuko wrote:
>
https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
> File content/browser/frame_host/navigation_request.cc (right):
> 
>
https://codereview.chromium.org/1399363004/diff/20001/content/browser/frame_h...
> content/browser/frame_host/navigation_request.cc:277:
> (frame_tree_node_->current_replication_state().sandbox_flags &
> On 2015/10/21 16:33:48, clamy wrote:
> > On 2015/10/20 23:07:42, nasko (slow to review) wrote:
> > > On 2015/10/20 22:12:53, michaeln wrote:
> > > > i'm wondering about potential errors stemming from the sandbox_flags in
> the
> > > > renderer and browser not matching or the flag changing between when
we're
> > > > sampleing it (here vs in the ctor)
> > > 
> > > There is important behavior here. Sandbox flags updating works by updating
> > > sandbox_flags, but keeping effective_sandbox_flags around. Security checks
> > > usually need to be performed against the effective_sandbox_flags. That
said,
> I
> > > can't find where any spec talks about the interaction of the
> allow-same-origin
> > > flag and ServiceWorker. Can you point me to that?
> > 
> > So should I change to checking against effective_sandbox_flags?
> > 
> > For the effects of it changing between when we're sampleing it:
> > - allowed to not allowed: we pre-created a SWProviderHost, but will not
create
> > the corresponding renderer object. The SWProviderHost will be destroyed when
> the
> > navigation commits.
> > - not to allowed to allowed: we did not pre-create a SWProviderHost for the
> > navigation. However, we will create a SWNetworkProvider in the renderer when
> the
> > navigation commits.
> > 
> > Is that acceptable behavior for ServiceWorker?
> 
> In the latter case the first main resource load may not be controlled but
> subsequent resource loads could be controlled?  That might feel slightly
weird,
> though I don't strongly feel we should make it behave differently. (The spec
> text for dynamically changing sandbox flag seems to be a little unclear too)
> 
> It'd be still probably worth noting this behavior in a comment?

I just had a chat with slightlyoff@ to clarify intended behavior. We should be
using the latest known value, so in this case *not* the effective flags, but
what we get from from sandbox_flags().

Also, the clear behavior is that when the allow-same-origin flag is present, SW
is allowed to handle the request. In terms of state transitions:
* changing from not allow-same-origin to allow-same-origin: SW is allowed to
handle.
* changing from allow-same-origin to not allow-same-origin: SW is not allowed

He will file a spec bug and ensure the spec is fixed to reflect this behavior.

[nasko, 2015-10-22 22:41:17]

With the sandboxed flags and SW interaction issue resolved, this now LGTM.

[kinuko, 2015-10-23 01:02:28]

Thanks for checking.  What is/was not entirely clear to me is how immediately
sandbox flag change should be reflected, because in regular iframe load spec
example says it's not immediately reflected, and not allowed -> allowed
transition could be made at a very racy timing in terms of loading.  I will
comment on github once it's filed.

[kinuko, 2015-10-23 02:37:45]

lgtm

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1399363004/diff/20001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:767: }
On 2015/10/20 22:12:53, michaeln wrote:
> On 2015/10/20 14:46:07, kinuko wrote:
> > Since this logic has bloated quite a bit I feel it'd be probably better to
> > factor out the SW-specific part into a method in SW-specific file/class.
> 
> we're in a sw specific class, do you mean to move it to a different class?

Sorry, I was going to put this comment for the changes in render_frame_impl..

https://codereview.chromium.org/1399363004/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_context_core.h (right):

https://codereview.chromium.org/1399363004/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_context_core.h:233: }
(Personally I prefer providing utility methods over exposing mutable map as is,
but it'd probably bloat the code size...)

mutable_navigation_handle_cores() maybe?

https://codereview.chromium.org/1399363004/diff/60001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/1399363004/diff/60001/content/renderer/render...
content/renderer/render_frame_impl.cc:2645: }
Could this part be maybe factored out as a utility method preferably in
SW-specific class/file?

[clamy, 2015-10-27 14:46:13]

Thanks!

https://codereview.chromium.org/1399363004/diff/60001/content/browser/service...
File content/browser/service_worker/service_worker_context_core.h (right):

https://codereview.chromium.org/1399363004/diff/60001/content/browser/service...
content/browser/service_worker/service_worker_context_core.h:233: }
On 2015/10/23 02:37:45, kinuko wrote:
> (Personally I prefer providing utility methods over exposing mutable map as
is,
> but it'd probably bloat the code size...)
> 
> mutable_navigation_handle_cores() maybe?

I changed to utility methods. The code is slightly longer but more readable for
the callers.

https://codereview.chromium.org/1399363004/diff/60001/content/renderer/render...
File content/renderer/render_frame_impl.cc (right):

https://codereview.chromium.org/1399363004/diff/60001/content/renderer/render...
content/renderer/render_frame_impl.cc:2645: }
On 2015/10/23 02:37:45, kinuko wrote:
> Could this part be maybe factored out as a utility method preferably in
> SW-specific class/file?

Done. This is now a static method in ServiceWorkerNetworkProvider.

[clamy, 2015-10-27 14:46:19]

The CQ bit was checked by clamy@chromium.org

[clamy, 2015-10-27 14:46:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaeln@chromium.org, kinuko@chromium.org, nasko@chromium.org
Link to the patchset: https://codereview.chromium.org/1399363004/#ps100001
(title: "Addressed Kinuko's comments")

[commit-bot: I haz the power, 2015-10-27 14:46:42]

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

[commit-bot: I haz the power, 2015-10-27 16:20:57]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-10-27 16:22:25]

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

[danakj, 2015-10-27 18:20:26]

https://codereview.chromium.org/1399363004/diff/100001/content/browser/servic...
File content/browser/service_worker/service_worker_provider_host.cc (right):

https://codereview.chromium.org/1399363004/diff/100001/content/browser/servic...
content/browser/service_worker/service_worker_provider_host.cc:82:
CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
Hi, I randomly came across this. Please use DCHECK instead of CHECK whenever it
is possible. CHECK adds strings and code to the binary we ship to our users.

Generally you should only use CHECK while fishing out stack traces for a bug,
and switch back to DCHECK after.

https://www.chromium.org/developers/coding-style#TOC-CHECK-DCHECK-and-NOTREAC...

[danakj, 2015-10-27 18:20:37]

Description was changed from

==========
PlzNavigate: Make ServiceWorker work with PlzNavigate.

This CL makes the ServiceWorker architecture work with PlzNavigate enabled.
This is done by allowing the browser to pre-create a ServiceWorkerProviderHost
during navigations if needed. If the navigation is successful, the renderer
chosen to commit the navigation will create a matching
ServiceWorkerNetworkProvider. If no ServiceWorkerProviderHost was pre-created,
the renderer creates a ServiceWorkerNetworkProvider in the traditional way.

This is a continuation of https://codereview.chromium.org/1294243004/ by
fdegans@ following his departure from the team.

BUG=440463

Committed: https://crrev.com/2a7a25b565fd4b702bb12ce40f16284d965a11ad
Cr-Commit-Position: refs/heads/master@{#356310}
==========

to

==========
PlzNavigate: Make ServiceWorker work with PlzNavigate.

This CL makes the ServiceWorker architecture work with PlzNavigate enabled.
This is done by allowing the browser to pre-create a ServiceWorkerProviderHost
during navigations if needed. If the navigation is successful, the renderer
chosen to commit the navigation will create a matching
ServiceWorkerNetworkProvider. If no ServiceWorkerProviderHost was pre-created,
the renderer creates a ServiceWorkerNetworkProvider in the traditional way.

This is a continuation of https://codereview.chromium.org/1294243004/ by
fdegans@ following his departure from the team.

BUG=440463

Committed: https://crrev.com/2a7a25b565fd4b702bb12ce40f16284d965a11ad
Cr-Commit-Position: refs/heads/master@{#356310}
==========