Revert 283572 "Changes to RenderFrameProxy:"

ASan failure (use-after-free) looks like it's due to this change):

http://build.chromium.org/p/chromium.memory/builders/Linux%20ASan%20LSan%20Te...

E.g.:

@@@STEP_LOG_LINE@ReloadEmbedder@SUMMARY: AddressSanitizer: heap-use-after-free content/renderer/render_frame_proxy.cc:168 content::RenderFrameProxy::OnMessageReceived(IPC::Message const&)@@@j

> Changes to RenderFrameProxy:
> 
> - Add accessors: web_frame(), routing_id(), render_view().
> - Remove accessor: render_frame(). Where we do need to touch the
>   RenderFrame, we'll look it up by its routing ID.
> - Small change to the CompositingHelper to use the new getters.
> - Add a map to allow finding a RenderFrameProxy by its associated
>   blink::WebFrame.
> - Introduce a second factory function and differentiate the two
>   factory functions according to the two ways RenderFrameProxies will
>   be created. The first is for when an extant local RenderFrame is
>   being swapped out and replaced with a new RenderFrameProxy. The
>   second is for when a RenderFrameProxy needs to be created without
>   displacing an existing RenderFrame, as shall occur once we mirror
>   the frame tree.
> - This second factory function, which is uncalled at the moment, will
>   create WebRemoteFrames. Also there is stubbed out code in the first
>   factory function to create WebRemoteFrames. This code is in
>   preparation for eliminating the RenderFrame (and its attendant
>   WebLocalFrame) and having instead just a RenderFrameProxy.
> - Add some defensive checks to prepare for when the render frame may
>   not exist, as will happen once the second factory function
>   enters use.
> - Add an Init function so that code can be shared between the two
>   factory functions.
> 
> As an adminstrative note, this patch is a chunk of nasko's 
> larger "use RenderFrameProxyHost" effor (issue 241223002)
> 
> BUG=357747
> TEST=browsertests, http://csreis.github.io/tests/cross-site-iframe.html renders after going cross-site under --site-per-process
> 
> Review URL: https://codereview.chromium.org/357043006

TBR=nick@chromium.org

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

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

1 files changed, 25 insertions, 111 deletions

