Introduce RenderFrameProxyHost object and use it in RFHM.

content/browser/frame_host/render_frame_host_manager_unittest.cc

 // 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 "base/files/file_path.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigator.h"
@@ skipping 107 matching lines @@
 
  private:
   int process_id_;
   int routing_id_;
   bool deleted_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderViewHostDeletedObserver);
 };
 
 
+// This observer keeps track of the last deleted RenderViewHost to avoid
+// accessing it and causing use-after-free condition.
+class RenderFrameHostDeletedObserver : public WebContentsObserver {
+ public:
+  RenderFrameHostDeletedObserver(RenderFrameHost* rfh)
+      : WebContentsObserver(WebContents::FromRenderFrameHost(rfh)),
+        process_id_(rfh->GetProcess()->GetID()),
+        routing_id_(rfh->GetRoutingID()),
+        deleted_(false) {
+  }
+
+  virtual void RenderFrameDeleted(RenderFrameHost* render_frame_host) OVERRIDE {
+    if (render_frame_host->GetProcess()->GetID() == process_id_ &&
+        render_frame_host->GetRoutingID() == routing_id_) {
+      deleted_ = true;
+    }
+  }
+
+  bool deleted() {
+    return deleted_;
+  }
+
+ private:
+  int process_id_;
+  int routing_id_;
+  bool deleted_;
+
+  DISALLOW_COPY_AND_ASSIGN(RenderFrameHostDeletedObserver);
+};
+
+
 // This observer is used to check whether IPC messages are being filtered for
 // swapped out RenderFrameHost objects. It observes the plugin crash and favicon
 // update events, which the FilterMessagesWhileSwappedOut test simulates being
 // sent. The test is successful if the event is not observed.
 // See http://crbug.com/351815
 class PluginFaviconMessageObserver : public WebContentsObserver {
  public:
   PluginFaviconMessageObserver(WebContents* web_contents)
       : WebContentsObserver(web_contents),
         plugin_crashed_(false),
@@ skipping 1620 matching lines @@
   EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, rvh1->rvh_state());
 
   // Simulate the swap out ack.
   rvh1->OnSwappedOut(false);
 
   // rvh1 should be swapped out.
   EXPECT_FALSE(rvh_deleted_observer.deleted());
   EXPECT_TRUE(rvh1->IsSwappedOut());
 }
 
+// Test that a RenderFrameHost is properly deleted or swapped out when a
+// cross-site navigation is cancelled.
+TEST_F(RenderFrameHostManagerTest,
+       CancelPendingProperlyDeletesOrSwaps) {
+  const GURL kUrl1("http://www.google.com/");
+  const GURL kUrl2("http://www.chromium.org/");
+  RenderFrameHostImpl* pending_rfh;

[Charlie Reis, 2014/04/11 17:42:53]

 = NULL;

[nasko, 2014/04/11 18:11:21]

Done.

+  base::TimeTicks now = base::TimeTicks::Now();
+
+  // Navigate to the first page.
+  contents()->NavigateAndCommit(kUrl1);
+  TestRenderViewHost* rvh1 = test_rvh();
+  EXPECT_EQ(RenderViewHostImpl::STATE_DEFAULT, rvh1->rvh_state());
+
+  // Navigate to a new site, starting a cross-site navigation.
+  controller().LoadURL(kUrl2, Referrer(), PAGE_TRANSITION_LINK, std::string());
+  {
+    pending_rfh = contents()->GetFrameTree()->root()->render_manager()
+        ->pending_frame_host();

[Charlie Reis, 2014/04/11 17:42:53]

I would have put -> on the previous line, but I defer to clang_format.

[nasko, 2014/04/11 18:11:21]

clang-format dictates this :( I dislike it too.

+    RenderFrameHostDeletedObserver rvh_deleted_observer(pending_rfh);
+
+    // Cancel the navigation by simulating user cancelled through beforeunload

[Charlie Reis, 2014/04/11 17:42:53]

by simulating a declined beforeunload dialog.

[nasko, 2014/04/11 18:11:21]

Done.

+    // event handler.
+    main_test_rfh()->OnMessageReceived(
+        FrameHostMsg_BeforeUnload_ACK(0, false, now, now));
+    EXPECT_FALSE(contents()->cross_navigation_pending());
+
+    // Since the pending RFH is the only one for the new SiteInstance, it should
+    // be deleted.
+    EXPECT_TRUE(rvh_deleted_observer.deleted());
+  }
+
+  // Start another cross-site navigation

[Charlie Reis, 2014/04/11 17:42:53]

nit: End with period.

[nasko, 2014/04/11 18:11:21]

Done.

+  controller().LoadURL(kUrl2, Referrer(), PAGE_TRANSITION_LINK, std::string());
+  {
+    pending_rfh = contents()->GetFrameTree()->root()->render_manager()
+        ->pending_frame_host();
+    RenderFrameHostDeletedObserver rvh_deleted_observer(pending_rfh);
+
+    // Increment the number of active views in the new SiteInstance, which will
+    // cause the pending RFH to be swapped out instead of deleted.
+    static_cast<SiteInstanceImpl*>(
+        pending_rfh->GetSiteInstance())->increment_active_view_count();
+
+    main_test_rfh()->OnMessageReceived(
+        FrameHostMsg_BeforeUnload_ACK(0, false, now, now));
+    EXPECT_FALSE(contents()->cross_navigation_pending());
+    EXPECT_FALSE(rvh_deleted_observer.deleted());
+  }
+}
+
 }  // namespace content