First step to refactor ServiceWorkerVersion to make event dispatching more modular.

First step to refactor ServiceWorkerVersion to make event dispatching more modular.

As described in https://docs.google.com/document/d/1k3xQuV2cQgygJXMpD4Jf77H88040Vcnixz-XW5o6GMM/edit?pli=1#heading=h.ysf7bzmgf12b,
this CL adds API to allow other code to inform SWVersion about events in
progress, and let SWVersion deal with timeouts and service worker lifetime
management in relation to these events.

On top of that this CL also adds a more generic mechanism for events that
use mojo services to connect to these mojo services (and have connections
to those services automatically close when the worker is stopped).

Finally this CL updates the navigator.connect call to use this new way of
dispatching events. Follow up CLs will update other mojo based events, and
will introduce the infrastructure to also move IPC based events out of SWVersion.

BUG=570820
Committed: https://crrev.com/24f20a4f6da8507e47ace8dd6a43e698d241e647
Cr-Commit-Position: refs/heads/master@{#368353}

[Marijn Kruisselbrink, 2015-12-08 20:05:24]

Patchset #1 (id:1) has been deleted

[Marijn Kruisselbrink, 2015-12-08 20:05:30]

Patchset #1 (id:20001) has been deleted

[Marijn Kruisselbrink, 2015-12-08 20:05:40]

Patchset #1 (id:40001) has been deleted

[Marijn Kruisselbrink, 2015-12-08 20:05:59]

Patchset #1 (id:60001) has been deleted

[Marijn Kruisselbrink, 2015-12-08 20:06:04]

Patchset #1 (id:80001) has been deleted

[Marijn Kruisselbrink, 2015-12-08 20:06:09]

Patchset #1 (id:100001) has been deleted

[Marijn Kruisselbrink, 2015-12-08 20:06:13]

Patchset #1 (id:120001) has been deleted

[Marijn Kruisselbrink, 2015-12-08 20:06:18]

Patchset #1 (id:140001) has been deleted

[Marijn Kruisselbrink, 2015-12-16 19:08:35]

Description was changed from

==========
WIP refactor ServiceWorkerVersion to make event dispatching more modular.

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

to

==========
WIP refactor ServiceWorkerVersion to make event dispatching more modular.
https://docs.google.com/document/d/1k3xQuV2cQgygJXMpD4Jf77H88040Vcnixz-XW5o6G...

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

[Marijn Kruisselbrink, 2015-12-17 22:34:41]

Description was changed from

==========
WIP refactor ServiceWorkerVersion to make event dispatching more modular.
https://docs.google.com/document/d/1k3xQuV2cQgygJXMpD4Jf77H88040Vcnixz-XW5o6G...

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

to

==========
WIP refactor ServiceWorkerVersion to make event dispatching more modular.
https://docs.google.com/document/d/1k3xQuV2cQgygJXMpD4Jf77H88040Vcnixz-XW5o6G...

BUG=570820
==========

[Marijn Kruisselbrink, 2015-12-17 22:38:40]

Description was changed from

==========
WIP refactor ServiceWorkerVersion to make event dispatching more modular.
https://docs.google.com/document/d/1k3xQuV2cQgygJXMpD4Jf77H88040Vcnixz-XW5o6G...

BUG=570820
==========

to

==========
First step to refactor ServiceWorkerVersion to make event dispatching more
modular.

This CL adds API to allow other code to inform SWVersion about events in
progress, and let SWVersion deal with timeouts and service worker lifetime
management in relation to these events.

On top of that this CL also adds a more generic mechanism for events that
use mojo services to connect to these mojo services (and have connections
to those services automatically close when the worker is stopped).

Finally this CL updates the navigator.connect call to use this new way of
dispatching events. Follow up CLs will update other mojo based events, and
will introduce the infrastructure to also move IPC based events out of
SWVersion.

BUG=570820
==========

[Marijn Kruisselbrink, 2015-12-17 22:39:16]

Description was changed from

==========
First step to refactor ServiceWorkerVersion to make event dispatching more
modular.

This CL adds API to allow other code to inform SWVersion about events in
progress, and let SWVersion deal with timeouts and service worker lifetime
management in relation to these events.

On top of that this CL also adds a more generic mechanism for events that
use mojo services to connect to these mojo services (and have connections
to those services automatically close when the worker is stopped).

Finally this CL updates the navigator.connect call to use this new way of
dispatching events. Follow up CLs will update other mojo based events, and
will introduce the infrastructure to also move IPC based events out of
SWVersion.

BUG=570820
==========

to

==========
First step to refactor ServiceWorkerVersion to make event dispatching more
modular.

As described in
https://docs.google.com/document/d/1k3xQuV2cQgygJXMpD4Jf77H88040Vcnixz-XW5o6G...,
this CL adds API to allow other code to inform SWVersion about events in
progress, and let SWVersion deal with timeouts and service worker lifetime
management in relation to these events.

On top of that this CL also adds a more generic mechanism for events that
use mojo services to connect to these mojo services (and have connections
to those services automatically close when the worker is stopped).

Finally this CL updates the navigator.connect call to use this new way of
dispatching events. Follow up CLs will update other mojo based events, and
will introduce the infrastructure to also move IPC based events out of
SWVersion.

BUG=570820
==========

[Marijn Kruisselbrink, 2015-12-17 22:40:02]

mek@chromium.org changed reviewers:
+ nhiroki@chromium.org, rockot@chromium.org

[Marijn Kruisselbrink, 2015-12-17 22:40:03]

nhiroki: PTAL
rockot: to make sure the mojo specific bits of this CL make sense

[nhiroki, 2016-01-05 09:33:24]

I came back from vacation.

Sorry, I couldn't take enough time to read this patch today (due to a bunch of
emails etc...). I'll closely review this tomorrow.

