New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 789111 link

Starred by 3 users

Issue metadata

Status: Assigned
Merged: issue 788698
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

A race bug in navigation is causing Service Worker API failures.

Project Member Reported by hbos@chromium.org, Nov 28 2017

Issue description

http/tests/devtools/service-workers/service-worker-v8-cache.js
has been flaky recently, and the virtual/mojo-loading version.

The first or one of the first failures:
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Trusty%20MSAN/builds/4486

Please take a look or re-triage.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 28 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66d28a05de9bdfae0aa358ea6b96af43d072ca14

commit 66d28a05de9bdfae0aa358ea6b96af43d072ca14
Author: Henrik Boström <hbos@chromium.org>
Date: Tue Nov 28 11:21:51 2017

Expecting service-worker-v8-cache.js to be flaky.

Bug filed: https://crbug.com/789111

TBR=horo@chromium.org,chenwilliam@chromium.org

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 789111
Change-Id: I63b3f9af581d8f6778b25b6cd4b7540182a5f87c
Reviewed-on: https://chromium-review.googlesource.com/793038
Reviewed-by: Henrik Boström <hbos@chromium.org>
Commit-Queue: Henrik Boström <hbos@chromium.org>
Cr-Commit-Position: refs/heads/master@{#519654}
[modify] https://crrev.com/66d28a05de9bdfae0aa358ea6b96af43d072ca14/third_party/WebKit/LayoutTests/TestExpectations

Comment 3 by horo@chromium.org, Nov 28 2017

Mergedinto: 788698
Status: Duplicate (was: Assigned)
Cc: chiniforooshan@chromium.org horo@chromium.org
Owner: ----
Status: Untriaged (was: Duplicate)
I suspect this is a different issue than  crbug.com/788698 . That one was about a DCHECK failure in tracing_controller_impl.cc. Here, the bot fails because the test times out. Looks like time outs started around refs/heads/master@{#518823}.

I suggest we track this issue separately.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 30 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4e197153a40aa5b298784c6132cac985d42a471e

commit 4e197153a40aa5b298784c6132cac985d42a471e
Author: henrika <henrika@chromium.org>
Date: Thu Nov 30 12:52:55 2017

service-worker-v8-cache.js is flaky on WebKit Linux Trusty MSAN

https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Trusty%20MSAN/builds/4616

TBR=horo@chromium.org,chenwilliam@chromium.org

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 789111
Change-Id: I219436a1d99f78f6b3e48b07634eec9c5668d6d9
Reviewed-on: https://chromium-review.googlesource.com/800172
Reviewed-by: Henrik Andreasson <henrika@chromium.org>
Commit-Queue: Henrik Andreasson <henrika@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520521}
[modify] https://crrev.com/4e197153a40aa5b298784c6132cac985d42a471e/third_party/WebKit/LayoutTests/TestExpectations

Components: Platform>DevTools
Owner: horo@chromium.org
Status: Assigned (was: Untriaged)
horo: probably caused by recently changed to the test? https://chromium-review.googlesource.com/737595 or https://chromium-review.googlesource.com/778706
Project Member

Comment 8 by bugdroid1@chromium.org, Dec 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bfc2241dc3f6c977f2aaaf2437c86910f0526db0

commit bfc2241dc3f6c977f2aaaf2437c86910f0526db0
Author: Derek Cheng <imcheng@chromium.org>
Date: Tue Dec 05 00:24:41 2017

[Sheriff] Mark service-worker-v8-cache.js as Timeout in TestExpectations

No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 789111
Change-Id: I7df38ee8bfce5eb135e6c42e2b6e6d7788a38ac7
Reviewed-on: https://chromium-review.googlesource.com/807524
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Commit-Queue: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521545}
[modify] https://crrev.com/bfc2241dc3f6c977f2aaaf2437c86910f0526db0/third_party/WebKit/LayoutTests/TestExpectations

Comment 9 by horo@chromium.org, Dec 18 2017

