Reland "Use typesafe helpers to pass WebEmbeddedWorker Mojo pipes across Blink public API"

This reverts commit 19d1314e.

Reason for revert: The speculative revert did not result in
improvement.

Original change's description:
> Revert "Use typesafe helpers to pass WebEmbeddedWorker Mojo pipes across Blink public API"
> 
> This reverts commit da054331.
> The code review was https://chromium-review.googlesource.com/c/chromium/src/+/2227103
> 
> Super speculative revert since we have no other clues about the perf
> regression which started in a range the CL landed in.
> 
> Bug: 1096423, 1059157
> Tbr: dcheng
> Change-Id: Ia291b0c16c0cacfcdd40387a6b8badd135fcf7b6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2355630
> Commit-Queue: Matt Falkenhagen <falken@chromium.org>
> Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#798067}

TBR=falken@chromium.org,dcheng@chromium.org,shimazu@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1096423
Bug: 1059157
Change-Id: Ibaa28a209cc2ac79d6e60b78c5b1d0860d65f4c9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2366172Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799963}

content/renderer/service_worker/embedded_worker_instance_client_impl.cc

@@ -108,8 +108,8 @@ void EmbeddedWorkerInstanceClientImpl::StartWorker(
     installed_scripts_manager_params =
         std::make_unique<blink::WebServiceWorkerInstalledScriptsManagerParams>(
             std::move(params->installed_scripts_info->installed_urls),
-            params->installed_scripts_info->manager_receiver.PassPipe(),
-            params->installed_scripts_info->manager_host_remote.PassPipe());
+            std::move(params->installed_scripts_info->manager_receiver),
+            std::move(params->installed_scripts_info->manager_host_remote));
   }
 
   auto worker =
@@ -117,8 +117,8 @@ void EmbeddedWorkerInstanceClientImpl::StartWorker(
   service_worker_context_client_->StartWorkerContextOnInitiatorThread(
       std::move(worker), std::move(start_data),
       std::move(installed_scripts_manager_params),
-      params->content_settings_proxy.PassPipe(), cache_storage.PassPipe(),
-      browser_interface_broker.PassPipe());
+      std::move(params->content_settings_proxy), std::move(cache_storage),
+      std::move(browser_interface_broker));
 }
 
 void EmbeddedWorkerInstanceClientImpl::StopWorker() {

content/renderer/service_worker/service_worker_context_client.cc

@@ -47,6 +47,7 @@
 #include "third_party/blink/public/common/service_worker/service_worker_utils.h"
 #include "third_party/blink/public/mojom/blob/blob.mojom.h"
 #include "third_party/blink/public/mojom/blob/blob_registry.mojom.h"
+#include "third_party/blink/public/mojom/browser_interface_broker.mojom.h"
 #include "third_party/blink/public/mojom/loader/request_context_frame_type.mojom.h"
 #include "third_party/blink/public/mojom/service_worker/service_worker.mojom.h"
 #include "third_party/blink/public/mojom/service_worker/service_worker_client.mojom.h"
@@ -178,14 +179,16 @@ void ServiceWorkerContextClient::StartWorkerContextOnInitiatorThread(
     std::unique_ptr<blink::WebEmbeddedWorkerStartData> start_data,
     std::unique_ptr<blink::WebServiceWorkerInstalledScriptsManagerParams>
         installed_scripts_manager_params,
-    mojo::ScopedMessagePipeHandle content_settings_handle,
-    mojo::ScopedMessagePipeHandle cache_storage,
-    mojo::ScopedMessagePipeHandle browser_interface_broker) {
+    mojo::PendingRemote<blink::mojom::WorkerContentSettingsProxy>
+        content_settings,
+    mojo::PendingRemote<blink::mojom::CacheStorage> cache_storage,
+    mojo::PendingRemote<blink::mojom::BrowserInterfaceBroker>
+        browser_interface_broker) {
   DCHECK(initiator_thread_task_runner_->RunsTasksInCurrentSequence());
   worker_ = std::move(worker);
   worker_->StartWorkerContext(
       std::move(start_data), std::move(installed_scripts_manager_params),
-      std::move(content_settings_handle), std::move(cache_storage),
+      std::move(content_settings), std::move(cache_storage),
       std::move(browser_interface_broker), initiator_thread_task_runner_);
 }
 

content/renderer/service_worker/service_worker_context_client.h

@@ -36,6 +36,8 @@
 #include "third_party/blink/public/mojom/service_worker/service_worker_provider.mojom-forward.h"
 #include "third_party/blink/public/mojom/service_worker/service_worker_registration.mojom-forward.h"
 #include "third_party/blink/public/mojom/worker/subresource_loader_updater.mojom-forward.h"
+#include "third_party/blink/public/mojom/worker/worker_content_settings_proxy.mojom-forward.h"
+#include "third_party/blink/public/mojom/worker/worker_content_settings_proxy.mojom.h"
 #include "third_party/blink/public/platform/modules/service_worker/web_service_worker_error.h"
 #include "third_party/blink/public/web/modules/service_worker/web_service_worker_context_client.h"
 #include "third_party/blink/public/web/modules/service_worker/web_service_worker_context_proxy.h"
@@ -120,9 +122,11 @@ class CONTENT_EXPORT ServiceWorkerContextClient
       std::unique_ptr<blink::WebEmbeddedWorker> worker,
       std::unique_ptr<blink::WebEmbeddedWorkerStartData> start_data,
       std::unique_ptr<blink::WebServiceWorkerInstalledScriptsManagerParams>,
-      mojo::ScopedMessagePipeHandle content_settings_handle,
-      mojo::ScopedMessagePipeHandle cache_storage,
-      mojo::ScopedMessagePipeHandle browser_interface_broker);
+      mojo::PendingRemote<blink::mojom::WorkerContentSettingsProxy>
+          content_settings,
+      mojo::PendingRemote<blink::mojom::CacheStorage> cache_storage,
+      mojo::PendingRemote<blink::mojom::BrowserInterfaceBroker>
+          browser_interface_broker);
 
   // Called on the initiator thread.
   blink::WebEmbeddedWorker& worker();

