Convert ExtensionMsg_SetActivityLoggingEnabled to mojo

This CL converts ExtensionMsg_SetActivityLoggingEnabled to a
mojom::Renderer message.

Bug: 1146101
Change-Id: Idf6520590b576c3c8ffe95477193dc1f6d9ee8ca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2525887Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#826661}

chrome/browser/extensions/activity_log/activity_log.cc

@@ -45,8 +45,10 @@
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extension_system_provider.h"
 #include "extensions/browser/extensions_browser_client.h"
+#include "extensions/browser/renderer_startup_helper.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_messages.h"
+#include "extensions/common/mojom/renderer.mojom.h"
 #include "third_party/re2/src/re2/re2.h"
 #include "url/gurl.h"
 
@@ -785,9 +787,14 @@ void ActivityLog::CheckActive(bool use_cached) {
            content::RenderProcessHost::AllHostsIterator());
        !iter.IsAtEnd(); iter.Advance()) {
     content::RenderProcessHost* host = iter.GetCurrentValue();
-    if (profile_->IsSameOrParent(
+    if (host->IsInitializedAndNotDead() &&
+        profile_->IsSameOrParent(
             Profile::FromBrowserContext(host->GetBrowserContext()))) {
-      host->Send(new ExtensionMsg_SetActivityLoggingEnabled(is_active_));
+      mojom::Renderer* renderer =
+          RendererStartupHelperFactory::GetForBrowserContext(
+              host->GetBrowserContext())
+              ->GetRenderer(host);
+      renderer->SetActivityLoggingEnabled(is_active_);
     }
   }
 }

chrome/browser/extensions/activity_log/activity_log_unittest.cc

@@ -28,10 +28,13 @@
 #include "components/no_state_prefetch/browser/prerender_manager.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_task_environment.h"
+#include "content/public/test/mock_render_process_host.h"
 #include "extensions/browser/extension_registry.h"
+#include "extensions/browser/renderer_startup_helper.h"
 #include "extensions/browser/uninstall_reason.h"
 #include "extensions/common/dom_action_types.h"
 #include "extensions/common/extension_builder.h"
+#include "mojo/public/cpp/bindings/associated_receiver_set.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -55,6 +58,31 @@ const char* const kUrlApiCalls[] = {
 
 namespace extensions {
 
+// Class that implements the binding of a new Renderer mojom interface and
+// can receive callbacks on it for testing validation.
+class InterceptingRendererStartupHelper : public RendererStartupHelper,
+                                          public mojom::Renderer {
+ public:
+  explicit InterceptingRendererStartupHelper(
+      content::BrowserContext* browser_context)
+      : RendererStartupHelper(browser_context) {}
+
+ protected:
+  mojo::PendingAssociatedRemote<mojom::Renderer> BindNewRendererRemote(
+      content::RenderProcessHost* process) override {
+    mojo::AssociatedRemote<mojom::Renderer> remote;
+    receivers_.Add(this, remote.BindNewEndpointAndPassDedicatedReceiver());
+    return remote.Unbind();
+  }
+
+ private:
+  // mojom::Renderer implementation:
+  void ActivateExtension(const std::string& extension_id) override {}
+  void SetActivityLoggingEnabled(bool enabled) override {}
+
+  mojo::AssociatedReceiverSet<mojom::Renderer> receivers_;
+};
+
 class ActivityLogTest : public ChromeRenderViewHostTestHarness {
  protected:
   virtual bool enable_activity_logging_switch() const { return true; }
@@ -74,9 +102,24 @@ class ActivityLogTest : public ChromeRenderViewHostTestHarness {
     extension_service_ = static_cast<TestExtensionSystem*>(
         ExtensionSystem::Get(profile()))->CreateExtensionService
             (&command_line, base::FilePath(), false);
+
+    RendererStartupHelperFactory::GetForBrowserContext(profile())
+        ->OnRenderProcessHostCreated(
+            static_cast<content::RenderProcessHost*>(process()));
+
     base::RunLoop().RunUntilIdle();
   }
 
+  static std::unique_ptr<KeyedService> BuildFakeRendererStartupHelper(
+      content::BrowserContext* context) {
+    return std::make_unique<InterceptingRendererStartupHelper>(context);
+  }
+
+  TestingProfile::TestingFactories GetTestingFactories() const override {
+    return {{RendererStartupHelperFactory::GetInstance(),
+             base::BindRepeating(&BuildFakeRendererStartupHelper)}};
+  }
+
   void TearDown() override {
     base::RunLoop().RunUntilIdle();
     SetActivityLogTaskRunnerForTesting(nullptr);

extensions/browser/renderer_startup_helper.cc

@@ -114,10 +114,8 @@ void RendererStartupHelper::InitializeProcess(
       client->IsActivityLoggingEnabled(process->GetBrowserContext());
   // We only send the ActivityLoggingEnabled message if it is enabled; otherwise
   // the default (not enabled) is correct.
-  if (activity_logging_enabled) {
-    process->Send(
-        new ExtensionMsg_SetActivityLoggingEnabled(activity_logging_enabled));
-  }
+  if (activity_logging_enabled)
+    renderer->SetActivityLoggingEnabled(activity_logging_enabled);
 
   // Extensions need to know the channel and the session type for API
   // restrictions. The values are sent to all renderers, as the non-extension
@@ -318,6 +316,11 @@ RendererStartupHelper::BindNewRendererRemote(
   return renderer_interface.Unbind();
 }
 
+mojom::Renderer* RendererStartupHelper::GetRenderer(
+    content::RenderProcessHost* process) {
+  DCHECK(base::Contains(process_mojo_map_, process));
+  return process_mojo_map_.find(process)->second.get();
+}
 //////////////////////////////////////////////////////////////////////////////
 
 // static

extensions/browser/renderer_startup_helper.h

@@ -68,6 +68,10 @@ class RendererStartupHelper : public KeyedService,
   void OnExtensionUnloaded(const Extension& extension);
   void OnExtensionLoaded(const Extension& extension);
 
+  // Returns mojom::Renderer* corresponding to |process|. Note that the callers
+  // should pass a valid content::RenderProcessHost*.
+  mojom::Renderer* GetRenderer(content::RenderProcessHost* process);
+
  protected:
   // Provide ability for tests to override.
   virtual mojo::PendingAssociatedRemote<mojom::Renderer> BindNewRendererRemote(

extensions/browser/renderer_startup_helper_unittest.cc

@@ -43,6 +43,7 @@ class InterceptingRendererStartupHelper : public RendererStartupHelper,
   void ActivateExtension(const std::string& extension_id) override {
     activated_extensions_.push_back(extension_id);
   }
+  void SetActivityLoggingEnabled(bool enabled) override {}
 
   std::vector<std::string> activated_extensions_;
   mojo::AssociatedReceiverSet<mojom::Renderer> receivers_;

extensions/common/extension_messages.h

@@ -991,11 +991,6 @@ IPC_MESSAGE_ROUTED3(ExtensionHostMsg_AutomationQuerySelector_Result,
                     ExtensionHostMsg_AutomationQuerySelector_Error /* error */,
                     int /* result_acc_obj_id */)
 
-// Tells the renderer whether or not activity logging is enabled. This is only
-// sent if logging is or was previously enabled; not being enabled is assumed
-// otherwise.
-IPC_MESSAGE_CONTROL1(ExtensionMsg_SetActivityLoggingEnabled, bool /* enabled */)
-
 // Messages related to Extension Service Worker.
 #undef IPC_MESSAGE_START
 #define IPC_MESSAGE_START ExtensionWorkerMsgStart

extensions/common/mojom/renderer.mojom

@@ -12,4 +12,9 @@ interface Renderer {
   // have more privileges than other extension content that might end up running
   // in the process (e.g. because of iframes or content scripts).
   ActivateExtension(string extension_id);
+
+  // Tells the renderer whether or not activity logging is enabled. This is only
+  // sent if logging is or was previously enabled; not being enabled is assumed
+  // otherwise.
+  SetActivityLoggingEnabled(bool enabled);
 };

extensions/renderer/dispatcher.cc

@@ -888,8 +888,6 @@ bool Dispatcher::OnControlMessageReceived(const IPC::Message& message) {
                       OnUpdateTabSpecificPermissions)
   IPC_MESSAGE_HANDLER(ExtensionMsg_ClearTabSpecificPermissions,
                       OnClearTabSpecificPermissions)
-  IPC_MESSAGE_HANDLER(ExtensionMsg_SetActivityLoggingEnabled,
-                      OnSetActivityLoggingEnabled)
   IPC_MESSAGE_FORWARD(ExtensionMsg_WatchPages,
                       content_watcher_.get(),
                       ContentWatcher::OnWatchPages)
@@ -1290,7 +1288,7 @@ void Dispatcher::OnClearTabSpecificPermissions(
   }
 }
 
-void Dispatcher::OnSetActivityLoggingEnabled(bool enabled) {
+void Dispatcher::SetActivityLoggingEnabled(bool enabled) {
   activity_logging_enabled_ = enabled;
   if (enabled) {
     for (const std::string& id : active_extension_ids_)

extensions/renderer/dispatcher.h

@@ -215,6 +215,7 @@ class Dispatcher : public content::RenderThreadObserver,
 
   // mojom::Renderer implementation:
   void ActivateExtension(const std::string& extension_id) override;
+  void SetActivityLoggingEnabled(bool enabled) override;
 
   void OnRendererAssociatedRequest(
       mojo::PendingAssociatedReceiver<mojom::Renderer> receiver);
@@ -263,8 +264,6 @@ class Dispatcher : public content::RenderThreadObserver,
       bool update_origin_whitelist,
       int tab_id);
 
-  void OnSetActivityLoggingEnabled(bool enabled);
-
   // UserScriptSetManager::Observer implementation.
   void OnUserScriptsUpdated(const std::set<HostID>& changed_hosts) override;