ServiceWorker: More accurate StartWorker result and UMA

ServiceWorker: More accurate StartWorker result and UMA

This adds SERVICE_WORKER_ERROR_DISK_CACHE to indicate
failure to read from the disk cache. It also re-enables
logging of StartWorker for redundant workers. This was
previously disabled because we got a lot of "failed" spam
for updates that get aborted when there is no script change,
so I've changed StartWorker to fail with SW_ERROR_EXISTS in
that case.

This patch is a step in the direction of evicting a SW
that can't be read from disk cache.

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

[falken, 2015-04-23 08:48:21]

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

[falken, 2015-04-23 08:48:22]

PTAL, PatchSet 1 shows WIP for also deleting the SW when it can't be read if you
want to see how this is planning to end up. This patch just basically just
adjusts UMA though.

[falken, 2015-04-24 04:01:16]

The CQ bit was checked by falken@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-04-24 04:01:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1054033004/40001

[nhiroki, 2015-04-24 04:58:28]

Looks good overall.

https://codereview.chromium.org/1054033004/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/1054033004/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_version.h:550: Status
prestart_status_ = NEW;
How about binding "prestart status" to DidEnsureLiveRegistrationForStartWorker
and RecordStartWorkerResult, and then removing this field? This class already
has many states and I'd rather not add such fields more...

[commit-bot: I haz the power, 2015-04-24 05:00:50]

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

[commit-bot: I haz the power, 2015-04-24 05:00:51]

Dry run: This issue passed the CQ dry run.

[kinuko, 2015-04-24 05:06:15]

lgtm

[falken, 2015-04-24 05:08:26]

https://codereview.chromium.org/1054033004/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/1054033004/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_version.h:550: Status
prestart_status_ = NEW;
On 2015/04/24 04:58:28, nhiroki wrote:
> How about binding "prestart status" to DidEnsureLiveRegistrationForStartWorker
> and RecordStartWorkerResult, and then removing this field? This class already
> has many states and I'd rather not add such fields more...

I originally wanted to do this but there are RecordStartWorkerResult callsites
like in the dtor that aren't binded. I agree about the states, maybe there's
opportunities to split some of these out into helper classes later.

[nhiroki, 2015-04-24 05:23:57]

https://codereview.chromium.org/1054033004/diff/40001/content/browser/service...
File content/browser/service_worker/service_worker_version.h (right):

https://codereview.chromium.org/1054033004/diff/40001/content/browser/service...
content/browser/service_worker/service_worker_version.h:550: Status
prestart_status_ = NEW;
On 2015/04/24 05:08:26, falken wrote:
> On 2015/04/24 04:58:28, nhiroki wrote:
> > How about binding "prestart status" to
DidEnsureLiveRegistrationForStartWorker
> > and RecordStartWorkerResult, and then removing this field? This class
already
> > has many states and I'd rather not add such fields more...
> 
> I originally wanted to do this but there are RecordStartWorkerResult callsites
> like in the dtor that aren't binded. I agree about the states, maybe there's
> opportunities to split some of these out into helper classes later.
> 

Oh... I see. Thank you for explanation. LGTM

[falken, 2015-04-24 05:28:56]

falken@chromium.org changed reviewers:
+ asvitkine@chromium.org, peter@chromium.org

[falken, 2015-04-24 05:28:56]

Adding OWNERS
+peter: notifications and push
+asvitkine: histograms

[johnme, 2015-04-24 08:30:40]

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

[johnme, 2015-04-24 08:30:41]

push+notifications lgtm

[Alexei Svitkine (slow), 2015-04-24 16:43:04]

lgtm

[falken, 2015-04-24 22:58:52]

The CQ bit was checked by falken@chromium.org

[commit-bot: I haz the power, 2015-04-24 22:59:25]

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

[commit-bot: I haz the power, 2015-04-24 23:00:39]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-04-24 23:01:26]

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