Shut down the ServiceWorker system as the browser's shutting down.

Shut down the ServiceWorker system as the browser's shutting down.

This avoids a DCHECK in ProfileDestroyer::DestroyProfileWhenAppropriate.

I also added a Chrome-side browsertest for the Service Worker system, where we
can check other things that only show up with this embedder.

I tried to move the DCHECK into ~BrowserContext so that we wouldn't need to
expose the Terminate() function across the Content API, but:
* BrowserContext doesn't know whether it's a "testing" profile.
* There is no BrowserContext::GetOffTheRecordProfile with which to check that
  transitive processes have been shut down.
* (Minor) GetHostsForProfile() works by iterating over all hosts, and moving the
  DCHECK to ~BrowserContext would have doubled the number of iterations needed.

BUG=368570
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=273741

[Jeffrey Yasskin, 2014-05-28 17:43:34]

This takes option 2 from
https://code.google.com/p/chromium/issues/detail?id=368570#c7 for the reasons
mentioned in the CL description. 

Is there a naming or placement convention for the new browser test? Should I add
the SW folks as per-file owners for the test?

Is there someone better to review the BrowserProcessImpl::ReleaseModule()
change? That looks like very old code, so there might be a better place to put
this hook that we haven't found.

[Jeffrey Yasskin, 2014-05-28 17:51:33]

On 2014/05/28 17:43:34, Jeffrey Yasskin wrote:
> This takes option 2 from
> https://code.google.com/p/chromium/issues/detail?id=368570#c7 for the reasons
> mentioned in the CL description. 
> 
> Is there a naming or placement convention for the new browser test? Should I
add
> the SW folks as per-file owners for the test?
> 
> Is there someone better to review the BrowserProcessImpl::ReleaseModule()
> change? That looks like very old code, so there might be a better place to put
> this hook that we haven't found.

Oh, this also depends on https://codereview.chromium.org/298023008/ in order to
avoid using the BrowserContext while it's being destroyed. You'll see in the
content_browsertests results.

[jam, 2014-05-28 18:57:56]

lgtm

On 2014/05/28 17:43:34, Jeffrey Yasskin wrote:
> This takes option 2 from
> https://code.google.com/p/chromium/issues/detail?id=368570#c7 for the reasons
> mentioned in the CL description. 
> 
> Is there a naming or placement convention for the new browser test? Should I
add
> the SW folks as per-file owners for the test?

since there are no service_worker directory under chrome/browser, this is fine
for now

> 
> Is there someone better to review the BrowserProcessImpl::ReleaseModule()
> change? That looks like very old code, so there might be a better place to put
> this hook that we haven't found.

there are probably various places that would work, if this works seems fine to
me

[falken, 2014-05-29 02:50:27]

lgtm
horo@ may want to take a look too

https://codereview.chromium.org/305723003/diff/10001/content/browser/service_...
File content/browser/service_worker/service_worker_job_coordinator.cc (right):

https://codereview.chromium.org/305723003/diff/10001/content/browser/service_...
content/browser/service_worker/service_worker_job_coordinator.cc:46:
jobs_.clear();
Looks like you can remove the clear() as STLDeleteElements does it.

[horo, 2014-05-29 04:06:17]

lgtm

[Jeffrey Yasskin, 2014-05-29 21:00:23]

https://codereview.chromium.org/305723003/diff/10001/content/browser/service_...
File content/browser/service_worker/service_worker_job_coordinator.cc (right):

https://codereview.chromium.org/305723003/diff/10001/content/browser/service_...
content/browser/service_worker/service_worker_job_coordinator.cc:46:
jobs_.clear();
On 2014/05/29 02:50:27, falken wrote:
> Looks like you can remove the clear() as STLDeleteElements does it.

Ah, right. Done, thanks.

[Jeffrey Yasskin, 2014-05-29 21:53:54]

The CQ bit was checked by jyasskin@chromium.org

[commit-bot: I haz the power, 2014-05-29 21:55:34]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jyasskin@chromium.org/305723003/30001

[commit-bot: I haz the power, 2014-05-30 00:20:27]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[commit-bot: I haz the power, 2014-05-30 00:44:47]

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

[commit-bot: I haz the power, 2014-05-30 00:44:48]

Try jobs failed on following builders:
  android_dbg_triggered_tests on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...)

[horo, 2014-05-30 01:12:56]

The CQ bit was checked by horo@chromium.org

[commit-bot: I haz the power, 2014-05-30 01:14:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/jyasskin@chromium.org/305723003/30001

[commit-bot: I haz the power, 2014-05-30 07:01:56]

Change committed as 273741