https://codereview.chromium.org/1264333002/diff/300001/content/browser/naviga...
File content/browser/navigator_connect/navigator_connect_context_impl.cc
(right):

https://codereview.chromium.org/1264333002/diff/300001/content/browser/naviga...
content/browser/navigator_connect/navigator_connect_context_impl.cc:169:
DCHECK(ports_.find(service_port_id) != ports_.end());
nit: ContainsKey() in stl_util.h would be more readable.

DCHECK(ContainsKey(ports_, client_port_id));
DCHECK(ContainsKey(ports_, service_port_id));

https://codereview.chromium.org/1264333002/diff/300001/content/browser/servic...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/1264333002/diff/300001/content/browser/servic...
content/browser/service_worker/service_worker_version.h:403: REQUEST_CUSTOM,
I think it'd be better that each event has a unique type for UMA as is instead
of a general one. As you noted in your docs, how about passing the request type
into StartRequest()?

[Marijn Kruisselbrink, 2016-01-05 23:23:16]

Don't worry about the delay, I was on vacation myself too.

https://codereview.chromium.org/1264333002/diff/300001/content/browser/naviga...
File content/browser/navigator_connect/navigator_connect_context_impl.cc
(right):

https://codereview.chromium.org/1264333002/diff/300001/content/browser/naviga...
content/browser/navigator_connect/navigator_connect_context_impl.cc:169:
DCHECK(ports_.find(service_port_id) != ports_.end());
On 2016/01/05 at 09:33:24, nhiroki wrote:
> nit: ContainsKey() in stl_util.h would be more readable.
> 
> DCHECK(ContainsKey(ports_, client_port_id));
> DCHECK(ContainsKey(ports_, service_port_id));

Done

https://codereview.chromium.org/1264333002/diff/300001/content/browser/servic...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/1264333002/diff/300001/content/browser/servic...
content/browser/service_worker/service_worker_version.h:403: REQUEST_CUSTOM,
On 2016/01/05 at 09:33:24, nhiroki wrote:
> I think it'd be better that each event has a unique type for UMA as is instead
of a general one. As you noted in your docs, how about passing the request type
into StartRequest()?