Status: Started (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Dec 18 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/70edab04af3e4109312ab50728cedf08b6cb70ef

commit 70edab04af3e4109312ab50728cedf08b6cb70ef
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Mon Dec 18 05:23:39 2017

Update service-worker-v8-cache-expected.txt

After this cl https://chromium-review.googlesource.com/c/v8/v8/+/806741,
printTimelineRecordsWithDetails() outputs the file name of v8-cache-script.js
without the port number and the directory path.

So this test is continuously failing.
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_tests&tests=service-worker-v8-cache

This CL doesn't completely fix the issue.
But I want to update the expectation first to monitor the flakiness dashboard.

Bug: 789111
Change-Id: I936f260c2bc73271cc76f67a99d691415f615103
Reviewed-on: https://chromium-review.googlesource.com/831374
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524653}
[modify] https://crrev.com/70edab04af3e4109312ab50728cedf08b6cb70ef/third_party/WebKit/LayoutTests/virtual/pwa-full-code-cache/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt

Project Member

Comment 11 by bugdroid1@chromium.org, Dec 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d73318878fc2afffd5c8bede63ce6682ccecb3df

commit d73318878fc2afffd5c8bede63ce6682ccecb3df
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Tue Dec 19 08:02:58 2017

Move v8-cache-script.js from /devtools/service-workers/resources/ to /devtools/resources/.

When pwa-full-code-cache is enabled, printTimelineRecordsWithDetails() at line
35 of service-worker-v8-cache.js is expected to print the script name as
"v8-cache-script.js".

But if the script resource is GCed before printTimelineRecordsWithDetails() is
called, ":8000/devtools/service-workers/resources/v8-cache-script.js" is printed
out. That is the result of url.trimURL(parsedURL.host) at line 84 of
Bindings.displayNameForURL() in ResourceUtils.js
https://chromium.googlesource.com/chromium/src/+/5459ad6/third_party/WebKit/Source/devtools/front_end/bindings/ResourceUtils.js

We can reproduce this by calling CollectAllGarbage() from
debug::GetLoadedScripts() which was removed by
https://chromium-review.googlesource.com/c/v8/v8/+/806741,

This can cause the test flakiness. To avoid the flakiness, this CL moves
v8-cache-script.js from /devtools/service-workers/resources/ to
/devtools/resources/. The main frame of the test is
/devtools/resources/inspected-page.html, so Bindings.displayNameForURL() in
ResourceUtils.js can return url.substring(index) at line 78.

Bug: 789111
Change-Id: I9e8f5c112a017e78ffb2c291eac89f70837a6eac
Reviewed-on: https://chromium-review.googlesource.com/832601
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524967}
[rename] https://crrev.com/d73318878fc2afffd5c8bede63ce6682ccecb3df/third_party/WebKit/LayoutTests/http/tests/devtools/resources/v8-cache-script.js
[modify] https://crrev.com/d73318878fc2afffd5c8bede63ce6682ccecb3df/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/resources/v8-cache-worker.js
[modify] https://crrev.com/d73318878fc2afffd5c8bede63ce6682ccecb3df/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt
[modify] https://crrev.com/d73318878fc2afffd5c8bede63ce6682ccecb3df/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache.js
[modify] https://crrev.com/d73318878fc2afffd5c8bede63ce6682ccecb3df/third_party/WebKit/LayoutTests/virtual/pwa-full-code-cache/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt

Project Member

Comment 12 by bugdroid1@chromium.org, Dec 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66e5ea2898491b73a2a27adb4767e7c9dba2d029

commit 66e5ea2898491b73a2a27adb4767e7c9dba2d029
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Wed Dec 20 06:47:52 2017

Add service-worker-v8-cache.js to SlowTests

To know whether the test has a bug or the test is just slow.
https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&tests=service-worker-v8-cache.js

Bug: 789111
Change-Id: I7aa771ca9712882c261e46d6af48241dac465a0c
Reviewed-on: https://chromium-review.googlesource.com/835967
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525280}
[modify] https://crrev.com/66e5ea2898491b73a2a27adb4767e7c9dba2d029/third_party/WebKit/LayoutTests/SlowTests

