Always create FrameTreeNodes and RenderFrameHosts for every frame.

Patch Set 1 and 2 are import of the original CL: https://codereview.chromium.org/82033002/

BUG=245126

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

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

10 files changed, 270 insertions, 198 deletions

diff --git a/content/browser/frame_host/frame_tree.cc b/content/browser/frame_host/frame_tree.cc
index cb0f453..f26c46c 100644
--- a/content/browser/frame_host/frame_tree.cc
+++ b/content/browser/frame_host/frame_tree.cc
@@ -110,9 +110,10 @@ RenderFrameHostImpl* FrameTree::AddFrame(int frame_routing_id,
   scoped_ptr<FrameTreeNode> node(new FrameTreeNode(
       this, parent->navigator(), render_frame_delegate_, render_view_delegate_,
       render_widget_delegate_, manager_delegate_, frame_id, frame_name));
-  RenderFrameHostImpl* render_frame = node->current_frame_host();
+  FrameTreeNode* node_ptr = node.get();
+  // AddChild is what creates the RenderFrameHost.
   parent->AddChild(node.Pass(), frame_routing_id);
-  return render_frame;
+  return node_ptr->current_frame_host();
 }
 
 void FrameTree::RemoveFrame(RenderFrameHostImpl* render_frame_host,
diff --git a/content/browser/frame_host/frame_tree_browsertest.cc b/content/browser/frame_host/frame_tree_browsertest.cc
new file mode 100644
index 0000000..9a67097
--- /dev/null
+++ b/content/browser/frame_host/frame_tree_browsertest.cc
@@ -0,0 +1,170 @@
+// 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.
+
+#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/browser_test_utils.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.
+  RenderProcessHostWatcher crash_observer(
+      shell()->web_contents(),
+      RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+  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/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index db3a43c..1523ec5 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -444,150 +444,6 @@ 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());
-}
-
-// Test that we can navigate away if the previous renderer doesn't clean up its
-// child frames.
-IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest, 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.
-  RenderProcessHostWatcher crash_observer(
-      shell()->web_contents(),
-      RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
-  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(SitePerProcessBrowserTest, 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());
-}
-
 // 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/content_tests.gypi b/content/content_tests.gypi
index ac616f9..3f768d0 100644
--- a/content/content_tests.gypi
+++ b/content/content_tests.gypi
@@ -964,6 +964,7 @@
             '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/frame_host/render_frame_host_manager_browsertest.cc',
             'browser/gpu/compositor_util_browsertest.cc',
             'browser/gpu/gpu_ipc_browsertests.cc',
diff --git a/content/public/test/mock_render_thread.cc b/content/public/test/mock_render_thread.cc
index 54758ea..bca80f2 100644
--- a/content/public/test/mock_render_thread.cc
+++ b/content/public/test/mock_render_thread.cc
@@ -5,6 +5,7 @@
 #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"
@@ -19,7 +20,8 @@ MockRenderThread::MockRenderThread()
       surface_id_(0),
       opener_id_(0),
       new_window_routing_id_(0),
