Revert 237119 "Always create FrameTreeNodes and RenderFrameHosts..."

> Always create FrameTreeNodes and RenderFrameHosts for every frame.
> 
> This allows us to run the FrameTreeShape tests without a flag.
> 
> BUG=245126
> TEST=FrameTreeBrowserTest.FrameTreeShape*
> 
> Review URL: https://codereview.chromium.org/82033002

TBR=creis@chromium.org

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

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

11 files changed, 124 insertions, 244 deletions

diff --git a/content/browser/frame_host/frame_tree_browsertest.cc b/content/browser/frame_host/frame_tree_browsertest.cc
deleted file mode 100644
index 9bc72e9..0000000
--- a/content/browser/frame_host/frame_tree_browsertest.cc
+++ /dev/null
@@ -1,169 +0,0 @@
-// Copyright 2013 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.
-
-#include "content/browser/frame_host/frame_tree.h"
-#include "content/browser/frame_host/frame_tree_node.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_service.h"
-#include "content/public/browser/notification_types.h"
-#include "content/public/common/url_constants.h"
-#include "content/public/test/test_navigation_observer.h"
-#include "content/public/test/test_utils.h"
-#include "content/shell/browser/shell.h"
-#include "content/test/content_browser_test.h"
-#include "content/test/content_browser_test_utils.h"
-#include "net/dns/mock_host_resolver.h"
-
-namespace content {
-
-class FrameTreeBrowserTest : public ContentBrowserTest {
- public:
-  FrameTreeBrowserTest() {}
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(FrameTreeBrowserTest);
-};
-
-// Ensures FrameTree correctly reflects page structure during navigations.
-IN_PROC_BROWSER_TEST_F(FrameTreeBrowserTest, FrameTreeShape) {
-  host_resolver()->AddRule("*", "127.0.0.1");
-  ASSERT_TRUE(test_server()->Start());
-
-  GURL base_url = test_server()->GetURL("files/site_isolation/");
-  GURL::Replacements replace_host;
-  std::string host_str("A.com");  // Must stay in scope with replace_host.
-  replace_host.SetHostStr(host_str);
-  base_url = base_url.ReplaceComponents(replace_host);
-
-  // Load doc without iframes. Verify FrameTree just has root.
-  // Frame tree:
-  //   Site-A Root
-  NavigateToURL(shell(), base_url.Resolve("blank.html"));
-  FrameTreeNode* root =
-      static_cast<WebContentsImpl*>(shell()->web_contents())->
-      GetFrameTree()->root();
-  EXPECT_EQ(0U, root->child_count());
-
-  // Add 2 same-site frames. Verify 3 nodes in tree with proper names.
-  // Frame tree:
-  //   Site-A Root -- Site-A frame1
-  //              \-- Site-A frame2
-  WindowedNotificationObserver observer1(
-      content::NOTIFICATION_LOAD_STOP,
-      content::Source<NavigationController>(
-          &shell()->web_contents()->GetController()));
-  NavigateToURL(shell(), base_url.Resolve("frames-X-X.html"));
-  observer1.Wait();
-  ASSERT_EQ(2U, root->child_count());
-  EXPECT_EQ(0U, root->child_at(0)->child_count());
-  EXPECT_EQ(0U, root->child_at(1)->child_count());
-}
-
-// TODO(ajwong): Talk with nasko and merge this functionality with
-// FrameTreeShape.
-IN_PROC_BROWSER_TEST_F(FrameTreeBrowserTest, FrameTreeShape2) {
-  ASSERT_TRUE(test_server()->Start());
-  NavigateToURL(shell(),
-                test_server()->GetURL("files/frame_tree/top.html"));
-
-  WebContentsImpl* wc = static_cast<WebContentsImpl*>(shell()->web_contents());
-  RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(
-      wc->GetRenderViewHost());
-  FrameTreeNode* root = wc->GetFrameTree()->root();
-
-  // Check that the root node is properly created with the frame id of the
-  // initial navigation.
-  ASSERT_EQ(3UL, root->child_count());
-  EXPECT_EQ(std::string(), root->frame_name());
-  EXPECT_EQ(rvh->main_frame_id(), root->frame_id());
-
-  ASSERT_EQ(2UL, root->child_at(0)->child_count());
-  EXPECT_STREQ("1-1-name", root->child_at(0)->frame_name().c_str());
-
-  // Verify the deepest node exists and has the right name.
-  ASSERT_EQ(2UL, root->child_at(2)->child_count());
-  EXPECT_EQ(1UL, root->child_at(2)->child_at(1)->child_count());
-  EXPECT_EQ(0UL, root->child_at(2)->child_at(1)->child_at(0)->child_count());
-  EXPECT_STREQ("3-1-id",
-      root->child_at(2)->child_at(1)->child_at(0)->frame_name().c_str());
-
-  // Navigate to about:blank, which should leave only the root node of the frame
-  // tree in the browser process.
-  NavigateToURL(shell(), test_server()->GetURL("files/title1.html"));
-
-  root = wc->GetFrameTree()->root();
-  EXPECT_EQ(0UL, root->child_count());
-  EXPECT_EQ(std::string(), root->frame_name());
-  EXPECT_EQ(rvh->main_frame_id(), root->frame_id());
-}
-
-// Test that we can navigate away if the previous renderer doesn't clean up its
-// child frames.
-IN_PROC_BROWSER_TEST_F(FrameTreeBrowserTest, FrameTreeAfterCrash) {
-  ASSERT_TRUE(test_server()->Start());
-  NavigateToURL(shell(),
-                test_server()->GetURL("files/frame_tree/top.html"));
-
-  // Crash the renderer so that it doesn't send any FrameDetached messages.
-  WindowedNotificationObserver crash_observer(
-      NOTIFICATION_RENDERER_PROCESS_CLOSED,
-      NotificationService::AllSources());
-  NavigateToURL(shell(), GURL(kChromeUICrashURL));
-  crash_observer.Wait();
-
-  // The frame tree should be cleared, and the frame ID should be reset.
-  WebContentsImpl* wc = static_cast<WebContentsImpl*>(shell()->web_contents());
-  RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(
-      wc->GetRenderViewHost());
-  FrameTreeNode* root = wc->GetFrameTree()->root();
-  EXPECT_EQ(0UL, root->child_count());
-  EXPECT_EQ(FrameTreeNode::kInvalidFrameId, root->frame_id());
-  EXPECT_EQ(rvh->main_frame_id(), root->frame_id());
-
-  // Navigate to a new URL.
-  NavigateToURL(shell(), test_server()->GetURL("files/title1.html"));
-
-  // The frame ID should now be set.
-  EXPECT_EQ(0UL, root->child_count());
-  EXPECT_NE(FrameTreeNode::kInvalidFrameId, root->frame_id());
-  EXPECT_EQ(rvh->main_frame_id(), root->frame_id());
-}
-
-// Test that we can navigate away if the previous renderer doesn't clean up its
-// child frames.
-IN_PROC_BROWSER_TEST_F(FrameTreeBrowserTest, NavigateWithLeftoverFrames) {
-  host_resolver()->AddRule("*", "127.0.0.1");
-  ASSERT_TRUE(test_server()->Start());
-
-  GURL base_url = test_server()->GetURL("files/site_isolation/");
-  GURL::Replacements replace_host;
-  std::string host_str("A.com");  // Must stay in scope with replace_host.
-  replace_host.SetHostStr(host_str);
-  base_url = base_url.ReplaceComponents(replace_host);
-
-  NavigateToURL(shell(),
-                test_server()->GetURL("files/frame_tree/top.html"));
-
-  // Hang the renderer so that it doesn't send any FrameDetached messages.
-  // (This navigation will never complete, so don't wait for it.)
-  shell()->LoadURL(GURL(kChromeUIHangURL));
-
-  // Check that the frame tree still has children.
-  WebContentsImpl* wc = static_cast<WebContentsImpl*>(shell()->web_contents());
-  FrameTreeNode* root = wc->GetFrameTree()->root();
-  ASSERT_EQ(3UL, root->child_count());
-
-  // Navigate to a new URL.  We use LoadURL because NavigateToURL will try to
-  // wait for the previous navigation to stop.
-  TestNavigationObserver tab_observer(wc, 1);
-  shell()->LoadURL(base_url.Resolve("blank.html"));
-  tab_observer.Wait();
-
-  // The frame tree should now be cleared, and the frame ID should be valid.
-  EXPECT_EQ(0UL, root->child_count());
-  EXPECT_NE(FrameTreeNode::kInvalidFrameId, root->frame_id());
-}
-
-}  // namespace content
diff --git a/content/browser/frame_host/frame_tree_node.cc b/content/browser/frame_host/frame_tree_node.cc
index 0d77618..ce35775 100644
--- a/content/browser/frame_host/frame_tree_node.cc
+++ b/content/browser/frame_host/frame_tree_node.cc
@@ -56,13 +56,11 @@ void FrameTreeNode::RemoveChild(FrameTreeNode* child) {
 
 void FrameTreeNode::ResetForMainFrame(
     RenderFrameHostImpl* new_render_frame_host) {
+  DCHECK_EQ(0UL, children_.size());
+
   owns_render_frame_host_ = false;
   frame_id_ = kInvalidFrameId;
   current_url_ = GURL();
-
-  // The children may not have been cleared if a cross-process navigation
-  // commits before the old process cleans everything up.  Make sure the child
-  // nodes get deleted.
   children_.clear();
 
   render_frame_host_ = new_render_frame_host;
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 70db572..2cfbc40 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -1388,9 +1388,8 @@ void RenderViewHostImpl::OnRenderProcessGone(int status, int exit_code) {
   render_view_termination_status_ =
       static_cast<base::TerminationStatus>(status);
 
-  // Reset frame tree state.
+  // Reset state.
   main_frame_id_ = -1;
-  delegate_->GetFrameTree()->SwapMainFrame(main_render_frame_host_.get());
 
   // Our base class RenderWidgetHost needs to reset some stuff.
   RendererExited(render_view_termination_status_, exit_code);
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index af645e0..cf92072 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -420,6 +420,83 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   }
 }
 
+// Ensures FrameTree correctly reflects page structure during navigations.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       FrameTreeShape) {
+  host_resolver()->AddRule("*", "127.0.0.1");
+  ASSERT_TRUE(test_server()->Start());
+
+  GURL base_url = test_server()->GetURL("files/site_isolation/");
+  GURL::Replacements replace_host;
+  std::string host_str("A.com");  // Must stay in scope with replace_host.
+  replace_host.SetHostStr(host_str);
+  base_url = base_url.ReplaceComponents(replace_host);
+
+  // Load doc without iframes. Verify FrameTree just has root.
+  // Frame tree:
+  //   Site-A Root
+  NavigateToURL(shell(), base_url.Resolve("blank.html"));
+  FrameTreeNode* root =
+      static_cast<WebContentsImpl*>(shell()->web_contents())->
+      GetFrameTree()->root();
+  EXPECT_EQ(0U, root->child_count());
+
+  // Add 2 same-site frames. Verify 3 nodes in tree with proper names.
+  // Frame tree:
+  //   Site-A Root -- Site-A frame1
+  //              \-- Site-A frame2
+  WindowedNotificationObserver observer1(
+      content::NOTIFICATION_LOAD_STOP,
+      content::Source<NavigationController>(
+          &shell()->web_contents()->GetController()));
+  NavigateToURL(shell(), base_url.Resolve("frames-X-X.html"));
+  observer1.Wait();
+  ASSERT_EQ(2U, root->child_count());
+  EXPECT_EQ(0U, root->child_at(0)->child_count());
+  EXPECT_EQ(0U, root->child_at(1)->child_count());
+}
+
+// TODO(ajwong): Talk with nasko and merge this functionality with
+// FrameTreeShape.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       FrameTreeShape2) {
+  host_resolver()->AddRule("*", "127.0.0.1");
+  ASSERT_TRUE(test_server()->Start());
+
+  NavigateToURL(shell(),
+                test_server()->GetURL("files/frame_tree/top.html"));
+
+  WebContentsImpl* wc = static_cast<WebContentsImpl*>(shell()->web_contents());
+  RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(
+      wc->GetRenderViewHost());
+  FrameTreeNode* root = wc->GetFrameTree()->root();
+
+  // Check that the root node is properly created with the frame id of the
+  // initial navigation.
+  ASSERT_EQ(3UL, root->child_count());
+  EXPECT_EQ(std::string(), root->frame_name());
+  EXPECT_EQ(rvh->main_frame_id(), root->frame_id());
+
+  ASSERT_EQ(2UL, root->child_at(0)->child_count());
+  EXPECT_STREQ("1-1-name", root->child_at(0)->frame_name().c_str());
+
+  // Verify the deepest node exists and has the right name.
+  ASSERT_EQ(2UL, root->child_at(2)->child_count());
+  EXPECT_EQ(1UL, root->child_at(2)->child_at(1)->child_count());
+  EXPECT_EQ(0UL, root->child_at(2)->child_at(1)->child_at(0)->child_count());
+  EXPECT_STREQ("3-1-id",
+      root->child_at(2)->child_at(1)->child_at(0)->frame_name().c_str());
+
+  // Navigate to about:blank, which should leave only the root node of the frame
+  // tree in the browser process.
+  NavigateToURL(shell(), test_server()->GetURL("files/title1.html"));
+
+  root = wc->GetFrameTree()->root();
+  EXPECT_EQ(0UL, root->child_count());
+  EXPECT_EQ(std::string(), root->frame_name());
+  EXPECT_EQ(rvh->main_frame_id(), root->frame_id());
+}
+
 // Tests that the |should_replace_current_entry| flag persists correctly across
 // request transfers that began with a cross-process navigation.
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
diff --git a/content/common/frame_messages.h b/content/common/frame_messages.h
index ed26831..c374241 100644
--- a/content/common/frame_messages.h
+++ b/content/common/frame_messages.h
@@ -7,7 +7,6 @@
 
 #include "content/common/content_export.h"
 #include "ipc/ipc_message_macros.h"
