Convert ExtensionMsg_SetTabId to a LocalFrame message

This CL introduces |local_frame_map_| and
GetLocalFrame(content::RenderFrameHost*) in
ExtensionWebContentsObserver in order to get
extensions::mojom::LocalFrame corresponding RenderFrameHost
from //chrome/browser/extensions or //extensions/browser and
converts ExtensionMsg_SetTabId to SetTabId in
extensions::mojom::LocalFrame.

Bug: 1178998
Change-Id: Ia0fa454688c70a53254abe02d844739a651bacf0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2703817
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#856202}

chrome/browser/extensions/tab_helper.cc

@@ -82,6 +82,9 @@ TabHelper::TabHelper(content::WebContents* web_contents)
   // The ActiveTabPermissionManager requires a session ID; ensure this
   // WebContents has one.
   CreateSessionServiceTabHelper(web_contents);
+  // We need an ExtensionWebContentsObserver, so make sure one exists (this is
+  // a no-op if one already does).
+  ChromeExtensionWebContentsObserver::CreateForWebContents(web_contents);
   // The Unretained() is safe because ForEachFrame() is synchronous.
   web_contents->ForEachFrame(
       base::BindRepeating(&TabHelper::SetTabId, base::Unretained(this)));
@@ -95,9 +98,6 @@ TabHelper::TabHelper(content::WebContents* web_contents)
     registry->MonitorWebContentsForRuleEvaluation(this->web_contents());
   });
 
-  // We need an ExtensionWebContentsObserver, so make sure one exists (this is
-  // a no-op if one already does).
-  ChromeExtensionWebContentsObserver::CreateForWebContents(web_contents);
   ExtensionWebContentsObserver::GetForWebContents(web_contents)->dispatcher()->
       set_delegate(this);
 
@@ -328,9 +328,9 @@ void TabHelper::SetTabId(content::RenderFrameHost* render_frame_host) {
   // We should wait for RenderFrameCreated() to happen, to avoid sending this
   // message twice.
   if (render_frame_host->IsRenderFrameCreated()) {
-    render_frame_host->Send(new ExtensionMsg_SetTabId(
-        render_frame_host->GetRoutingID(),
-        sessions::SessionTabHelper::IdForTab(web_contents()).id()));
+    ExtensionWebContentsObserver::GetForWebContents(web_contents())
+        ->GetLocalFrame(render_frame_host)
+        ->SetTabId(sessions::SessionTabHelper::IdForTab(web_contents()).id());
   }
 }
 

extensions/browser/extension_web_contents_observer.cc

@@ -25,7 +25,6 @@
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_messages.h"
 #include "extensions/common/view_type.h"
-#include "mojo/public/cpp/bindings/associated_remote.h"
 #include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/mojom/autoplay/autoplay.mojom.h"
 #include "url/origin.h"
