[Backport] Merge to 2840 "[DevTools] Avoid current_ and pending_ being the same host in RenderFrameDevToolsAgentHost."

> [DevTools] Avoid current_ and pending_ being the same host in RenderFrameDevToolsAgentHost.
>
> This happens when we resue RenderFrameHost after crash.
> The fix is to commit immediately to avoid pending. This is safe because
> we don't even have two RenderFrameHosts.
>
> BUG=644963
>
> Review-Url: https://codereview.chromium.org/2366773003
> Cr-Commit-Position: refs/heads/master@{#421697}
(cherry picked from commit f07d611e4c7c7e0b38521119c42166b6104bba1b)
TBR=pfeldman

Review URL: https://codereview.chromium.org/2400863002 .
(CVE-2016-5186)

Change-Id: I850747c1b1c9ee2f1313309e505216c746061158
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>

3 files changed, 73 insertions, 6 deletions

diff --git a/chromium/content/browser/devtools/protocol/devtools_protocol_browsertest.cc b/chromium/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
index 62f7a35aade..fa9f8bbc540 100644
--- a/chromium/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
+++ b/chromium/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
@@ -149,7 +149,7 @@ class DevToolsProtocolTest : public ContentBrowserTest,
   }
 
   void AgentHostClosed(DevToolsAgentHost* agent_host, bool replaced) override {
-    EXPECT_TRUE(false);
+    DCHECK(false);
   }
 
   std::string waiting_for_notification_;
@@ -474,6 +474,27 @@ IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, InspectDuringFrameSwap) {
   EXPECT_TRUE(success);
 }
 
