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}

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),
-            std::move(params->installed_scripts_info->manager_receiver),
-            std::move(params->installed_scripts_info->manager_host_remote));
+            params->installed_scripts_info->manager_receiver.PassPipe(),
+            params->installed_scripts_info->manager_host_remote.PassPipe());
   }
 
   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),
-      std::move(params->content_settings_proxy), std::move(cache_storage),
-      std::move(browser_interface_broker));
+      params->content_settings_proxy.PassPipe(), cache_storage.PassPipe(),
+      browser_interface_broker.PassPipe());
 }
 
 void EmbeddedWorkerInstanceClientImpl::StopWorker() {

content/renderer/service_worker/service_worker_context_client.cc

@@ -47,7 +47,6 @@
 #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"
@@ -179,16 +178,14 @@ void ServiceWorkerContextClient::StartWorkerContextOnInitiatorThread(
     std::unique_ptr<blink::WebEmbeddedWorkerStartData> start_data,
     std::unique_ptr<blink::WebServiceWorkerInstalledScriptsManagerParams>
         installed_scripts_manager_params,
-    mojo::PendingRemote<blink::mojom::WorkerContentSettingsProxy>
-        content_settings,
-    mojo::PendingRemote<blink::mojom::CacheStorage> cache_storage,
-    mojo::PendingRemote<blink::mojom::BrowserInterfaceBroker>
-        browser_interface_broker) {
+    mojo::ScopedMessagePipeHandle content_settings_handle,
+    mojo::ScopedMessagePipeHandle cache_storage,
+    mojo::ScopedMessagePipeHandle 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), std::move(cache_storage),
+      std::move(content_settings_handle), std::move(cache_storage),
       std::move(browser_interface_broker), initiator_thread_task_runner_);
 }
 

content/renderer/service_worker/service_worker_context_client.h

@@ -36,8 +36,6 @@
 #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"
@@ -122,11 +120,9 @@ class CONTENT_EXPORT ServiceWorkerContextClient
       std::unique_ptr<blink::WebEmbeddedWorker> worker,
       std::unique_ptr<blink::WebEmbeddedWorkerStartData> start_data,
       std::unique_ptr<blink::WebServiceWorkerInstalledScriptsManagerParams>,
-      mojo::PendingRemote<blink::mojom::WorkerContentSettingsProxy>
-          content_settings,
-      mojo::PendingRemote<blink::mojom::CacheStorage> cache_storage,
-      mojo::PendingRemote<blink::mojom::BrowserInterfaceBroker>
-          browser_interface_broker);
+      mojo::ScopedMessagePipeHandle content_settings_handle,
+      mojo::ScopedMessagePipeHandle cache_storage,
+      mojo::ScopedMessagePipeHandle browser_interface_broker);
 
   // Called on the initiator thread.
   blink::WebEmbeddedWorker& worker();

third_party/blink/public/web/web_embedded_worker.h

@@ -33,11 +33,7 @@
 
 #include <memory>
 
-#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 "mojo/public/cpp/system/message_pipe.h"
 #include "third_party/blink/public/platform/web_common.h"
 #include "third_party/blink/public/platform/web_vector.h"
 
@@ -47,25 +43,23 @@ 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,
-      CrossVariantMojoReceiver<
-          mojom::ServiceWorkerInstalledScriptsManagerInterfaceBase>
-          manager_receiver,
-      CrossVariantMojoRemote<
-          mojom::ServiceWorkerInstalledScriptsManagerHostInterfaceBase>
-          manager_host_remote);
+      mojo::ScopedMessagePipeHandle manager_receiver,
+      mojo::ScopedMessagePipeHandle manager_host_remote);
   ~WebServiceWorkerInstalledScriptsManagerParams() = default;
 
   WebVector<WebURL> installed_scripts_urls;
-  CrossVariantMojoReceiver<
-      mojom::ServiceWorkerInstalledScriptsManagerInterfaceBase>
-      manager_receiver;
-  CrossVariantMojoRemote<
-      mojom::ServiceWorkerInstalledScriptsManagerHostInterfaceBase>
-      manager_host_remote;
+  // 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;
 };
 
 // An interface to start and terminate an embedded worker.