Comment 13 by horo@chromium.org, Jan 5 2018

This test is still sometimes failing with this error:
"Error: Uncaught (in promise)".

https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=webkit_layout_tests&tests=service-worker-v8-cache.js

Comment 14 by horo@chromium.org, Jan 5 2018

Cc: falken@chromium.org clamy@chromium.org
I can reproduce this failure by this command.
  ./blink/tools/run_layout_tests.py  -t Linux --exit-after-n-failures=1 --iterations=1000 http/tests/devtools/service-workers/

navigator.serviceWorker.register() is sometime failing because ServiceWorkerProviderHost's |document_url| is empty.
https://chromium.googlesource.com/chromium/src/+/2452ce0/content/browser/service_worker/service_worker_provider_host.cc#1248

I don't understand why this happens yet, but when the test failed:
- The ServiceWorkerDispatcherHost which had been created by ServiceWorkerProviderHost::PreCreateNavigationHost() has been deleted.
- And ServiceWorkerDispatcherHost::OnProviderCreated() created a new ServiceWorkerProviderHost because GetNavigationHandleCore() returns null.
  https://chromium.googlesource.com/chromium/src/+/2452ce0/content/browser/service_worker/service_worker_dispatcher_host.cc#287
- And ServiceWorkerProviderHost::Register() is called before |document_url_| is set. And CanServeContainerHostMethods() rejects the callback with kNoDocumentURLErrorMessage.

Issue 703972 and this cl https://codereview.chromium.org/2774513003 look related to this failure.

falken@
Do you have any idea about it?
crbug.com/776408 may also related to this.

Comment 15 by horo@chromium.org, Jan 9 2018

Components: UI>Browser>Navigation
Labels: Proj-PlzNavigate
Owner: clamy@chromium.org
Status: Assigned (was: Started)
I understand what is causing this problem.
The root cause of the bug is that |navigation_handle_| in RenderFrameHostImpl could be cleared by the FrameHostMsg_DidStopLoading IPC message from the previous load.

I created a patch which sends the navigation_id via IPC:
- [Browser] NavigationRequest::CommitNavigation() => [Renderer] RenderFrameImpl::CommitNavigation()
- [Renderer] RenderFrameImpl::DidStopLoading() => [Browser] RenderFrameHostImpl::OnDidStopLoading()
And I checked the navigation_id of the IPC message and |navigation_handle_| in RenderFrameHostImpl::OnDidStopLoading().
https://chromium-review.googlesource.com/c/chromium/src/+/856026/1/content/browser/frame_host/render_frame_host_impl.cc#2695

When the test failed while executing run_layout_tests.py with "--exit-after-n-failures=1 --iterations=1000 http/tests/devtools/service-workers/", I can see the error log "NavigationID missmatch".

When I add the return statement at line 2696, these tests didn't fail.


I think this is a bug of PlzNavigate.

clamy@
Could you please look into this issue?
This is a known race, but we didn't think it would affect ServiceWorker (though reading the description, it makes sense it does). We're working on a fix in issue 784904. However it involves quite a bit of refactoring as this race is a bit complicated to fix.
Project Member

Comment 17 by bugdroid1@chromium.org, Jan 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6d188105276e44cf8dfa3d40da10ac3ed400811

commit d6d188105276e44cf8dfa3d40da10ac3ed400811
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Wed Jan 10 16:32:27 2018

Print more detailed error log in ApplicationTestRunner.registerServiceWorker

Currently we can only see "Error: Uncaught (in promise)" in the error log of
failed LayoutTests of crbug.com/789111.
With this patch, we can see more detailed error log.
"Error: Uncaught (in promise) Error: Service Worker registration error:
SecurityError: Failed to register a ServiceWorker: No URL is associated
with the caller's document."

Bug: 789111
Change-Id: Ie3fd794e463f7f3283efb71e154012ba43c8b97b
Reviewed-on: https://chromium-review.googlesource.com/858536
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528337}
[modify] https://crrev.com/d6d188105276e44cf8dfa3d40da10ac3ed400811/third_party/WebKit/Source/devtools/front_end/application_test_runner/ServiceWorkersTestRunner.js