-      new_window_main_frame_routing_id_(0) {
+      new_window_main_frame_routing_id_(0),
+      new_frame_routing_id_(0) {
 }
 
 MockRenderThread::~MockRenderThread() {
@@ -231,6 +233,15 @@ 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;
@@ -250,6 +261,7 @@ 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 fd3f58a..2f2e364 100644
--- a/content/public/test/mock_render_thread.h
+++ b/content/public/test/mock_render_thread.h
@@ -100,6 +100,10 @@ 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.
@@ -129,6 +133,13 @@ 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);
@@ -153,6 +164,7 @@ 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 0a7354c..e6555d0 100644
--- a/content/public/test/render_view_test.cc
+++ b/content/public/test/render_view_test.cc
@@ -41,6 +41,7 @@ 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
@@ -140,6 +141,7 @@ 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 55b2536..88224ae 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/command_line.h"
+#include "base/debug/alias.h"
 #include "base/i18n/char_iterator.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/time/time.h"
@@ -96,7 +97,7 @@ namespace content {
 namespace {
 
 typedef std::map<blink::WebFrame*, RenderFrameImpl*> FrameMap;
-base::LazyInstance<FrameMap> g_child_frame_map = LAZY_INSTANCE_INITIALIZER;
+base::LazyInstance<FrameMap> g_frame_map = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
@@ -116,8 +117,8 @@ RenderFrameImpl* RenderFrameImpl::Create(RenderViewImpl* render_view,
 
 RenderFrameImpl* RenderFrameImpl::FindByWebFrame(blink::WebFrame* web_frame) {
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess)) {
-    FrameMap::iterator iter = g_child_frame_map.Get().find(web_frame);
-    if (iter != g_child_frame_map.Get().end())
+    FrameMap::iterator iter = g_frame_map.Get().find(web_frame);
+    if (iter != g_frame_map.Get().end())
       return iter->second;
   }
 
@@ -154,7 +155,18 @@ RenderFrameImpl::~RenderFrameImpl() {
   RenderThread::Get()->RemoveRoute(routing_id_);
 }
 
+// TODO(nasko): Overload the delete operator to overwrite the freed
+// RenderFrameImpl object and help detect potential use-after-free bug.
+// See https://crbug.com/245126#c34.
+void RenderFrameImpl::operator delete(void* ptr) {
+  memset(ptr, 0xAF, sizeof(RenderFrameImpl));
+}
+
 void RenderFrameImpl::MainWebFrameCreated(blink::WebFrame* frame) {
+  std::pair<FrameMap::iterator, bool> result = g_frame_map.Get().insert(
+      std::make_pair(frame, this));
+  CHECK(result.second) << "Inserting a duplicate item.";
+
   FOR_EACH_OBSERVER(RenderFrameObserver, observers_,
                     WebFrameCreated(frame));
 }
@@ -664,32 +676,32 @@ 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(routing_id_,
-                                           parent->identifier(),
-                                           child_frame_identifier,
-                                           base::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;
+  Send(new FrameHostMsg_CreateChildFrame(routing_id_,
+                                         parent->identifier(),
+                                         child_frame_identifier,
+                                         base::UTF16ToUTF8(name),
+                                         &routing_id));
+  CHECK_NE(routing_id, MSG_ROUTING_NONE);
+  RenderFrameImpl* child_render_frame = RenderFrameImpl::Create(render_view_,
+                                                                routing_id);
+  // TODO(nasko): Over-conservative check for debugging.
+  CHECK(child_render_frame);
   blink::WebFrame* web_frame = WebFrame::create(child_render_frame,
                                                 child_frame_identifier);
+  // TODO(nasko): Over-conservative check for debugging.
+  CHECK(web_frame);
+  child_render_frame->SetWebFrame(web_frame);
 
-  if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess)) {
-    child_render_frame->SetWebFrame(web_frame);
-    g_child_frame_map.Get().insert(
-        std::make_pair(web_frame, child_render_frame));
-  } else {
-    FOR_EACH_OBSERVER(RenderFrameObserver, observers_,
-                      WebFrameCreated(web_frame));
-  }
+  std::pair<FrameMap::iterator, bool> result = g_frame_map.Get().insert(
+      std::make_pair(web_frame, child_render_frame));
+  CHECK(result.second) << "Inserting a duplicate item.";
 
+  FOR_EACH_OBSERVER(RenderFrameObserver, observers_,
+                    WebFrameCreated(web_frame));
   return web_frame;
 }
 
@@ -702,46 +714,45 @@ void RenderFrameImpl::frameDetached(blink::WebFrame* frame) {
   // the parent frame.  This is different from createChildFrame() which is
   // called on the parent frame.
   CHECK(!is_detaching_);
+  // TODO(nasko): Remove all debug::Alias lines after diagnosing failures.
+  base::debug::Alias(frame);
+
+  bool is_subframe = !!frame->parent();
+  base::debug::Alias(&is_subframe);
 
   int64 parent_frame_id = -1;
-  if (frame->parent())
+  base::debug::Alias(&parent_frame_id);
+  if (is_subframe)
     parent_frame_id = frame->parent()->identifier();
 
   Send(new FrameHostMsg_Detach(routing_id_, 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
+  // sent before setting |is_detaching_| to true. In contrast, Observers
+  // should only be notified afterwards so they cannot call back into here 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 only
+  // removal from the map is needed and no deletion.
+  FrameMap::iterator it = g_frame_map.Get().find(frame);
+  CHECK(it != g_frame_map.Get().end());
+  CHECK_EQ(it->second, this);
+  g_frame_map.Get().erase(it);
+
+  // |frame| is invalid after here.
   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 (is_subframe) {
+    delete this;
+    // Object is invalid after this point.
   }
 }
 
diff --git a/content/renderer/render_frame_impl.h b/content/renderer/render_frame_impl.h
index 274c597..bcdfcfa 100644
--- a/content/renderer/render_frame_impl.h
+++ b/content/renderer/render_frame_impl.h
@@ -64,6 +64,9 @@ class CONTENT_EXPORT RenderFrameImpl
       RenderFrameImpl* (*create_render_frame_impl)(RenderViewImpl*, int32));
 
   virtual ~RenderFrameImpl();
+  // TODO(nasko): Remove when no longer needed.
+  // See comment on the implementation of this method for more details.
+  void operator delete(void*);
 
   bool is_swapped_out() const {
     return is_swapped_out_;
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index 0faf360..a10ae54 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -949,7 +949,7 @@ void RenderViewImpl::Initialize(RenderViewImplParams* params) {
   main_render_frame_.reset(
       RenderFrameImpl::Create(this, params->main_frame_routing_id));
   // The main frame WebFrame object is closed by
-  // RenderViewImpl::frameDetached().
+  // RenderFrameImpl::frameDetached().
   webview()->setMainFrame(WebFrame::create(main_render_frame_.get()));
   main_render_frame_->MainWebFrameCreated(webview()->mainFrame());
   main_render_frame_->SetWebFrame(webview()->mainFrame());
@@ -3081,6 +3081,10 @@ 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 owned by |main_render_frame_|.
+
   FOR_EACH_OBSERVER(RenderViewObserver, observers_, FrameDetached(frame));
 }