-#include "url/gurl.h"
 
 #undef IPC_MESSAGE_EXPORT
 #define IPC_MESSAGE_EXPORT CONTENT_EXPORT
diff --git a/content/content_tests.gypi b/content/content_tests.gypi
index db03b54..3fc00e9 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -946,7 +946,6 @@
             'browser/download/mhtml_generation_browsertest.cc',
             'browser/download/save_package_browsertest.cc',
             'browser/fileapi/file_system_browsertest.cc',
-            'browser/frame_host/frame_tree_browsertest.cc',
             'browser/gpu/compositor_util_browsertest.cc',
             'browser/gpu/gpu_ipc_browsertests.cc',
             'browser/indexed_db/indexed_db_browsertest.cc',
diff --git a/content/public/test/mock_render_thread.cc b/content/public/test/mock_render_thread.cc
index 848a710..4862675 100644
--- a/content/public/test/mock_render_thread.cc
+++ b/content/public/test/mock_render_thread.cc
@@ -5,7 +5,6 @@
 #include "content/public/test/mock_render_thread.h"
 
 #include "base/message_loop/message_loop_proxy.h"
-#include "content/common/frame_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/renderer/render_process_observer.h"
 #include "ipc/ipc_message_utils.h"
@@ -20,8 +19,7 @@ MockRenderThread::MockRenderThread()
       surface_id_(0),
       opener_id_(0),
       new_window_routing_id_(0),
