ServiceWorker: Implement ServiceWorkerRegistration.update() (2)

ServiceWorker: Implement ServiceWorkerRegistration.update() (2)

Spec: https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#service-worker-registration-update

(1) Blink: https://codereview.chromium.org/1177913002/
(2) Chromium: THIS PATCH
(3) Blink: https://codereview.chromium.org/1177563005

BUG=450507
TEST=will be added by (3)

Committed: https://crrev.com/f1f9ab588bd0efb7c1eff9e1bebe69a00d50c92c
Cr-Commit-Position: refs/heads/master@{#334394}

[nhiroki, 2015-06-10 00:16:43]

Patchset #1 (id:1) has been deleted

[nhiroki, 2015-06-10 03:39:26]

Patchset #1 (id:20001) has been deleted

[nhiroki, 2015-06-10 03:40:37]

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

[nhiroki, 2015-06-10 03:40:38]

PTAL

[falken, 2015-06-10 04:04:40]

lgtm

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

https://codereview.chromium.org/1175823002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:710: // This
can happen if update() is called during the script evaluation. Abort
nit: during initial script evaluation

https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
File content/child/service_worker/service_worker_dispatcher.cc (right):

https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
content/child/service_worker/service_worker_dispatcher.cc:133: return;
Hmm, should/can we surface this error somehow? Like as a console error?

[falken, 2015-06-10 04:08:49]

Oops forgot one thing.

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

https://codereview.chromium.org/1175823002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:715: false /*
force_bypass_cache */);
Ah this should be true, as per spec's non-normative note: "update() pings the
server for an updated version of this script without consulting caches. This is
conceptually the same operation that UA does maximum once per every 24 hours."

It'd be nice to have test coverage for busting the cache on update() too.

[nhiroki, 2015-06-10 06:36:32]

Thank you for reviewing! Updated.

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

https://codereview.chromium.org/1175823002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:710: // This
can happen if update() is called during the script evaluation. Abort
On 2015/06/10 04:04:40, falken wrote:
> nit: during initial script evaluation

Done.

https://codereview.chromium.org/1175823002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_dispatcher_host.cc:715: false /*
force_bypass_cache */);
On 2015/06/10 04:08:48, falken wrote:
> Ah this should be true, as per spec's non-normative note: "update() pings the
> server for an updated version of this script without consulting caches. This
is
> conceptually the same operation that UA does maximum once per every 24 hours."

Good catch! Fixed.

> It'd be nice to have test coverage for busting the cache on update() too.

serviceworker/update.html to be added by the CL(3) will cover the case.

https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
File content/child/service_worker/service_worker_dispatcher.cc (right):

https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
content/child/service_worker/service_worker_dispatcher.cc:133: return;
On 2015/06/10 04:04:40, falken wrote:
> Hmm, should/can we surface this error somehow? Like as a console error?

I'm not really sure how to print this and other errors emitted in the
browser-side in the console, so let me leave a TODO comment for now.

[nhiroki, 2015-06-10 06:39:36]

nhiroki@chromium.org changed reviewers:
+ asvitkine@chromium.org, nasko@chromium.org

[nhiroki, 2015-06-10 06:39:36]

+nasko@ for bad_message.h and service_worker_messages.h
+asvitkine@ for histograms.xml

PTAL, thanks!

[kinuko, 2015-06-10 06:47:17]

kinuko@chromium.org changed reviewers:
+ kinuko@chromium.org

[kinuko, 2015-06-10 06:47:17]

https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
File content/child/service_worker/service_worker_dispatcher.cc (right):