+// CrashTab() works differently on Windows, leading to RFH removal before
+// RenderProcessGone is called. TODO(dgozman): figure out the problem.
+#if defined(OS_WIN)
+#define MAYBE_DoubleCrash DISABLED_DoubleCrash
+#else
+#define MAYBE_DoubleCrash DoubleCrash
+#endif
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, MAYBE_DoubleCrash) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL test_url = embedded_test_server()->GetURL("/devtools/navigation.html");
+  NavigateToURLBlockUntilNavigationsComplete(shell(), GURL("about:blank"), 1);
+  Attach();
+  SendCommand("ServiceWorker.enable", nullptr);
+  NavigateToURLBlockUntilNavigationsComplete(shell(), test_url, 1);
+  CrashTab(shell()->web_contents());
+  NavigateToURLBlockUntilNavigationsComplete(shell(), test_url, 1);
+  CrashTab(shell()->web_contents());
+  NavigateToURLBlockUntilNavigationsComplete(shell(), GURL("about:blank"), 1);
+  // Should not crash at this point.
+}
+
 IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, ReloadBlankPage) {
   Shell* window =  Shell::CreateNewWindow(
       shell()->web_contents()->GetBrowserContext(),
diff --git a/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc b/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc
index cb1be38ea98..1a19fc1c57d 100644
--- a/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc
+++ b/chromium/content/browser/devtools/render_frame_devtools_agent_host.cc
@@ -320,6 +320,7 @@ void RenderFrameDevToolsAgentHost::OnCancelPendingNavigation(
   if (agent_host->pending_ && agent_host->pending_->host() == pending) {
     DCHECK(agent_host->current_ && agent_host->current_->host() == current);
     agent_host->DiscardPending();
+    DCHECK(agent_host->CheckConsistency());
   }
 }
 
@@ -348,6 +349,7 @@ RenderFrameDevToolsAgentHost::RenderFrameDevToolsAgentHost(
       emulation_handler_(nullptr),
       frame_trace_recorder_(nullptr),
       protocol_handler_(new DevToolsProtocolHandler(this)),
+      handlers_frame_host_(nullptr),
       current_frame_crashed_(false),
       pending_handle_(nullptr),
       in_navigation_(0),
@@ -543,7 +545,14 @@ void RenderFrameDevToolsAgentHost::ReadyToCommitNavigation(
     if (current_->host() != render_frame_host_impl || current_frame_crashed_) {
       SetPending(render_frame_host_impl);
       pending_handle_ = navigation_handle;
+      // Commit when navigating the same frame after crash, avoiding the same
+      // host in current_ and pending_.
+      if (current_->host() == render_frame_host_impl) {
+        pending_handle_ = nullptr;
+        CommitPending();
+      }
     }
+    DCHECK(CheckConsistency());
   }
 }
 
@@ -569,6 +578,7 @@ void RenderFrameDevToolsAgentHost::DidFinishNavigation(
     DispatchBufferedProtocolMessagesIfNecessary();
   }
 
+  DCHECK(CheckConsistency());
   if (navigation_handle->HasCommitted())
     service_worker_handler_->UpdateHosts();
 }
@@ -580,12 +590,21 @@ void RenderFrameDevToolsAgentHost::AboutToNavigateRenderFrame(
     return;
 
   DCHECK(!pending_ || pending_->host() != old_host);
-  if (!current_ || current_->host() != old_host)
+  if (!current_ || current_->host() != old_host) {
+    DCHECK(CheckConsistency());
     return;
-  if (old_host == new_host && !current_frame_crashed_)
+  }
+  if (old_host == new_host && !current_frame_crashed_) {
+    DCHECK(CheckConsistency());
     return;
+  }
   DCHECK(!pending_);
   SetPending(static_cast<RenderFrameHostImpl*>(new_host));
+  // Commit when navigating the same frame after crash, avoiding the same
+  // host in current_ and pending_.
+  if (old_host == new_host)
+    CommitPending();
+  DCHECK(CheckConsistency());
 }
 
 void RenderFrameDevToolsAgentHost::RenderFrameHostChanged(
@@ -595,21 +614,24 @@ void RenderFrameDevToolsAgentHost::RenderFrameHostChanged(
     return;
 
   DCHECK(!pending_ || pending_->host() != old_host);
-  if (!current_ || current_->host() != old_host)
+  if (!current_ || current_->host() != old_host) {
+    DCHECK(CheckConsistency());
     return;
+  }
 
   // AboutToNavigateRenderFrame was not called for renderer-initiated
   // navigation.
   if (!pending_)
     SetPending(static_cast<RenderFrameHostImpl*>(new_host));
-
   CommitPending();
+  DCHECK(CheckConsistency());
 }
 
 void RenderFrameDevToolsAgentHost::FrameDeleted(RenderFrameHost* rfh) {
   if (pending_ && pending_->host() == rfh) {
     if (!browser_side_navigation)
       DiscardPending();
+    DCHECK(CheckConsistency());
     return;
   }
 
@@ -620,6 +642,8 @@ void RenderFrameDevToolsAgentHost::FrameDeleted(RenderFrameHost* rfh) {
 void RenderFrameDevToolsAgentHost::RenderFrameDeleted(RenderFrameHost* rfh) {
   if (!current_frame_crashed_)
     FrameDeleted(rfh);
+  else
+    DCHECK(CheckConsistency());
 }
 
 void RenderFrameDevToolsAgentHost::DestroyOnRenderFrameGone() {
@@ -637,6 +661,18 @@ void RenderFrameDevToolsAgentHost::DestroyOnRenderFrameGone() {
   Release();
 }
 
+bool RenderFrameDevToolsAgentHost::CheckConsistency() {
+  if (current_ && pending_ && current_->host() == pending_->host())
+    return false;
+  if (IsBrowserSideNavigationEnabled())
+    return true;
+  if (!frame_tree_node_)
+    return !handlers_frame_host_;
+  RenderFrameHostManager* manager = frame_tree_node_->render_manager();
+  return handlers_frame_host_ == manager->current_frame_host() ||
+      handlers_frame_host_ == manager->pending_frame_host();
+}
+
 void RenderFrameDevToolsAgentHost::RenderProcessGone(
     base::TerminationStatus status) {
   switch(status) {
@@ -657,6 +693,7 @@ void RenderFrameDevToolsAgentHost::RenderProcessGone(
       inspector_handler_->TargetDetached("Render process gone.");
       break;
   }
+  DCHECK(CheckConsistency());
 }
 
 bool RenderFrameDevToolsAgentHost::OnMessageReceived(
@@ -696,12 +733,15 @@ void RenderFrameDevToolsAgentHost::DidAttachInterstitialPage() {
     page_handler_->DidAttachInterstitialPage();
 
   // TODO(dgozman): this may break for cross-process subframes.
-  if (!pending_)
+  if (!pending_) {
+    DCHECK(CheckConsistency());
     return;
+  }
   // Pending set in AboutToNavigateRenderFrame turned out to be interstitial.
   // Connect back to the real one.
   DiscardPending();
   pending_handle_ = nullptr;
+  DCHECK(CheckConsistency());
 }
 
 void RenderFrameDevToolsAgentHost::DidDetachInterstitialPage() {
@@ -717,6 +757,7 @@ void RenderFrameDevToolsAgentHost::DidCommitProvisionalLoadForFrame(
     return;
   if (pending_ && pending_->host() == render_frame_host)
     CommitPending();
+  DCHECK(CheckConsistency());
   service_worker_handler_->UpdateHosts();
 }
 
@@ -730,6 +771,7 @@ void RenderFrameDevToolsAgentHost::DidFailProvisionalLoad(
     return;
   if (pending_ && pending_->host() == render_frame_host)
     DiscardPending();
+  DCHECK(CheckConsistency());
 }
 
 void RenderFrameDevToolsAgentHost::
@@ -746,6 +788,7 @@ void RenderFrameDevToolsAgentHost::
 
 void RenderFrameDevToolsAgentHost::UpdateProtocolHandlers(
     RenderFrameHostImpl* host) {
+  handlers_frame_host_ = host;
   dom_handler_->SetRenderFrameHost(host);
   if (emulation_handler_)
     emulation_handler_->SetRenderFrameHost(host);
diff --git a/chromium/content/browser/devtools/render_frame_devtools_agent_host.h b/chromium/content/browser/devtools/render_frame_devtools_agent_host.h
index 68409344de8..acfb77b0c90 100644
--- a/chromium/content/browser/devtools/render_frame_devtools_agent_host.h
+++ b/chromium/content/browser/devtools/render_frame_devtools_agent_host.h
@@ -133,6 +133,8 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost
   void OnSwapCompositorFrame(const IPC::Message& message);
   void DestroyOnRenderFrameGone();
 
+  bool CheckConsistency();
+
   bool MatchesMyTreeNode(NavigationHandle* navigation_handle);
 
   class FrameHostHolder;
@@ -159,6 +161,7 @@ class CONTENT_EXPORT RenderFrameDevToolsAgentHost
   scoped_ptr<PowerSaveBlockerImpl> power_save_blocker_;
 #endif
   scoped_ptr<DevToolsProtocolHandler> protocol_handler_;
+  RenderFrameHostImpl* handlers_frame_host_;
   bool current_frame_crashed_;
 
   // PlzNavigate