-      new_window_main_frame_routing_id_(0),
-      new_frame_routing_id_(0) {
+      new_window_main_frame_routing_id_(0) {
 }
 
 MockRenderThread::~MockRenderThread() {
@@ -230,15 +228,6 @@ void MockRenderThread::OnCreateWindow(
   *cloned_session_storage_namespace_id = 0;
 }
 
-// The Frame expects to be returned a valid route_id different from its own.
-void MockRenderThread::OnCreateChildFrame(int new_frame_routing_id,
-                                          int64 parent_frame_id,
-                                          int64 frame_id,
-                                          const std::string& frame_name,
-                                          int* new_render_frame_id) {
-  *new_render_frame_id = new_frame_routing_id_;
-}
-
 bool MockRenderThread::OnControlMessageReceived(const IPC::Message& msg) {
   ObserverListBase<RenderProcessObserver>::Iterator it(observers_);
   RenderProcessObserver* observer;
@@ -258,7 +247,6 @@ bool MockRenderThread::OnMessageReceived(const IPC::Message& msg) {
   IPC_BEGIN_MESSAGE_MAP_EX(MockRenderThread, msg, msg_is_ok)
     IPC_MESSAGE_HANDLER(ViewHostMsg_CreateWidget, OnCreateWidget)
     IPC_MESSAGE_HANDLER(ViewHostMsg_CreateWindow, OnCreateWindow)
-    IPC_MESSAGE_HANDLER(FrameHostMsg_CreateChildFrame, OnCreateChildFrame)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP_EX()
   return handled;
diff --git a/content/public/test/mock_render_thread.h b/content/public/test/mock_render_thread.h
index 803cc2a..ec03c78 100644
--- a/content/public/test/mock_render_thread.h
+++ b/content/public/test/mock_render_thread.h
@@ -98,10 +98,6 @@ class MockRenderThread : public RenderThread {
     new_window_routing_id_ = id;
   }
 
-  void set_new_frame_routing_id(int32 id) {
-    new_frame_routing_id_ = id;
-  }
-
   // Simulates the Widget receiving a close message. This should result
   // on releasing the internal reference counts and destroying the internal
   // state.
@@ -131,13 +127,6 @@ class MockRenderThread : public RenderThread {
     int* surface_id,
     int64* cloned_session_storage_namespace_id);
 
-  // The Frame expects to be returned a valid route_id different from its own.
-  void OnCreateChildFrame(int new_frame_routing_id,
-                          int64 parent_frame_id,
-                          int64 frame_id,
-                          const std::string& frame_name,
-                          int* new_render_frame_id);
-
 #if defined(OS_WIN)
   void OnDuplicateSection(base::SharedMemoryHandle renderer_handle,
                           base::SharedMemoryHandle* browser_handle);
@@ -162,7 +151,6 @@ class MockRenderThread : public RenderThread {
   // Routing id that will be assigned to a CreateWindow Widget.
   int32 new_window_routing_id_;
   int32 new_window_main_frame_routing_id_;
-  int32 new_frame_routing_id_;
 
   // The last known good deserializer for sync messages.
   scoped_ptr<IPC::MessageReplyDeserializer> reply_deserializer_;
diff --git a/content/public/test/render_view_test.cc b/content/public/test/render_view_test.cc
index 97b1d13..2d96a85 100644
--- a/content/public/test/render_view_test.cc
+++ b/content/public/test/render_view_test.cc
@@ -41,7 +41,6 @@ const int32 kOpenerId = -2;
 const int32 kRouteId = 5;
 const int32 kMainFrameRouteId = 6;
 const int32 kNewWindowRouteId = 7;
-const int32 kNewFrameRouteId = 10;
 const int32 kSurfaceId = 42;
 
 }  // namespace
@@ -141,7 +140,6 @@ void RenderViewTest::SetUp() {
   render_thread_->set_routing_id(kRouteId);
   render_thread_->set_surface_id(kSurfaceId);
   render_thread_->set_new_window_routing_id(kNewWindowRouteId);
-  render_thread_->set_new_frame_routing_id(kNewFrameRouteId);
 
   command_line_.reset(new CommandLine(CommandLine::NO_PROGRAM));
   params_.reset(new MainFunctionParams(*command_line_));
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 6231d59..b2b7373 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -7,6 +7,7 @@
 #include <map>
 #include <string>
 
+#include "base/command_line.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
 #include "content/child/appcache/appcache_dispatcher.h"
@@ -219,25 +220,27 @@ 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();
-  // Synchronously notify the browser of a child frame creation to get the
-  // routing_id for the RenderFrame.
-  int routing_id = MSG_ROUTING_NONE;
-  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);
+  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);
+  }
 
   blink::WebFrame* web_frame = WebFrame::create(child_render_frame,
                                                 child_frame_identifier);
 
-  g_child_frame_map.Get().insert(
-      std::make_pair(web_frame, child_render_frame));
+  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess)) {
+    g_child_frame_map.Get().insert(
+        std::make_pair(web_frame, child_render_frame));
+  }
 
   return web_frame;
 }
