Navigation: Adding NavigationClient path to history subframe navigations.

When navigating back to a frame that has subframes, we should try to
restore the subframes to their latest history item. This is currently
done by OpenURL and should be done by BeginNavigation instead.

Design doc:
https://docs.google.com/document/d/13Rqdg1HBmtfnYMUXvg7SpUl3bJisTElfyMc1Ze8hLdE/edit#

This is patch 2/2.

This patch only modifies the behavior for
IsPerNavigationMojoInterface on.
See https://chromium-review.googlesource.com/c/chromium/src/+/1596448 for
trybots run with the flag on.

Bug: 962518
Change-Id: If153cbcb8e1d11bd75be519839c7f36c3907b1dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1594576
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664234}

content/browser/frame_host/navigation_controller_impl.cc

@@ -2137,7 +2137,8 @@ void NavigationControllerImpl::NotifyUserActivation() {
 
 bool NavigationControllerImpl::StartHistoryNavigationInNewSubframe(
     RenderFrameHostImpl* render_frame_host,
-    const GURL& default_url) {
+    const GURL& default_url,
+    mojom::NavigationClientAssociatedPtrInfo* navigation_client) {
   NavigationEntryImpl* entry =
       GetEntryWithUniqueID(render_frame_host->nav_entry_id());
   if (!entry)
@@ -2176,6 +2177,9 @@ bool NavigationControllerImpl::StartHistoryNavigationInNewSubframe(
   if (!request)
     return false;
 
+  request->SetNavigationClient(std::move(*navigation_client),
+                               render_frame_host->GetSiteInstance()->GetId());
+
   render_frame_host->frame_tree_node()->navigator()->Navigate(
       std::move(request), ReloadType::NONE, RestoreType::NONE);
 

content/browser/frame_host/navigation_controller_impl.h

@@ -99,7 +99,8 @@ class CONTENT_EXPORT NavigationControllerImpl : public NavigationController {
   // navigation to |default_url| should be done instead.
   bool StartHistoryNavigationInNewSubframe(
       RenderFrameHostImpl* render_frame_host,
-      const GURL& default_url);
+      const GURL& default_url,
+      mojom::NavigationClientAssociatedPtrInfo* navigation_client);
 
   // Navigates to a specified offset from the "current entry". Currently records
   // a histogram indicating whether the session history navigation would only

content/browser/frame_host/navigation_request.cc

@@ -638,14 +638,8 @@ NavigationRequest::NavigationRequest(
 
     if (IsPerNavigationMojoInterfaceEnabled()) {
       DCHECK(navigation_client.is_valid());
-      request_navigation_client_ = mojom::NavigationClientAssociatedPtr();
-      request_navigation_client_.Bind(std::move(navigation_client));
-      // Binds the OnAbort callback
-      HandleInterfaceDisconnection(
-          &request_navigation_client_,
-          base::BindOnce(&NavigationRequest::OnRendererAbortedNavigation,
-                         base::Unretained(this)));
-      associated_site_instance_id_ = source_site_instance_->GetId();
+      SetNavigationClient(std::move(navigation_client),
+                          source_site_instance_->GetId());
     }
   } else if (entry) {
     DCHECK(!navigation_client.is_valid());
@@ -2761,4 +2755,24 @@ void NavigationRequest::UpdateStateFollowingRedirect(
   navigation_handle_->SetCompleteCallback(std::move(callback));
 }
 
+void NavigationRequest::SetNavigationClient(
+    mojom::NavigationClientAssociatedPtrInfo navigation_client,
+    int32_t associated_site_instance_id) {
+  DCHECK(from_begin_navigation_ ||
+         common_params_.is_history_navigation_in_new_child_frame);
+  DCHECK(!request_navigation_client_);
+  if (!navigation_client.is_valid())
+    return;
+
+  request_navigation_client_ = mojom::NavigationClientAssociatedPtr();
+  request_navigation_client_.Bind(std::move(navigation_client));
+
+  // Binds the OnAbort callback
+  HandleInterfaceDisconnection(
+      &request_navigation_client_,
+      base::BindOnce(&NavigationRequest::OnRendererAbortedNavigation,
+                     base::Unretained(this)));
+  associated_site_instance_id_ = associated_site_instance_id;
+}
+
 }  // namespace content

content/browser/frame_host/navigation_request.h

@@ -419,6 +419,14 @@ class CONTENT_EXPORT NavigationRequest : public NavigationURLLoaderDelegate,
 
   Referrer& sanitized_referrer() { return sanitized_referrer_; }
 
+  // This should be a private method. The only valid reason to be used
+  // outside of the class constructor is in the case of an initial history
+  // navigation in a subframe. This allows a browser-initiated NavigationRequest
+  // to be canceled by the renderer.
+  void SetNavigationClient(
+      mojom::NavigationClientAssociatedPtrInfo navigation_client,
+      int32_t associated_site_instance_id);
+
  private:
   // TODO(clamy): Transform NavigationHandleImplTest into NavigationRequestTest
   // once NavigationHandleImpl has become a wrapper around NavigationRequest.

content/browser/frame_host/navigator.cc

@@ -20,7 +20,8 @@ NavigationController* Navigator::GetController() {
 
 bool Navigator::StartHistoryNavigationInNewSubframe(
     RenderFrameHostImpl* render_frame_host,
-    const GURL& default_url) {
+    const GURL& default_url,
+    mojom::NavigationClientAssociatedPtrInfo* navigation_client) {
   return false;
 }
 

content/browser/frame_host/navigator.h

@@ -88,7 +88,8 @@ class CONTENT_EXPORT Navigator : public base::RefCounted<Navigator> {
   // cases that we use a different URL from history than the frame's src.
   virtual bool StartHistoryNavigationInNewSubframe(
       RenderFrameHostImpl* render_frame_host,
-      const GURL& default_url);
+      const GURL& default_url,
+      mojom::NavigationClientAssociatedPtrInfo* navigation_client);
 
   // Navigation requests -------------------------------------------------------
 

content/browser/frame_host/navigator_impl.cc

@@ -171,9 +171,10 @@ void NavigatorImpl::DidFailLoadWithError(
 
 bool NavigatorImpl::StartHistoryNavigationInNewSubframe(
     RenderFrameHostImpl* render_frame_host,
-    const GURL& default_url) {
-  return controller_->StartHistoryNavigationInNewSubframe(render_frame_host,
-                                                          default_url);
+    const GURL& default_url,
+    mojom::NavigationClientAssociatedPtrInfo* navigation_client) {
+  return controller_->StartHistoryNavigationInNewSubframe(
+      render_frame_host, default_url, navigation_client);
 }
 
 void NavigatorImpl::DidNavigate(
@@ -599,7 +600,8 @@ void NavigatorImpl::OnBeginNavigation(
     // on the frame's unique name.  If this can't be found, fall back to the
     // default path below.
     if (frame_tree_node->navigator()->StartHistoryNavigationInNewSubframe(
-            frame_tree_node->current_frame_host(), common_params.url)) {
+            frame_tree_node->current_frame_host(), common_params.url,
+            &navigation_client)) {
       return;
     }
   }

content/browser/frame_host/navigator_impl.h

@@ -56,7 +56,8 @@ class CONTENT_EXPORT NavigatorImpl : public Navigator {
                    bool was_within_same_document) override;
   bool StartHistoryNavigationInNewSubframe(
       RenderFrameHostImpl* render_frame_host,
-      const GURL& default_url) override;
+      const GURL& default_url,
+      mojom::NavigationClientAssociatedPtrInfo* navigation_client) override;
   void Navigate(std::unique_ptr<NavigationRequest> request,
                 ReloadType reload_type,
                 RestoreType restore_type) override;