Replace useless call to ValidateSpeculativeRenderFrameHostForBug1146573.

The call in SetRenderFrameHost was useless because the speculative RFH
has already been move out of place so the validation exits early. This
change moves the validation into a static method that takes 2 frames
as arguments, so that it can be used even in this case.

https://crrev.com/c/2564933/1 shows that this passes all tests even with
SkipEarlyCommitPendingForCrashedFrame.

Bug: 1146573
Change-Id: I59574bd78a93534ec7e0ae603589e82c2e87a119
Cq-Do-Not-Cancel-Tryjobs: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2564933Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Fergal Daly <fergal@chromium.org>
Auto-Submit: Fergal Daly <fergal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834005}

content/browser/renderer_host/render_frame_host_manager.cc

@@ -739,30 +739,34 @@ void RenderFrameHostManager::DidCreateNavigationRequest(
 }
 
 void RenderFrameHostManager::ValidateSpeculativeRenderFrameHostForBug1146573() {
+  ValidateSpeculativeRenderFrameHostForBug1146573(
+      render_frame_host_.get(), speculative_render_frame_host_.get());
+}
+
+// Static
+void RenderFrameHostManager::ValidateSpeculativeRenderFrameHostForBug1146573(
+    RenderFrameHostImpl* current,
+    RenderFrameHostImpl* pending) {
   // TODO(https://crbug.com/1146573): Remove this when the bug is closed.
   if (ShouldCreateNewHostForSameSiteSubframe())
     return;
   // This can happen during destruction after the RFH has been cleared.
-  if (!render_frame_host_)
+  if (!current)
     return;
-  if (!speculative_render_frame_host_)
+  if (!pending)
     return;
-  if (speculative_render_frame_host_->GetSiteInstance() ==
-      render_frame_host_->GetSiteInstance()) {
+  if (pending->GetSiteInstance() == current->GetSiteInstance()) {
     // This should never be true.
-    SCOPED_CRASH_KEY_BOOL(ValidateSpeculative, HostsEqual,
-                          speculative_render_frame_host_ == render_frame_host_);
-    DCHECK_NE(speculative_render_frame_host_, render_frame_host_);
+    SCOPED_CRASH_KEY_BOOL(ValidateSpeculative, HostsEqual, pending == current);
+    DCHECK_NE(pending, current);
     SCOPED_CRASH_KEY_BOOL(ValidateSpeculative, Live,
-                          render_frame_host_->IsRenderFrameLive());
-    SCOPED_CRASH_KEY_STRING256(
-        ValidateSpeculative, OldSiteInstance,
-        render_frame_host_->GetSiteInstance()->GetSiteURL().spec());
-    SCOPED_CRASH_KEY_STRING256(
-        ValidateSpeculative, NewSiteInstance,
-        speculative_render_frame_host_->GetSiteInstance()->GetSiteURL().spec());
+                          current->IsRenderFrameLive());
+    SCOPED_CRASH_KEY_STRING256(ValidateSpeculative, OldSiteInstance,
+                               current->GetSiteInstance()->GetSiteURL().spec());
+    SCOPED_CRASH_KEY_STRING256(ValidateSpeculative, NewSiteInstance,
+                               pending->GetSiteInstance()->GetSiteURL().spec());
     SCOPED_CRASH_KEY_BOOL(ValidateSpeculative, MustBeReplaced,
-                          render_frame_host_->must_be_replaced());
+                          current->must_be_replaced());
     NOTREACHED();
     base::debug::DumpWithoutCrashing();
   }
@@ -3207,6 +3211,13 @@ void RenderFrameHostManager::CommitPending(
 
 std::unique_ptr<RenderFrameHostImpl> RenderFrameHostManager::SetRenderFrameHost(
     std::unique_ptr<RenderFrameHostImpl> render_frame_host) {
+  // It's expected that a same-site swap will occur via here when
+  // ShouldSkipEarlyCommitPendingForCrashedFrame is true, so only check when
+  // that's not the case.
+  if (!(render_frame_host_ && render_frame_host_->must_be_replaced())) {
+    RenderFrameHostManager::ValidateSpeculativeRenderFrameHostForBug1146573(
+        render_frame_host_.get(), render_frame_host.get());
+  }
   // Swap the two.
   std::unique_ptr<RenderFrameHostImpl> old_render_frame_host =
       std::move(render_frame_host_);

content/browser/renderer_host/render_frame_host_manager.h

@@ -329,6 +329,9 @@ class CONTENT_EXPORT RenderFrameHostManager
   // Validate the speculative RFH and DumpWithoutCrashing if it's invalid.
   // TODO(https://crbug.com/1146573): Remove this when the bug is closed.
   void ValidateSpeculativeRenderFrameHostForBug1146573();
+  static void ValidateSpeculativeRenderFrameHostForBug1146573(
+      RenderFrameHostImpl* current,
+      RenderFrameHostImpl* pending);
 
   // Called (possibly several times) during a navigation to select or create an
   // appropriate RenderFrameHost for the provided URL. The returned pointer will