https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
content/child/service_worker/service_worker_dispatcher.cc:131: const GURL&
pattern) {
I'm curious why we supply pattern / scopeURL in the renderer process?

[nhiroki, 2015-06-10 06:55:26]

https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
File content/child/service_worker/service_worker_dispatcher.cc (right):

https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
content/child/service_worker/service_worker_dispatcher.cc:131: const GURL&
pattern) {
On 2015/06/10 06:47:17, kinuko wrote:
> I'm curious why we supply pattern / scopeURL in the renderer process?

It's used for finding a registration. Is there any alternative to retrieve it?

[kinuko, 2015-06-10 07:01:41]

On 2015/06/10 06:55:26, nhiroki wrote:
>
https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
> File content/child/service_worker/service_worker_dispatcher.cc (right):
> 
>
https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
> content/child/service_worker/service_worker_dispatcher.cc:131: const GURL&
> pattern) {
> On 2015/06/10 06:47:17, kinuko wrote:
> > I'm curious why we supply pattern / scopeURL in the renderer process?
> 
> It's used for finding a registration. Is there any alternative to retrieve it?

Hmm, I thought we could be calling this via, say,
WebServiceWorkerRegistrationImpl as it's implementing
ServiceWorkerRegistration#update() method, where pattern should be already
known.  Also is there any value we're checking max URL char value here?  It
should be the valid URL here, otherwise the renderer could be compromised and
checking here has no value?

[nhiroki, 2015-06-10 07:28:35]

On 2015/06/10 07:01:41, kinuko wrote:
> On 2015/06/10 06:55:26, nhiroki wrote:
> >
>
https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
> > File content/child/service_worker/service_worker_dispatcher.cc (right):
> > 
> >
>
https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
> > content/child/service_worker/service_worker_dispatcher.cc:131: const GURL&
> > pattern) {
> > On 2015/06/10 06:47:17, kinuko wrote:
> > > I'm curious why we supply pattern / scopeURL in the renderer process?
> > 
> > It's used for finding a registration. Is there any alternative to retrieve
it?
> 
> Hmm, I thought we could be calling this via, say,
> WebServiceWorkerRegistrationImpl as it's implementing
> ServiceWorkerRegistration#update() method, where pattern should be already
> known.

Yeah, it seems really straightforward. Actually, I tried it for unregister()
before. IIRC, provider_id handling was a pain point. There was no way to lookup
provider_id by reg_handle like "ServiceWorkerDispatcher::worker_to_provider_".

> Also is there any value we're checking max URL char value here?  It
> should be the valid URL here, otherwise the renderer could be compromised and
> checking here has no value?

Agree. Register/Unregister/GetRegistration do similar checks for document_url
and scope. Probably we are able to move them into the browser-side if necessary.

[kinuko, 2015-06-10 08:44:01]

On 2015/06/10 07:28:35, nhiroki wrote:
> On 2015/06/10 07:01:41, kinuko wrote:
> > On 2015/06/10 06:55:26, nhiroki wrote:
> > >
> >
>
https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
> > > File content/child/service_worker/service_worker_dispatcher.cc (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1175823002/diff/40001/content/child/service_w...
> > > content/child/service_worker/service_worker_dispatcher.cc:131: const GURL&
> > > pattern) {
> > > On 2015/06/10 06:47:17, kinuko wrote:
> > > > I'm curious why we supply pattern / scopeURL in the renderer process?
> > > 
> > > It's used for finding a registration. Is there any alternative to retrieve
> it?
> > 
> > Hmm, I thought we could be calling this via, say,
> > WebServiceWorkerRegistrationImpl as it's implementing
> > ServiceWorkerRegistration#update() method, where pattern should be already
> > known.
> 
> Yeah, it seems really straightforward. Actually, I tried it for unregister()
> before. IIRC, provider_id handling was a pain point. There was no way to
lookup
> provider_id by reg_handle like "ServiceWorkerDispatcher::worker_to_provider_".

Could WebServiceWorkerRegistration.update() or unregister() take
WebServiceWorkerProvider, since ServiceWorkerRegistration holds it?

> > Also is there any value we're checking max URL char value here?  It
> > should be the valid URL here, otherwise the renderer could be compromised
and
> > checking here has no value?
> 
> Agree. Register/Unregister/GetRegistration do similar checks for document_url
> and scope. Probably we are able to move them into the browser-side if
necessary.

For Register or GetRegistration I can understand because the pattern's given by
a user script (though the check seems pretty uncommon).

[nhiroki, 2015-06-10 13:16:36]

nasko@, asvitkine@: sorry, please hold on. I'll remake this CL to incorporate
kinuko@'s comments tomorrow.

[nhiroki, 2015-06-10 13:16:54]

nhiroki@chromium.org changed reviewers:
- asvitkine@chromium.org, nasko@chromium.org

[kinuko, 2015-06-10 13:27:03]

On 2015/06/10 13:16:36, nhiroki wrote:
> nasko@, asvitkine@: sorry, please hold on. I'll remake this CL to incorporate
> kinuko@'s comments tomorrow.

(I'm ok for going with this direction for now and clean up later if you want,
btw)

[michaeln, 2015-06-10 19:43:19]

drive by: the object being operated on is the registation, maybe refer to it by
its unique id

 122 IPC_MESSAGE_CONTROL2(ServiceWorkerHostMsg_UpdateServiceWorker,
 123                      int /* provider_id */,
 124                      int64 /* registration_id */)

so we have what to operate on and who is trying to do that operation

[nhiroki, 2015-06-11 00:01:07]

Patchset #3 (id:80001) has been deleted

[nhiroki, 2015-06-11 00:04:57]

kinuko@, michaeln@: Thank you for your comments! Updated the CL to route a
request through WebServiceWorkerRegistrationImpl. Could you take another look?
If this is OK, I'll re-land the first Blink CL.

[nhiroki, 2015-06-11 00:19:56]

On 2015/06/11 00:04:57, nhiroki wrote:
> kinuko@, michaeln@: Thank you for your comments! Updated the CL to route a
> request through WebServiceWorkerRegistrationImpl. Could you take another look?
> If this is OK, I'll re-land the first Blink CL.

retry patch: https://codereview.chromium.org/1177913002/

[kinuko, 2015-06-11 03:50:36]

Looking nicer to me.

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:383: if
(!OriginCanAccessServiceWorkers(provider_host->document_url())) {
This check's not necessary now?

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:392: }
This check's not necessary now?

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:701: }
It looks we don't check if the registration's for the given origin when we
return from LiveRegistrations in FindRegistrationForId, is that right?  Can we
check the origin in that case too?

https://codereview.chromium.org/1175823002/diff/120001/content/child/service_...
File content/child/service_worker/service_worker_dispatcher.h (right):

https://codereview.chromium.org/1175823002/diff/120001/content/child/service_...
content/child/service_worker/service_worker_dispatcher.h:71: // Corresponds to
ServiceWorkerRegistration.update()
nit: It's not only in your change but it's good to have period at the end of
comments

[nhiroki, 2015-06-11 05:44:58]

Patchset #6 (id:160001) has been deleted

[nhiroki, 2015-06-11 05:48:34]

Thanks! Updated.

falken@: I have a question about AllowServiceWorker. Can you take a look at a
comment on
https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
?

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:383: if
(!OriginCanAccessServiceWorkers(provider_host->document_url())) {
On 2015/06/11 03:50:36, kinuko wrote:
> This check's not necessary now?

Removed.

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:392: }
On 2015/06/11 03:50:36, kinuko wrote:
> This check's not necessary now?

Hmm... we may have to respect Cookie setting changed during a session...??

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:525:
provider_host->document_url(), provider_host->topmost_frame_url(),
Question (maybe falken@ knows an answer?): According to a header comment on
AllowServiceWorker(), the first argument should be 'scope', but this is
'document_url'. Is this correct?

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:701: }
On 2015/06/11 03:50:36, kinuko wrote:
> It looks we don't check if the registration's for the given origin when we
> return from LiveRegistrations in FindRegistrationForId, is that right?  Can we
> check the origin in that case too?

