DevTools: Be more reliable when the navigation commits after RFH swap.

The main motivation behind this CL is to land:
https://chromium-review.googlesource.com/c/chromium/src/+/1688957
It makes every navigations from a crashed RenderFrameHost to use a new
RenderFrameHost. Previously, only the cross-origin navigations where using a
new RenderFrameHost.

DevToolsProtocolTest was only used with same-process navigation, this CL adds
a regression test:
DevToolsProtocolTest.InspectorTargetCrashedNavigateCrossProcess
This test fails without this patch and works with it.

Context:

RenderFrameDevToolsAgentHost follows the current document in a given frame.
This translates by following a RenderFrameHost in a given FrameTreeNode.

To do this, most observers of a WebContents uses:
WebContentsObserver::RenderFrameHostChanged(old_rfh, new_rfh). They updates the
current RenderFrameHost after the navigation has been done and the old and new
RenderFrameHost have been swapped.

The RenderFrameDevToolsAgenHost way of doing things is slightly different, it
wants to follow the new RenderFrameHost as soon as possible. It happens when the
navigation is ReadyToCommit().

To handle canceled navigation and crashed RenderFrameHost, the
RenderFrameDevToolsAgentHost observes the following "events":
 * ReadyToCommitNavigation
 * DidFinishNavigation
 * RenderFrameHostChanged.
Please take a look at their implementations.

Depending on the case, they are called in a different order and produces
different results.

Let's explore several examples with a cross-process navigation from A to B:

────────────────────────────────────────────────────────────────────────────────

Case 1: Successful navigation from a living RenderFrameHost.
  ┌─┬────────────────────────┬───────────────────┬───────────────────┐
  │#│ Event                  │ before patch      │ after patch       │
  ├─┼────────────────────────┼───────────────────┼───────────────────┤
  │1│ ReadyToCommitNavigation│ UpdateFrameHost(B)│ UpdateFrameHost(B)│
  ├─┼────────────────────────┼───────────────────┼───────────────────┤
  │2│ RenderFrameHostChanged │                   │ UpdateFrameHost(B)│
  ├─┼────────────────────────┼───────────────────┼───────────────────┤
  │3│ DidFinishNavigation    │ UpdateFrameHost(B)│ UpdateFrameHost(B)│
  └─┴────────────────────────┴───────────────────┴───────────────────┘

  Observation: This produces the same effects, UpdateFrameHost called several
  time in a row with the same RenderFrameHost is essentially a no-op.

────────────────────────────────────────────────────────────────────────────────

Case 2: Successful navigation from a dead RenderFrameHost.
  ┌─┬────────────────────────┬─────────────────────────┬───────────────────┐
  │#│ Event                  │ before patch            │ after patch       │
  ├─┼────────────────────────┼─────────────────────────┼───────────────────┤
  │1│ RenderFrameHostChanged │ UpdateFrameHost(nullptr)│ UpdateFrameHost(B)│
  ├─┼────────────────────────┼─────────────────────────┼───────────────────┤
  │2│ ReadyToCommitNavigation│ UpdateFrameHost(B)      │ UpdateFrameHost(B)│
  ├─┼────────────────────────┼─────────────────────────┼───────────────────┤
  │3│ DidFinishNavigation    │ UpdateFrameHost(B)      │ UpdateFrameHost(B)│
  └─┴────────────────────────┴─────────────────────────┴───────────────────┘

  Observation: Without this patch, when the new RenderFrameHost starts loading,
  it temporarily switch to nullptr for a short time. It is no more the case.

────────────────────────────────────────────────────────────────────────────────

Case 3: Navigation canceled after ReadyToCommitNavigation.
  ┌─┬────────────────────────┬───────────────────┬───────────────────┐
  │#│ Event                  │ before patch      │ after patch       │
  ├─┼────────────────────────┼───────────────────┼───────────────────┤
  │1│ ReadyToCommitNavigation│ UpdateFrameHost(B)│ UpdateFrameHost(B)│
  ├─┼────────────────────────┼───────────────────┼───────────────────┤
  │2│ DidFinishNavigation    │ UpdateFrameHost(A)│ UpdateFrameHost(A)│
  └─┴────────────────────────┴───────────────────┴───────────────────┘

  Observation: Nothing changed.

────────────────────────────────────────────────────────────────────────────────

The previous way of "following" RenderFrameHost by the
RenderFrameDevToolsAgentHost can "temporary" follow nullptr in case 2. This is
no more the case.

Bug: 990315
Change-Id: I0703e1602ef47f8964a5041cc509ed0344bf4e55
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1731489Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#684259}

content/browser/devtools/protocol/devtools_protocol_browsertest.cc

@@ -863,8 +863,35 @@ IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, CrossSiteCrash) {
 
 IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, InspectorTargetCrashedNavigate) {
   set_agent_host_can_close();
-  GURL url = GURL("data:text/html,<body></body>");
-  NavigateToURLBlockUntilNavigationsComplete(shell(), url, 1);
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL url_a = embedded_test_server()->GetURL("a.com", "/title1.html");
+
+  NavigateToURLBlockUntilNavigationsComplete(shell(), url_a, 1);
+  Attach();
+  SendCommand("Inspector.enable", nullptr);
+
+  {
+    ScopedAllowRendererCrashes scoped_allow_renderer_crashes(shell());
+    shell()->LoadURL(GURL(content::kChromeUICrashURL));
+    WaitForNotification("Inspector.targetCrashed");
+  }
+
+  ClearNotifications();
+  shell()->LoadURL(url_a);
+  WaitForNotification("Inspector.targetReloadedAfterCrash", true);
+}
+
+// Same as in DevToolsProtocolTest.InspectorTargetCrashedNavigate, but with a
+// cross-process navigation at the end.
+// Regression test for https://crbug.com/990315
+IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest,
+                       InspectorTargetCrashedNavigateCrossProcess) {
+  set_agent_host_can_close();
+  ASSERT_TRUE(embedded_test_server()->Start());
+  GURL url_a = embedded_test_server()->GetURL("a.com", "/title1.html");
+  GURL url_b = embedded_test_server()->GetURL("b.com", "/title1.html");
+
+  NavigateToURLBlockUntilNavigationsComplete(shell(), url_a, 1);
   Attach();
   SendCommand("Inspector.enable", nullptr);
 
@@ -875,7 +902,7 @@ IN_PROC_BROWSER_TEST_F(DevToolsProtocolTest, InspectorTargetCrashedNavigate) {
   }
 
   ClearNotifications();
-  shell()->LoadURL(url);
+  shell()->LoadURL(url_b);
   WaitForNotification("Inspector.targetReloadedAfterCrash", true);
 }
 

content/browser/devtools/render_frame_devtools_agent_host.cc

@@ -484,10 +484,11 @@ void RenderFrameDevToolsAgentHost::DidStartNavigation(
 void RenderFrameDevToolsAgentHost::RenderFrameHostChanged(
     RenderFrameHost* old_host,
     RenderFrameHost* new_host) {
-  if (old_host != frame_host_)
+  auto* new_host_impl = static_cast<RenderFrameHostImpl*>(new_host);
+  FrameTreeNode* frame_tree_node = new_host_impl->frame_tree_node();
+  if (frame_tree_node != frame_tree_node_)
     return;
-
-  UpdateFrameHost(nullptr);
+  UpdateFrameHost(new_host_impl);
   // UpdateFrameHost may destruct |this|.
 }