[Extensions] Use mojom::EventListener for basic service worker listeners

Use mojom::EventListener in EventRouter::AddListenerForServiceWorker() and EventRouter::RemoveListenerForServiceWorker(). This CL has no behavior change. Bug: 1470045 Change-Id: I01a6ddf23b543dc5f79e50ac5ed63197efefe1f0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4950756 Reviewed-by: Alex Gough <ajgo@chromium.org> Commit-Queue: Devlin Cronin <rdevlin.cronin@chromium.org> Cr-Commit-Position: refs/heads/main@{#1211647}
chromium · Oct 18, 2023 · df33428 · df33428
1 parent c9db1ff
commit df33428
Show file tree

Hide file tree

Showing 7 changed files with 84 additions and 120 deletions.
diff --git a/extensions/browser/event_router.cc b/extensions/browser/event_router.cc
@@ -361,23 +361,25 @@ void EventRouter::AddListenerForMainThread(
 }
 
 void EventRouter::AddListenerForServiceWorker(
-    const std::string& extension_id,
-    const std::string& event_name,
-    mojom::ServiceWorkerContextPtr service_worker_context) {
+    mojom::EventListenerPtr event_listener) {
   auto* process = GetRenderProcessHostForCurrentReceiver();
   if (!process)
     return;
 
-  if (crx_file::id_util::IdIsValid(extension_id)) {
-    if (!service_worker_context->scope_url.is_valid()) {
-      mojo::ReportBadMessage(kAddEventListenerWithInvalidWorkerScopeURL);
-      return;
-    }
-    AddServiceWorkerEventListener(event_name, process, extension_id,
-                                  std::move(service_worker_context));
-  } else {
+  const mojom::EventListenerOwner& listener_owner =
+      *event_listener->listener_owner;
+  if (!listener_owner.is_extension_id() ||
+      !crx_file::id_util::IdIsValid(listener_owner.get_extension_id())) {
     mojo::ReportBadMessage(kAddEventListenerWithInvalidExtensionID);
+    return;
+  }
+
+  if (!event_listener->service_worker_context->scope_url.is_valid()) {
+    mojo::ReportBadMessage(kAddEventListenerWithInvalidWorkerScopeURL);
+    return;
   }
+
+  AddServiceWorkerEventListener(std::move(event_listener), process);
 }
 
 void EventRouter::AddLazyListenerForMainThread(const std::string& extension_id,
@@ -457,23 +459,25 @@ void EventRouter::RemoveListenerForMainThread(
 }
 
 void EventRouter::RemoveListenerForServiceWorker(
-    const std::string& extension_id,
-    const std::string& event_name,
-    mojom::ServiceWorkerContextPtr service_worker_context) {
+    mojom::EventListenerPtr event_listener) {
   auto* process = GetRenderProcessHostForCurrentReceiver();
   if (!process)
     return;
 
-  if (crx_file::id_util::IdIsValid(extension_id)) {
-    if (!service_worker_context->scope_url.is_valid()) {
-      mojo::ReportBadMessage(kRemoveEventListenerWithInvalidWorkerScopeURL);
-      return;
-    }
-    RemoveServiceWorkerEventListener(event_name, process, extension_id,
-                                     std::move(service_worker_context));
-  } else {
+  const mojom::EventListenerOwner& listener_owner =
+      *event_listener->listener_owner;
+  if (!listener_owner.is_extension_id() ||
+      !crx_file::id_util::IdIsValid(listener_owner.get_extension_id())) {
     mojo::ReportBadMessage(kRemoveEventListenerWithInvalidExtensionID);
+    return;
   }
+
+  if (!event_listener->service_worker_context->scope_url.is_valid()) {
+    mojo::ReportBadMessage(kRemoveEventListenerWithInvalidWorkerScopeURL);
+    return;
+  }
+
+  RemoveServiceWorkerEventListener(std::move(event_listener), process);
 }
 
 void EventRouter::RemoveLazyListenerForMainThread(
@@ -540,14 +544,15 @@ void EventRouter::AddEventListener(const std::string& event_name,
 }
 
 void EventRouter::AddServiceWorkerEventListener(
-    const std::string& event_name,
-    RenderProcessHost* process,
-    const ExtensionId& extension_id,
-    mojom::ServiceWorkerContextPtr service_worker_context) {
+    mojom::EventListenerPtr event_listener,
+    RenderProcessHost* process) {
+  const mojom::ServiceWorkerContext& service_worker =
+      *event_listener->service_worker_context;
   listeners_.AddListener(EventListener::ForExtensionServiceWorker(
-      event_name, extension_id, process, process->GetBrowserContext(),
-      service_worker_context->scope_url, service_worker_context->version_id,
-      service_worker_context->thread_id, absl::nullopt));
+      event_listener->event_name,
+      event_listener->listener_owner->get_extension_id(), process,
+      process->GetBrowserContext(), service_worker.scope_url,
+      service_worker.version_id, service_worker.thread_id, absl::nullopt));
   CHECK(base::Contains(observed_process_set_, process));
 }
 
@@ -560,15 +565,16 @@ void EventRouter::RemoveEventListener(const std::string& event_name,
 }
 
 void EventRouter::RemoveServiceWorkerEventListener(
-    const std::string& event_name,
-    RenderProcessHost* process,
-    const ExtensionId& extension_id,
-    mojom::ServiceWorkerContextPtr service_worker_context) {
+    mojom::EventListenerPtr event_listener,
+    RenderProcessHost* process) {
+  const mojom::ServiceWorkerContext& service_worker =
+      *event_listener->service_worker_context;
   std::unique_ptr<EventListener> listener =
       EventListener::ForExtensionServiceWorker(
-          event_name, extension_id, process, process->GetBrowserContext(),
-          service_worker_context->scope_url, service_worker_context->version_id,
-          service_worker_context->thread_id, absl::nullopt);
+          event_listener->event_name,
+          event_listener->listener_owner->get_extension_id(), process,
+          process->GetBrowserContext(), service_worker.scope_url,
+          service_worker.version_id, service_worker.thread_id, absl::nullopt);
   listeners_.RemoveListener(listener.get());
 }
 

diff --git a/extensions/browser/event_router.h b/extensions/browser/event_router.h
@@ -166,9 +166,7 @@ class EventRouter : public KeyedService,
       mojom::EventListenerPtr event_listener) override;
 
   void AddListenerForServiceWorker(
-      const std::string& extension_id,
-      const std::string& name,
-      mojom::ServiceWorkerContextPtr service_worker_context) override;
+      mojom::EventListenerPtr event_listener) override;
 
   void AddLazyListenerForMainThread(const std::string& extension_id,
                                     const std::string& name) override;
