New issue
Advanced search Search tips

Issue 845341 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 849539



Sign in to add a comment

Eliminate ServiceWorkerDispatcherHost

Project Member Reported by leon....@intel.com, May 22 2018

Issue description

Make ServiceWorkerDispatcherHost not a BrowserMessageFilter, i.e. decouple content::mojom::ServiceWorkerDispatcherHost from the world of legacy IPC Channel-associated interfaces.
Thus all those Mojo interfaces associated with content::mojom::ServiceWorkerDispatcherHost will be de-associated from the legacy IPC Channel.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 24 2018

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

commit fac8e63e6d7fa91493ba3ad53d5b09e0d38901a2
Author: Han Leon <leon.han@intel.com>
Date: Thu May 24 06:18:54 2018

[ServiceWorker] Eliminate ServiceWorkerDispatcherHost::resource_context_

This CL lets ServiceWorkerProviderHost get the resource context from
ServiceWorkerContextWrapper directly, rather than getting from
ServiceWorkerDispatcherHost, then we no longer need to keep a resource
context in each ServiceWorkerDispatcherHost.

BUG=845341

Change-Id: Ia6dd603055607b3e2d53f5bbc7d5095c9514f954
Reviewed-on: https://chromium-review.googlesource.com/1068268
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Han Leon <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#561401}
[modify] https://crrev.com/fac8e63e6d7fa91493ba3ad53d5b09e0d38901a2/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/fac8e63e6d7fa91493ba3ad53d5b09e0d38901a2/content/browser/service_worker/embedded_worker_test_helper.cc
[modify] https://crrev.com/fac8e63e6d7fa91493ba3ad53d5b09e0d38901a2/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/fac8e63e6d7fa91493ba3ad53d5b09e0d38901a2/content/browser/service_worker/service_worker_dispatcher_host.h
[modify] https://crrev.com/fac8e63e6d7fa91493ba3ad53d5b09e0d38901a2/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc
[modify] https://crrev.com/fac8e63e6d7fa91493ba3ad53d5b09e0d38901a2/content/browser/service_worker/service_worker_handle_unittest.cc
[modify] https://crrev.com/fac8e63e6d7fa91493ba3ad53d5b09e0d38901a2/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/fac8e63e6d7fa91493ba3ad53d5b09e0d38901a2/content/browser/service_worker/service_worker_registration_unittest.cc

Project Member

Comment 2 by bugdroid1@chromium.org, May 28 2018

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

commit 43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1
Author: Han Leon <leon.han@intel.com>
Date: Mon May 28 06:53:14 2018

[ServiceWorker] ServiceWorkerProviderHost no longer needs ServiceWorkerDispatcherHost

In legacy IPCs era, ServiceWorkerProviderHost was relying on
ServiceWorkerDispatcherHost to send legacy IPCs to the renderer
process, now it has switched to Mojo and no longer needs
ServiceWorkerDispatcherHost to do any work.

This CL removes such a dependency aiming to make code logic around
ServiceWorkerDispatcherHost clearer, which will facilitate the future
refactor work on ServiceWorkerDispatcherHost.