@@ -147,6 +146,7 @@ void ExtensionWebContentsObserver::RenderFrameCreated(
 void ExtensionWebContentsObserver::RenderFrameDeleted(
     content::RenderFrameHost* render_frame_host) {
   DCHECK(initialized_);
+  local_frame_map_.erase(render_frame_host);
   ProcessManager::Get(browser_context_)
       ->UnregisterRenderFrameHost(render_frame_host);
   ExtensionApiFrameIdMap::Get()->OnRenderFrameDeleted(render_frame_host);
@@ -306,6 +306,17 @@ const Extension* ExtensionWebContentsObserver::GetExtensionFromFrame(
   return extension;
 }
 
+mojom::LocalFrame* ExtensionWebContentsObserver::GetLocalFrame(
+    content::RenderFrameHost* render_frame_host) {
+  mojo::AssociatedRemote<mojom::LocalFrame>& remote =
+      local_frame_map_[render_frame_host];
+  if (!remote.is_bound()) {
+    render_frame_host->GetRemoteAssociatedInterfaces()->GetInterface(
+        remote.BindNewEndpointAndPassReceiver());
+  }
+  return remote.get();
+}
+
 void ExtensionWebContentsObserver::OnRequest(
     content::RenderFrameHost* render_frame_host,
     const ExtensionHostMsg_Request_Params& params) {

extensions/browser/extension_web_contents_observer.h

@@ -5,12 +5,15 @@
 #ifndef EXTENSIONS_BROWSER_EXTENSION_WEB_CONTENTS_OBSERVER_H_
 #define EXTENSIONS_BROWSER_EXTENSION_WEB_CONTENTS_OBSERVER_H_
 
+#include <map>
 #include <string>
 
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "extensions/browser/extension_function_dispatcher.h"
+#include "extensions/common/mojom/frame.mojom.h"
+#include "mojo/public/cpp/bindings/associated_remote.h"
 
 namespace content {
 class BrowserContext;
@@ -68,6 +71,11 @@ class ExtensionWebContentsObserver
       content::RenderFrameHost* render_frame_host,
       bool verify_url) const;
 
+  // Returns mojom::LocalFrame* corresponding |render_frame_host|. It emplaces
+  // AssociatedRemote<mojom::LocalFrame> to |local_frame_map_| if the map
+  // doesn't have it. Note that it does not return nullptr.
+  mojom::LocalFrame* GetLocalFrame(content::RenderFrameHost* render_frame_host);
+
  protected:
   explicit ExtensionWebContentsObserver(content::WebContents* web_contents);
   ~ExtensionWebContentsObserver() override;
@@ -120,6 +128,10 @@ class ExtensionWebContentsObserver
   // Whether this object has been initialized.
   bool initialized_;
 
+  // A map of render frame host to mojo remotes.
+  std::map<content::RenderFrameHost*, mojo::AssociatedRemote<mojom::LocalFrame>>
+      local_frame_map_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionWebContentsObserver);
 };
 

extensions/browser/guest_view/web_view/web_view_guest.cc

@@ -52,6 +52,7 @@
 #include "extensions/browser/bad_message.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/extension_util.h"
+#include "extensions/browser/extension_web_contents_observer.h"
 #include "extensions/browser/extensions_browser_client.h"
 #include "extensions/browser/guest_view/web_view/web_view_constants.h"
 #include "extensions/browser/guest_view/web_view/web_view_content_script_manager.h"
@@ -68,7 +69,6 @@
 #include "net/base/escape.h"
 #include "net/base/net_errors.h"
 #include "net/cookies/canonical_cookie.h"
-#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
 #include "third_party/blink/public/common/logging/logging_utils.h"
 #include "third_party/blink/public/common/mediastream/media_stream_request.h"
 #include "third_party/blink/public/common/page/page_zoom.h"
@@ -585,19 +585,12 @@ void WebViewGuest::GuestDestroyed() {
       web_contents()->GetMainFrame()->GetRenderViewHost()->GetRoutingID());
 }
 
