Convert ExtensionMsg_Unloaded to a Renderer mojom message

This CL migrates ExtensionMsg_Unloaded to extensions.mojom.Renderer
interface. Additionally, this CL renames the name of the parameter
from |id| to |extension_id| to be aligned with other functions.

Bug: 1146099
Change-Id: I9c5463a6331ed4a87e39996f72dabb4b0ac40931
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2536669Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Karan Bhatia <karandeepb@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Cr-Commit-Position: refs/heads/master@{#828567}

chrome/browser/extensions/activity_log/activity_log_unittest.cc

@@ -79,6 +79,7 @@ class InterceptingRendererStartupHelper : public RendererStartupHelper,
   // mojom::Renderer implementation:
   void ActivateExtension(const std::string& extension_id) override {}
   void SetActivityLoggingEnabled(bool enabled) override {}
+  void UnloadExtension(const std::string& extension_id) override {}
 
   mojo::AssociatedReceiverSet<mojom::Renderer> receivers_;
 };

extensions/browser/renderer_startup_helper.cc

@@ -291,7 +291,7 @@ void RendererStartupHelper::OnExtensionUnloaded(const Extension& extension) {
       extension_process_map_[extension.id()];
   for (content::RenderProcessHost* process : loaded_process_set) {
     DCHECK(base::Contains(process_mojo_map_, process));
-    process->Send(new ExtensionMsg_Unloaded(extension.id()));
+    GetRenderer(process)->UnloadExtension(extension.id());
   }
 
   // Resets registered origin access lists in the BrowserContext asynchronously.

extensions/browser/renderer_startup_helper_unittest.cc

@@ -30,6 +30,8 @@ class InterceptingRendererStartupHelper : public RendererStartupHelper,
 
   size_t num_activated_extensions() { return activated_extensions_.size(); }
 
+  size_t num_unloaded_extensions() { return unloaded_extensions_.size(); }
+
  protected:
   mojo::PendingAssociatedRemote<mojom::Renderer> BindNewRendererRemote(
       content::RenderProcessHost* process) override {
@@ -45,7 +47,12 @@ class InterceptingRendererStartupHelper : public RendererStartupHelper,
   }
   void SetActivityLoggingEnabled(bool enabled) override {}
 
+  void UnloadExtension(const std::string& extension_id) override {
+    unloaded_extensions_.push_back(extension_id);
+  }
+
   std::vector<std::string> activated_extensions_;
+  std::vector<std::string> unloaded_extensions_;
   mojo::AssociatedReceiverSet<mojom::Renderer> receivers_;
 };
 
@@ -210,9 +217,8 @@ TEST_F(RendererStartupHelperTest, NormalExtensionLifecycle) {
   RemoveExtensionFromRegistry(extension_);
   helper_->OnExtensionUnloaded(*extension_);
   EXPECT_FALSE(IsExtensionLoaded(*extension_));
-  ASSERT_EQ(1u, sink.message_count());
-  EXPECT_EQ(static_cast<uint32_t>(ExtensionMsg_Unloaded::ID),
-            sink.GetMessageAt(0)->type());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(1u, helper_->num_unloaded_extensions());
 
   // Extension enabled again.
   sink.ClearMessages();
@@ -359,11 +365,10 @@ TEST_F(RendererStartupHelperTest, ExtensionInIncognitoRenderer) {
             incognito_sink.GetMessageAt(0)->type());
   // The extension would be first unloaded and then loaded from the normal
   // renderer.
-  ASSERT_EQ(2u, sink.message_count());
-  EXPECT_EQ(static_cast<uint32_t>(ExtensionMsg_Unloaded::ID),
-            sink.GetMessageAt(0)->type());
+  base::RunLoop().RunUntilIdle();
+  ASSERT_EQ(1u, helper_->num_unloaded_extensions());
   EXPECT_EQ(static_cast<uint32_t>(ExtensionMsg_Loaded::ID),
-            sink.GetMessageAt(1)->type());
+            sink.GetMessageAt(0)->type());
 }
 
 // Tests that platform apps are always loaded in an incognito renderer.

extensions/common/extension_messages.h