@@ -252,41 +255,45 @@ void RenderFrameImpl::frameDetached(blink::WebFrame* frame) {
   // called on the parent frame.
   CHECK(!is_detaching_);
 
-  bool is_subframe = !!frame->parent();
   int64 parent_frame_id = -1;
-  if (is_subframe)
+  if (frame->parent())
     parent_frame_id = frame->parent()->identifier();
 
   Send(new FrameHostMsg_Detach(GetRoutingID(), parent_frame_id,
                                frame->identifier()));
 
-  // 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;
+  // 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;
+  }
 
   // Call back to RenderViewImpl for observers to be notified.
   // TODO(nasko): Remove once we have RenderFrameObserver.
   render_view_->frameDetached(frame);
 
-  // We need to clean up subframes by removing them from the map and deleting
-  // the RenderFrameImpl.  In contrast, the main frame is owned by its
-  // containing RenderViewHost (so that they have the same lifetime), so it does
-  // not require any cleanup here.
-  if (is_subframe) {
-    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);
-  }
-
-  // |frame| is invalid after here.
   frame->close();
 
-  if (is_subframe) {
-    delete this;
-    // Object is invalid after this point.
+  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.
+    }
   }
 }
 
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index fe683d5..b21d9e0 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -3167,10 +3167,6 @@ void RenderViewImpl::didDisownOpener(blink::WebFrame* frame) {
 }
 
 void RenderViewImpl::frameDetached(WebFrame* frame) {
-  // NOTE: We may get here for either the main frame or for subframes.  The
-  // RenderFrameImpl will be deleted immediately after this call for subframes
-  // but not for the main frame, which is kept around in a scoped_ptr.
-
   FOR_EACH_OBSERVER(RenderViewObserver, observers_, FrameDetached(frame));
 }