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

Issue 790933 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 715640



Sign in to add a comment

NetworkService: ServiceWorker URLLoader are destroyed before the end of a navigation.

Project Member Reported by arthurso...@chromium.org, Dec 1 2017

Issue description

The NavigationURLLoaderNetworkService owns (more or less directly) the (appcache?)/serviceworker URLLoader. That's a problem.

When the navigation reaches the ReadyToCommit stage (when the headers are received), the navigation is transferred to the renderer and the NavigationURLLoaderNetworkService is deleted. It means the ServiceWorker URLLoader may be deleted before it has written the response's body. Several tests are flaky because of this.

I got this stacktrace (simplified):
#1 ~ServiceWorkerURLLoaderJob()
#2 ~ServiceWorkerURLJobWrapper()
#3 ~ServiceWorkerControlleeRequestHandler()
#4 std::__1::vector<>::~vector()
#5 NavigationURLLoaderNetworkService::URLLoaderRequestController:: ~URLLoaderRequestController()

The other URLLoader either owns themselves (See the FileURLLoader) or are owned by an object that will persist until the end of the request (in net::URLRequest for instance).

Note: patch https://chromium-review.googlesource.com/753738 will make ReadyToCommit happen earlier, several test will become flaky.
 
Owner: kinuko@chromium.org
I think appache's probably fine. I could take this.
Blocking: 715640
Project Member

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

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

commit eaa26226676f72947c83aad1da7c05c2760b6234
Author: Matt Falkenhagen <falken@chromium.org>
Date: Fri Dec 08 02:27:52 2017

Gardening: A new WPT service worker test is flaky in Network Service.

fetch-cors-exposed-header-names.https.html is a new test from r522082
that likely got flaky after r522470 like many other service worker
tests.

Bug:  790933 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: If44363e09a0ef519a95107d9ab9b07c1657c48a1
TBR: kinuko
NOTRY: true
Reviewed-on: https://chromium-review.googlesource.com/816475
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522683}
[modify] https://crrev.com/eaa26226676f72947c83aad1da7c05c2760b6234/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Status: Assigned (was: Untriaged)
Project Member

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

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

commit 4790a9c6b2b0b66fdb4a554bfd2b3ea3cdbf5d48
Author: Matt Falkenhagen <falken@chromium.org>
Date: Fri Dec 15 01:23:43 2017

Gardening: Network Service: Add another service worker test timeout.

WPT test navigation-redirect.https.html has been failing for a few
days, prrobably the same bug as the others.

Also update the expectations BUG format to the conventional style.

Bug:  790933 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Idc1cc54565f930ad5e8a87d8b73911b609252f27
TBR: kinuko
NOTRY: true
Reviewed-on: https://chromium-review.googlesource.com/828420
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524282}
[modify] https://crrev.com/4790a9c6b2b0b66fdb4a554bfd2b3ea3cdbf5d48/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 15 2017

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

commit 91a20d220cda899c9470ffae774e678850b6cc5a
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Fri Dec 15 14:03:28 2017

S13nServiceWorker: do not destroy SW's main resource loader too early

After the main resource loader is bound to a loader request
the loader shouldn't be deleted when the request handler is
deleted. (The loader should be able to outlive as far as the
loader's Mojo pipe is held by its client)

Bug:  790933 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: I81dab1599d1b0af3aa03f2acfed5f8edc4dd53dd
Reviewed-on: https://chromium-review.googlesource.com/828225
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524373}
[modify] https://crrev.com/91a20d220cda899c9470ffae774e678850b6cc5a/content/browser/service_worker/service_worker_url_job_wrapper.cc
[modify] https://crrev.com/91a20d220cda899c9470ffae774e678850b6cc5a/content/browser/service_worker/service_worker_url_loader_job.cc
[modify] https://crrev.com/91a20d220cda899c9470ffae774e678850b6cc5a/content/browser/service_worker/service_worker_url_loader_job.h
[modify] https://crrev.com/91a20d220cda899c9470ffae774e678850b6cc5a/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Comment 7 by kinuko@chromium.org, Dec 18 2017

Status: Fixed (was: Assigned)

Sign in to add a comment