third_party/blink/public/web/web_embedded_worker.h

@@ -33,7 +33,11 @@
 
 #include <memory>
 
-#include "mojo/public/cpp/system/message_pipe.h"
+#include "third_party/blink/public/mojom/browser_interface_broker.mojom-shared.h"
+#include "third_party/blink/public/mojom/cache_storage/cache_storage.mojom-shared.h"
+#include "third_party/blink/public/mojom/service_worker/service_worker_installed_scripts_manager.mojom-shared.h"
+#include "third_party/blink/public/mojom/worker/worker_content_settings_proxy.mojom-shared.h"
+#include "third_party/blink/public/platform/cross_variant_mojo_util.h"
 #include "third_party/blink/public/platform/web_common.h"
 #include "third_party/blink/public/platform/web_vector.h"
 
@@ -43,23 +47,25 @@ class WebServiceWorkerContextClient;
 class WebURL;
 struct WebEmbeddedWorkerStartData;
 
-// TODO(crbug.com/1096423): Use CrossVariantMojoRemote if the perf regression is
-// not caused by it.
 struct BLINK_EXPORT WebServiceWorkerInstalledScriptsManagerParams {
   WebServiceWorkerInstalledScriptsManagerParams() = delete;
   WebServiceWorkerInstalledScriptsManagerParams(
       WebVector<WebURL> installed_scripts_urls,
-      mojo::ScopedMessagePipeHandle manager_receiver,
-      mojo::ScopedMessagePipeHandle manager_host_remote);
+      CrossVariantMojoReceiver<
+          mojom::ServiceWorkerInstalledScriptsManagerInterfaceBase>
+          manager_receiver,
+      CrossVariantMojoRemote<
+          mojom::ServiceWorkerInstalledScriptsManagerHostInterfaceBase>
+          manager_host_remote);
   ~WebServiceWorkerInstalledScriptsManagerParams() = default;
 
   WebVector<WebURL> installed_scripts_urls;
-  // A handle for
-  // mojo::PendingReceiver<mojom::blink::ServiceWorkerInstalledScriptsManager>.
-  mojo::ScopedMessagePipeHandle manager_receiver;
-  // A handle for
-  // mojo::PendingRemote<mojom::blink::ServiceWorkerInstalledScriptsManagerHost>.
-  mojo::ScopedMessagePipeHandle manager_host_remote;
+  CrossVariantMojoReceiver<
+      mojom::ServiceWorkerInstalledScriptsManagerInterfaceBase>
+      manager_receiver;
+  CrossVariantMojoRemote<
+      mojom::ServiceWorkerInstalledScriptsManagerHostInterfaceBase>
+      manager_host_remote;
 };
 
 // An interface to start and terminate an embedded worker.