Project Member

Comment 18 by bugdroid1@chromium.org, Jan 11 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3fff4fc04f8b5c87c62aa0b5fb6fcc759b9bb354

commit 3fff4fc04f8b5c87c62aa0b5fb6fcc759b9bb354
Author: Tsuyoshi Horo <horo@chromium.org>
Date: Thu Jan 11 08:03:37 2018

Remove non-PlzNavigate code from ServiceWorkerDispatcherHostTest

This test is too complicated to support non-PlzNavigate and PlzNavigate.
But now we don't need to support non-PlzNavigate mode.
So this CL cleans up the test.

I noticed this while investigating the race issue in navigation logic:
https//crbug.com/789111#c15

Bug: 789111
Change-Id: I84af80b92b9e3eb315f8bd9b3abb3673931f4dc9
Reviewed-on: https://chromium-review.googlesource.com/860979
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#528577}
[modify] https://crrev.com/3fff4fc04f8b5c87c62aa0b5fb6fcc759b9bb354/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc

Comment 19 by horo@chromium.org, Jan 11 2018

Summary: A race bug in navigation is causeing Service Worker API failures. (was: service-worker-v8-cache.js is flaky on WebKit Linux Trusty MSAN)
clamy@
When do you think this will be fixed by using Mojo?
This issue is causing problems in the real world.
I think non-ChromeOS case of the issue 776408 is caused by the race.

If it takes time to fix using Mojo, I want change the legacy IPC to fix it:
https://chromium-review.googlesource.com/c/chromium/src/+/858857

Comment 20 by horo@chromium.org, Jan 11 2018

Summary: A race bug in navigation is causing Service Worker API failures. (was: A race bug in navigation is causeing Service Worker API failures.)

Comment 21 by clamy@chromium.org, Jan 11 2018

@horo: the issue is not using Mojo. It's that we made an assumption when writing the navigation code - that we only have one navigation about to commit - which turns out to be not true. This require quite a lot of changes in the navigation code to fix (and the fix is made simpler by switching to Mojo). We have ahemery working on it full time, and I am also looking at alleviating some of the races around DidStopLoading in the meantime (this won't fully fix the races, but should make them less likely to happen). I'm hoping my work will be done in the coming weeks. Arthur's may take between 1 and 2 months.

Comment 22 by horo@chromium.org, Jan 12 2018

I created a cl to mitigate the race problem (https://chromium-review.googlesource.com/c/chromium/src/+/858857) and it passed all tests.
But I'm not a professional of navigations, and you are already working on the issue.
So I'm willing to abandon the cl.

Thank you.
PlzNavigate was recently enabled for google3 webdriver tests and is causing quite a few test flakes, possibly related to service workers, e.g., b/72180655.

I'm glad to see there's some ideas for diagnosing the root cause. But, given that the fix might take a few months (and hence releases), is it possible to disable PlzNavigate until the fixes land? 

It seems worthwhile to avoid other users and developers needing to repeat the diagnosis independently.

Comment 24 by clamy@chromium.org, Jan 29 2018

There is no support for non-PlzNavigate code paths starting M65. So disabling it is a very temporary solution at best. Also it means taht you are not testing your code with the version of Chrome that actually ships to users.

Comment 25 by nasko@chromium.org, Jan 29 2018

It would be great if we can isolate a standalone set of documents/service worker scripts that can reproduce the issue independent of internal test infrastructure. I think this step will be crucial in investigating the issue and producing a fix in a timely manner.

Comment 26 by horo@chromium.org, Apr 26 2018

 Issue 836680  has been merged into this issue.
Project Member

Comment 27 by bugdroid1@chromium.org, Jun 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b92fcfb1cf296130f6cb38213aa52a0324c9e46

commit 6b92fcfb1cf296130f6cb38213aa52a0324c9e46
Author: clamy <clamy@chromium.org>
Date: Fri Jun 01 13:51:37 2018

Do not reset NavigationRequest in DidStopLoading

This CL fixes a race condition in which a NavigationRequest pending
commit is wrongly deleted when the RenderFrameHost receives a
DidStopLoading IPC. The NavigationRequest is now never deleted in
DidStopLoading. Instead, the RenderFrameHost passes a callback to

be called when the commit has been processed or cancelled. It's this
callback that deletes the NavigationRequest when the commit has been
cancelled.

FrameNavigationControl: :CommitNavigation and CommitFailedNavigation to
Bug: 789111,738177
Change-Id: Ib34b4b2d189ece0b23988f9bee6c5814b2e58356
Reviewed-on: https://chromium-review.googlesource.com/1064114
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Commit-Queue: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563610}
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/chrome/browser/captive_portal/captive_portal_tab_helper_unittest.cc
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/chrome/browser/page_load_metrics/observers/core_page_load_metrics_observer_unittest.cc
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/browser/frame_host/render_frame_host_impl.h
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/common/frame.mojom
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/public/test/navigation_simulator.cc
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/public/test/navigation_simulator.h
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/renderer/navigation_client.cc
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/renderer/navigation_state_impl.cc
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/renderer/navigation_state_impl.h
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/renderer/render_frame_impl.h
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/test/test_render_frame.cc
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/test/test_render_frame_host.cc
[modify] https://crrev.com/6b92fcfb1cf296130f6cb38213aa52a0324c9e46/content/test/test_render_frame_host.h

