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

Issue 772788 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug


Sign in to add a comment

Impl mojo interface ServiceWorkerObject

Project Member Reported by leon....@intel.com, Oct 9 2017

Issue description

Put into third_party/WebKit/public/platform/modules/serviceworker/service_worker_object.mojom:
'
interface ServiceWorkerObject {
  // Replace ServiceWorkerMsg_ServiceWorkerStateChanged.
  StateChanged(blink.mojom.ServiceWorkerState state);
};
'
 

Comment 1 by leon....@intel.com, Nov 6 2017

Blocking: 384119
Recording Some ideas at this doc:
https://docs.google.com/document/d/1D0yLQ_7IV48JBCpb0k8O8yBIAs7f_QFM9bbMqDgsHmw/edit#
Although partly done, PTAL to help check whether it makes sense, Thanks.

Comment 2 by leon....@intel.com, Nov 8 2017

Blocking: 745327

Comment 3 by leon....@intel.com, Nov 9 2017

Blocking: 783071

Comment 4 by leon....@intel.com, Jan 30 2018

Blocking: 807126
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 9 2018

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

commit 29e56b134e4066a8197b00e5d3b4e6f97ea0986c
Author: Han Leon <leon.han@intel.com>
Date: Fri Feb 09 05:21:02 2018

[ServiceWorker] Correct the context of ExtendableMessageEvent's source sw object

If an ExtendableMessageEvent originates from a sw execution context,
when the browser side tries to dispatch it, the source should be a sw
object corresponding to the source ServiceWorkerVersion.

Previously, GetOrCreateServiceWorkerHandle() of the source service
worker provider host is used for making the service worker info.
However, the object info should be associated with the destination's
provider host since the corresponding JavaScript object is used in the
destination.

In this CL, GetOrCreateServiceWorkerHandle() is called for the
destination's provider host instead of the source's one.

BUG= 772788 

Change-Id: Ifb8b6e1f8d94e460a66664c43e9336d2018de43a
Reviewed-on: https://chromium-review.googlesource.com/903105
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#535650}
[modify] https://crrev.com/29e56b134e4066a8197b00e5d3b4e6f97ea0986c/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/29e56b134e4066a8197b00e5d3b4e6f97ea0986c/content/browser/service_worker/service_worker_dispatcher_host.h
[modify] https://crrev.com/29e56b134e4066a8197b00e5d3b4e6f97ea0986c/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc

Comment 6 by leon....@intel.com, Mar 18 2018

Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 17 2018

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

commit d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5
Author: Han Leon <leon.han@intel.com>
Date: Tue Apr 17 15:39:25 2018

[ServiceWorker] Enter the pure Mojo world

This CL aims to mojofy the last one legacy IPC
ServiceWorkerMsg_ServiceWorkerStateChanged, then service worker code
finally enter the pure Mojo world!

Currently ServiceWorkerRegistrationObject and ServiceWorkerContainer are
channel-associated interfaces. For correctness, ServiceWorkerObject
needs to have FIFO ordering with those interfaces, which means it would
need to be a channel-associated interface as well. But when used for
service worker execution contexts, we have been binding
channel-associated interfaces on the IO thread rather than the service
worker thread due to constraints of channel-associated interfaces.[1]
But we want ServiceWorkerObject to be bound on the service worker
thread. Actually we want SWContainer and SWRO to be bound on the service
worker thread too instead of bouncing to the IO thread. Until now, we
have worked around this using hacks like CallbackWrapperOnWorkerThread
to mediate between the IO thread and service worker thread. But we do
not want to keep doing these hacks. Since we no longer have legacy IPCs,
there is no need for these interfaces to be channel-associated
interfaces. So this CL makes these no longer channel-associated
interfaces so they can be bound on the service worker thread directly.
[1] https://groups.google.com/a/chromium.org/forum/#!topic/chromium-mojo/RDO46Py0F3s

Based on the above analysis, this CL goes a long way to introduce
ServiceWorkerObject interface. This CL:
ps#1 - firstly decouples EmbeddedWorkerInstanceClient interface from the
   world of Channel-associated interfaces, makes it use a dedicated Mojo
   message pipe, this turns out to make its several attached interfaces
   not be Channel-associated interfaces, either, like
   ServiceWorkerRegistrationObject, ServiceWorkerHost etc.
ps#2 - then, for service worker execution contexts, starts to bind
   endpoints for interfaces like ServiceWorkerHost,
   ServiceWorkerObject[Host], ServiceWorkerRegistrationObject[Host] etc.
   on the service worker thread directly, rather than binding them on
   the IO thread and using PostTask to actually send/receive Mojo
   messages on the worker thread, this had been forced to be so because
   they were Channel-associated interfaces before.
ps#6 - after the above preparations, introduces ServiceWorkerObject
   interface and mojofies the state changed legacy IPC, it's also
   associated with EmbeddedWorkerInstanceClient interface and now we can
   just bind its receiver endpoint directly on the service worker
   thread.

Follow-up work:
  - CL https://chromium-review.googlesource.com/c/chromium/src/+/981898
    Associate all interfaces living on the service worker thread with
    ServiceWorkerEventDispatcher interface rather than current
    EmbeddedWorkerInstanceClient interface. This will not only guarantee
    the FIFO ordering with ServiceWorkerEventDispatcher, but also make
    all Mojo messages dispatch skip the main thread completely to avoid
    possible congestion.
  - Fix the layout test
    external/wpt/service-workers/service-worker/skip-waiting-installed.https.html
    which becomes flaky due to this CL.
  - Remove channel-associated interfaces entirely for service worker
    code. This CL makes interfaces for SW execution contexts to be not
    channel-associated. A follow-up CL will make the same change for
    interfaces for SW client contexts.

BUG= 772788 , 783071 

Change-Id: I3a6a666a41b4edc2289dceee6e071ef9510e2012
Reviewed-on: https://chromium-review.googlesource.com/967863
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Han Leon <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#551343}
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/embedded_worker_instance.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/embedded_worker_instance.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/embedded_worker_instance_unittest.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/embedded_worker_test_helper.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/embedded_worker_test_helper.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_dispatcher_host.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_handle.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_handle.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_handle_unittest.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_provider_host.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_registration_object_host.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_registration_object_host.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_registration_unittest.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_test_utils.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/browser/service_worker/service_worker_version.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/common/renderer.mojom
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/common/service_worker/service_worker_messages.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/render_thread_impl.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/embedded_worker_instance_client_impl.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/embedded_worker_instance_client_impl.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/service_worker_context_client.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/service_worker_dispatcher.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/service_worker_dispatcher.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/service_worker_dispatcher_unittest.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/service_worker_message_filter.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/service_worker_message_filter.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/service_worker_provider_context.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/service_worker_provider_context.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/service_worker_provider_context_unittest.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/web_service_worker_impl.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/web_service_worker_impl.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/web_service_worker_registration_impl.cc
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/content/renderer/service_worker/web_service_worker_registration_impl.h
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/third_party/blink/public/mojom/service_worker/service_worker_object.mojom
[modify] https://crrev.com/d24f8b2abbb47b3b76e3b3cf3c2af24b96ad37e5/third_party/blink/public/mojom/service_worker/service_worker_registration.mojom

Comment 8 by leon....@intel.com, May 15 2018

Status: Fixed (was: Started)

Sign in to add a comment