@@ -194,9 +192,7 @@ class EventRouter : public KeyedService,
       mojom::EventListenerPtr event_listener) override;
 
   void RemoveListenerForServiceWorker(
-      const std::string& extension_id,
-      const std::string& event_name,
-      mojom::ServiceWorkerContextPtr service_worker_context) override;
+      mojom::EventListenerPtr event_listener) override;
 
   void RemoveLazyListenerForMainThread(const std::string& extension_id,
                                        const std::string& name) override;
@@ -226,11 +222,8 @@ class EventRouter : public KeyedService,
   void RemoveEventListener(const std::string& event_name,
                            content::RenderProcessHost* process,
                            const ExtensionId& extension_id);
-  void RemoveServiceWorkerEventListener(
-      const std::string& event_name,
-      content::RenderProcessHost* process,
-      const ExtensionId& extension_id,
-      mojom::ServiceWorkerContextPtr service_worker_context);
+  void RemoveServiceWorkerEventListener(mojom::EventListenerPtr event_listener,
+                                        content::RenderProcessHost* process);
 
   // Add or remove a URL as an event listener for |event_name|.
   void AddEventListenerForURL(const std::string& event_name,
@@ -416,11 +409,8 @@ class EventRouter : public KeyedService,
   void AddEventListener(const std::string& event_name,
                         content::RenderProcessHost* process,
                         const ExtensionId& extension_id);
-  void AddServiceWorkerEventListener(
-      const std::string& event_name,
-      content::RenderProcessHost* process,
-      const ExtensionId& extension_id,
-      mojom::ServiceWorkerContextPtr service_worker_context);
+  void AddServiceWorkerEventListener(mojom::EventListenerPtr event_listener,
+                                     content::RenderProcessHost* process);
 
   // Returns or sets the list of events for which the given extension has
   // registered.

diff --git a/extensions/browser/event_router_unittest.cc b/extensions/browser/event_router_unittest.cc
@@ -814,8 +814,11 @@ TEST_F(EventRouterDispatchTest, DISABLED_TestDispatchCallback) {
   const int sw_thread_id = 100;
   MockEventDispatcher sw_event_dispatcher;
   event_router()->AddServiceWorkerEventListener(
-      event_name, process4.get(), ext3,
-      mojom::ServiceWorkerContext::New(GURL(), sw_version_id, sw_thread_id));
+      mojom::EventListener::New(
+          mojom::EventListenerOwner::NewExtensionId(ext3), event_name,
+          mojom::ServiceWorkerContext::New(GURL(), sw_version_id, sw_thread_id),
+          /*event_filter=*/absl::nullopt),
+      process4.get());
   event_router()->BindServiceWorkerEventDispatcher(
       process4->GetID(), sw_thread_id, sw_event_dispatcher.BindAndPassRemote());
 

diff --git a/extensions/common/mojom/event_router.mojom b/extensions/common/mojom/event_router.mojom
@@ -47,9 +47,7 @@ interface EventRouter {
 
   // Notifies the browser that the given extension added a listener to an event
   // in a service worker.
-  AddListenerForServiceWorker(string extension_id,
-                              string event_name,
-                              ServiceWorkerContext service_worker_context);
+  AddListenerForServiceWorker(EventListener event_listener);
 
   // Notifies the browser that the given extension added a listener to an event
   // from a lazy background page.
@@ -87,9 +85,7 @@ interface EventRouter {
 
   // Notifies the browser that the given extension removed a listener from an
   // event in a service worker.
-  RemoveListenerForServiceWorker(string extension_id,
-                                 string event_name,
-                                 ServiceWorkerContext service_worker_context);
+  RemoveListenerForServiceWorker(EventListener event_listener);
 
   // Notifies the browser that the given extension is no longer interested in
   // receiving the given event from a lazy background page.

diff --git a/extensions/renderer/ipc_message_sender.cc b/extensions/renderer/ipc_message_sender.cc
@@ -392,20 +392,19 @@ class WorkerThreadIPCMessageSender : public IPCMessageSender {
     DCHECK_NE(blink::mojom::kInvalidServiceWorkerVersionId,
               context->service_worker_version_id());
 
+    auto event_listener = mojom::EventListener::New(
+        mojom::EventListenerOwner::NewExtensionId(context->GetExtensionID()),
+        event_name,
+        mojom::ServiceWorkerContext::New(context->service_worker_scope(),
+                                         context->service_worker_version_id(),
+                                         content::WorkerThread::GetCurrentId()),
+        /*event_filter=*/absl::nullopt);
 #if BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
-    dispatcher_->SendAddEventListener(
-        context->GetExtensionID(), context->service_worker_scope(), event_name,
-        context->service_worker_version_id(),
-        content::WorkerThread::GetCurrentId());
+    dispatcher_->SendAddEventListener(std::move(event_listener));
 #else
     WorkerThreadDispatcher::GetServiceWorkerData()
         ->GetEventRouter()
-        ->AddListenerForServiceWorker(
-            context->GetExtensionID(), event_name,
-            mojom::ServiceWorkerContext::New(
-                context->service_worker_scope(),
-                context->service_worker_version_id(),
-                content::WorkerThread::GetCurrentId()));
+        ->AddListenerForServiceWorker(std::move(event_listener))
 #endif
   }
 
@@ -417,20 +416,20 @@ class WorkerThreadIPCMessageSender : public IPCMessageSender {
     DCHECK_NE(blink::mojom::kInvalidServiceWorkerVersionId,
               context->service_worker_version_id());
 
+    auto event_listener = mojom::EventListener::New(
+        mojom::EventListenerOwner::NewExtensionId(context->GetExtensionID()),
+        event_name,
+        mojom::ServiceWorkerContext::New(context->service_worker_scope(),
+                                         context->service_worker_version_id(),
+                                         content::WorkerThread::GetCurrentId()),
+        /*event_filter=*/absl::nullopt);
+
 #if BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
-    dispatcher_->SendRemoveEventListener(
-        context->GetExtensionID(), context->service_worker_scope(), event_name,
-        context->service_worker_version_id(),
-        content::WorkerThread::GetCurrentId());
+    dispatcher_->SendRemoveEventListener(std::move(event_listener));
 #else
     WorkerThreadDispatcher::GetServiceWorkerData()
         ->GetEventRouter()
-        ->RemoveListenerForServiceWorker(
-            context->GetExtensionID(), event_name,
-            mojom::ServiceWorkerContext::New(
-                context->service_worker_scope(),
-                context->service_worker_version_id(),
-                content::WorkerThread::GetCurrentId()));
+        ->RemoveListenerForServiceWorker(std::move(event_listener));
 #endif
   }
 

diff --git a/extensions/renderer/worker_thread_dispatcher.cc b/extensions/renderer/worker_thread_dispatcher.cc
@@ -53,30 +53,18 @@ void BindAutomationOnIO(
 
 // Calls mojom::EventRouter::AddListenerForServiceWorker(). It should be called
 // on the IO thread.
-void AddEventListenerOnIO(const std::string& extension_id,
-                          const GURL& scope,
-                          const std::string& event_name,
-                          int64_t service_worker_version_id,
-                          int worker_thread_id) {
+void AddEventListenerOnIO(mojom::EventListenerPtr event_listener) {
   auto* dispatcher = WorkerThreadDispatcher::Get();
   dispatcher->GetEventRouterOnIO()->AddListenerForServiceWorker(
-      extension_id, event_name,
-      mojom::ServiceWorkerContext::New(scope, service_worker_version_id,
-                                       worker_thread_id));
+      std::move(event_listener));
 }
 
 // Calls mojom::EventRouter::RemoveListenerForServiceWorker(). It should be
 // called on the IO thread.
-void RemoveEventListenerOnIO(const std::string& extension_id,
-                             const GURL& scope,
-                             const std::string& event_name,
-                             int64_t service_worker_version_id,
-                             int worker_thread_id) {
+void RemoveEventListenerOnIO(mojom::EventListenerPtr event_listener) {
   auto* dispatcher = WorkerThreadDispatcher::Get();
   dispatcher->GetEventRouterOnIO()->RemoveListenerForServiceWorker(
-      extension_id, event_name,
-      mojom::ServiceWorkerContext::New(scope, service_worker_version_id,
-                                       worker_thread_id));
+      std::move(event_listener));
 }
 
 // Calls mojom::EventRouter::AddLazyListenerForServiceWorker(). It should be
@@ -275,15 +263,10 @@ bool WorkerThreadDispatcher::UpdateBindingsForWorkers(
 
 #if BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
 void WorkerThreadDispatcher::SendAddEventListener(
-    const std::string& extension_id,
-    const GURL& scope,
-    const std::string& event_name,
-    int64_t service_worker_version_id,
-    int worker_thread_id) {
+    mojom::EventListenerPtr event_listener) {
   io_task_runner_->PostTask(
       FROM_HERE,
-      base::BindOnce(&AddEventListenerOnIO, extension_id, scope, event_name,
-                     service_worker_version_id, worker_thread_id));
+      base::BindOnce(&AddEventListenerOnIO, std::move(event_listener)));
 }
 
 void WorkerThreadDispatcher::SendAddEventLazyListener(
@@ -318,15 +301,10 @@ void WorkerThreadDispatcher::SendBindAutomation(
 }
 
 void WorkerThreadDispatcher::SendRemoveEventListener(
-    const std::string& extension_id,
-    const GURL& scope,
-    const std::string& event_name,
-    int64_t service_worker_version_id,
-    int worker_thread_id) {
+    mojom::EventListenerPtr event_listener) {
   io_task_runner_->PostTask(
       FROM_HERE,
-      base::BindOnce(&RemoveEventListenerOnIO, extension_id, scope, event_name,
-                     service_worker_version_id, worker_thread_id));
+      base::BindOnce(&RemoveEventListenerOnIO, std::move(event_listener)));
 }
 
 void WorkerThreadDispatcher::SendRemoveEventLazyListener(

diff --git a/extensions/renderer/worker_thread_dispatcher.h b/extensions/renderer/worker_thread_dispatcher.h
@@ -120,11 +120,7 @@ class WorkerThreadDispatcher : public content::RenderThreadObserver,
 #if BUILDFLAG(ENABLE_EXTENSIONS_LEGACY_IPC)
   // Posts mojom::EventRouter::AddListenerForServiceWorker to the IO thread to
   // call it with GetEventRouterOnIO().
-  void SendAddEventListener(const std::string& extension_id,
-                            const GURL& scope,
-                            const std::string& event_name,
-                            int64_t service_worker_version_id,
-                            int worker_thread_id);
+  void SendAddEventListener(mojom::EventListenerPtr event_listener);
 
   // Posts mojom::EventRouter::AddLazyListenerForServiceWorker to the IO thread
   // to call it with GetEventRouterOnIO().
@@ -149,11 +145,7 @@ class WorkerThreadDispatcher : public content::RenderThreadObserver,
 
   // Posts mojom::EventRouter::RemoveListenerForServiceWorker to the IO thread
   // to call it with GetEventRouterOnIO().
-  void SendRemoveEventListener(const std::string& extension_id,
-                               const GURL& scope,
-                               const std::string& event_name,
-                               int64_t service_worker_version_id,
-                               int worker_thread_id);
+  void SendRemoveEventListener(mojom::EventListenerPtr event_listener);
 
   // Posts mojom::EventRouter::RemoveLazyListenerForServiceWorker to the IO
   // thread to call it with GetEventRouterOnIO().