@@ -78,9 +84,11 @@ class BLINK_EXPORT WebEmbeddedWorker {
   virtual void StartWorkerContext(
       std::unique_ptr<WebEmbeddedWorkerStartData>,
       std::unique_ptr<WebServiceWorkerInstalledScriptsManagerParams>,
-      mojo::ScopedMessagePipeHandle content_settings_handle,
-      mojo::ScopedMessagePipeHandle cache_storage,
-      mojo::ScopedMessagePipeHandle browser_interface_broker,
+      CrossVariantMojoRemote<mojom::WorkerContentSettingsProxyInterfaceBase>
+          content_settings,
+      CrossVariantMojoRemote<mojom::CacheStorageInterfaceBase> cache_storage,
+      CrossVariantMojoRemote<mojom::BrowserInterfaceBrokerInterfaceBase>
+          browser_interface_broker,
       scoped_refptr<base::SingleThreadTaskRunner>
           initiator_thread_task_runner) = 0;
   virtual void TerminateWorkerContext() = 0;

third_party/blink/renderer/modules/exported/web_embedded_worker_impl.cc

@@ -76,8 +76,12 @@ namespace blink {
 WebServiceWorkerInstalledScriptsManagerParams::
     WebServiceWorkerInstalledScriptsManagerParams(
         WebVector<WebURL> installed_scripts_urls,
-        mojo::ScopedMessagePipeHandle manager_receiver,
-        mojo::ScopedMessagePipeHandle manager_host_remote)
+        CrossVariantMojoReceiver<
+            mojom::blink::ServiceWorkerInstalledScriptsManagerInterfaceBase>
+            manager_receiver,
+        CrossVariantMojoRemote<
+            mojom::blink::ServiceWorkerInstalledScriptsManagerHostInterfaceBase>
+            manager_host_remote)
     : installed_scripts_urls(std::move(installed_scripts_urls)),
       manager_receiver(std::move(manager_receiver)),
       manager_host_remote(std::move(manager_host_remote)) {
@@ -105,9 +109,12 @@ void WebEmbeddedWorkerImpl::StartWorkerContext(
     std::unique_ptr<WebEmbeddedWorkerStartData> worker_start_data,
     std::unique_ptr<WebServiceWorkerInstalledScriptsManagerParams>
         installed_scripts_manager_params,
-    mojo::ScopedMessagePipeHandle content_settings_handle,
-    mojo::ScopedMessagePipeHandle cache_storage,
-    mojo::ScopedMessagePipeHandle browser_interface_broker,
+    CrossVariantMojoRemote<
+        mojom::blink::WorkerContentSettingsProxyInterfaceBase> content_settings,
+    CrossVariantMojoRemote<mojom::blink::CacheStorageInterfaceBase>
+        cache_storage,
+    CrossVariantMojoRemote<mojom::blink::BrowserInterfaceBrokerInterfaceBase>
+        browser_interface_broker,
     scoped_refptr<base::SingleThreadTaskRunner> initiator_thread_task_runner) {
   DCHECK(!asked_to_terminate_);
 
@@ -138,15 +145,8 @@ void WebEmbeddedWorkerImpl::StartWorkerContext(
   StartWorkerThread(
       std::move(worker_start_data), std::move(installed_scripts_manager),
       std::make_unique<ServiceWorkerContentSettingsProxy>(
-          // Chrome doesn't use interface versioning.
-          // TODO(falken): Is that comment about versioning correct?
-          mojo::PendingRemote<mojom::blink::WorkerContentSettingsProxy>(
-              std::move(content_settings_handle), 0u)),
-      mojo::PendingRemote<mojom::blink::CacheStorage>(
-          std::move(cache_storage), mojom::blink::CacheStorage::Version_),
-      mojo::PendingRemote<mojom::blink::BrowserInterfaceBroker>(
-          std::move(browser_interface_broker),
-          mojom::blink::BrowserInterfaceBroker::Version_),
+          std::move(content_settings)),
+      std::move(cache_storage), std::move(browser_interface_broker),
       std::move(initiator_thread_task_runner));
 }
 

third_party/blink/renderer/modules/exported/web_embedded_worker_impl.h

@@ -66,9 +66,13 @@ class MODULES_EXPORT WebEmbeddedWorkerImpl final : public WebEmbeddedWorker {
   void StartWorkerContext(
       std::unique_ptr<WebEmbeddedWorkerStartData>,
       std::unique_ptr<WebServiceWorkerInstalledScriptsManagerParams>,
-      mojo::ScopedMessagePipeHandle content_settings_handle,
-      mojo::ScopedMessagePipeHandle cache_storage,
-      mojo::ScopedMessagePipeHandle browser_interface_broker,
+      CrossVariantMojoRemote<
+          mojom::blink::WorkerContentSettingsProxyInterfaceBase>
+          content_settings,
+      CrossVariantMojoRemote<mojom::blink::CacheStorageInterfaceBase>
+          cache_storage,
+      CrossVariantMojoRemote<mojom::blink::BrowserInterfaceBrokerInterfaceBase>
+          browser_interface_broker,
       scoped_refptr<base::SingleThreadTaskRunner> initiator_thread_task_runner)
       override;
   void TerminateWorkerContext() override;

third_party/blink/renderer/modules/service_worker/service_worker_installed_scripts_manager.cc

@@ -262,10 +262,7 @@ ServiceWorkerInstalledScriptsManager::ServiceWorkerInstalledScriptsManager(
   DCHECK(installed_scripts_manager_params->manager_host_remote);
   manager_host_ = mojo::SharedRemote<
       mojom::blink::ServiceWorkerInstalledScriptsManagerHost>(
-      mojo::PendingRemote<
-          mojom::blink::ServiceWorkerInstalledScriptsManagerHost>(
-          std::move(installed_scripts_manager_params->manager_host_remote),
-          mojom::blink::ServiceWorkerInstalledScriptsManagerHost::Version_));
+      std::move(installed_scripts_manager_params->manager_host_remote));
 
   // Don't touch |installed_urls_| after this point. We're on the initiator
   // thread now, but |installed_urls_| will be accessed on the

third_party/blink/renderer/modules/service_worker/service_worker_installed_scripts_manager_test.cc

@@ -4,6 +4,8 @@
 
 #include "third_party/blink/renderer/modules/service_worker/service_worker_installed_scripts_manager.h"
 
+#include <utility>
+
 #include "base/run_loop.h"
 #include "base/synchronization/waitable_event.h"
 #include "mojo/public/cpp/bindings/receiver.h"
@@ -144,8 +146,8 @@ class ServiceWorkerInstalledScriptsManagerTest : public testing::Test {
     auto installed_scripts_manager_params =
         std::make_unique<WebServiceWorkerInstalledScriptsManagerParams>(
             std::move(installed_scripts_info->installed_urls),
-            installed_scripts_info->manager_receiver.PassPipe(),
-            installed_scripts_info->manager_host_remote.PassPipe());
+            std::move(installed_scripts_info->manager_receiver),
+            std::move(installed_scripts_info->manager_host_remote));
     installed_scripts_manager_ =
         std::make_unique<ServiceWorkerInstalledScriptsManager>(
             std::move(installed_scripts_manager_params),

third_party/blink/renderer/modules/service_worker/web_embedded_worker_impl_test.cc

@@ -299,9 +299,9 @@ TEST_F(WebEmbeddedWorkerImplTest, TerminateSoonAfterStart) {
   worker_->StartWorkerContext(
       CreateStartData(),
       /*installed_scripts_manager_params=*/nullptr,
-      /*content_settings_proxy=*/mojo::ScopedMessagePipeHandle(),
-      /*cache_storage_remote=*/mojo::ScopedMessagePipeHandle(),
-      browser_interface_broker.BindNewPipeAndPassRemote().PassPipe(),
+      /*content_settings_proxy=*/mojo::NullRemote(),
+      /*cache_storage_remote=*/mojo::NullRemote(),
+      browser_interface_broker.BindNewPipeAndPassRemote(),
       Thread::Current()->GetTaskRunner());
   testing::Mock::VerifyAndClearExpectations(mock_client_.get());
 
@@ -318,9 +318,9 @@ TEST_F(WebEmbeddedWorkerImplTest, TerminateWhileWaitingForDebugger) {
   worker_->StartWorkerContext(
       std::move(start_data),
       /*installed_scripts_manager_params=*/nullptr,
-      /*content_settings_proxy=*/mojo::ScopedMessagePipeHandle(),
-      /*cache_storage_remote=*/mojo::ScopedMessagePipeHandle(),
-      browser_interface_broker.BindNewPipeAndPassRemote().PassPipe(),
+      /*content_settings_proxy=*/mojo::NullRemote(),
+      /*cache_storage_remote=*/mojo::NullRemote(),
+      browser_interface_broker.BindNewPipeAndPassRemote(),
       Thread::Current()->GetTaskRunner());
   testing::Mock::VerifyAndClearExpectations(mock_client_.get());
 
@@ -340,9 +340,9 @@ TEST_F(WebEmbeddedWorkerImplTest, ScriptNotFound) {
   worker_->StartWorkerContext(
       std::move(start_data),
       /*installed_scripts_manager_params=*/nullptr,
-      /*content_settings_proxy=*/mojo::ScopedMessagePipeHandle(),
-      /*cache_storage_remote=*/mojo::ScopedMessagePipeHandle(),
-      browser_interface_broker.BindNewPipeAndPassRemote().PassPipe(),
+      /*content_settings_proxy=*/mojo::NullRemote(),
+      /*cache_storage_remote=*/mojo::NullRemote(),
+      browser_interface_broker.BindNewPipeAndPassRemote(),
       Thread::Current()->GetTaskRunner());
   testing::Mock::VerifyAndClearExpectations(mock_client_.get());