S13nServiceWorker: http/tests/serviceworker/chromium/usecounter.html is flaky. |
|||
Issue description
What steps will reproduce the problem?
(1) Enable the the network service (or maybe only ServiceWorkerServicification ???)
(2) Add a delay − usleep(100'000) in ServiceWorkerProviderHost::SendSetControllerServiceWorker()
(3) Run http/tests/serviceworker/chromium/usecounter.html
What is the expected result?
Test PASS
What happens instead?
Test FAIL.
Location: in promise_test('UseCounter on ServiceWorkerGlobalScope') at the last line:
```
.then(win => {
assert_true(isUseCounted(win, kFeature));
assert_true(isUseCounted(win, kDeprecatedFeature)); <---- HERE
});
```
I have the same kind of flakiness with NavigationMojoResponse. This is the last remaining flaky test with service worker I need to fix.
The vector of used features is sent in ServiceWorkerProviderHost::SendSetControllerServiceWorker(), where the usleep(100'000) is added. This message may or may not arrive before the renderer has loaded the new document. So it's fundamentally flaky.
Kinuko: I am not really aware of what is it about. Do you think the vector of used feature really needs to be sent before the document is loaded? In this case, it looks like it must be sent in CommitNavigation(), in the other case, maybe we can just update the test and wait until it is updated. Please let me know.
,
Feb 5 2018
Thanks! So I will update the test.
,
Feb 6 2018
Sounds good to fix the test. As I wrote on the code review the long term fix would be fixing this TODO: // In S13nServiceWorker case the controller is already sent in navigation // commit, but we still need this for S13nServiceWorker case for setting the // use counter correctly. // TODO(kinuko): Stop doing this in S13nServiceWorker case. SendSetControllerServiceWorker(false /* notify_controllerchange */); https://cs.chromium.org/chromium/src/content/browser/service_worker/service_worker_provider_host.cc?l=622&rcl=64ace9566d83786c4928cc7a40b853727e674be7 https://chromium-review.googlesource.com/c/chromium/src/+/901610
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/933489bc251bdb3ca89882c91600832c094e355f commit 933489bc251bdb3ca89882c91600832c094e355f Author: arthursonzogni <arthursonzogni@chromium.org> Date: Tue Feb 06 09:30:07 2018 Service worker: Update usecounter.html test. This test is flaky with the ServiceWorker servicification More details in https://crbug.com/808964 . The previous test was checking the UseCounter immediatly after a new window loads. It is racy because the UseCounter state is sent in ServiceWorkerProviderHost::SendSetControllerServiceWorker(). In the case of a very slow browser process, it can happens after the new window loads. Instead of checking the UseCounter state after a window loads, it uses an observer to wait until its state is updated. Bug: 808964 Change-Id: I53c2bf2775a837a3a4b0c6cf9b196f26fe7b7f5e Reviewed-on: https://chromium-review.googlesource.com/901610 Reviewed-by: Matt Falkenhagen <falken@chromium.org> Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org> Cr-Commit-Position: refs/heads/master@{#534667} [modify] https://crrev.com/933489bc251bdb3ca89882c91600832c094e355f/third_party/WebKit/LayoutTests/http/tests/serviceworker/chromium/usecounter.html
,
Feb 6 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by shimazu@chromium.org
, Feb 5 2018