Currently my approach was to add necessary functionality as I'm migrating
existing events. So given that fetch is currently the only event for which UMA
stats are collected by ServiceWorkerVersion I didn't yet add support for that. I
do still plan to add a ServiceWorkerMetrics::EventType parameter to
StartRequest, although I'm not quite sure yet how to deal with the potential
explosion of parameters to this method (especially where many of the parameters
should at least conceptually be optional. For background sync StartRequest will
need custom timeout behavior for example. But I don't really want to end up
with:
StartRequest(callback)
StartRequestWithEventType(callback, event_type)
StartRequestWithCustomTimeout(callback, timeout)
StartRequestWithEventTypeAndCustomTimeout(callback, event_type, timeout)
and who knows what other attributes eventually might apply to a request...
(or just having one overloaded method, or even one method with default
parameters, both of which aren't recommended by the styleguide).

So I figured I'd ignore this problem for now and deal with it when I actually
have to.

[nhiroki, 2016-01-06 10:13:57]

Looks good. Let me discuss UMA stats just a bit more...

https://codereview.chromium.org/1264333002/diff/300001/content/browser/servic...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/1264333002/diff/300001/content/browser/servic...
content/browser/service_worker/service_worker_version.h:403: REQUEST_CUSTOM,
Thank you for the explanation.

On 2016/01/05 23:23:16, Marijn Kruisselbrink wrote:
> On 2016/01/05 at 09:33:24, nhiroki wrote:
> > I think it'd be better that each event has a unique type for UMA as is
instead
> of a general one. As you noted in your docs, how about passing the request
type
> into StartRequest()?
> 
> Currently my approach was to add necessary functionality as I'm migrating
> existing events. So given that fetch is currently the only event for which UMA
> stats are collected by ServiceWorkerVersion I didn't yet add support for that.
I

You mean "ServiceWorker.EventHandledRatioType"? It's collected only for fetch
event, but there is other UMA stat collected for all event types
("ServiceWorker.RequestTimeouts.Count").

> do still plan to add a ServiceWorkerMetrics::EventType parameter to
> StartRequest, although I'm not quite sure yet how to deal with the potential
> explosion of parameters to this method (especially where many of the
parameters
> should at least conceptually be optional. For background sync StartRequest
will
> need custom timeout behavior for example. But I don't really want to end up
> with:
> StartRequest(callback)
> StartRequestWithEventType(callback, event_type)
> StartRequestWithCustomTimeout(callback, timeout)
> StartRequestWithEventTypeAndCustomTimeout(callback, event_type, timeout)
> and who knows what other attributes eventually might apply to a request...
> (or just having one overloaded method, or even one method with default
> parameters, both of which aren't recommended by the styleguide).
> 
> So I figured I'd ignore this problem for now and deal with it when I actually
> have to.

I think we may need to make |event_type| a required parameter to collect the
timeout stats. |timeout| would be optional.

[Marijn Kruisselbrink, 2016-01-07 00:59:08]

https://codereview.chromium.org/1264333002/diff/300001/content/browser/servic...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/1264333002/diff/300001/content/browser/servic...
content/browser/service_worker/service_worker_version.h:403: REQUEST_CUSTOM,
On 2016/01/06 at 10:13:57, nhiroki wrote:
> Thank you for the explanation.
> 
> On 2016/01/05 23:23:16, Marijn Kruisselbrink wrote:
> > On 2016/01/05 at 09:33:24, nhiroki wrote:
> > > I think it'd be better that each event has a unique type for UMA as is
instead
> > of a general one. As you noted in your docs, how about passing the request
type
> > into StartRequest()?
> > 
> > Currently my approach was to add necessary functionality as I'm migrating
> > existing events. So given that fetch is currently the only event for which
UMA
> > stats are collected by ServiceWorkerVersion I didn't yet add support for
that. I
> 
> You mean "ServiceWorker.EventHandledRatioType"? It's collected only for fetch
event, but there is other UMA stat collected for all event types
("ServiceWorker.RequestTimeouts.Count").

Ah yes, not sure how I missed the "Used for UMA" comment on RequestType. In that
case yes, this is something I need to deal with here already. In the latest
version I changed the code to no longer use RequestType for UMA stats, but
instead use ServiceWorkerMetrics::EventType (and I added all the existing values
from RequestType in the same order/with the same values to ensure histograms
keep being valid). That makes the existing code in SWVersion temporarily a bit
more complicated, as now all the event dispatching code has to specify both a
RequestType and an EventType, but in the future when all event dispatching is
done using StartRequest we should be able to completely get rid of RequestType
(and make various other simplifications as well).
As suggested, EventType now is also a required parameter to StartRequest, so
histograms should remain working the same. RecordEventHandledRatio still only
works (and is only called) for fetch events; I imagine if we'd want similar
histograms for other event types we could add some kind of event handled status
to FinishRequest, which would then collect the correct metrics.

[nhiroki, 2016-01-07 05:06:09]

LGTM. Thank you!

https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
File content/browser/service_worker/embedded_worker_test_helper.cc (right):

https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
content/browser/service_worker/embedded_worker_test_helper.cc:55: static void
Create(base::WeakPtr<EmbeddedWorkerTestHelper> helper,
nit: const base::WeakPtr<>&

https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
content/browser/service_worker/embedded_worker_test_helper.cc:71:
MockEmbeddedWorkerSetup(base::WeakPtr<EmbeddedWorkerTestHelper> helper,
ditto

[Marijn Kruisselbrink, 2016-01-07 18:30:39]

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

[Marijn Kruisselbrink, 2016-01-07 18:30:39]

rockot: could you make sure the mojo code (particularly in
service_worker_version and embedded_worker_test_helper) doesn't do anything
crazy/wrong?
+mpearson for histogram OWNERS

[Mark P, 2016-01-07 19:47:53]

https://codereview.chromium.org/1264333002/diff/380001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1264333002/diff/380001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:43717: +   
enum="ServiceWorkerMetrics.EventType">
It looks like enums are equivalent.  Is that correct?  If so, why are you
bothering to rename the enum name?

Also, is this histogram with this new(-ish) enum type being emitted under
exactly the same conditions / same time as the previous histogram?  I ask
because you've touched a lot of code and it's unclear whether all paths through
the code are identical.

[Marijn Kruisselbrink, 2016-01-07 20:33:44]

https://codereview.chromium.org/1264333002/diff/380001/tools/metrics/histogra...
File tools/metrics/histograms/histograms.xml (right):

https://codereview.chromium.org/1264333002/diff/380001/tools/metrics/histogra...
tools/metrics/histograms/histograms.xml:43717: +   
enum="ServiceWorkerMetrics.EventType">
On 2016/01/07 at 19:47:53, Mark P wrote:
> It looks like enums are equivalent.  Is that correct?  If so, why are you
bothering to rename the enum name?
Yes, the enums are equivalent. I renamed it here to match the same change in the
code that emits this histograms. Previously that code used
ServiceWorkerVersion::RequestType, now the code uses
ServiceWorkerMetrics::EventType (with new EventType having the same values as
old RequestType, but new RequestType no longer being the same as old
RequestType, and eventually going away entirely).

> Also, is this histogram with this new(-ish) enum type being emitted under
exactly the same conditions / same time as the previous histogram?  I ask
because you've touched a lot of code and it's unclear whether all paths through
the code are identical.
It is certainly the intention that this enum is still being emitted under
exactly the same conditions with exactly the same values as before this change.
Ensuring that was primarily the reason for using a different enum for this
histogram.

[Mark P, 2016-01-07 20:35:02]

in those cases, histograms.xml lgtm

[Ken Rockot(use gerrit already), 2016-01-07 20:36:47]

rockot@chromium.org changed reviewers:
- mpearson@chromium.org

[Ken Rockot(use gerrit already), 2016-01-07 20:36:48]

I don't see anything fundamentally wrong about what you're doing, but I also
don't quite understand the point of storing a map of arbitrary interface
proxies, as opposed to simply having fields for each interface type you care
about. You could still build generic error handling without storing the proxies
in a map, no?

https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
content/browser/service_worker/service_worker_version.h:444: class
CONTENT_EXPORT BaseMojoService {
nit: I find the name a little confusing since it's really a proxy to a service,
not a service per se

https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
content/browser/service_worker/service_worker_version.h:452: };
nit: DISALLOW_COPY_AND_ASSIGN

[Marijn Kruisselbrink, 2016-01-07 21:33:05]

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

[Marijn Kruisselbrink, 2016-01-07 21:33:06]

On 2016/01/07 at 20:36:48, rockot wrote:
> I don't see anything fundamentally wrong about what you're doing, but I also
don't quite understand the point of storing a map of arbitrary interface
proxies, as opposed to simply having fields for each interface type you care
about. You could still build generic error handling without storing the proxies
in a map, no?

The reason to store arbitrary interface proxies is that the whole point of this
refactoring is that ServiceWorkerVersion should not have to know what
events/interfaces somebody else might care about. All it provides is a way for
some subsystem to connect to a service exposed by a service worker, while
ensuring the correct lifetime and error handling for the connection to that
service. So just like you shouldn't need to add code to ServiceProvider for
every concrete service type you might want to connect to you also shouldn't need
to add code to ServiceWorkerVersion for each mojo service exposed by a service
worker you might want to connect to.

https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
content/browser/service_worker/service_worker_version.h:444: class
CONTENT_EXPORT BaseMojoService {
On 2016/01/07 at 20:36:48, Ken Rockot wrote:
> nit: I find the name a little confusing since it's really a proxy to a
service, not a service per se

renamed it to (Base)MojoServiceWrapper

https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
content/browser/service_worker/service_worker_version.h:452: };
On 2016/01/07 at 20:36:48, Ken Rockot wrote:
> nit: DISALLOW_COPY_AND_ASSIGN

Done

[Marijn Kruisselbrink, 2016-01-07 21:39:31]

mek@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[Marijn Kruisselbrink, 2016-01-07 21:39:31]

+phajdan.jr for content/public/test OWNERS

[Ken Rockot(use gerrit already), 2016-01-07 22:27:28]

On 2016/01/07 at 21:33:06, mek wrote:
> On 2016/01/07 at 20:36:48, rockot wrote:
> > I don't see anything fundamentally wrong about what you're doing, but I also
don't quite understand the point of storing a map of arbitrary interface
proxies, as opposed to simply having fields for each interface type you care
about. You could still build generic error handling without storing the proxies
in a map, no?
> 
> The reason to store arbitrary interface proxies is that the whole point of
this refactoring is that ServiceWorkerVersion should not have to know what
events/interfaces somebody else might care about. All it provides is a way for
some subsystem to connect to a service exposed by a service worker, while
ensuring the correct lifetime and error handling for the connection to that
service. So just like you shouldn't need to add code to ServiceProvider for
every concrete service type you might want to connect to you also shouldn't need
to add code to ServiceWorkerVersion for each mojo service exposed by a service
worker you might want to connect to.

I see - sorry, I'm not too familiar with service worker code and I forgot what
ServiceWorkerVersion was all about. I was thinking these were only going to be
services used by the ServiceWorkerVersion implementation directly.

So IIUC this is something worker feature implementations can use to acquire
arbitrary service proxies which have some browser-side context tying them to a
particular worker.  Out of curiosity, is there a specific reason why you don't
want to just give implementations a handle to the worker's service registry and
let them directly acquire the services they need?

In any case LGTM.

> 
>
https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
> File content/browser/service_worker/service_worker_version.h (right):
> 
>
https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
> content/browser/service_worker/service_worker_version.h:444: class
CONTENT_EXPORT BaseMojoService {
> On 2016/01/07 at 20:36:48, Ken Rockot wrote:
> > nit: I find the name a little confusing since it's really a proxy to a
service, not a service per se
> 
> renamed it to (Base)MojoServiceWrapper
> 
>
https://codereview.chromium.org/1264333002/diff/360001/content/browser/servic...
> content/browser/service_worker/service_worker_version.h:452: };
> On 2016/01/07 at 20:36:48, Ken Rockot wrote:
> > nit: DISALLOW_COPY_AND_ASSIGN
> 
> Done

[Marijn Kruisselbrink, 2016-01-08 01:32:52]

On 2016/01/07 at 22:27:28, rockot wrote:
> On 2016/01/07 at 21:33:06, mek wrote:
> > On 2016/01/07 at 20:36:48, rockot wrote:
> > > I don't see anything fundamentally wrong about what you're doing, but I
also don't quite understand the point of storing a map of arbitrary interface
proxies, as opposed to simply having fields for each interface type you care
about. You could still build generic error handling without storing the proxies
in a map, no?
> > 
> > The reason to store arbitrary interface proxies is that the whole point of
this refactoring is that ServiceWorkerVersion should not have to know what
events/interfaces somebody else might care about. All it provides is a way for
some subsystem to connect to a service exposed by a service worker, while
ensuring the correct lifetime and error handling for the connection to that
service. So just like you shouldn't need to add code to ServiceProvider for
every concrete service type you might want to connect to you also shouldn't need
to add code to ServiceWorkerVersion for each mojo service exposed by a service
worker you might want to connect to.
> 
> I see - sorry, I'm not too familiar with service worker code and I forgot what
ServiceWorkerVersion was all about. I was thinking these were only going to be
services used by the ServiceWorkerVersion implementation directly.
> 
> So IIUC this is something worker feature implementations can use to acquire
arbitrary service proxies which have some browser-side context tying them to a
particular worker.  Out of curiosity, is there a specific reason why you don't
want to just give implementations a handle to the worker's service registry and
let them directly acquire the services they need?

Something like that could work, although it would probably mean that every event
would require a new connection to a service, which could be a bit annoying (in
particular because it would mean we'd lose all ordering guarantees even between
events of the same type). Also I'd still like to keep it possible for
ServiceWorkerVersion to force close all the connections to services exposed by a
service worker when it decides the worker needs to be killed. Of course
eventually these connections would close anyway, but the browser process side
code would get more complicated if it could still receive replies from a worker
that was killed for some time after the browser process side code decided to
kill said worker. So SWVersion would need to somehow keep a reference to the
service/connection anyway. So the simplest thing seemed to be to let SWVersion
own the connections to mojo services, and only hand out weak pointers to
interfaces externally (some kind of more generic copyable weak reference to a
mojo service would be even nicer of course, if it would allow things like
attaching error handlers, but I'm not sure if that is really necessary, so
simple weak pointers work fine for now).

[Paweł Hajdan Jr., 2016-01-08 16:53:22]

content/public/test LGTM

[Marijn Kruisselbrink, 2016-01-08 16:53:56]

The CQ bit was checked by mek@chromium.org

[Marijn Kruisselbrink, 2016-01-08 16:53:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
nhiroki@chromium.org, mpearson@chromium.org
Link to the patchset: https://codereview.chromium.org/1264333002/#ps400001
(title: "rockot nits")

[commit-bot: I haz the power, 2016-01-08 16:54:15]

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

[commit-bot: I haz the power, 2016-01-08 16:59:25]

Committed patchset #13 (id:400001)

[commit-bot: I haz the power, 2016-01-08 17:00:28]

Description was changed from

==========
First step to refactor ServiceWorkerVersion to make event dispatching more
modular.

As described in
https://docs.google.com/document/d/1k3xQuV2cQgygJXMpD4Jf77H88040Vcnixz-XW5o6G...,
this CL adds API to allow other code to inform SWVersion about events in
progress, and let SWVersion deal with timeouts and service worker lifetime
management in relation to these events.

On top of that this CL also adds a more generic mechanism for events that
use mojo services to connect to these mojo services (and have connections
to those services automatically close when the worker is stopped).

Finally this CL updates the navigator.connect call to use this new way of
dispatching events. Follow up CLs will update other mojo based events, and
will introduce the infrastructure to also move IPC based events out of
SWVersion.

BUG=570820
==========

to

==========
First step to refactor ServiceWorkerVersion to make event dispatching more
modular.

As described in
https://docs.google.com/document/d/1k3xQuV2cQgygJXMpD4Jf77H88040Vcnixz-XW5o6G...,
this CL adds API to allow other code to inform SWVersion about events in
progress, and let SWVersion deal with timeouts and service worker lifetime
management in relation to these events.

On top of that this CL also adds a more generic mechanism for events that
use mojo services to connect to these mojo services (and have connections
to those services automatically close when the worker is stopped).

Finally this CL updates the navigator.connect call to use this new way of
dispatching events. Follow up CLs will update other mojo based events, and
will introduce the infrastructure to also move IPC based events out of
SWVersion.

BUG=570820

Committed: https://crrev.com/24f20a4f6da8507e47ace8dd6a43e698d241e647
Cr-Commit-Position: refs/heads/master@{#368353}
==========

[commit-bot: I haz the power, 2016-01-08 17:00:29]

Patchset 13 (id:??) landed as
https://crrev.com/24f20a4f6da8507e47ace8dd6a43e698d241e647
Cr-Commit-Position: refs/heads/master@{#368353}