Good point. Added an origin check here.

https://codereview.chromium.org/1175823002/diff/120001/content/child/service_...
File content/child/service_worker/service_worker_dispatcher.h (right):

https://codereview.chromium.org/1175823002/diff/120001/content/child/service_...
content/child/service_worker/service_worker_dispatcher.h:71: // Corresponds to
ServiceWorkerRegistration.update()
On 2015/06/11 03:50:36, kinuko wrote:
> nit: It's not only in your change but it's good to have period at the end of
> comments

Done.

[falken, 2015-06-11 06:26:55]

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:525:
provider_host->document_url(), provider_host->topmost_frame_url(),
On 2015/06/11 05:48:33, nhiroki wrote:
> Question (maybe falken@ knows an answer?): According to a header comment on
> AllowServiceWorker(), the first argument should be 'scope', but this is
> 'document_url'. Is this correct?

Mmm... good question. In practice, in the Chrome browser, they are equivalent,
Chrome's content setting policies are at the origin-level not path, if you trace
through AllowServiceWorker -> IsSettingCookieAllowed -> ... ->
ContentSettingsPattern::Match. But there could be embedders other than Chrome
that use the path. I don't know if it makes sense to worry about those as the
general web security/privacy model is considered per-origin.

I guess the question is, if SW is blocked for //a/foo*, should //a/foobar be
able to get/register/unregister at scope //a/fo? Probably not. But currently we
allow register/unregister and block get. That's kinda inconsistent.

On the other hand, if SW is blocked for //a/foo* should //a/f be able to
get/register/unregister at scope //a/foobar. I'd say definitely not. 

