(Reland) Evict Service Worker when reading it from disk cache fails.

(Reland) Evict Service Worker when reading it from disk cache fails.

If reading the SW resources from the disk cache fails, we should evict
the worker or else we will keep getting the same failure.

This patch unsets the worker from the live registration, and then
deletes the registration if the SW was the stored version.

This relands the original commit (64e3ffb1c), and fixes a problem where
the test terminated before the eviction, which starts on the IO thread,
then hops to the database task runner, finishes. Now the test ends by
looking up the registration in storage, which hops to the database task
runner, so it can only finish once eviction finishes.

Original patch: https://codereview.chromium.org/1098083003/

BUG=448003
Committed: https://crrev.com/0e476caf67493d79b6dc40891e7ef8868871983e
Cr-Commit-Position: refs/heads/master@{#327260}

[falken, 2015-04-28 07:34:14]

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

[falken, 2015-04-28 07:34:15]

PTAL

Patchset #1 is the original patch.

I think what happened (after discussion with nhiroki@) is the browser test ended
while the db task was still running. The last reference to the SWRegistration
was held by the db task closure, so when that died, the SWRegistration dtor
cried that it wasn't called on the IO thread.

[kinuko, 2015-04-28 08:34:05]

lgtm

These core modules are really tightly coupled, needing to have a lot of fixture
is a bit sad but the code looks fine

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

https://codereview.chromium.org/1108253002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_browsertest.cc:590:
DCHECK_CURRENTLY_ON(BrowserThread::IO);
nit: I think we prefer ASSERT when in tests

[nhiroki, 2015-04-28 08:41:57]

lgtm

[falken, 2015-04-28 09:00:11]

Thanks!

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

https://codereview.chromium.org/1108253002/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_browsertest.cc:590:
DCHECK_CURRENTLY_ON(BrowserThread::IO);
On 2015/04/28 08:34:05, kinuko wrote:
> nit: I think we prefer ASSERT when in tests

Oh, didn't know that... done.

[falken, 2015-04-28 09:00:24]

The CQ bit was checked by falken@chromium.org

[falken, 2015-04-28 09:00:24]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
nhiroki@chromium.org
Link to the patchset: https://codereview.chromium.org/1108253002/#ps60001
(title: "review comments")

[commit-bot: I haz the power, 2015-04-28 09:00:53]

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

[commit-bot: I haz the power, 2015-04-28 10:43:35]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-04-28 10:44:32]

Patchset 4 (id:??) landed as
https://crrev.com/0e476caf67493d79b6dc40891e7ef8868871983e
Cr-Commit-Position: refs/heads/master@{#327260}