@@ -84,11 +78,9 @@ class BLINK_EXPORT WebEmbeddedWorker {
   virtual void StartWorkerContext(
       std::unique_ptr<WebEmbeddedWorkerStartData>,
       std::unique_ptr<WebServiceWorkerInstalledScriptsManagerParams>,
-      CrossVariantMojoRemote<mojom::WorkerContentSettingsProxyInterfaceBase>
-          content_settings,
-      CrossVariantMojoRemote<mojom::CacheStorageInterfaceBase> cache_storage,
-      CrossVariantMojoRemote<mojom::BrowserInterfaceBrokerInterfaceBase>
-          browser_interface_broker,
+      mojo::ScopedMessagePipeHandle content_settings_handle,
+      mojo::ScopedMessagePipeHandle cache_storage,
+      mojo::ScopedMessagePipeHandle 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,12 +76,8 @@ namespace blink {
 WebServiceWorkerInstalledScriptsManagerParams::
     WebServiceWorkerInstalledScriptsManagerParams(
         WebVector<WebURL> installed_scripts_urls,
-        CrossVariantMojoReceiver<
-            mojom::blink::ServiceWorkerInstalledScriptsManagerInterfaceBase>
-            manager_receiver,
-        CrossVariantMojoRemote<
-            mojom::blink::ServiceWorkerInstalledScriptsManagerHostInterfaceBase>
-            manager_host_remote)
+        mojo::ScopedMessagePipeHandle manager_receiver,
+        mojo::ScopedMessagePipeHandle 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)) {
@@ -109,12 +105,9 @@ void WebEmbeddedWorkerImpl::StartWorkerContext(
     std::unique_ptr<WebEmbeddedWorkerStartData> worker_start_data,
     std::unique_ptr<WebServiceWorkerInstalledScriptsManagerParams>
         installed_scripts_manager_params,
-    CrossVariantMojoRemote<
-        mojom::blink::WorkerContentSettingsProxyInterfaceBase> content_settings,
-    CrossVariantMojoRemote<mojom::blink::CacheStorageInterfaceBase>
-        cache_storage,
-    CrossVariantMojoRemote<mojom::blink::BrowserInterfaceBrokerInterfaceBase>
-        browser_interface_broker,
+    mojo::ScopedMessagePipeHandle content_settings_handle,
+    mojo::ScopedMessagePipeHandle cache_storage,
+    mojo::ScopedMessagePipeHandle browser_interface_broker,
     scoped_refptr<base::SingleThreadTaskRunner> initiator_thread_task_runner) {
   DCHECK(!asked_to_terminate_);
 
@@ -145,8 +138,15 @@ void WebEmbeddedWorkerImpl::StartWorkerContext(
   StartWorkerThread(
       std::move(worker_start_data), std::move(installed_scripts_manager),
       std::make_unique<ServiceWorkerContentSettingsProxy>(
-          std::move(content_settings)),
-      std::move(cache_storage), std::move(browser_interface_broker),
+          // 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(initiator_thread_task_runner));
 }
 

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

@@ -66,13 +66,9 @@ class MODULES_EXPORT WebEmbeddedWorkerImpl final : public WebEmbeddedWorker {
   void StartWorkerContext(
       std::unique_ptr<WebEmbeddedWorkerStartData>,
       std::unique_ptr<WebServiceWorkerInstalledScriptsManagerParams>,
-      CrossVariantMojoRemote<
-          mojom::blink::WorkerContentSettingsProxyInterfaceBase>
-          content_settings,
-      CrossVariantMojoRemote<mojom::blink::CacheStorageInterfaceBase>
-          cache_storage,
-      CrossVariantMojoRemote<mojom::blink::BrowserInterfaceBrokerInterfaceBase>
-          browser_interface_broker,
+      mojo::ScopedMessagePipeHandle content_settings_handle,
+      mojo::ScopedMessagePipeHandle cache_storage,
+      mojo::ScopedMessagePipeHandle 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,7 +262,10 @@ ServiceWorkerInstalledScriptsManager::ServiceWorkerInstalledScriptsManager(
   DCHECK(installed_scripts_manager_params->manager_host_remote);
   manager_host_ = mojo::SharedRemote<
       mojom::blink::ServiceWorkerInstalledScriptsManagerHost>(
-      std::move(installed_scripts_manager_params->manager_host_remote));
+      mojo::PendingRemote<
+          mojom::blink::ServiceWorkerInstalledScriptsManagerHost>(
+          std::move(installed_scripts_manager_params->manager_host_remote),
+          mojom::blink::ServiceWorkerInstalledScriptsManagerHost::Version_));
 
   // 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,8 +4,6 @@
 
 #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"
@@ -146,8 +144,8 @@ class ServiceWorkerInstalledScriptsManagerTest : public testing::Test {
     auto installed_scripts_manager_params =
         std::make_unique<WebServiceWorkerInstalledScriptsManagerParams>(
             std::move(installed_scripts_info->installed_urls),
-            std::move(installed_scripts_info->manager_receiver),
-            std::move(installed_scripts_info->manager_host_remote));
+            installed_scripts_info->manager_receiver.PassPipe(),
+            installed_scripts_info->manager_host_remote.PassPipe());
     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::NullRemote(),
-      /*cache_storage_remote=*/mojo::NullRemote(),
-      browser_interface_broker.BindNewPipeAndPassRemote(),
+      /*content_settings_proxy=*/mojo::ScopedMessagePipeHandle(),
+      /*cache_storage_remote=*/mojo::ScopedMessagePipeHandle(),
+      browser_interface_broker.BindNewPipeAndPassRemote().PassPipe(),
       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::NullRemote(),
-      /*cache_storage_remote=*/mojo::NullRemote(),
-      browser_interface_broker.BindNewPipeAndPassRemote(),
+      /*content_settings_proxy=*/mojo::ScopedMessagePipeHandle(),
+      /*cache_storage_remote=*/mojo::ScopedMessagePipeHandle(),
+      browser_interface_broker.BindNewPipeAndPassRemote().PassPipe(),
       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::NullRemote(),
-      /*cache_storage_remote=*/mojo::NullRemote(),
-      browser_interface_broker.BindNewPipeAndPassRemote(),
+      /*content_settings_proxy=*/mojo::ScopedMessagePipeHandle(),
+      /*cache_storage_remote=*/mojo::ScopedMessagePipeHandle(),
+      browser_interface_broker.BindNewPipeAndPassRemote().PassPipe(),
       Thread::Current()->GetTaskRunner());
   testing::Mock::VerifyAndClearExpectations(mock_client_.get());