So maybe the right thing to do is AllowServiceWorker(scope, topmost_frame) &&
AllowServiceWorker(doc_url, topmost_frame) for get/register/unregister.

Everyone, WDYT? Maybe better to split this to another bug/codereview...

[kinuko, 2015-06-11 06:33:06]

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:392: }
On 2015/06/11 05:48:33, nhiroki wrote:
> On 2015/06/11 03:50:36, kinuko wrote:
> > This check's not necessary now?
> 
> Hmm... we may have to respect Cookie setting changed during a session...??

Um, wasn't really reading the code. Let's keep it. I'd stick to origin-based
check though.

https://codereview.chromium.org/1175823002/diff/180001/content/browser/servic...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1175823002/diff/180001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:391:
GetContext()->storage()->FindRegistrationForId(
If this comes from WebServiceWorkerRegistration the registration should be
always live (assuming that IPC reordering doesn't happen), is it true?  Do we
need to look up the storage?

https://codereview.chromium.org/1175823002/diff/180001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:767: if
(registration->pattern().GetOrigin() != document_url.GetOrigin()) {
I wonder if we should perform this check in FindRegistrationForId?  Currently it
checks origin in some cases, but not always, and documentation of the method's
unclear about that.

[michaeln, 2015-06-11 21:40:47]

michaeln@chromium.org changed reviewers:
+ michaeln@chromium.org

[michaeln, 2015-06-11 21:40:48]

https://codereview.chromium.org/1175823002/diff/180001/content/browser/servic...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1175823002/diff/180001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:391:
GetContext()->storage()->FindRegistrationForId(
On 2015/06/11 06:33:06, kinuko wrote:
> If this comes from WebServiceWorkerRegistration the registration should be
> always live (assuming that IPC reordering doesn't happen), is it true?  Do we
> need to look up the storage?

Yes, it should be valid to look this up in the live map. If its not in the map
or the origins don't match, i think that should count as a bad message.

Hooray for less and simpler code!

https://codereview.chromium.org/1175823002/diff/180001/content/child/service_...
File content/child/service_worker/web_service_worker_registration_impl.h
(right):

https://codereview.chromium.org/1175823002/diff/180001/content/child/service_...
content/child/service_worker/web_service_worker_registration_impl.h:64:
scoped_ptr<ServiceWorkerRegistrationHandleReference> handle_ref_;
Given non-hacked renderers, it should be safe to assume the reg is live when
handling any ipc we send from the renderer prior to this ref being deleted. If
that's not the case, then we have a handle/refcounting bug that should be fixed.

[nhiroki, 2015-06-12 08:47:41]

Patchset #7 (id:190001) has been deleted

[nhiroki, 2015-06-12 08:50:58]

Updated. Can you take another look?

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1175823002/diff/120001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:525:
provider_host->document_url(), provider_host->topmost_frame_url(),
On 2015/06/11 06:26:55, falken wrote:
> On 2015/06/11 05:48:33, nhiroki wrote:
> > Question (maybe falken@ knows an answer?): According to a header comment on
> > AllowServiceWorker(), the first argument should be 'scope', but this is
> > 'document_url'. Is this correct?
> 
> Mmm... good question. In practice, in the Chrome browser, they are equivalent,
> Chrome's content setting policies are at the origin-level not path, if you
trace
> through AllowServiceWorker -> IsSettingCookieAllowed -> ... ->
> ContentSettingsPattern::Match. But there could be embedders other than Chrome
> that use the path. I don't know if it makes sense to worry about those as the
> general web security/privacy model is considered per-origin.
> 
> I guess the question is, if SW is blocked for //a/foo*, should //a/foobar be
> able to get/register/unregister at scope //a/fo? Probably not. But currently
we
> allow register/unregister and block get. That's kinda inconsistent.
> 
> On the other hand, if SW is blocked for //a/foo* should //a/f be able to
> get/register/unregister at scope //a/foobar. I'd say definitely not. 
> 
> So maybe the right thing to do is AllowServiceWorker(scope, topmost_frame) &&
> AllowServiceWorker(doc_url, topmost_frame) for get/register/unregister.
> 
> Everyone, WDYT? Maybe better to split this to another bug/codereview...

Thank you for your reply. Probably I got the point of this... in my opinion, the
per-origin model seems sufficient and we might have to explicitly mention the
model on a header comment. I'll open another bug/codereview later.

https://codereview.chromium.org/1175823002/diff/180001/content/browser/servic...
File content/browser/service_worker/service_worker_dispatcher_host.cc (right):

https://codereview.chromium.org/1175823002/diff/180001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:391:
GetContext()->storage()->FindRegistrationForId(
On 2015/06/11 21:40:48, michaeln wrote:
> On 2015/06/11 06:33:06, kinuko wrote:
> > If this comes from WebServiceWorkerRegistration the registration should be
> > always live (assuming that IPC reordering doesn't happen), is it true?  Do
we
> > need to look up the storage?
> 
> Yes, it should be valid to look this up in the live map. If its not in the map
> or the origins don't match, i think that should count as a bad message.
> 
> Hooray for less and simpler code!

You're right. This registration should be in the live map. Removed storage
access :)

https://codereview.chromium.org/1175823002/diff/180001/content/browser/servic...
content/browser/service_worker/service_worker_dispatcher_host.cc:767: if
(registration->pattern().GetOrigin() != document_url.GetOrigin()) {
On 2015/06/11 06:33:06, kinuko wrote:
> I wonder if we should perform this check in FindRegistrationForId?  Currently
it
> checks origin in some cases, but not always, and documentation of the method's
> unclear about that.

Sounds reasonable. I'll make it in a separate CL (because FindRegistraionForId
call is gone from this CL).

https://codereview.chromium.org/1175823002/diff/180001/content/child/service_...
File content/child/service_worker/web_service_worker_registration_impl.h
(right):

https://codereview.chromium.org/1175823002/diff/180001/content/child/service_...
content/child/service_worker/web_service_worker_registration_impl.h:64:
scoped_ptr<ServiceWorkerRegistrationHandleReference> handle_ref_;
On 2015/06/11 21:40:48, michaeln wrote:
> Given non-hacked renderers, it should be safe to assume the reg is live when
> handling any ipc we send from the renderer prior to this ref being deleted. If
> that's not the case, then we have a handle/refcounting bug that should be
fixed.

Acknowledged.

[kinuko, 2015-06-12 09:21:16]

lgtm

[nhiroki, 2015-06-12 09:24:22]

nhiroki@chromium.org changed reviewers:
+ asvitkine@chromium.org, mkwst@chromium.org

[nhiroki, 2015-06-12 09:25:32]

+mkwst@ for service_worker_messages.h
+asvitkine@ for histograms.xml

PTAL, thanks!

[Mike West, 2015-06-12 13:02:07]

https://codereview.chromium.org/1175823002/diff/230001/content/common/service...
File content/common/service_worker/service_worker_messages.h (right):

https://codereview.chromium.org/1175823002/diff/230001/content/common/service...
content/common/service_worker/service_worker_messages.h:130: GURL /* scope */)
What does this change imply?

[nhiroki, 2015-06-12 14:59:50]

https://codereview.chromium.org/1175823002/diff/230001/content/common/service...
File content/common/service_worker/service_worker_messages.h (right):

https://codereview.chromium.org/1175823002/diff/230001/content/common/service...
content/common/service_worker/service_worker_messages.h:130: GURL /* scope */)
On 2015/06/12 13:02:06, Mike West wrote:
> What does this change imply?

Just a cleanup. In the early SW spec, “pattern” was commonly used, but “scope”
became a formal term. “pattern” should be gradually replaced.

Btw, I’m now making a separate CL to refactor UnregisterServiceWorker, so I
moved this cleanup into the CL.

[Mike West, 2015-06-13 08:07:02]

IPC LGTM, thanks for the explanation.

[Alexei Svitkine (slow), 2015-06-15 14:32:01]

lgtm

[nhiroki, 2015-06-15 14:43:04]

The CQ bit was checked by nhiroki@chromium.org

[nhiroki, 2015-06-15 14:43:04]

The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org,
kinuko@chromium.org, mkwst@chromium.org, asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/1175823002/#ps260001
(title: "rebase")

[commit-bot: I haz the power, 2015-06-15 14:43:24]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1175823002/260001

[commit-bot: I haz the power, 2015-06-15 16:23:00]

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

[commit-bot: I haz the power, 2015-06-15 16:23:01]

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

[nhiroki, 2015-06-15 16:23:57]

The CQ bit was checked by nhiroki@chromium.org

[commit-bot: I haz the power, 2015-06-15 16:24:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1175823002/260001

[commit-bot: I haz the power, 2015-06-15 16:58:42]

Committed patchset #10 (id:260001)

[commit-bot: I haz the power, 2015-06-15 16:59:47]

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