New issue
Advanced search Search tips

Issue 879069 link

Starred by 1 user
Status: Fixed
Owner:
shimazu@chromium.org
Closed: Nov 5
Cc:
wanderview@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug



Sign in to add a comment

Service worker with inflight events directly dispatched may be stopped when activation

Project Member Reported by shimazu@chromium.org, Aug 30 
Activation needs to happens when a service worker isn't handling events. However, currently SWRegistration::IsReadyToActivate() only sees tasks in the browser, so activation might happen even when the worker is having tasks.

I found this when I'm seeing the code, so didn't check if it's really happen. We need to make a simple example (or test?) to reproduce it.

Comment 1 by falken@chromium.org, Aug 30 

I think I saw something similar, see the end of the commit description to https://chromium-review.googlesource.com/1125714

Comment 2 by wanderview@chromium.org, Aug 30

Cc: wanderview@chromium.org
Do you mean any task or just sw extendable events?  I think the spec refers to only extendable events blocking activation:

https://w3c.github.io/ServiceWorker/#service-worker-has-no-pending-events

Comment 3 by falken@chromium.org, Aug 30 

Yes I think we're talking about extendable events here (specifically fetch events).

Comment 4 by shimazu@chromium.org, Sep 5

Status: Started (was: Assigned)
Project Member

Comment 5 by bugdroid1@chromium.org, Oct 10 

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

commit 7bf1887dcf830f011222920693982db8f52ff9ad
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Wed Oct 10 06:13:13 2018

ServiceWorker: Triggers activation only when no event is handled

Currently we check only "HasNoWorkInBrowser()" when starting activation in
SWRegistration::IsReadyToActivate(). This CL is changing it to check tasks in
the renderer. Along with that, |idle_timer_fired_in_renderer_| kept in SWVersion
is renamed to |renderer_is_idle_| to make things clearer.

Bug:  879069 ,  878667 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ia57d5275af977d275f9a6eadabf82c97b74c2012
Reviewed-on: https://chromium-review.googlesource.com/c/1209134
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598221}
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/browser/service_worker/embedded_worker_instance.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/browser/service_worker/service_worker_job_unittest.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/browser/service_worker/service_worker_navigation_loader_unittest.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/browser/service_worker/service_worker_provider_host_unittest.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/browser/service_worker/service_worker_registration.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/browser/service_worker/service_worker_registration_unittest.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/browser/service_worker/service_worker_url_request_job_unittest.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/browser/service_worker/service_worker_version.h
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/browser/service_worker/service_worker_version_unittest.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/renderer/service_worker/service_worker_context_client.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/renderer/service_worker/service_worker_timeout_timer.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/renderer/service_worker/service_worker_timeout_timer.h
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/content/renderer/service_worker/service_worker_timeout_timer_unittest.cc
[modify] https://crrev.com/7bf1887dcf830f011222920693982db8f52ff9ad/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 10 

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

commit e2c7f32b61f695ec9be6f5c34278a4804fc558d5
Author: Makoto Shimazu <shimazu@chromium.org>
Date: Wed Oct 10 10:29:48 2018

S13nServiceWorker: Trigger activation even when devtools is open

This allows SWVersion to call StopWorkerIfIdle() even when DevTools is attached.
For making the SWVersion aware of activation, this CL introduces
TriggerIdleTerminationASAP() to SWVersion and it marks a flag to know the
service worker is going to be stopped soon.

This makes ServiceWorkerActivationTest added at crrev.com/c/1209134 pass with
S13nServiceWorker flag.

Test: content_unittests --enable-features=ServiceWorkerServicification '--gtest_filter=ServiceWorkerActivationTest*'

Bug:  879069 
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I20afa22f665d9e703e44dd731e6f8799b29fd813
Reviewed-on: https://chromium-review.googlesource.com/c/1245020
Commit-Queue: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598279}
[modify] https://crrev.com/e2c7f32b61f695ec9be6f5c34278a4804fc558d5/content/browser/service_worker/service_worker_registration.cc
[modify] https://crrev.com/e2c7f32b61f695ec9be6f5c34278a4804fc558d5/content/browser/service_worker/service_worker_registration_unittest.cc
[modify] https://crrev.com/e2c7f32b61f695ec9be6f5c34278a4804fc558d5/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/e2c7f32b61f695ec9be6f5c34278a4804fc558d5/content/browser/service_worker/service_worker_version.h
[modify] https://crrev.com/e2c7f32b61f695ec9be6f5c34278a4804fc558d5/third_party/WebKit/LayoutTests/FlagExpectations/enable-features=NetworkService

Comment 7 by falken@chromium.org, Oct 23 

shimazu: Is there anything left to do here?

Comment 8 by shimazu@chromium.org, Nov 5

Status: Fixed (was: Started)
oops, I missed this. It's fixed.
►

Sign in to add a comment