@@ -573,10 +573,6 @@ IPC_MESSAGE_CONTROL2(ExtensionMsg_SetSystemFont,
 IPC_MESSAGE_CONTROL1(ExtensionMsg_Loaded,
                      std::vector<ExtensionMsg_Loaded_Params>)
 
-// Notifies the renderer that an extension was unloaded in the browser.
-IPC_MESSAGE_CONTROL1(ExtensionMsg_Unloaded,
-                     std::string)
-
 // Updates the scripting allowlist for extensions in the render process. This is
 // only used for testing.
 IPC_MESSAGE_CONTROL1(ExtensionMsg_SetScriptingAllowlist,

extensions/common/mojom/renderer.mojom

@@ -17,4 +17,7 @@ interface Renderer {
   // sent if logging is or was previously enabled; not being enabled is assumed
   // otherwise.
   SetActivityLoggingEnabled(bool enabled);
+
+  // Notifies the renderer that an extension was unloaded in the browser.
+  UnloadExtension(string extension_id);
 };

extensions/renderer/dispatcher.cc

@@ -881,7 +881,6 @@ bool Dispatcher::OnControlMessageReceived(const IPC::Message& message) {
   IPC_MESSAGE_HANDLER(ExtensionMsg_ShouldSuspend, OnShouldSuspend)
   IPC_MESSAGE_HANDLER(ExtensionMsg_Suspend, OnSuspend)
   IPC_MESSAGE_HANDLER(ExtensionMsg_TransferBlobs, OnTransferBlobs)
-  IPC_MESSAGE_HANDLER(ExtensionMsg_Unloaded, OnUnloaded)
   IPC_MESSAGE_HANDLER(ExtensionMsg_UpdatePermissions, OnUpdatePermissions)
   IPC_MESSAGE_HANDLER(ExtensionMsg_UpdateDefaultPolicyHostRestrictions,
                       OnUpdateDefaultPolicyHostRestrictions)
@@ -957,6 +956,57 @@ void Dispatcher::ActivateExtension(const std::string& extension_id) {
   UpdateActiveExtensions();
 }
 
+void Dispatcher::UnloadExtension(const std::string& extension_id) {
+  // See comment in OnLoaded for why it would be nice, but perhaps incorrect,
+  // to CHECK here rather than guarding.
+  // TODO(devlin): This may be fixed by crbug.com/528026. Monitor, and
+  // consider making this a release CHECK.
+  if (!RendererExtensionRegistry::Get()->Remove(extension_id)) {
+    NOTREACHED();
+    return;
+  }
+
+  ExtensionsRendererClient::Get()->OnExtensionUnloaded(extension_id);
+
+  bindings_system_->OnExtensionRemoved(extension_id);
+
+  active_extension_ids_.erase(extension_id);
+
+  script_injection_manager_->OnExtensionUnloaded(extension_id);
+
+  // If the extension is later reloaded with a different set of permissions,
+  // we'd like it to get a new isolated world ID, so that it can pick up the
+  // changed origin whitelist.
+  ScriptInjection::RemoveIsolatedWorld(extension_id);
+
+  // Inform the bindings system that the contexts will be removed to allow time
+  // to clear out context-specific data, and then remove the contexts
+  // themselves.
+  script_context_set_->ForEach(
+      extension_id, nullptr,
+      base::Bind(&NativeExtensionBindingsSystem::WillReleaseScriptContext,
+                 base::Unretained(bindings_system_.get())));
+  script_context_set_->OnExtensionUnloaded(extension_id);
+
+  // Update the available bindings for the remaining contexts. These may have
+  // changed if an externally_connectable extension is unloaded and a webpage
+  // is no longer accessible.
+  UpdateAllBindings();
+
+  // Invalidates the messages map for the extension in case the extension is
+  // reloaded with a new messages map.
+  EraseL10nMessagesMap(extension_id);
+
+  // Update the origin access map so that any content scripts injected no longer
+  // have dedicated allow/block lists for extra origins.
+  WebSecurityPolicy::ClearOriginAccessListForOrigin(
+      Extension::GetBaseURLFromExtensionId(extension_id));
+
+  // We don't do anything with existing platform-app stylesheets. They will
+  // stay resident, but the URL pattern corresponding to the unloaded
+  // extension's URL just won't match anything anymore.
+}
+
 void Dispatcher::OnCancelSuspend(const std::string& extension_id) {
   DispatchEvent(extension_id, kOnSuspendCanceledEvent, base::ListValue(),
                 nullptr);
@@ -1160,57 +1210,6 @@ void Dispatcher::OnTransferBlobs(const std::vector<std::string>& blob_uuids) {
   RenderThread::Get()->Send(new ExtensionHostMsg_TransferBlobsAck(blob_uuids));
 }
 
-void Dispatcher::OnUnloaded(const std::string& id) {
-  // See comment in OnLoaded for why it would be nice, but perhaps incorrect,
-  // to CHECK here rather than guarding.
-  // TODO(devlin): This may be fixed by crbug.com/528026. Monitor, and
-  // consider making this a release CHECK.
-  if (!RendererExtensionRegistry::Get()->Remove(id)) {
-    NOTREACHED();
-    return;
-  }
-
-  ExtensionsRendererClient::Get()->OnExtensionUnloaded(id);
-
-  bindings_system_->OnExtensionRemoved(id);
-
-  active_extension_ids_.erase(id);
-
-  script_injection_manager_->OnExtensionUnloaded(id);
-
-  // If the extension is later reloaded with a different set of permissions,
-  // we'd like it to get a new isolated world ID, so that it can pick up the
-  // changed origin whitelist.
-  ScriptInjection::RemoveIsolatedWorld(id);
-
-  // Inform the bindings system that the contexts will be removed to allow time
-  // to clear out context-specific data, and then remove the contexts
-  // themselves.
-  script_context_set_->ForEach(
-      id, nullptr,
-      base::Bind(&NativeExtensionBindingsSystem::WillReleaseScriptContext,
-                 base::Unretained(bindings_system_.get())));
-  script_context_set_->OnExtensionUnloaded(id);
-
-  // Update the available bindings for the remaining contexts. These may have
-  // changed if an externally_connectable extension is unloaded and a webpage
-  // is no longer accessible.
-  UpdateAllBindings();
-
-  // Invalidates the messages map for the extension in case the extension is
-  // reloaded with a new messages map.
-  EraseL10nMessagesMap(id);
-
-  // Update the origin access map so that any content scripts injected no longer
-  // have dedicated allow/block lists for extra origins.
-  WebSecurityPolicy::ClearOriginAccessListForOrigin(
-      Extension::GetBaseURLFromExtensionId(id));
-
-  // We don't do anything with existing platform-app stylesheets. They will
-  // stay resident, but the URL pattern corresponding to the unloaded
-  // extension's URL just won't match anything anymore.
-}
-
 void Dispatcher::OnUpdateDefaultPolicyHostRestrictions(
     const ExtensionMsg_UpdateDefaultPolicyHostRestrictions_Params& params) {
   PermissionsData::SetDefaultPolicyHostRestrictions(

extensions/renderer/dispatcher.h

@@ -216,6 +216,7 @@ class Dispatcher : public content::RenderThreadObserver,
   // mojom::Renderer implementation:
   void ActivateExtension(const std::string& extension_id) override;
   void SetActivityLoggingEnabled(bool enabled) override;
+  void UnloadExtension(const std::string& extension_id) override;
 
   void OnRendererAssociatedRequest(
       mojo::PendingAssociatedReceiver<mojom::Renderer> receiver);
@@ -250,7 +251,6 @@ class Dispatcher : public content::RenderThreadObserver,
   void OnShouldSuspend(const std::string& extension_id, uint64_t sequence_id);
   void OnSuspend(const std::string& extension_id);
   void OnTransferBlobs(const std::vector<std::string>& blob_uuids);
-  void OnUnloaded(const std::string& id);
   void OnUpdatePermissions(const ExtensionMsg_UpdatePermissions_Params& params);
   void OnUpdateDefaultPolicyHostRestrictions(
       const ExtensionMsg_UpdateDefaultPolicyHostRestrictions_Params& params);