-extensions::mojom::LocalFrame* WebViewGuest::GetLocalFrame() {
-  if (!local_frame_.is_bound()) {
-    content::RenderFrameHost* main_frame = web_contents()->GetMainFrame();
-    main_frame->GetRemoteAssociatedInterfaces()->GetInterface(
-        local_frame_.BindNewEndpointAndPassReceiver());
-  }
-  return local_frame_.get();
-}
-
 void WebViewGuest::GuestReady() {
   // The guest RenderView should always live in an isolated guest process.
   CHECK(web_contents()->GetMainFrame()->GetProcess()->IsForGuestsOnly());
-  GetLocalFrame()->SetFrameName(name_);
+  ExtensionWebContentsObserver::GetForWebContents(web_contents())
+      ->GetLocalFrame(web_contents()->GetMainFrame())
+      ->SetFrameName(name_);
 
   // We don't want to accidentally set the opacity of an interstitial page.
   // WebContents::GetRenderWidgetHostView will return the RWHV of an
@@ -1289,14 +1282,18 @@ void WebViewGuest::SetName(const std::string& name) {
     return;
   name_ = name;
 
-  GetLocalFrame()->SetFrameName(name_);
+  ExtensionWebContentsObserver::GetForWebContents(web_contents())
+      ->GetLocalFrame(web_contents()->GetMainFrame())
+      ->SetFrameName(name_);
 }
 
 void WebViewGuest::SetSpatialNavigationEnabled(bool enabled) {
   if (is_spatial_navigation_enabled_ == enabled)
     return;
   is_spatial_navigation_enabled_ = enabled;
-  GetLocalFrame()->SetSpatialNavigationEnabled(enabled);
+  ExtensionWebContentsObserver::GetForWebContents(web_contents())
+      ->GetLocalFrame(web_contents()->GetMainFrame())
+      ->SetSpatialNavigationEnabled(enabled);
 }
 
 bool WebViewGuest::IsSpatialNavigationEnabled() const {

extensions/browser/guest_view/web_view/web_view_guest.h

@@ -22,7 +22,6 @@
 #include "extensions/browser/guest_view/web_view/web_view_permission_types.h"
 #include "extensions/browser/script_executor.h"
 #include "extensions/common/mojom/frame.mojom.h"
-#include "mojo/public/cpp/bindings/associated_remote.h"
 #include "third_party/blink/public/mojom/frame/find_in_page.mojom.h"
 
 namespace content {
@@ -392,9 +391,6 @@ class WebViewGuest : public guest_view::GuestView<WebViewGuest> {
   // Store spatial navigation status.
   bool is_spatial_navigation_enabled_;
 
-  // Holder of Mojo connection with the LocalFrame.
-  mojo::AssociatedRemote<extensions::mojom::LocalFrame> local_frame_;
-
   // This is used to ensure pending tasks will not fire after this object is
   // destroyed.
   base::WeakPtrFactory<WebViewGuest> weak_ptr_factory_{this};

extensions/common/extension_messages.h

@@ -598,10 +598,6 @@ IPC_MESSAGE_ROUTED4(ExtensionMsg_ExecuteDeclarativeScript,
 IPC_MESSAGE_ROUTED1(ExtensionMsg_UpdateBrowserWindowId,
                     int /* id of browser window */)
 
-// Tell the render view what its tab ID is.
-IPC_MESSAGE_ROUTED1(ExtensionMsg_SetTabId,
-                    int /* id of tab */)
-
 // Tell the renderer to update an extension's permission set.
 IPC_MESSAGE_CONTROL1(ExtensionMsg_UpdatePermissions,
                      ExtensionMsg_UpdatePermissions_Params)

extensions/common/mojom/frame.mojom

@@ -12,4 +12,7 @@ interface LocalFrame {
 
   // Enables or disables spatial navigation.
   SetSpatialNavigationEnabled(bool spatial_nav_enabled);
+
+  // Tells the render view what its tab ID is.
+  SetTabId(int32 tab_id);
 };

extensions/renderer/extension_frame_helper.cc

@@ -118,11 +118,11 @@ ExtensionFrameHelper::ExtensionFrameHelper(content::RenderFrame* render_frame,
   if (render_frame->IsMainFrame()) {
     // Manages its own lifetime.
     new AutomationApiHelper(render_frame);
-
-    render_frame->GetAssociatedInterfaceRegistry()->AddInterface(
-        base::BindRepeating(&ExtensionFrameHelper::BindLocalFrame,
-                            weak_ptr_factory_.GetWeakPtr()));
   }
+
+  render_frame->GetAssociatedInterfaceRegistry()->AddInterface(
+      base::BindRepeating(&ExtensionFrameHelper::BindLocalFrame,
+                          weak_ptr_factory_.GetWeakPtr()));
 }
 
 ExtensionFrameHelper::~ExtensionFrameHelper() {
@@ -383,7 +383,6 @@ bool ExtensionFrameHelper::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ExtensionMsg_DeliverMessage, OnExtensionDeliverMessage)
     IPC_MESSAGE_HANDLER(ExtensionMsg_DispatchOnDisconnect,
                         OnExtensionDispatchOnDisconnect)
-    IPC_MESSAGE_HANDLER(ExtensionMsg_SetTabId, OnExtensionSetTabId)
     IPC_MESSAGE_HANDLER(ExtensionMsg_UpdateBrowserWindowId,
                         OnUpdateBrowserWindowId)
     IPC_MESSAGE_HANDLER(ExtensionMsg_NotifyRenderViewType,
@@ -441,7 +440,7 @@ void ExtensionFrameHelper::OnExtensionDispatchOnDisconnect(
           error_message, render_frame());
 }
 
-void ExtensionFrameHelper::OnExtensionSetTabId(int tab_id) {
+void ExtensionFrameHelper::SetTabId(int32_t tab_id) {
   CHECK_EQ(tab_id_, -1);
   CHECK_GE(tab_id, 0);
   tab_id_ = tab_id;

extensions/renderer/extension_frame_helper.h

@@ -102,6 +102,7 @@ class ExtensionFrameHelper
   // mojom::LocalFrame:
   void SetFrameName(const std::string& name) override;
   void SetSpatialNavigationEnabled(bool enabled) override;
+  void SetTabId(int32_t id) override;
 
   // Called when the document element has been inserted in this frame. This
   // method may invoke untrusted JavaScript code that invalidate the frame and
@@ -159,7 +160,6 @@ class ExtensionFrameHelper
   void OnExtensionDispatchOnDisconnect(int worker_thread_id,
                                        const PortId& id,
                                        const std::string& error_message);
-  void OnExtensionSetTabId(int tab_id);
   void OnUpdateBrowserWindowId(int browser_window_id);
   void OnNotifyRendererViewType(ViewType view_type);
   void OnExtensionResponse(int request_id,
@@ -207,7 +207,6 @@ class ExtensionFrameHelper
   // navigation happens, it is either the initial one or a reload.
   bool has_started_first_navigation_ = false;
 
-  // Note that it's bound only when this frame is a main frame.
   mojo::AssociatedReceiverSet<mojom::LocalFrame> local_frame_receivers_;
 
   base::WeakPtrFactory<ExtensionFrameHelper> weak_ptr_factory_{this};