Always create FrameTreeNodes and RenderFrameHosts for every frame.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 3165905917e543ec0117fb38f9e64a735917cd49..24b80e0005cbaf1996136a88f2dad40fb7628623 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -222,27 +222,25 @@ void RenderFrameImpl::didAccessInitialDocument(blink::WebFrame* frame) {
 blink::WebFrame* RenderFrameImpl::createChildFrame(
     blink::WebFrame* parent,
     const blink::WebString& name) {
-  RenderFrameImpl* child_render_frame = this;
   long long child_frame_identifier = WebFrame::generateEmbedderIdentifier();
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess)) {
-    // Synchronously notify the browser of a child frame creation to get the
-    // routing_id for the RenderFrame.
-    int routing_id;
-    Send(new FrameHostMsg_CreateChildFrame(GetRoutingID(),
-                                           parent->identifier(),
-                                           child_frame_identifier,
-                                           UTF16ToUTF8(name),
-                                           &routing_id));
-    child_render_frame = RenderFrameImpl::Create(render_view_, routing_id);
-  }
+  // Synchronously notify the browser of a child frame creation to get the
+  // routing_id for the RenderFrame.
+  int routing_id = MSG_ROUTING_NONE;

[Charlie Reis, 2013/11/21 23:59:15]

I added this since it didn't seem like a good idea to risk having it
uninitialized.  This follows the pattern from RenderViewImpl::createView.

[awong, 2013/11/22 01:07:15]

SGTM

+  Send(new FrameHostMsg_CreateChildFrame(GetRoutingID(),
+                                         parent->identifier(),
+                                         child_frame_identifier,
+                                         UTF16ToUTF8(name),
+                                         &routing_id));
+  if (routing_id == MSG_ROUTING_NONE)
+    return NULL;
+  RenderFrameImpl* child_render_frame = RenderFrameImpl::Create(render_view_,
+                                                                routing_id);
 
   blink::WebFrame* web_frame = WebFrame::create(child_render_frame,
                                                 child_frame_identifier);
 
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess)) {
-    g_child_frame_map.Get().insert(
-        std::make_pair(web_frame, child_render_frame));
-  }
+  g_child_frame_map.Get().insert(
+      std::make_pair(web_frame, child_render_frame));
 
   return web_frame;
 }
@@ -264,19 +262,11 @@ void RenderFrameImpl::frameDetached(blink::WebFrame* frame) {
   Send(new FrameHostMsg_Detach(GetRoutingID(), parent_frame_id,
                                frame->identifier()));
 
-  // Currently multiple WebCore::Frames can send frameDetached to a single
-  // RenderFrameImpl. This is legacy behavior from when RenderViewImpl served
-  // as a shared WebFrameClient for multiple Webcore::Frame objects. It also
-  // prevents this class from entering the |is_detaching_| state because
-  // even though one WebCore::Frame may have detached itself, others will
-  // still need to use this object.
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess)) {
-    // The |is_detaching_| flag disables Send(). FrameHostMsg_Detach must be
-    // sent before setting |is_detaching_| to true. In contrast, Observers
-    // should only be notified afterwards so they cannot call back into and
-    // have IPCs fired off.
-    is_detaching_ = true;
-  }
+  // The |is_detaching_| flag disables Send(). FrameHostMsg_Detach must be

[nasko, 2013/11/22 00:09:01]

Any specific reason why the existing comment about multiple calls to
frameDetached is now removed?

[Charlie Reis, 2013/11/22 00:13:21]

That comment no longer applies.  :)  There's always a 1:1 relationship between
blink::Frame and RenderFrameImpls now.

[nasko, 2013/11/22 00:25:04]

Albert, I recall you were getting multiple calls to frameDetached for the same
blink::Frame object. Is that still true? I thought the comment was related to
that.

[awong, 2013/11/22 01:07:15]

Daniel's reply is correct.

+  // sent before setting |is_detaching_| to true. In contrast, Observers
+  // should only be notified afterwards so they cannot call back into and
+  // have IPCs fired off.
+  is_detaching_ = true;
 
   // Call back to RenderViewImpl for observers to be notified.
   // TODO(nasko): Remove once we have RenderFrameObserver.
@@ -284,18 +274,16 @@ void RenderFrameImpl::frameDetached(blink::WebFrame* frame) {
 
   frame->close();
 
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess)) {
-    // If the frame does not have a parent, it is the main frame. The main
-    // frame is owned by the containing RenderViewHost so it does not require
-    // any cleanup here.
-    if (frame->parent()) {
-      FrameMap::iterator it = g_child_frame_map.Get().find(frame);
-      DCHECK(it != g_child_frame_map.Get().end());
-      DCHECK_EQ(it->second, this);
-      g_child_frame_map.Get().erase(it);
-      delete this;
-      // Object is invalid after this point.
-    }
+  // If the frame does not have a parent, it is the main frame. The main
+  // frame is owned by the containing RenderViewHost so it does not require
+  // any cleanup here.
+  if (frame->parent()) {
+    FrameMap::iterator it = g_child_frame_map.Get().find(frame);
+    DCHECK(it != g_child_frame_map.Get().end());
+    DCHECK_EQ(it->second, this);
+    g_child_frame_map.Get().erase(it);
+    delete this;
+    // Object is invalid after this point.
   }
 }