Navigation: Implement NavigationClient commit callback.

In the model of the NavigationClient navigation interface, the renderer
does not send a message notifying the browser that it committed. Instead,
it runs the mojo callback that was passed to it at commit.

In practice, this means that both SimulateCommitProcessed and
DidCommitProvisionalLoad are part of the callback, instead of just the
former.

Because changing the callback means changing signatures, significant
work has to be done to both be able to pass around the new callback and
maintain a sane architecture that avoids code duplication.

This patch only implements the callback change for SUCCESSFUL commits.
CommitFailedNavigation will be handled in a second patch.
The architecture should be the exact same, this is simply to keep test
failures to a manageable level while debugging/landing.

Note: This patch is fully behind the PerNavigationMojoInterface flag.

Please see
https://chromium-review.googlesource.com/c/chromium/src/+/1344107/
for the trybot runs with the PerNavigationMojoInterface flag on.

Please see
https://docs.google.com/document/d/1mXjxYJptb_bZ_EqGMF-c4LTSnhjt6Gn_WVvSrsinpq8/edit
for the design doc.

Bug: 784904
Change-Id: I9efdd25bfd289a2e76c5bf43e32e2dc0c589ad25
Reviewed-on: https://chromium-review.googlesource.com/c/1344091
Commit-Queue: Arthur Hemery <ahemery@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627978}

content/browser/frame_host/render_frame_host_impl.h

@@ -1118,6 +1118,17 @@ class CONTENT_EXPORT RenderFrameHostImpl
           validated_params,
       mojom::DidCommitProvisionalLoadInterfaceParamsPtr interface_params)
       override;
+
+  // This function mimics DidCommitProvisionalLoad but is a direct mojo
+  // callback from NavigationClient::CommitNavigation.
+  // This only used when PerNavigationMojoInterface is enabled, and will
+  // replace DidCommitProvisionalLoad in the long run.
+  void DidCommitPerNavigationMojoInterfaceNavigation(
+      NavigationRequest* committing_navigation_request,
+      std::unique_ptr<FrameHostMsg_DidCommitProvisionalLoad_Params>
+          validated_params,
+      mojom::DidCommitProvisionalLoadInterfaceParamsPtr interface_params);
+
   void DidCommitSameDocumentNavigation(
       std::unique_ptr<FrameHostMsg_DidCommitProvisionalLoad_Params>
           validated_params) override;
@@ -1408,12 +1419,21 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // an interstitial.
   void UpdateSiteURL(const GURL& url, bool url_is_unreachable);
 
+  // The actual implementation of DidCommitProvisionalLoad and
+  // DidCommitPerNavigationMojoInterfaceNavigation.
+  void DidCommitNavigation(
+      NavigationRequest* committing_navigation_request,
+      std::unique_ptr<FrameHostMsg_DidCommitProvisionalLoad_Params>
+          validated_params,
+      mojom::DidCommitProvisionalLoadInterfaceParamsPtr interface_params);
+
   // Called when we receive the confirmation that a navigation committed in the
   // renderer. Used by both DidCommitSameDocumentNavigation and
   // DidCommitNavigation.
   // Returns true if the navigation did commit properly, false if the commit
   // state should be restored to its pre-commit value.
   bool DidCommitNavigationInternal(
+      NavigationRequest* navigation_request,
       FrameHostMsg_DidCommitProvisionalLoad_Params* validated_params,
       bool is_same_document_navigation);
 
@@ -1686,6 +1706,12 @@ class CONTENT_EXPORT RenderFrameHostImpl
   // indexed by IDs. These are navigations that have passed ReadyToCommit stage
   // and are waiting for the renderer to send back a matching
   // OnCrossDocumentCommitProcessed.
+
+  // TODO(ahemery): We have this storage as a map because we actually want to
+  // find navigations by id with PerNavigationMojoInterface disabled.
+  // When the flag is always on, rework the structure to simply store an
+  // unindexed bunch of ongoing navigations and modify
+  // DidCommitNavigationInternal.
   std::map<int64_t, std::unique_ptr<NavigationRequest>> navigation_requests_;
 
   // Holds a same-document NavigationRequest while waiting for the navigation it