@horo, falken: the CL that has landed above should fix the issue reported in comment 15. We still have one race condition around multiple navigations trying to commit at the same time, and I'm working on a fix for this one.
Thanks for the update! That's great news.

Could the other race condition also contribute to this general bug? If so, I'll keep this open.

Do you think we can now (or after the other race is fixed) remove the weird path in ServiceWorkerDispatcherHost::OnProviderCreated:

    // If no host is found, create one.
    // TODO(crbug.com/789111#c14): This is probably not right, see bug.
    if (!provider_host) {
      GetContext()->AddProviderHost(ServiceWorkerProviderHost::Create(
          render_process_id_, std::move(info), GetContext()->AsWeakPtr()));
      return;
    }

I think we still need to keep the workaround until the other race condition is fixed. Hopefully, this should happen soon.
Project Member

Comment 31 by bugdroid1@chromium.org, Jun 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/302ab2ea33d59bf4ca3d9ab36f1f85984e322956

commit 302ab2ea33d59bf4ca3d9ab36f1f85984e322956
Author: Matt Falkenhagen <falken@chromium.org>
Date: Tue Jun 05 15:05:07 2018

Rebaseline service-worker-v8-cache.js

The test was mostly failing due to wrong text output. Probably it needed
rebaselining after enabling pwa-full-code cache (bug 788621).

But we didn't notice it since it had flaky TestExpectations due to bug
789111.

The test seems to also be crashing on Debug though, so I added
 bug 849670 .

Bug: 789111,  849670 , 788621
Change-Id: Id42679714a09ae1cb5c79324e4c224e56ae7a9ca
TBR: horo
Reviewed-on: https://chromium-review.googlesource.com/1086959
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564500}
[modify] https://crrev.com/302ab2ea33d59bf4ca3d9ab36f1f85984e322956/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/302ab2ea33d59bf4ca3d9ab36f1f85984e322956/third_party/WebKit/LayoutTests/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt

Project Member

Comment 32 by bugdroid1@chromium.org, Jun 5 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/52ab7ec9b6d3531ac11cde6f414ee8b434082a9c

commit 52ab7ec9b6d3531ac11cde6f414ee8b434082a9c
Author: Matt Falkenhagen <falken@chromium.org>
Date: Tue Jun 05 15:20:30 2018

Remove virtual/pwa-full-code-cache test suite.

pwa-full-code-cache is already enabled by default, so the virtual test
suite is redundant with the default test suite.

Note that the expectation file for service-worker-v8-cache.js differs
from the default. But both have TestExpectations for Pass Failure so
the expectations are essentially unused.

