Navigation: Separate same document navigation DidCommit path.

This patch aims to split the currently unique path for same document and
cross document navigation DidCommit. We introduce a new FrameHost
interface method DidCommitSameDocumentNavigation while preserving the
existing DidCommitProvisionalLoad IPC. Same document navigation will use
the new mojo interface method while cross document navigation will keep
on using DidCommitProvisionalLoad.

This change is part of the broader goal of providing a clear mojo
interface for renderer/RFH navigation communication. It would solve
a number of complex race conditions. This specific change helps the
Browser process classify navigations as a same document. This is a step
forward unifying browser and renderer initiated same document navigation
code paths.

Bug:784904

Change-Id: Ie937ece170643ec2aa651c763df27fae3c150ac2
Reviewed-on: https://chromium-review.googlesource.com/789857
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#534392}

content/browser/bad_message.h

@@ -213,7 +213,7 @@ enum BadMessageReason {
   CSDH_UNEXPECTED_OPERATION = 186,
   RMF_BAD_URL_CACHEABLE_METADATA = 187,
   RFH_INTERFACE_PROVIDER_MISSING = 188,
-  RFH_INTERFACE_PROVIDER_SUPERFLUOUS = 189,
+  OBSOLETE_RFH_INTERFACE_PROVIDER_SUPERFLUOUS = 189,
   AIRH_UNEXPECTED_BITSTREAM = 190,
   ARH_UNEXPECTED_BITSTREAM = 191,
   RDH_NULL_CLIENT = 192,

content/browser/frame_host/interstitial_page_navigator_impl.cc

@@ -41,7 +41,8 @@ void InterstitialPageNavigatorImpl::DidStartProvisionalLoad(
 void InterstitialPageNavigatorImpl::DidNavigate(
     RenderFrameHostImpl* render_frame_host,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& input_params,
-    std::unique_ptr<NavigationHandleImpl> navigation_handle) {
+    std::unique_ptr<NavigationHandleImpl> navigation_handle,
+    bool was_within_same_document) {
   // Do not proceed if the interstitial itself has been disabled.
   if (!enabled_)
     return;

content/browser/frame_host/interstitial_page_navigator_impl.h

@@ -35,7 +35,8 @@ class CONTENT_EXPORT InterstitialPageNavigatorImpl : public Navigator {
   void DidNavigate(
       RenderFrameHostImpl* render_frame_host,
       const FrameHostMsg_DidCommitProvisionalLoad_Params& input_params,
-      std::unique_ptr<NavigationHandleImpl> navigation_handle) override;
+      std::unique_ptr<NavigationHandleImpl> navigation_handle,
+      bool was_within_same_document) override;
 
   // Disables any further action when the interstitial page is preparing to
   // delete itself.

content/browser/frame_host/navigation_handle_impl.cc

@@ -473,7 +473,6 @@ void NavigationHandleImpl::CallDidCommitNavigationForTesting(const GURL& url) {
   params.searchable_form_encoding = std::string();
   params.did_create_new_entry = false;
   params.gesture = NavigationGestureUser;
-  params.was_within_same_document = false;
   params.method = "GET";
   params.page_state = PageState::CreateFromURL(url);
   params.contents_mime_type = std::string("text/html");

content/browser/frame_host/navigator.h

@@ -81,7 +81,8 @@ class CONTENT_EXPORT Navigator : public base::RefCounted<Navigator> {
   virtual void DidNavigate(
       RenderFrameHostImpl* render_frame_host,
       const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
-      std::unique_ptr<NavigationHandleImpl> navigation_handle) {}
+      std::unique_ptr<NavigationHandleImpl> navigation_handle,
+      bool was_within_same_document) {}
 
   // Called by the NavigationController to cause the Navigator to navigate
   // to the current pending entry. The NavigationController should be called

content/browser/frame_host/navigator_impl.cc

@@ -426,13 +426,13 @@ bool NavigatorImpl::NavigateNewChildFrame(
 void NavigatorImpl::DidNavigate(
     RenderFrameHostImpl* render_frame_host,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
-    std::unique_ptr<NavigationHandleImpl> navigation_handle) {
+    std::unique_ptr<NavigationHandleImpl> navigation_handle,
+    bool was_within_same_document) {
   FrameTreeNode* frame_tree_node = render_frame_host->frame_tree_node();
   FrameTree* frame_tree = frame_tree_node->frame_tree();
 
   bool is_same_document_navigation = controller_->IsURLSameDocumentNavigation(
-      params.url, params.origin, params.was_within_same_document,
-      render_frame_host);
+      params.url, params.origin, was_within_same_document, render_frame_host);
 
   // If a frame claims the navigation was same-document, it must be the current
   // frame, not a pending one.
@@ -457,7 +457,7 @@ void NavigatorImpl::DidNavigate(
         // want same-document navigations to be super fast, and taking a
         // screenshot currently blocks GPU for a longer time than we are willing
         // to tolerate in this use case.
-        if (!params.was_within_same_document)
+        if (!was_within_same_document)
           controller_->TakeScreenshot();
       }
 

content/browser/frame_host/navigator_impl.h

@@ -53,10 +53,10 @@ class CONTENT_EXPORT NavigatorImpl : public Navigator {
                             const GURL& url,
                             int error_code,
                             const base::string16& error_description) override;
-  void DidNavigate(
-      RenderFrameHostImpl* render_frame_host,
-      const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
-      std::unique_ptr<NavigationHandleImpl> navigation_handle) override;
+  void DidNavigate(RenderFrameHostImpl* render_frame_host,
+                   const FrameHostMsg_DidCommitProvisionalLoad_Params& params,
+                   std::unique_ptr<NavigationHandleImpl> navigation_handle,
+                   bool was_within_same_document) override;
   bool NavigateToPendingEntry(FrameTreeNode* frame_tree_node,
                               const FrameNavigationEntry& frame_entry,
                               ReloadType reload_type,

content/browser/frame_host/navigator_impl_unittest.cc

@@ -1160,16 +1160,6 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   }
 }
 
-namespace {
-void SetWithinSameDocument(
-    const GURL& url,
-    FrameHostMsg_DidCommitProvisionalLoad_Params* params) {
-  params->was_within_same_document = true;
-  params->url = url;
-  params->origin = url::Origin::Create(url);
-}
-}
-
 // A renderer process might try and claim that a cross site navigation was
 // within the same document by setting was_within_same_document = true in
 // FrameHostMsg_DidCommitProvisionalLoad_Params. Such case should be detected on
@@ -1178,17 +1168,14 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, CrossSiteClaimWithinPage) {
   const GURL kUrl1("http://www.chromium.org/");
   const GURL kUrl2("http://www.google.com/");
 
-  contents()->NavigateAndCommit(kUrl1);
-  FrameTreeNode* node = main_test_rfh()->frame_tree_node();
-
-  // Navigate to a different site.
-  int entry_id = RequestNavigation(node, kUrl2);
-  main_test_rfh()->PrepareForCommit();
+  NavigationSimulator::NavigateAndCommitFromBrowser(contents(), kUrl1);
 
-  // Claim that the navigation was within same page.
+  // Navigate to a different site and claim that the navigation was within same
+  // page.
   int bad_msg_count = process()->bad_msg_count();
-  GetSpeculativeRenderFrameHost(node)->SendNavigateWithModificationCallback(
-      entry_id, true, kUrl2, base::Bind(SetWithinSameDocument, kUrl1));
+  auto simulator =
+      NavigationSimulator::CreateRendererInitiated(kUrl2, main_test_rfh());
+  simulator->CommitSameDocument();
   EXPECT_EQ(process()->bad_msg_count(), bad_msg_count + 1);
 }
 
@@ -1323,7 +1310,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, FeaturePolicyNewChild) {
   FrameHostMsg_DidCommitProvisionalLoad_Params params;
   InitNavigateParams(&params, 1, false, kUrl2,
                      ui::PAGE_TRANSITION_AUTO_SUBFRAME);
-  subframe_rfh->SendNavigateWithParams(&params);
+  subframe_rfh->SendNavigateWithParams(&params, false);
 
   blink::FeaturePolicy* subframe_feature_policy =
       subframe_rfh->feature_policy();