Move ownership of CrossProcessFrameConnector to RenderFrameProxyHost.

BUG=357747

Review URL: https://codereview.chromium.org/310413004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278532 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/frame_host/cross_process_frame_connector.cc

@@ -4,7 +4,7 @@
 
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 
-#include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/browser/frame_host/render_frame_proxy_host.h"
 #include "content/browser/frame_host/render_widget_host_view_child_frame.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
@@ -15,11 +15,10 @@
 namespace content {
 
 CrossProcessFrameConnector::CrossProcessFrameConnector(
-    RenderFrameHostImpl* frame_proxy_in_parent_renderer)
+    RenderFrameProxyHost* frame_proxy_in_parent_renderer)
     : frame_proxy_in_parent_renderer_(frame_proxy_in_parent_renderer),
       view_(NULL),
       device_scale_factor_(1) {
-  frame_proxy_in_parent_renderer->set_cross_process_frame_connector(this);
 }
 
 CrossProcessFrameConnector::~CrossProcessFrameConnector() {
@@ -63,7 +62,7 @@ void CrossProcessFrameConnector::set_view(
 
 void CrossProcessFrameConnector::RenderProcessGone() {
   frame_proxy_in_parent_renderer_->Send(new FrameMsg_ChildFrameProcessGone(
-      frame_proxy_in_parent_renderer_->routing_id()));
+      frame_proxy_in_parent_renderer_->GetRoutingID()));
 }
 
 void CrossProcessFrameConnector::ChildFrameBuffersSwapped(
@@ -76,10 +75,8 @@ void CrossProcessFrameConnector::ChildFrameBuffersSwapped(
   params.gpu_route_id = gpu_params.route_id;
   params.gpu_host_id = gpu_host_id;
 
-  frame_proxy_in_parent_renderer_->Send(
-      new FrameMsg_BuffersSwapped(
-          frame_proxy_in_parent_renderer_->routing_id(),
-          params));
+  frame_proxy_in_parent_renderer_->Send(new FrameMsg_BuffersSwapped(
+      frame_proxy_in_parent_renderer_->GetRoutingID(), params));
 }
 
 void CrossProcessFrameConnector::ChildFrameCompositorFrameSwapped(
@@ -93,7 +90,7 @@ void CrossProcessFrameConnector::ChildFrameCompositorFrameSwapped(
   params.producing_route_id = route_id;
   params.producing_host_id = host_id;
   frame_proxy_in_parent_renderer_->Send(new FrameMsg_CompositorFrameSwapped(
-      frame_proxy_in_parent_renderer_->routing_id(), params));
+      frame_proxy_in_parent_renderer_->GetRoutingID(), params));
 }
 
 void CrossProcessFrameConnector::OnBuffersSwappedACK(
@@ -145,7 +142,7 @@ void CrossProcessFrameConnector::OnForwardInputEvent(
   RenderWidgetHostImpl* child_widget =
       RenderWidgetHostImpl::From(view_->GetRenderWidgetHost());
   RenderWidgetHostImpl* parent_widget =
-      frame_proxy_in_parent_renderer_->render_view_host();
+      frame_proxy_in_parent_renderer_->GetRenderViewHost();
 
   if (blink::WebInputEvent::isKeyboardEventType(event->type)) {
     if (!parent_widget->GetLastKeyboardEvent())

content/browser/frame_host/cross_process_frame_connector.h

@@ -22,7 +22,7 @@ struct FrameHostMsg_ReclaimCompositorResources_Params;
 struct GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params;
 
 namespace content {
-class RenderFrameHostImpl;
+class RenderFrameProxyHost;
 class RenderWidgetHostImpl;
 class RenderWidgetHostViewChildFrame;
 
@@ -31,8 +31,8 @@ class RenderWidgetHostViewChildFrame;
 // of RenderFrameHost.
 //
 // The RenderWidgetHostView of an out-of-process child frame needs to
-// communicate with the swapped out RenderFrameHost representing this frame
-// in the process of the parent frame. For example, assume you have this page:
+// communicate with the RenderFrameProxyHost representing this frame in the
+// process of the parent frame. For example, assume you have this page:
 //
 //   -----------------
 //   | frame 1       |
@@ -43,8 +43,8 @@ class RenderWidgetHostViewChildFrame;
 //
 // If frames 1 and 2 are in process A and B, there are 4 RenderFrameHosts:
 //   A1 - RFH for frame 1 in process A
-//   B1 - Swapped out RFH for frame 1 in process B
-//   A2 - Swapped out RFH for frame 2 in process A
+//   B1 - RFPH for frame 1 in process B
+//   A2 - RFPH for frame 2 in process A
 //   B2 - RFH for frame 2 in process B
 //
 // B2, having a parent frame in a different process, will have a
@@ -57,15 +57,16 @@ class RenderWidgetHostViewChildFrame;
 // (Note: B1 is only mentioned for completeness. It is not needed in this
 // example.)
 //
-// CrossProcessFrameConnector objects are owned by the child frame's
-// RenderFrameHostManager. When a child frame swaps, SetChildFrameView() is
-// called to update to the new view.
+// CrossProcessFrameConnector objects are owned by the RenderFrameProxyHost
+// in the child frame's RenderFrameHostManager corresponding to the parent's
+// SiteInstance, A2 in the picture above. When a child frame navigates in a new
+// process, set_view() is called to update to the new view.
 //
 class CrossProcessFrameConnector {
  public:
   // |frame_proxy_in_parent_renderer| corresponds to A2 in the example above.
   explicit CrossProcessFrameConnector(
-      RenderFrameHostImpl* frame_proxy_in_parent_renderer);
+      RenderFrameProxyHost* frame_proxy_in_parent_renderer);
   virtual ~CrossProcessFrameConnector();
 
   bool OnMessageReceived(const IPC::Message &msg);
@@ -73,6 +74,7 @@ class CrossProcessFrameConnector {
   // |view| corresponds to B2's RenderWidgetHostViewChildFrame in the example
   // above.
   void set_view(RenderWidgetHostViewChildFrame* view);
+  RenderWidgetHostViewChildFrame* get_view_for_testing() { return view_; }
 
   void RenderProcessGone();
 
@@ -104,11 +106,9 @@ class CrossProcessFrameConnector {
   void SetDeviceScaleFactor(float scale_factor);
   void SetSize(gfx::Rect frame_rect);
 
-  // The RenderFrameHost that routes messages to the parent frame's renderer
-  // process.
-  // TODO(kenrb): The type becomes RenderFrameProxyHost when that class comes
-  // to exist.
-  RenderFrameHostImpl* frame_proxy_in_parent_renderer_;
+  // The RenderFrameProxyHost that routes messages to the parent frame's
+  // renderer process.
+  RenderFrameProxyHost* frame_proxy_in_parent_renderer_;
 
   // The RenderWidgetHostView for the frame. Initially NULL.
   RenderWidgetHostViewChildFrame* view_;

content/browser/frame_host/render_frame_host_impl.cc

@@ -279,8 +279,10 @@ bool RenderFrameHostImpl::OnMessageReceived(const IPC::Message &msg) {
   if (delegate_->OnMessageReceived(this, msg))
     return true;
 
-  if (cross_process_frame_connector_ &&
-      cross_process_frame_connector_->OnMessageReceived(msg))
+  RenderFrameProxyHost* proxy =
+      frame_tree_node_->render_manager()->GetProxyToParent();
+  if (proxy && proxy->cross_process_frame_connector() &&
+      proxy->cross_process_frame_connector()->OnMessageReceived(msg))
     return true;
 
   bool handled = true;

content/browser/frame_host/render_frame_host_manager.cc

@@ -12,7 +12,6 @@
 #include "base/stl_util.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/devtools/render_view_devtools_agent_host.h"
-#include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
 #include "content/browser/frame_host/debug_urls.h"
 #include "content/browser/frame_host/interstitial_page_impl.h"
@@ -22,11 +21,9 @@
 #include "content/browser/frame_host/render_frame_host_factory.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/frame_host/render_frame_proxy_host.h"
-#include "content/browser/frame_host/render_widget_host_view_child_frame.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_factory.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
-#include "content/browser/renderer_host/render_widget_host_view_base.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/browser/webui/web_ui_impl.h"
@@ -80,7 +77,6 @@ RenderFrameHostManager::RenderFrameHostManager(
       render_view_delegate_(render_view_delegate),
       render_widget_delegate_(render_widget_delegate),
       interstitial_page_(NULL),
-      cross_process_frame_connector_(NULL),
       weak_factory_(this) {
   DCHECK(frame_tree_node_);
 }
@@ -89,9 +85,6 @@ RenderFrameHostManager::~RenderFrameHostManager() {
   if (pending_render_frame_host_)
     CancelPending();
 
-  if (cross_process_frame_connector_)
-    delete cross_process_frame_connector_;
-
   // We should always have a current RenderFrameHost except in some tests.
   SetRenderFrameHost(scoped_ptr<RenderFrameHostImpl>());
 
@@ -143,6 +136,22 @@ RenderWidgetHostView* RenderFrameHostManager::GetRenderWidgetHostView() const {
   return render_frame_host_->render_view_host()->GetView();
 }
 
+RenderFrameProxyHost* RenderFrameHostManager::GetProxyToParent() {
+  if (frame_tree_node_->IsMainFrame())
+    return NULL;
+
+  RenderFrameProxyHostMap::iterator iter =
+      proxy_hosts_.find(frame_tree_node_->parent()
+                            ->render_manager()
+                            ->current_frame_host()
+                            ->GetSiteInstance()
+                            ->GetId());
+  if (iter == proxy_hosts_.end())
+    return NULL;
+
+  return iter->second;
+}
+
 void RenderFrameHostManager::SetPendingWebUI(const NavigationEntryImpl& entry) {
   pending_web_ui_.reset(
       delegate_->CreateWebUIForRenderManager(entry.GetURL()));
@@ -496,24 +505,6 @@ void RenderFrameHostManager::SwapOutOldPage() {
   // no longer on the stack when we send the SwapOut message.
   delegate_->CancelModalDialogsForRenderManager();
 
-  if (!frame_tree_node_->IsMainFrame()) {
-    // The RenderFrameHost being swapped out becomes the proxy for this
-    // frame in its parent's process. CrossProcessFrameConnector
-    // initialization only needs to happen on an initial cross-process
-    // navigation, when the RenderFrame leaves the same process as its parent.
-    // The same CrossProcessFrameConnector is used for subsequent cross-
-    // process navigations, but it will be destroyed if the Frame is
-    // navigated back to the same site instance as its parent.
-    // TODO(kenrb): This will change when RenderFrameProxyHost is created.
-    // TODO(nasko): Move CrossProcessFrameConnector to be owned by
-    // RenderFrameProxyHost instead of RenderFrameHostManager once proxy
-    // support lands.
-    if (!cross_process_frame_connector_) {
-      cross_process_frame_connector_ =
-          new CrossProcessFrameConnector(render_frame_host_.get());
-    }
-  }
-
   // Create the RenderFrameProxyHost that will replace the
   // RenderFrameHost which is swapping out. If one exists, ensure it is deleted
   // from the map and not leaked.
@@ -921,8 +912,6 @@ int RenderFrameHostManager::CreateRenderFrame(
   // remove it from the list of swapped out hosts if it commits.
   RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
 
-  FrameTreeNode* parent_node = frame_tree_node_->parent();
-
   if (proxy) {
     routing_id = proxy->GetRenderViewHost()->GetRoutingID();
     // Delete the existing RenderFrameProxyHost, but reuse the RenderFrameHost.
@@ -947,15 +936,6 @@ int RenderFrameHostManager::CreateRenderFrame(
           rvh->AllowBindings(required_bindings);
         }
       }
-    } else {
-      // Detect if this is a cross-process child frame that is navigating
-      // back to the same SiteInstance as its parent.
-      if (parent_node && cross_process_frame_connector_ &&
-          render_frame_host_->GetSiteInstance() == parent_node->
-              render_manager()->current_frame_host()->GetSiteInstance()) {
-        delete cross_process_frame_connector_;
-        cross_process_frame_connector_ = NULL;
-      }
     }
   } else {
     // Create a new RenderFrameHost if we don't find an existing one.
@@ -1190,11 +1170,11 @@ void RenderFrameHostManager::CommitPending() {
       // created already and we just need to link it to the proper view in the
       // new process.
       if (!is_main_frame) {
-        RenderWidgetHostView* rwhv =
-            render_frame_host_->render_view_host()->GetView();
-        RenderWidgetHostViewChildFrame* rwhv_child =
-            static_cast<RenderWidgetHostViewChildFrame*>(rwhv);
-        cross_process_frame_connector_->set_view(rwhv_child);
+        RenderFrameProxyHost* proxy = GetProxyToParent();
+        if (proxy) {
+          proxy->SetChildRWHView(
+              render_frame_host_->render_view_host()->GetView());
+        }
       }
     }
   }

content/browser/frame_host/render_frame_host_manager.h

@@ -154,6 +154,8 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
   // there is no current one.
   RenderWidgetHostView* GetRenderWidgetHostView() const;
 
+  RenderFrameProxyHost* GetProxyToParent();
+
   // Returns the pending RenderFrameHost, or NULL if there is no pending one.
   RenderFrameHostImpl* pending_frame_host() const {
     return pending_render_frame_host_.get();
@@ -476,13 +478,6 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
 
   NotificationRegistrar registrar_;
 
-  // When |render_frame_host_| is in a different process from its parent in
-  // the frame tree, this class connects its associated RenderWidgetHostView
-  // to the proxy RenderFrameHost for the parent's renderer process. NULL
-  // when |render_frame_host_| is the frame tree root or is in the same
-  // process as its parent.
-  CrossProcessFrameConnector* cross_process_frame_connector_;
-
   base::WeakPtrFactory<RenderFrameHostManager> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderFrameHostManager);

content/browser/frame_host/render_frame_proxy_host.cc

@@ -4,8 +4,11 @@
 
 #include "content/browser/frame_host/render_frame_proxy_host.h"
 
+#include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
+#include "content/browser/frame_host/render_widget_host_view_child_frame.h"
+#include "content/browser/renderer_host/render_widget_host_view_base.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/common/frame_messages.h"
 #include "ipc/ipc_message.h"
@@ -18,6 +21,21 @@ RenderFrameProxyHost::RenderFrameProxyHost(SiteInstance* site_instance,
       site_instance_(site_instance),
       frame_tree_node_(frame_tree_node) {
   GetProcess()->AddRoute(routing_id_, this);
+
+  if (!frame_tree_node_->IsMainFrame() &&
+      frame_tree_node_->parent()
+              ->render_manager()
+              ->current_frame_host()
+              ->GetSiteInstance() == site_instance) {
+    // The RenderFrameHost navigating cross-process is destroyed and a proxy for
+    // it is created in the parent's process. CrossProcessFrameConnector
+    // initialization only needs to happen on an initial cross-process
+    // navigation, when the RenderFrameHost leaves the same process as its
+    // parent. The same CrossProcessFrameConnector is used for subsequent cross-
+    // process navigations, but it will be destroyed if the frame is
+    // navigated back to the same SiteInstance as its parent.
+    cross_process_frame_connector_.reset(new CrossProcessFrameConnector(this));
+  }
 }
 
 RenderFrameProxyHost::~RenderFrameProxyHost() {
@@ -27,6 +45,11 @@ RenderFrameProxyHost::~RenderFrameProxyHost() {
   GetProcess()->RemoveRoute(routing_id_);
 }
 
+void RenderFrameProxyHost::SetChildRWHView(RenderWidgetHostView* view) {
+  cross_process_frame_connector_->set_view(
+      static_cast<RenderWidgetHostViewChildFrame*>(view));
+}
+
 RenderViewHostImpl* RenderFrameProxyHost::GetRenderViewHost() {
   if (render_frame_host_.get())
     return render_frame_host_->render_view_host();
@@ -47,6 +70,10 @@ bool RenderFrameProxyHost::Send(IPC::Message *msg) {
 }
 
 bool RenderFrameProxyHost::OnMessageReceived(const IPC::Message& msg) {
+  if (cross_process_frame_connector_.get() &&
+      cross_process_frame_connector_->OnMessageReceived(msg))
+    return true;
+
   // TODO(nasko): This can be removed once we don't have a swapped out state on
   // RenderFrameHosts. See https://crbug.com/357747.
   if (render_frame_host_.get())

content/browser/frame_host/render_frame_proxy_host.h

@@ -11,12 +11,14 @@
 #include "ipc/ipc_listener.h"
 #include "ipc/ipc_sender.h"
 
+namespace content {
+
+class CrossProcessFrameConnector;
 class FrameTreeNode;
 class RenderProcessHost;
 class RenderFrameHostImpl;
 class RenderViewHostImpl;
-
-namespace content {
+class RenderWidgetHostView;
 
 // When a page's frames are rendered by multiple processes, each renderer has a
 // full copy of the frame tree. It has full RenderFrames for the frames it is
@@ -69,6 +71,8 @@ class RenderFrameProxyHost
     return site_instance_.get();
   }
 
+  void SetChildRWHView(RenderWidgetHostView* view);
+
   // TODO(nasko): The following methods should be removed once we don't have a
   // swapped out state on RenderFrameHosts. See https://crbug.com/357747.
   RenderFrameHostImpl* render_frame_host() {
@@ -85,10 +89,14 @@ class RenderFrameProxyHost
   // IPC::Sender
   virtual bool Send(IPC::Message* msg) OVERRIDE;
 
- private:
   // IPC::Listener
   virtual bool OnMessageReceived(const IPC::Message& msg) OVERRIDE;
 
+  CrossProcessFrameConnector* cross_process_frame_connector() {
+    return cross_process_frame_connector_.get();
+  }
+
+ private:
   // This RenderFrameProxyHost's routing id.
   int routing_id_;
 
@@ -98,6 +106,12 @@ class RenderFrameProxyHost
   // The node in the frame tree where this proxy is located.
   FrameTreeNode* frame_tree_node_;
 
+  // When a RenderFrameHost is in a different process from its parent in the
+  // frame tree, this class connects its associated RenderWidgetHostView
+  // to this RenderFrameProxyHost, which corresponds to the same frame in the
+  // parent's renderer process.
+  scoped_ptr<CrossProcessFrameConnector> cross_process_frame_connector_;
+
   // TODO(nasko): This can be removed once we don't have a swapped out state on
   // RenderFrameHosts. See https://crbug.com/357747.
   scoped_ptr<RenderFrameHostImpl> render_frame_host_;

content/browser/site_per_process_browsertest.cc

@@ -4,7 +4,10 @@
 
 #include "base/command_line.h"
 #include "base/strings/stringprintf.h"
+#include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/frame_tree.h"
+#include "content/browser/frame_host/render_frame_proxy_host.h"
+#include "content/browser/frame_host/render_widget_host_view_child_frame.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/notification_observer.h"
@@ -212,8 +215,9 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
   SitePerProcessWebContentsObserver observer(shell()->web_contents());
 
   // Load same-site page into iframe.
+  FrameTreeNode* child = root->child_at(0);
   GURL http_url(test_server()->GetURL("files/title1.html"));
-  NavigateFrameToURL(root->child_at(0), http_url);
+  NavigateFrameToURL(child, http_url);
   EXPECT_EQ(http_url, observer.navigation_url());
   EXPECT_TRUE(observer.navigation_succeeded());
   {
@@ -224,6 +228,10 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
             ->GetRenderWidgetHostViewsInTree();
     EXPECT_EQ(1U, views_set.size());
   }
+  RenderFrameProxyHost* proxy_to_parent =
+      child->render_manager()->GetRenderFrameProxyHost(
+          shell()->web_contents()->GetSiteInstance());
+  EXPECT_FALSE(proxy_to_parent);
 
   // These must stay in scope with replace_host.
   GURL::Replacements replace_host;
@@ -239,7 +247,6 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
 
   // Ensure that we have created a new process for the subframe.
   ASSERT_EQ(1U, root->child_count());
-  FrameTreeNode* child = root->child_at(0);
   SiteInstance* site_instance = child->current_frame_host()->GetSiteInstance();
   RenderViewHost* rvh = child->current_frame_host()->render_view_host();
   RenderProcessHost* rph = child->current_frame_host()->GetProcess();
@@ -254,6 +261,12 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
             ->GetRenderWidgetHostViewsInTree();
     EXPECT_EQ(2U, views_set.size());
   }
+  proxy_to_parent = child->render_manager()->GetProxyToParent();
+  EXPECT_TRUE(proxy_to_parent);
+  EXPECT_TRUE(proxy_to_parent->cross_process_frame_connector());
+  EXPECT_EQ(
+      rvh->GetView(),
+      proxy_to_parent->cross_process_frame_connector()->get_view_for_testing());
 
   // Load another cross-site page into the same iframe.
   cross_site_url = test_server()->GetURL("files/title3.html");
@@ -284,6 +297,11 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, CrossSiteIframe) {
             ->GetRenderWidgetHostViewsInTree();
     EXPECT_EQ(2U, views_set.size());
   }
+  EXPECT_EQ(proxy_to_parent, child->render_manager()->GetProxyToParent());
+  EXPECT_TRUE(proxy_to_parent->cross_process_frame_connector());
+  EXPECT_EQ(
+      child->current_frame_host()->render_view_host()->GetView(),
+      proxy_to_parent->cross_process_frame_connector()->get_view_for_testing());
 }
 
 // Crash a subframe and ensures its children are cleared from the FrameTree.