Bug: 788621, 789111,  480769 ,  847373 
Change-Id: I8c9d79ed104437a98edd6eba68ac3bef01e7f1a4
TBR: horo
Reviewed-on: https://chromium-review.googlesource.com/1086956
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564507}
[modify] https://crrev.com/52ab7ec9b6d3531ac11cde6f414ee8b434082a9c/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=HeapIncrementalMarkingStress
[modify] https://crrev.com/52ab7ec9b6d3531ac11cde6f414ee8b434082a9c/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees
[modify] https://crrev.com/52ab7ec9b6d3531ac11cde6f414ee8b434082a9c/third_party/WebKit/LayoutTests/SlowTests
[modify] https://crrev.com/52ab7ec9b6d3531ac11cde6f414ee8b434082a9c/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/52ab7ec9b6d3531ac11cde6f414ee8b434082a9c/third_party/WebKit/LayoutTests/VirtualTestSuites
[delete] https://crrev.com/829188c003b9048a6cd1c2bc4a1f4f68c8e940c3/third_party/WebKit/LayoutTests/virtual/pwa-full-code-cache/http/tests/devtools/service-workers/README.txt
[delete] https://crrev.com/829188c003b9048a6cd1c2bc4a1f4f68c8e940c3/third_party/WebKit/LayoutTests/virtual/pwa-full-code-cache/http/tests/devtools/service-workers/service-worker-v8-cache-expected.txt

 Issue 849856  has been merged into this issue.
Project Member

Comment 34 by bugdroid1@chromium.org, Jun 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ffa607a4bcb2d291da13b01547c391904eabd56e

commit ffa607a4bcb2d291da13b01547c391904eabd56e
Author: Matt Falkenhagen <falken@chromium.org>
Date: Wed Jun 06 00:25:32 2018

Gardening: Widen expectation for service-worker-v8-cache.js failures.

It's also failing on ASAN which I guess isn't part of [ Debug ].

Bug:  849670 , 789111
Change-Id: I93ceceb4c4c84390dc2253132a29c1cccde722fc
NOTRY: true
TBR: horo
Reviewed-on: https://chromium-review.googlesource.com/1087195
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564729}
[modify] https://crrev.com/ffa607a4bcb2d291da13b01547c391904eabd56e/third_party/WebKit/LayoutTests/TestExpectations

Project Member

Comment 35 by bugdroid1@chromium.org, Jun 6 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0385223775bcb0c8451af0450b8c2d1fed3b07ee

commit 0385223775bcb0c8451af0450b8c2d1fed3b07ee
Author: Chong Zhang <chongz@chromium.org>
Date: Wed Jun 06 18:59:12 2018

Disable http/tests/devtools/service-workers/service-worker-v8-cache.js on Mojo Linux

It's failing consistently on the Network Service bot, probably due to DCHECK.

Sample Build:
https://ci.chromium.org/buildbot/chromium.fyi/Mojo%20Linux/14273

TBR=falken@chromium.org,horo@chromium.org

Bug:  849670 , 789111
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I3d9da983aa3ba2006a75c27639da8e6fe716875c
Reviewed-on: https://chromium-review.googlesource.com/1089356
Commit-Queue: Chong Zhang <chongz@chromium.org>
Reviewed-by: Chong Zhang <chongz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564987}
[modify] https://crrev.com/0385223775bcb0c8451af0450b8c2d1fed3b07ee/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Components: -Platform>DevTools
clamy: I'm curious if there's been any update since comment 30:
> I think we still need to keep the workaround until the other race condition is fixed. Hopefully, this should happen soon.

Is there a bug about this race condition that this bug can be blocked on?
I've landed the fix for the other race condition. We still have an issue with some mismatch of checks at commit time between NavigationRequest and the commit message. This shouldn't happen for regular http/s navigations though, so I think from SW POV we should be fine.
c#38: Thanks, I tried to remove the workaround at https://chromium-review.googlesource.com/c/chromium/src/+/1391661 and found some reliably failing layout tests. Will continue to investigate.

Sign in to add a comment