Fix flaky SW registration in PushMessaging/BudgetManager browser tests

Fix flaky SW registration in PushMessaging/BudgetManager browser tests

There was a race condition between registering the Service Workers (and
having them install and activate) and reloading the page. Many tests
were assuming that reloading the page would be sufficient to ensure
the page was controlled by the Service Worker. This patch makes that
reliable, by waiting until the SW is ready.

It should fix the error mentioned in https://crbug.com/657202:
    Actual: "false - is not controlled"
  Expected: "true - is controlled"
which affected:
- BudgetManagerBrowserTest.BudgetInWorker
- PushMessagingBrowserTest.SubscribeWorker
- PushMessagingBrowserTest.SubscribeWorkerUsingManifest

Hopefully it'll also fix https://crbug.com/554003 which affected:
- PushMessagingBrowserTest.SubscribePersiste
since the errors there were unclear but that test exercises SW
registration heavily, so it might well have suffered from the same
problem. If that test (alone) turns out to still be flaky, please just
mark it flaky again rather than reverting this patch.

BUG=657202, 554003
Committed: https://crrev.com/8062ec7de6c712190aad47f398f853afbfb19e5d
Cr-Commit-Position: refs/heads/master@{#428996}

[johnme, 2016-10-26 18:28:01]

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

[johnme, 2016-10-26 18:28:21]

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

[commit-bot: I haz the power, 2016-10-26 18:29:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 20:09:41]

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

[commit-bot: I haz the power, 2016-10-26 20:09:41]

Dry run: This issue passed the CQ dry run.

[harkness, 2016-10-27 09:26:15]

lgtm  with comment. You can update the install event or not as you see fit.

https://codereview.chromium.org/2452833004/diff/1/chrome/test/data/budget_ser...
File chrome/test/data/budget_service/service_worker.js (right):

https://codereview.chromium.org/2452833004/diff/1/chrome/test/data/budget_ser...
chrome/test/data/budget_service/service_worker.js:6:
self.addEventListener('install', () => skipWaiting());
Don't you need the event.waitUntil() for this for completeness? I know the odds
that the service worker would be shut down before the skipWaiting finished are
pretty low, but it seems to be the best practice to add it.

[johnme, 2016-11-01 13:18:49]

The CQ bit was checked by johnme@chromium.org

[johnme, 2016-11-01 13:18:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
harkness@chromium.org
Link to the patchset: https://codereview.chromium.org/2452833004/#ps20001
(title: "Rebase")

[commit-bot: I haz the power, 2016-11-01 13:18:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[johnme, 2016-11-01 13:18:59]

https://codereview.chromium.org/2452833004/diff/1/chrome/test/data/budget_ser...
File chrome/test/data/budget_service/service_worker.js (right):

https://codereview.chromium.org/2452833004/diff/1/chrome/test/data/budget_ser...
chrome/test/data/budget_service/service_worker.js:6:
self.addEventListener('install', () => skipWaiting());
On 2016/10/27 09:26:15, harkness wrote:
> Don't you need the event.waitUntil() for this for completeness? I know the
odds
> that the service worker would be shut down before the skipWaiting finished are
> pretty low, but it seems to be the best practice to add it.

Jake says it's unnecessary:
https://github.com/GoogleChrome/samples/commit/dbca5f70c8beec398b7bde257983ed...

[commit-bot: I haz the power, 2016-11-01 14:16:09]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-01 14:17:46]

Description was changed from

==========
Fix flaky SW registration in PushMessaging/BudgetManager browser tests

There was a race condition between registering the Service Workers (and
having them install and activate) and reloading the page. Many tests
were assuming that reloading the page would be sufficient to ensure
the page was controlled by the Service Worker. This patch makes that
reliable, by waiting until the SW is ready.

It should fix the error mentioned in https://crbug.com/657202:
    Actual: "false - is not controlled"
  Expected: "true - is controlled"
which affected:
- BudgetManagerBrowserTest.BudgetInWorker
- PushMessagingBrowserTest.SubscribeWorker
- PushMessagingBrowserTest.SubscribeWorkerUsingManifest

Hopefully it'll also fix https://crbug.com/554003 which affected:
- PushMessagingBrowserTest.SubscribePersiste
since the errors there were unclear but that test exercises SW
registration heavily, so it might well have suffered from the same
problem. If that test (alone) turns out to still be flaky, please just
mark it flaky again rather than reverting this patch.

BUG=657202,554003
==========

to

==========
Fix flaky SW registration in PushMessaging/BudgetManager browser tests

There was a race condition between registering the Service Workers (and
having them install and activate) and reloading the page. Many tests
were assuming that reloading the page would be sufficient to ensure
the page was controlled by the Service Worker. This patch makes that
reliable, by waiting until the SW is ready.

It should fix the error mentioned in https://crbug.com/657202:
    Actual: "false - is not controlled"
  Expected: "true - is controlled"
which affected:
- BudgetManagerBrowserTest.BudgetInWorker
- PushMessagingBrowserTest.SubscribeWorker
- PushMessagingBrowserTest.SubscribeWorkerUsingManifest

Hopefully it'll also fix https://crbug.com/554003 which affected:
- PushMessagingBrowserTest.SubscribePersiste
since the errors there were unclear but that test exercises SW
registration heavily, so it might well have suffered from the same
problem. If that test (alone) turns out to still be flaky, please just
mark it flaky again rather than reverting this patch.

BUG=657202,554003

Committed: https://crrev.com/8062ec7de6c712190aad47f398f853afbfb19e5d
Cr-Commit-Position: refs/heads/master@{#428996}
==========

[commit-bot: I haz the power, 2016-11-01 14:17:47]

Patchset 2 (id:??) landed as
https://crrev.com/8062ec7de6c712190aad47f398f853afbfb19e5d
Cr-Commit-Position: refs/heads/master@{#428996}