content/common/frame.mojom

@@ -19,7 +19,6 @@ import "services/network/public/mojom/url_loader_factory.mojom";
 import "services/service_manager/public/mojom/interface_provider.mojom";
 import "services/viz/public/interfaces/compositing/surface_id.mojom";
 import "third_party/blink/public/mojom/blob/blob_url_store.mojom";
-import "third_party/blink/public/mojom/frame/document_interface_broker.mojom";
 import "third_party/blink/public/mojom/feature_policy/feature_policy.mojom";
 import "third_party/blink/public/mojom/frame/navigation_initiator.mojom";
 import "third_party/blink/public/mojom/loader/url_loader_factory_bundle.mojom";
@@ -268,12 +267,6 @@ struct CreateNewWindowReply {
 // the frame is detached. Used by resource requests with "keepalive" specified.
 interface KeepAliveHandle {};
 
-struct DidCommitProvisionalLoadInterfaceParams {
-  service_manager.mojom.InterfaceProvider& interface_provider_request;
-  blink.mojom.DocumentInterfaceBroker& document_interface_broker_content_request;
-  blink.mojom.DocumentInterfaceBroker& document_interface_broker_blink_request;
-};
-
 // Implemented by the frame server (i.e. the browser process). For messages that
 // must be associated with the IPC channel.
 interface FrameHost {

content/common/frame_messages.mojom

@@ -4,6 +4,16 @@
 
 module content.mojom;
 
+import "services/service_manager/public/mojom/interface_provider.mojom";
+import "third_party/blink/public/mojom/frame/document_interface_broker.mojom";
+
 [Native]
 struct DidCommitProvisionalLoadParams;
 
+struct DidCommitProvisionalLoadInterfaceParams {
+  service_manager.mojom.InterfaceProvider& interface_provider_request;
+  blink.mojom.DocumentInterfaceBroker&
+      document_interface_broker_content_request;
+  blink.mojom.DocumentInterfaceBroker& document_interface_broker_blink_request;
+};
+

content/common/navigation_client.mojom

@@ -6,6 +6,7 @@ module content.mojom;
 
 import "services/network/public/mojom/url_loader.mojom";
 import "services/network/public/mojom/url_loader_factory.mojom";
+import "content/common/frame_messages.mojom";
 import "content/public/common/transferrable_url_loader.mojom";
 import "mojo/public/mojom/base/unguessable_token.mojom";
 import "url/mojom/url.mojom";
@@ -62,7 +63,8 @@ interface NavigationClient {
       blink.mojom.ControllerServiceWorkerInfo? controller_service_worker_info,
       network.mojom.URLLoaderFactory? prefetch_loader_factory,
       mojo_base.mojom.UnguessableToken devtools_navigation_token)
-      => (blink.mojom.CommitResult commit_result);
+      => (DidCommitProvisionalLoadParams params,
+          DidCommitProvisionalLoadInterfaceParams? interface_params);
 
   // Tells the renderer that a failed navigation is ready to commit.
   //

content/renderer/navigation_client.cc

@@ -32,7 +32,7 @@ void NavigationClient::CommitNavigation(
   // race conditions leading to the early deletion of NavigationRequest would
   // unexpectedly abort the ongoing navigation. Remove when the races are fixed.
   ResetDisconnectionHandler();
-  render_frame_->CommitNavigation(
+  render_frame_->CommitPerNavigationMojoInterfaceNavigation(
       head, common_params, commit_params,
       std::move(url_loader_client_endpoints), std::move(subresource_loaders),
       std::move(subresource_overrides),

content/renderer/navigation_state.cc

@@ -4,7 +4,12 @@
 
 #include "content/renderer/navigation_state.h"
 
+#include <utility>
+
+#include "content/common/frame_messages.h"
+#include "content/public/common/navigation_policy.h"
 #include "content/renderer/internal_document_state_data.h"
+#include "third_party/blink/public/web/commit_result.mojom.h"
 
 namespace content {
 
@@ -18,10 +23,13 @@ std::unique_ptr<NavigationState> NavigationState::CreateBrowserInitiated(
     const CommitNavigationParams& commit_params,
     base::TimeTicks time_commit_requested,
     mojom::FrameNavigationControl::CommitNavigationCallback callback,
+    mojom::NavigationClient::CommitNavigationCallback
+        per_navigation_mojo_interface_callback,
     std::unique_ptr<NavigationClient> navigation_client) {
   return base::WrapUnique(new NavigationState(
       common_params, commit_params, time_commit_requested, false,
-      std::move(callback), std::move(navigation_client)));
+      std::move(callback), std::move(per_navigation_mojo_interface_callback),
+      std::move(navigation_client)));
 }
 
 // static
@@ -29,7 +37,7 @@ std::unique_ptr<NavigationState> NavigationState::CreateContentInitiated() {
   return base::WrapUnique(new NavigationState(
       CommonNavigationParams(), CommitNavigationParams(), base::TimeTicks(),
       true, content::mojom::FrameNavigationControl::CommitNavigationCallback(),
-      nullptr));
+      content::mojom::NavigationClient::CommitNavigationCallback(), nullptr));
 }
 
 // static
@@ -53,12 +61,24 @@ void NavigationState::RunCommitNavigationCallback(
     std::move(commit_callback_).Run(result);
 }
 
+void NavigationState::RunPerNavigationInterfaceCommitNavigationCallback(
+    std::unique_ptr<::FrameHostMsg_DidCommitProvisionalLoad_Params> params,
+    mojom::DidCommitProvisionalLoadInterfaceParamsPtr interface_params) {
+  DCHECK(IsPerNavigationMojoInterfaceEnabled());
+  if (per_navigation_mojo_interface_commit_callback_)
+    std::move(per_navigation_mojo_interface_commit_callback_)
+        .Run(std::move(params), std::move(interface_params));
+  navigation_client_.reset();
+}
+
 NavigationState::NavigationState(
     const CommonNavigationParams& common_params,
     const CommitNavigationParams& commit_params,
     base::TimeTicks time_commit_requested,
     bool is_content_initiated,
     mojom::FrameNavigationControl::CommitNavigationCallback callback,
+    mojom::NavigationClient::CommitNavigationCallback
+        per_navigation_mojo_interface_commit_callback,
     std::unique_ptr<NavigationClient> navigation_client)
     : request_committed_(false),
       was_within_same_document_(false),
@@ -67,6 +87,7 @@ NavigationState::NavigationState(
       commit_params_(commit_params),
       time_commit_requested_(time_commit_requested),
       navigation_client_(std::move(navigation_client)),
-      commit_callback_(std::move(callback)) {}
-
+      commit_callback_(std::move(callback)),
+      per_navigation_mojo_interface_commit_callback_(
+          std::move(per_navigation_mojo_interface_commit_callback)) {}
 }  // namespace content

content/renderer/navigation_state.h

@@ -5,16 +5,22 @@
 #ifndef CONTENT_RENDERER_NAVIGATION_STATE_H_
 #define CONTENT_RENDERER_NAVIGATION_STATE_H_
 
-#include <string>
+#include <memory>
 
 #include "base/macros.h"
+#include "base/time/time.h"
 #include "content/common/frame.mojom.h"
 #include "content/common/navigation_params.h"
 #include "content/renderer/navigation_client.h"
-#include "third_party/blink/public/web/commit_result.mojom.h"
+
+struct FrameHostMsg_DidCommitProvisionalLoad_Params;
 
 namespace blink {
 class WebDocumentLoader;
+
+namespace mojom {
+enum class CommitResult;
+}
 }
 
 namespace content {
@@ -28,6 +34,8 @@ class CONTENT_EXPORT NavigationState {
       const CommitNavigationParams& commit_params,
       base::TimeTicks time_commit_requested,
       mojom::FrameNavigationControl::CommitNavigationCallback callback,
+      mojom::NavigationClient::CommitNavigationCallback
+          per_navigation_mojo_interface_callback,
       std::unique_ptr<NavigationClient> navigation_client);
 
   static std::unique_ptr<NavigationState> CreateContentInitiated();
@@ -44,6 +52,9 @@ class CONTENT_EXPORT NavigationState {
   const CommonNavigationParams& common_params() const { return common_params_; }
   const CommitNavigationParams& commit_params() const { return commit_params_; }
   bool request_committed() const { return request_committed_; }
+  bool uses_per_navigation_mojo_interface() const {
+    return navigation_client_.get();
+  }
   void set_request_committed(bool value) { request_committed_ = value; }
   void set_was_within_same_document(bool value) {
     was_within_same_document_ = value;
@@ -69,6 +80,10 @@ class CONTENT_EXPORT NavigationState {
 
   void RunCommitNavigationCallback(blink::mojom::CommitResult result);
 
+  void RunPerNavigationInterfaceCommitNavigationCallback(
+      std::unique_ptr<::FrameHostMsg_DidCommitProvisionalLoad_Params> params,
+      mojom::DidCommitProvisionalLoadInterfaceParamsPtr interface_params);
+
  private:
   NavigationState(
       const CommonNavigationParams& common_params,
@@ -76,6 +91,8 @@ class CONTENT_EXPORT NavigationState {
       base::TimeTicks time_commit_requested,
       bool is_content_initiated,
       content::mojom::FrameNavigationControl::CommitNavigationCallback callback,
+      content::mojom::NavigationClient::CommitNavigationCallback
+          per_navigation_mojo_interface_callback,
       std::unique_ptr<NavigationClient> navigation_client);
 
   bool request_committed_;
@@ -111,6 +128,12 @@ class CONTENT_EXPORT NavigationState {
   // successful or not.
   mojom::FrameNavigationControl::CommitNavigationCallback commit_callback_;
 
+  // Temporary member meant to be used in place of |commit_callback_| when
+  // PerNavigationMojoInterface is enabled. Should eventually replace it
+  // completely.
+  mojom::NavigationClient::CommitNavigationCallback
+      per_navigation_mojo_interface_commit_callback_;
+
   DISALLOW_COPY_AND_ASSIGN(NavigationState);
 };
 

content/renderer/render_frame_impl.cc

@@ -815,6 +815,8 @@ std::unique_ptr<DocumentState> BuildDocumentStateFromParams(
     const CommitNavigationParams& commit_params,
     base::TimeTicks time_commit_requested,
     mojom::FrameNavigationControl::CommitNavigationCallback commit_callback,
+    mojom::NavigationClient::CommitNavigationCallback
+        per_navigation_mojo_interface_commit_callback,
     const network::ResourceResponseHead* head,
     std::unique_ptr<NavigationClient> navigation_client) {
   std::unique_ptr<DocumentState> document_state(new DocumentState());
@@ -875,7 +877,9 @@ std::unique_ptr<DocumentState> BuildDocumentStateFromParams(
   InternalDocumentStateData::FromDocumentState(document_state.get())
       ->set_navigation_state(NavigationState::CreateBrowserInitiated(
           common_params, commit_params, time_commit_requested,
-          std::move(commit_callback), std::move(navigation_client)));
+          std::move(commit_callback),
+          std::move(per_navigation_mojo_interface_commit_callback),
+          std::move(navigation_client)));
   return document_state;
 }
 
@@ -2833,7 +2837,8 @@ void RenderFrameImpl::LoadNavigationErrorPage(
     document_state = BuildDocumentStateFromParams(
         navigation_state->common_params(), navigation_state->commit_params(),
         base::TimeTicks(),  // Not used for failed navigation.
-        CommitNavigationCallback(), nullptr, nullptr);
+        mojom::FrameNavigationControl::CommitNavigationCallback(),
+        mojom::NavigationClient::CommitNavigationCallback(), nullptr, nullptr);
     FillMiscNavigationParams(navigation_state->common_params(),
                              navigation_state->commit_params(),
                              navigation_params.get());
@@ -3203,7 +3208,62 @@ void RenderFrameImpl::CommitNavigation(
     blink::mojom::ControllerServiceWorkerInfoPtr controller_service_worker_info,
     network::mojom::URLLoaderFactoryPtr prefetch_loader_factory,
     const base::UnguessableToken& devtools_navigation_token,
-    CommitNavigationCallback callback) {
+    CommitNavigationCallback commit_callback) {
+  DCHECK(!navigation_client_impl_);
+  // We can have a FrameNavigationControl::CommitNavigation with
+  // IsPerNavigationMojoInterfaceEnabled() == true, for non-committed
+  // interstitials where no NavigationRequest was created. Therefore, no DCHECK.
+  CommitNavigationInternal(
+      head, common_params, commit_params,
+      std::move(url_loader_client_endpoints),
+      std::move(subresource_loader_factories), std::move(subresource_overrides),
+      std::move(controller_service_worker_info),
+      std::move(prefetch_loader_factory), devtools_navigation_token,
+      std::move(commit_callback),
+      mojom::NavigationClient::CommitNavigationCallback());
+}
+
+void RenderFrameImpl::CommitPerNavigationMojoInterfaceNavigation(
+    const network::ResourceResponseHead& head,
+    const CommonNavigationParams& common_params,
+    const CommitNavigationParams& commit_params,
+    network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
+    std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
+        subresource_loader_factories,
+    base::Optional<std::vector<mojom::TransferrableURLLoaderPtr>>
+        subresource_overrides,
+    blink::mojom::ControllerServiceWorkerInfoPtr controller_service_worker_info,
+    network::mojom::URLLoaderFactoryPtr prefetch_loader_factory,
+    const base::UnguessableToken& devtools_navigation_token,
+    mojom::NavigationClient::CommitNavigationCallback
+        per_navigation_mojo_interface_callback) {
+  DCHECK(navigation_client_impl_);
+  DCHECK(IsPerNavigationMojoInterfaceEnabled());
+  CommitNavigationInternal(
+      head, common_params, commit_params,
+      std::move(url_loader_client_endpoints),
+      std::move(subresource_loader_factories), std::move(subresource_overrides),
+      std::move(controller_service_worker_info),
+      std::move(prefetch_loader_factory), devtools_navigation_token,
+      mojom::FrameNavigationControl::CommitNavigationCallback(),
+      std::move(per_navigation_mojo_interface_callback));
+}
+
+void RenderFrameImpl::CommitNavigationInternal(
+    const network::ResourceResponseHead& head,
+    const CommonNavigationParams& common_params,
+    const CommitNavigationParams& commit_params,
+    network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
+    std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
+        subresource_loader_factories,
+    base::Optional<std::vector<mojom::TransferrableURLLoaderPtr>>
+        subresource_overrides,
+    blink::mojom::ControllerServiceWorkerInfoPtr controller_service_worker_info,
+    network::mojom::URLLoaderFactoryPtr prefetch_loader_factory,
+    const base::UnguessableToken& devtools_navigation_token,
+    mojom::FrameNavigationControl::CommitNavigationCallback callback,
+    mojom::NavigationClient::CommitNavigationCallback
+        per_navigation_mojo_interface_callback) {
   DCHECK(!IsRendererDebugURL(common_params.url));
   DCHECK(
       !FrameMsg_Navigate_Type::IsSameDocument(common_params.navigation_type));
@@ -3214,7 +3274,11 @@ void RenderFrameImpl::CommitNavigation(
       browser_side_navigation_pending_url_ == commit_params.original_url &&
       commit_params.nav_entry_id == 0) {
     browser_side_navigation_pending_url_ = GURL();
-    std::move(callback).Run(blink::mojom::CommitResult::Aborted);
+    if (IsPerNavigationMojoInterfaceEnabled()) {
+      navigation_client_impl_.reset();
+    } else {
+      std::move(callback).Run(blink::mojom::CommitResult::Aborted);
+    }
     return;
   }
 
@@ -3244,7 +3308,8 @@ void RenderFrameImpl::CommitNavigation(
     response_head = &head;
   std::unique_ptr<DocumentState> document_state(BuildDocumentStateFromParams(
       common_params, commit_params, base::TimeTicks::Now(), std::move(callback),
-      response_head, std::move(navigation_client_impl_)));
+      std::move(per_navigation_mojo_interface_callback), response_head,
+      std::move(navigation_client_impl_)));
 
   blink::WebFrameLoadType load_type = NavigationTypeToLoadType(
       common_params.navigation_type, common_params.should_replace_current_entry,
@@ -3462,7 +3527,8 @@ void RenderFrameImpl::CommitFailedNavigation(
 
   std::unique_ptr<DocumentState> document_state = BuildDocumentStateFromParams(
       common_params, commit_params, base::TimeTicks(), std::move(callback),
-      nullptr, std::move(navigation_client_impl_));
+      mojom::NavigationClient::CommitNavigationCallback(), nullptr,
+      std::move(navigation_client_impl_));
 
   // The load of the error page can result in this frame being removed.
   // Use a WeakPtr as an easy way to detect whether this has occured. If so,
@@ -3524,7 +3590,8 @@ void RenderFrameImpl::CommitSameDocumentNavigation(
     internal_data->set_navigation_state(NavigationState::CreateBrowserInitiated(
         common_params, commit_params,
         base::TimeTicks(),  // Not used for same-document navigation.
-        CommitNavigationCallback(), nullptr));
+        mojom::FrameNavigationControl::CommitNavigationCallback(),
+        mojom::NavigationClient::CommitNavigationCallback(), nullptr));
 
     // Load the request.
     commit_status = frame_->CommitSameDocumentNavigation(
@@ -3568,12 +3635,13 @@ void RenderFrameImpl::HandleRendererDebugURL(const GURL& url) {
 }
 
 void RenderFrameImpl::UpdateSubresourceLoaderFactories(
-    std::unique_ptr<blink::URLLoaderFactoryBundleInfo> subresource_loaders) {
+    std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
+        subresource_loader_factories) {
   DCHECK(loader_factories_);
   // TODO(crbug/916625): CHECKing for crbug.com/916625.
   CHECK(loader_factories_->IsHostChildURLLoaderFactoryBundle());
   static_cast<HostChildURLLoaderFactoryBundle*>(loader_factories_.get())
-      ->UpdateThisAndAllClones(std::move(subresource_loaders));
+      ->UpdateThisAndAllClones(std::move(subresource_loader_factories));
 }
 
 void RenderFrameImpl::BindDevToolsAgent(
@@ -4371,7 +4439,10 @@ void RenderFrameImpl::DidCommitProvisionalLoad(
   if (media_permission_dispatcher_)
     media_permission_dispatcher_->OnNavigation();
 
-  navigation_state->RunCommitNavigationCallback(blink::mojom::CommitResult::Ok);
+  if (!navigation_state->uses_per_navigation_mojo_interface()) {
+    navigation_state->RunCommitNavigationCallback(
+        blink::mojom::CommitResult::Ok);
+  }
 
   ui::PageTransition transition = GetTransitionType(frame_->GetDocumentLoader(),
                                                     frame_, true /* loading */);
@@ -5780,9 +5851,16 @@ void RenderFrameImpl::DidCommitNavigationInternal(
     GetFrameHost()->DidCommitSameDocumentNavigation(
         MakeDidCommitProvisionalLoadParams(commit_type, transition));
   } else {
-    GetFrameHost()->DidCommitProvisionalLoad(
-        MakeDidCommitProvisionalLoadParams(commit_type, transition),
-        std::move(interface_params));
+    auto params = MakeDidCommitProvisionalLoadParams(commit_type, transition);
+    NavigationState* navigation_state =
+        NavigationState::FromDocumentLoader(frame_->GetDocumentLoader());
+    if (navigation_state->uses_per_navigation_mojo_interface()) {
+      navigation_state->RunPerNavigationInterfaceCommitNavigationCallback(
+          std::move(params), std::move(interface_params));
+    } else {
+      GetFrameHost()->DidCommitProvisionalLoad(std::move(params),
+                                               std::move(interface_params));
+    }
   }
 }
 

content/renderer/render_frame_impl.h

@@ -562,21 +562,44 @@ class CONTENT_EXPORT RenderFrameImpl
       const CommonNavigationParams& common_params,
       const CommitNavigationParams& commit_params,
       network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
-      std::unique_ptr<blink::URLLoaderFactoryBundleInfo> subresource_loaders,
+      std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
+          subresource_loader_factories,
       base::Optional<std::vector<mojom::TransferrableURLLoaderPtr>>
           subresource_overrides,
       blink::mojom::ControllerServiceWorkerInfoPtr
           controller_service_worker_info,
       network::mojom::URLLoaderFactoryPtr prefetch_loader_factory,
       const base::UnguessableToken& devtools_navigation_token,
-      CommitNavigationCallback callback) override;
+      mojom::FrameNavigationControl::CommitNavigationCallback commit_callback)
+      override;
+
+  // This is the version to be used with PerNavigationMojoInterface enabled.
+  // It essentially works the same way, except the navigation callback is
+  // the one from NavigationClient mojo interface.
+  void CommitPerNavigationMojoInterfaceNavigation(
+      const network::ResourceResponseHead& head,
+      const CommonNavigationParams& common_params,
+      const CommitNavigationParams& commit_params,
+      network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
+      std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
+          subresource_loader_factories,
+      base::Optional<std::vector<mojom::TransferrableURLLoaderPtr>>
+          subresource_overrides,
+      blink::mojom::ControllerServiceWorkerInfoPtr
+          controller_service_worker_info,
+      network::mojom::URLLoaderFactoryPtr prefetch_loader_factory,
+      const base::UnguessableToken& devtools_navigation_token,
+      mojom::NavigationClient::CommitNavigationCallback
+          per_navigation_mojo_interface_callback);
+
   void CommitFailedNavigation(
       const CommonNavigationParams& common_params,
       const CommitNavigationParams& commit_params,
       bool has_stale_copy_in_cache,
       int error_code,
       const base::Optional<std::string>& error_page_content,
-      std::unique_ptr<blink::URLLoaderFactoryBundleInfo> subresource_loaders,
+      std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
+          subresource_loader_factories,
       CommitFailedNavigationCallback callback) override;
   void CommitSameDocumentNavigation(
       const CommonNavigationParams& common_params,
@@ -584,8 +607,8 @@ class CONTENT_EXPORT RenderFrameImpl
       CommitSameDocumentNavigationCallback callback) override;
   void HandleRendererDebugURL(const GURL& url) override;
   void UpdateSubresourceLoaderFactories(
-      std::unique_ptr<blink::URLLoaderFactoryBundleInfo> subresource_loaders)
-      override;
+      std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
+          subresource_loader_factories) override;
   void BindDevToolsAgent(
       blink::mojom::DevToolsAgentHostAssociatedPtrInfo host,
       blink::mojom::DevToolsAgentAssociatedRequest request) override;
@@ -1327,6 +1350,25 @@ class CONTENT_EXPORT RenderFrameImpl
       blink::mojom::ControllerServiceWorkerInfoPtr
           controller_service_worker_info);
 
+  // This function avoid duplication between CommitNavigation and
+  // CommitPerNavigationMojoInterfaceNavigation.
+  void CommitNavigationInternal(
+      const network::ResourceResponseHead& head,
+      const CommonNavigationParams& common_params,
+      const CommitNavigationParams& commit_params,
+      network::mojom::URLLoaderClientEndpointsPtr url_loader_client_endpoints,
+      std::unique_ptr<blink::URLLoaderFactoryBundleInfo>
+          subresource_loader_factories,
+      base::Optional<std::vector<mojom::TransferrableURLLoaderPtr>>
+          subresource_overrides,
+      blink::mojom::ControllerServiceWorkerInfoPtr
+          controller_service_worker_info,
+      network::mojom::URLLoaderFactoryPtr prefetch_loader_factory,
+      const base::UnguessableToken& devtools_navigation_token,
+      mojom::FrameNavigationControl::CommitNavigationCallback callback,
+      mojom::NavigationClient::CommitNavigationCallback
+          per_navigation_mojo_interface_callback);
+
   // Stores the WebLocalFrame we are associated with.  This is null from the
   // constructor until BindToFrame() is called, and it is null after
   // FrameDetached() is called until destruction (which is asynchronous in the

content/test/navigation_simulator_impl.cc

@@ -575,6 +575,14 @@ void NavigationSimulatorImpl::Commit() {
     document_interface_broker_blink_request_ = nullptr;
   }
 
+  if (handle_) {
+    scoped_refptr<net::HttpResponseHeaders> response_headers =
+        new net::HttpResponseHeaders(std::string());
+    response_headers->AddHeader(std::string("Content-Type: ") +
+                                contents_mime_type_);
+    handle_->set_response_headers_for_testing(response_headers);
+  }
+
   auto params = BuildDidCommitProvisionalLoadParams(
       false /* same_document */, false /* failed_navigation */);
   render_frame_host_->SimulateCommitProcessed(

content/test/test_render_frame_host.cc

@@ -16,7 +16,6 @@
 #include "content/browser/frame_host/navigator_impl.h"
 #include "content/browser/frame_host/render_frame_host_delegate.h"
 #include "content/browser/web_contents/web_contents_impl.h"
-#include "content/common/frame_messages.h"
 #include "content/common/frame_owner_properties.h"
 #include "content/public/browser/navigation_throttle.h"
 #include "content/public/common/navigation_policy.h"
@@ -294,7 +293,7 @@ void TestRenderFrameHost::SendNavigateWithParamsAndInterfaceParams(
     FrameHostMsg_DidCommitProvisionalLoad_Params* params,
     mojom::DidCommitProvisionalLoadInterfaceParamsPtr interface_params,
     bool was_within_same_document) {
-  if (GetNavigationHandle()) {
+  if (GetNavigationHandle() && !GetNavigationHandle()->GetResponseHeaders()) {
     scoped_refptr<net::HttpResponseHeaders> response_headers =
         new net::HttpResponseHeaders(std::string());
     response_headers->AddHeader(std::string("Content-Type: ") +
@@ -444,6 +443,7 @@ void TestRenderFrameHost::SimulateCommitProcessed(
   CHECK(params);
   blink::mojom::CommitResult result = blink::mojom::CommitResult::Ok;
 
+  bool did_commit = false;
   if (!same_document) {
     // Note: Although the code does not prohibit the running of multiple
     // callbacks, no more than 1 callback will ever run, because navigation_id
@@ -455,8 +455,15 @@ void TestRenderFrameHost::SimulateCommitProcessed(
     }
     {
       auto callback_it = navigation_client_commit_callback_.find(navigation_id);
-      if (callback_it != navigation_client_commit_callback_.end())
-        std::move(callback_it->second).Run(result);
+      if (callback_it != navigation_client_commit_callback_.end()) {
+        std::move(callback_it->second)
+            .Run(std::move(params),
+                 mojom::DidCommitProvisionalLoadInterfaceParams::New(
+                     std::move(interface_provider_request),
+                     std::move(document_interface_broker_content_request),
+                     std::move(document_interface_broker_blink_request)));
+        did_commit = true;
+      }
     }
     {
       auto callback_it = commit_failed_callback_.find(navigation_id);
@@ -471,13 +478,15 @@ void TestRenderFrameHost::SimulateCommitProcessed(
     }
   }
 
-  SendNavigateWithParamsAndInterfaceParams(
-      params.release(),
-      mojom::DidCommitProvisionalLoadInterfaceParams::New(
-          std::move(interface_provider_request),
-          std::move(document_interface_broker_content_request),
-          std::move(document_interface_broker_blink_request)),
-      same_document);
+  if (!did_commit) {
+    SendNavigateWithParamsAndInterfaceParams(
+        params.release(),
+        mojom::DidCommitProvisionalLoadInterfaceParams::New(
+            std::move(interface_provider_request),
+            std::move(document_interface_broker_content_request),
+            std::move(document_interface_broker_blink_request)),
+        same_document);
+  }
 }
 
 WebBluetoothServiceImpl*