BUG=845341

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo
Change-Id: Ibcf4d0aca0f3413def03ef1ddcf5862c6c994597
Reviewed-on: https://chromium-review.googlesource.com/1068514
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@{#562197}
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/embedded_worker_test_helper.cc
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/embedded_worker_test_helper.h
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_context_unittest.cc
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_dispatcher_host.h
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_handle_unittest.cc
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_provider_host.cc
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_provider_host.h
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_provider_host_unittest.cc
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_registration_unittest.cc
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_test_utils.cc
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_test_utils.h
[modify] https://crrev.com/43f2d98ad1066244e0f2f7a1ffbb3dd6f597a7f1/content/browser/service_worker/service_worker_version.cc

Project Member

Comment 3 by bugdroid1@chromium.org, May 31 2018

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

commit 64b2aa35c993c2a068dfa50b4245462ef4be5ac4
Author: Han Leon <leon.han@intel.com>
Date: Thu May 31 00:42:54 2018

[ServiceWorker] ServiceWorkerContextCore no longer needs to manage SWDispatcherHosts

After https://chromium-review.googlesource.com/c/chromium/src/+/1068514,
no code will try to find the specific ServiceWorkerDispatcherHost for a
given renderer process, so this CL removes the mapping management of
ServiceWorkerDispatcherHosts inside ServiceWorkerContextCore, in the
meantime simplifies ServiceWorkerDispatcherHost a little.

BUG=845341

Change-Id: Ie7747846570f64a7a4eda960878bbc0c97042952
Reviewed-on: https://chromium-review.googlesource.com/1075849
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Han Leon <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#563090}
[modify] https://crrev.com/64b2aa35c993c2a068dfa50b4245462ef4be5ac4/content/browser/background_sync/background_sync_manager_unittest.cc
[modify] https://crrev.com/64b2aa35c993c2a068dfa50b4245462ef4be5ac4/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/64b2aa35c993c2a068dfa50b4245462ef4be5ac4/content/browser/service_worker/service_worker_context_core.cc
[modify] https://crrev.com/64b2aa35c993c2a068dfa50b4245462ef4be5ac4/content/browser/service_worker/service_worker_context_core.h
[modify] https://crrev.com/64b2aa35c993c2a068dfa50b4245462ef4be5ac4/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/64b2aa35c993c2a068dfa50b4245462ef4be5ac4/content/browser/service_worker/service_worker_dispatcher_host.h
[modify] https://crrev.com/64b2aa35c993c2a068dfa50b4245462ef4be5ac4/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc

Comment 4 by leon....@intel.com, May 31 2018

Summary: Eliminate ServiceWorkerDispatcherHost (was: Decouple ServiceWorkerDispatcherHost from legacy IPC world)
Once we make clear about how to do what content::mojom::ServiceWorkerDispatcherHost.OnProviderCreated() does now in another way, then we can eliminate ServiceWorkerDispatcherHost class.
Blockedon: 849539
Project Member

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

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

commit 74c810c13e9ba0eebcc5fe4e72d750c6d50b54c3
Author: Han Leon <leon.han@intel.com>
Date: Thu Jun 21 13:06:28 2018

[ServiceWorker] Make ServiceWorkerDispatcherHost not a BrowserMessageFilter any more

As Service Worker IPCs are all in Mojo now, we no longer use
ServiceWorkerDispatcherHost to handle any legacy IPCs, its only usage is
to implement mojom::ServiceWorkerDispatcherHost interface, so, it no
longer needs to be a BrowserMessageFilter.
This CL makes ServiceWorkerDispatcherHost not a BrowserMessageFilter any
more, but still keeps its Mojo interface being associated with the
legacy IPC channel.

In future once we make clear of those potential races with some legacy
IPCs like navigation IPCs, we can consider putting
mojom::ServiceWorkerDispatcherHost on a dedicated Mojo message pipe, or
we'd remove mojom::ServiceWorkerDispatcherHost completely if we can find
an alternative/clear way to achieve the same goal.

BUG=845341

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I9f764017832a5e2f9ff15aff9b49b4c2a6834b78
Reviewed-on: https://chromium-review.googlesource.com/1105623
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Han Leon <leon.han@intel.com>
Cr-Commit-Position: refs/heads/master@{#569232}
[modify] https://crrev.com/74c810c13e9ba0eebcc5fe4e72d750c6d50b54c3/content/browser/bad_message.h
[modify] https://crrev.com/74c810c13e9ba0eebcc5fe4e72d750c6d50b54c3/content/browser/renderer_host/render_process_host_impl.cc
[modify] https://crrev.com/74c810c13e9ba0eebcc5fe4e72d750c6d50b54c3/content/browser/renderer_host/render_process_host_impl.h
[modify] https://crrev.com/74c810c13e9ba0eebcc5fe4e72d750c6d50b54c3/content/browser/service_worker/service_worker_dispatcher_host.cc
[modify] https://crrev.com/74c810c13e9ba0eebcc5fe4e72d750c6d50b54c3/content/browser/service_worker/service_worker_dispatcher_host.h
[modify] https://crrev.com/74c810c13e9ba0eebcc5fe4e72d750c6d50b54c3/content/browser/service_worker/service_worker_dispatcher_host_unittest.cc
[modify] https://crrev.com/74c810c13e9ba0eebcc5fe4e72d750c6d50b54c3/tools/metrics/histograms/enums.xml

Sign in to add a comment