Create RenderFrameProxyHost at time of swap-out instead of commit.

content/browser/frame_host/render_frame_proxy_host.h

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CONTENT_BROWSER_FRAME_HOST_RENDER_FRAME_PROXY_HOST_H_
 #define CONTENT_BROWSER_FRAME_HOST_RENDER_FRAME_PROXY_HOST_H_
 
 #include "base/memory/scoped_ptr.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/site_instance_impl.h"
+#include "ipc/ipc_listener.h"
+#include "ipc/ipc_sender.h"
 
+class FrameTreeNode;
 class RenderProcessHost;
 class RenderFrameHostImpl;
 class RenderViewHostImpl;
 
 namespace content {
 
 // 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
 // responsible for rendering and placeholder objects (i.e., RenderFrameProxies)
 // for frames rendered by other processes.
@@ skipping 18 matching lines @@
 // * When navigating cross-process and there is already a RenderFrameProxyHost
 // for the new SiteInstance. A pending RenderFrameHost is created, but it is
 // not used until it commits. At that point, RenderFrameHostManager transitions
 // the pending RenderFrameHost to the active one and deletes the proxy.
 // * When navigating cross-process and the existing document has an unload
 // event handler. When the new navigation commits, RenderFrameHostManager
 // creates a RenderFrameProxyHost for the old SiteInstance and uses it going
 // forward. It also instructs the RenderFrameHost to run the unload event
 // handler and is kept alive for the duration. Once the event handling is
 // complete, the RenderFrameHost is deleted.
-class RenderFrameProxyHost {
+class RenderFrameProxyHost
+    : public IPC::Listener,
+      public IPC::Sender {
  public:
-  explicit RenderFrameProxyHost(
-       scoped_ptr<RenderFrameHostImpl> render_frame_host);
-  ~RenderFrameProxyHost();
+  RenderFrameProxyHost(SiteInstance* site_instance,
+                       FrameTreeNode* frame_tree_node);
+  virtual ~RenderFrameProxyHost();
 
   RenderProcessHost* GetProcess() {
-    return render_frame_host_->GetProcess();
+    return site_instance_->GetProcess();
+  }
+
+  int GetRoutingID() {
+    return routing_id_;
   }
 
   SiteInstance* GetSiteInstance() {
     return site_instance_.get();
   }
 
+  // This method deletes the RenderFrameProxy object in the renderer process,

[Charlie Reis, 2014/05/15 00:32:50]

nit: Remove comma

[nasko, 2014/05/15 18:47:13]

Done.

+  // which corresponds to this RenderFrameProxyHost. Must be called before

[Charlie Reis, 2014/05/15 00:32:50]

I'm a bit concerned about missing cases where the host is deleted and leaking it
in the renderer process.  Is there a reason we can't send the IPC message from
the RFPH destructor?

[nasko, 2014/05/15 18:47:13]

In the case of a renderer process crashing, we delete all proxies for that
process, so we will be sending IPCs to non-existing process.
After chatting about it, it can be moved to the destructor and gated on
GetProcess()->HasConnection().

+  // deleting the RenderFrameProxyHost if the renderer process exists and is
+  // responsive.
+  void DeleteRendererProxy();

[Charlie Reis, 2014/05/15 00:32:50]

nit: DeleteRenderFrameProxy()

[nasko, 2014/05/15 18:47:13]

Done.

+
   // TODO(nasko): The following methods should be removed when swapping out
   // of RenderFrameHosts is no longer used.
   RenderFrameHostImpl* render_frame_host() {
     return render_frame_host_.get();
   }
   RenderViewHostImpl* render_view_host() {

[Charlie Reis, 2014/05/15 00:32:50]

This isn't a one liner anymore, so it should be in the .cc file.

[nasko, 2014/05/15 18:47:13]

This is no longer needed.

-    return render_frame_host_->render_view_host();
+    if (render_frame_host_.get())
+      return render_frame_host_->render_view_host();
+    return NULL;
   }
-  scoped_ptr<RenderFrameHostImpl> PassFrameHost() {
+  void TakeFrameHostOwnership(
+      scoped_ptr<RenderFrameHostImpl> render_frame_host) {
+    render_frame_host_ = render_frame_host.Pass();
+  }
+  scoped_ptr<RenderFrameHostImpl> PassFrameHostOwnership() {

[Charlie Reis, 2014/05/15 00:32:50]

nit: Move implementation to .cc file.

[nasko, 2014/05/15 18:47:13]

Done.

+    DeleteRendererProxy();
     return render_frame_host_.Pass();
   }
 
+  // IPC::Sender
+  virtual bool Send(IPC::Message* msg) OVERRIDE;
+
  private:
+  // IPC::Listener
+  virtual bool OnMessageReceived(const IPC::Message& msg) OVERRIDE;
+
+  // This RenderFrameProxyHost's routing id.
+  int routing_id_;
+
+  // The SiteInstance this proxy is associated with.
   scoped_refptr<SiteInstance> site_instance_;
 
+  // The node in the frame tree where this proxy is located.
+  FrameTreeNode* frame_tree_node_;
+
   // TODO(nasko): For now, hide the RenderFrameHost inside the proxy, but remove
   // it once we have all the code support for proper proxy objects.
   scoped_ptr<RenderFrameHostImpl> render_frame_host_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderFrameProxyHost);
 };
 
 }  // namespace
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_RENDER_FRAME_PROXY_HOST_H_