diff --git a/content/renderer/render_frame_proxy.cc b/content/renderer/render_frame_proxy.cc
index 1eeaab2..2e85a52 100644
--- a/content/renderer/render_frame_proxy.cc
+++ b/content/renderer/render_frame_proxy.cc
@@ -19,72 +19,18 @@ namespace content {
 
 namespace {
 
-// Facilitates lookup of RenderFrameProxy by routing_id.
 typedef std::map<int, RenderFrameProxy*> RoutingIDProxyMap;
 static base::LazyInstance<RoutingIDProxyMap> g_routing_id_proxy_map =
     LAZY_INSTANCE_INITIALIZER;
 
-// Facilitates lookup of RenderFrameProxy by WebFrame.
-typedef std::map<blink::WebFrame*, RenderFrameProxy*> FrameMap;
-base::LazyInstance<FrameMap> g_frame_map = LAZY_INSTANCE_INITIALIZER;
-
 }  // namespace
 
 // static
-RenderFrameProxy* RenderFrameProxy::CreateProxyToReplaceFrame(
-    RenderFrameImpl* frame_to_replace,
-    int routing_id) {
-  CHECK_NE(routing_id, MSG_ROUTING_NONE);
-
-  scoped_ptr<RenderFrameProxy> proxy(
-      new RenderFrameProxy(routing_id, frame_to_replace->GetRoutingID()));
-#if 0
-  // TODO(nick): Enable this code when we're ready to create WebRemoteFrames.
-  blink::WebRemoteFrame* web_frame = NULL;
-  if (frame_to_replace->GetWebFrame()->parent() &&
-      frame_to_replace->GetWebFrame()->parent()->isWebRemoteFrame()) {
-    blink::WebRemoteFrame* parent_web_frame =
-        frame_to_replace->GetWebFrame()->parent()->toWebRemoteFrame();
-    web_frame = parent_web_frame->createRemoteChild("", proxy.get());
-  } else {
-    web_frame = blink::WebRemoteFrame::create(proxy.get());
-  }
-#else
-  blink::WebFrame* web_frame = frame_to_replace->GetWebFrame();
-#endif
-  proxy->Init(web_frame, frame_to_replace->render_view());
-  return proxy.release();
-}
-
-RenderFrameProxy* RenderFrameProxy::CreateFrameProxy(
-    int routing_id,
-    int parent_routing_id,
-    int render_view_routing_id) {
-  scoped_ptr<RenderFrameProxy> proxy(
-      new RenderFrameProxy(routing_id, MSG_ROUTING_NONE));
-  RenderViewImpl* render_view = NULL;
-  blink::WebRemoteFrame* web_frame = NULL;
-  if (parent_routing_id == MSG_ROUTING_NONE) {
-    // Create a top level frame.
-    render_view = RenderViewImpl::FromRoutingID(render_view_routing_id);
-    web_frame = blink::WebRemoteFrame::create(proxy.get());
-    render_view->webview()->setMainFrame(web_frame);
-  } else {
-    // Create a frame under an existing parent. The parent is always expected to
-    // be a RenderFrameProxy, because navigations initiated by local frames
-    // should not wind up here.
-    RenderFrameProxy* parent =
-        RenderFrameProxy::FromRoutingID(parent_routing_id);
-    CHECK(parent);
-    CHECK(parent->web_frame()->isWebRemoteFrame());
-    web_frame = parent->web_frame()->toWebRemoteFrame()->createRemoteChild(
-        "", proxy.get());
-    render_view = parent->render_view();
-  }
-
-  proxy->Init(web_frame, render_view);
-
-  return proxy.release();
+RenderFrameProxy* RenderFrameProxy::CreateFrameProxy(int routing_id,
+                                                     int frame_routing_id) {
+  DCHECK_NE(routing_id, MSG_ROUTING_NONE);
+  RenderFrameProxy* proxy = new RenderFrameProxy(routing_id, frame_routing_id);
+  return proxy;
 }
 
 // static
@@ -94,58 +40,27 @@ RenderFrameProxy* RenderFrameProxy::FromRoutingID(int32 routing_id) {
   return it == proxies->end() ? NULL : it->second;
 }
 
-// static
-RenderFrameProxy* RenderFrameProxy::FromWebFrame(blink::WebFrame* web_frame) {
-  FrameMap::iterator iter = g_frame_map.Get().find(web_frame);
-  if (iter != g_frame_map.Get().end()) {
-    RenderFrameProxy* proxy = iter->second;
-    DCHECK_EQ(web_frame, proxy->web_frame());
-    return proxy;
-  }
-  return NULL;
-}
-
 RenderFrameProxy::RenderFrameProxy(int routing_id, int frame_routing_id)
     : routing_id_(routing_id),
-      frame_routing_id_(frame_routing_id),
-      web_frame_(NULL),
-      render_view_(NULL) {
+      frame_routing_id_(frame_routing_id) {
   std::pair<RoutingIDProxyMap::iterator, bool> result =
-      g_routing_id_proxy_map.Get().insert(std::make_pair(routing_id_, this));
+    g_routing_id_proxy_map.Get().insert(std::make_pair(routing_id_, this));
   CHECK(result.second) << "Inserting a duplicate item.";
   RenderThread::Get()->AddRoute(routing_id_, this);
+
+  render_frame_ = RenderFrameImpl::FromRoutingID(frame_routing_id);
+  CHECK(render_frame_);
+  render_frame_->render_view()->RegisterRenderFrameProxy(this);
 }
 
 RenderFrameProxy::~RenderFrameProxy() {
-  render_view()->UnregisterRenderFrameProxy(this);
-
-  FrameMap::iterator it = g_frame_map.Get().find(web_frame_);
-  CHECK(it != g_frame_map.Get().end());
-  CHECK_EQ(it->second, this);
-  g_frame_map.Get().erase(it);
-
+  render_frame_->render_view()->UnregisterRenderFrameProxy(this);
   RenderThread::Get()->RemoveRoute(routing_id_);
   g_routing_id_proxy_map.Get().erase(routing_id_);
-
-  // TODO(nick): Call close unconditionally when web_frame() is always remote.
-  if (web_frame()->isWebRemoteFrame())
-    web_frame()->close();
 }
 
-void RenderFrameProxy::Init(blink::WebFrame* web_frame,
-                            RenderViewImpl* render_view) {
-  CHECK(web_frame);
-  CHECK(render_view);
-
-  web_frame_ = web_frame;
-  render_view_ = render_view;
-
-  // TODO(nick): Should all RenderFrameProxies remain observers of their views?
-  render_view_->RegisterRenderFrameProxy(this);
-
-  std::pair<FrameMap::iterator, bool> result =
-      g_frame_map.Get().insert(std::make_pair(web_frame_, this));
-  CHECK(result.second) << "Inserted a duplicate item.";
+blink::WebFrame* RenderFrameProxy::GetWebFrame() {
+  return render_frame_->GetWebFrame();
 }
 
 void RenderFrameProxy::DidCommitCompositorFrame() {
@@ -164,10 +79,8 @@ bool RenderFrameProxy::OnMessageReceived(const IPC::Message& msg) {
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
-  RenderFrameImpl* render_frame =
-      RenderFrameImpl::FromRoutingID(frame_routing_id_);
-  if (!handled && render_frame)
-    return render_frame->OnMessageReceived(msg);
+  if (!handled)
+    return render_frame_->OnMessageReceived(msg);
 
   return handled;
 }
@@ -184,9 +97,8 @@ bool RenderFrameProxy::Send(IPC::Message* message) {
 void RenderFrameProxy::OnDeleteProxy() {
   RenderFrameImpl* render_frame =
       RenderFrameImpl::FromRoutingID(frame_routing_id_);
-
-  if (render_frame)
-    render_frame->set_render_frame_proxy(NULL);
+  CHECK(render_frame);
+  render_frame->set_render_frame_proxy(NULL);
 
   delete this;
 }
@@ -198,9 +110,10 @@ void RenderFrameProxy::OnChildFrameProcessGone() {
 
 void RenderFrameProxy::OnBuffersSwapped(
     const FrameMsg_BuffersSwapped_Params& params) {
-  if (!compositing_helper_) {
+  if (!compositing_helper_.get()) {
     compositing_helper_ =
-        ChildFrameCompositingHelper::CreateForRenderFrameProxy(this);
+        ChildFrameCompositingHelper::CreateCompositingHelperForRenderFrame(
+            GetWebFrame(), this, routing_id_);
     compositing_helper_->EnableCompositing(true);
   }
   compositing_helper_->OnBuffersSwapped(
@@ -208,7 +121,7 @@ void RenderFrameProxy::OnBuffersSwapped(
       params.mailbox,
       params.gpu_route_id,
       params.gpu_host_id,
-      web_frame()->view()->deviceScaleFactor());
+      render_frame_->render_view()->GetWebView()->deviceScaleFactor());
 }
 
 void RenderFrameProxy::OnCompositorFrameSwapped(const IPC::Message& message) {
@@ -219,9 +132,10 @@ void RenderFrameProxy::OnCompositorFrameSwapped(const IPC::Message& message) {
   scoped_ptr<cc::CompositorFrame> frame(new cc::CompositorFrame);
   param.a.frame.AssignTo(frame.get());
 
-  if (!compositing_helper_) {
+  if (!compositing_helper_.get()) {
     compositing_helper_ =
-        ChildFrameCompositingHelper::CreateForRenderFrameProxy(this);
+        ChildFrameCompositingHelper::CreateCompositingHelperForRenderFrame(
+            GetWebFrame(), this, routing_id_);
     compositing_helper_->EnableCompositing(true);
   }
   compositing_helper_->OnCompositorFrameSwapped(frame.Pass(),