PlzNavigate: Use the NavigationUrlLoaderNetworkService.

Use the NavigationURLLoaderNetworkService when NavigationMojoResponse is
enabled, even if the network service isn’t.

The feature NavigationMojoResponse is currently disabled by default, so
this patch shouldn't introduce any new regression.

See the others CLs in this series:
[1/3] https://chromium-review.googlesource.com/c/739502
[2/3] this CL.
[3/3] https://chromium-review.googlesource.com/c/753738

This is part 2 of the implementation plan.
Design doc: https://goo.gl/Rrrc7n

Bug: 705744
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_mojo;master.tryserver.chromium.linux:linux_site_isolation
Change-Id: Ia95527dce4be7b43747fec09ffe61bc51b2195a6
Reviewed-on: https://chromium-review.googlesource.com/741237
Commit-Queue: Arthur Sonzogni <arthursonzogni@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523373}

content/browser/browser_side_navigation_browsertest.cc

@@ -582,4 +582,205 @@ IN_PROC_BROWSER_TEST_F(BrowserSideNavigationBaseBrowserTest,
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
 }
 
+// TODO(arthursonzogni): Remove these tests once NavigationMojoResponse has
+// launched.
+class NavigationMojoResponseBrowserTest : public ContentBrowserTest {
+ public:
+  NavigationMojoResponseBrowserTest() {}
+
+ protected:
+  void SetUp() override {
+    base::test::ScopedFeatureList().InitAndEnableFeature(
+        features::kNavigationMojoResponse);
+    ContentBrowserTest::SetUp();
+  }
+
+  void SetUpOnMainThread() override {
+    host_resolver()->AddRule("*", "127.0.0.1");
+    ASSERT_TRUE(embedded_test_server()->Start());
+  }
+};
+
+// Ensure that browser initiated basic navigations work with browser side
+// navigation.
+// TODO(arthursonzogni): Remove this test once NavigationMojoResponse has
+// launched.
+IN_PROC_BROWSER_TEST_F(NavigationMojoResponseBrowserTest,
+                       BrowserInitiatedNavigations) {
+  // Perform a navigation with no live renderer.
+  {
+    TestNavigationObserver observer(shell()->web_contents());
+    GURL url(embedded_test_server()->GetURL("/title1.html"));
+    NavigateToURL(shell(), url);
+    EXPECT_EQ(url, observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+  }
+
+  RenderFrameHost* initial_rfh =
+      static_cast<WebContentsImpl*>(shell()->web_contents())
+          ->GetFrameTree()
+          ->root()
+          ->current_frame_host();
+
+  // Perform a same site navigation.
+  {
+    TestNavigationObserver observer(shell()->web_contents());
+    GURL url(embedded_test_server()->GetURL("/title2.html"));
+    NavigateToURL(shell(), url);
+    EXPECT_EQ(url, observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+  }
+
+  // The RenderFrameHost should not have changed.
+  EXPECT_EQ(initial_rfh, static_cast<WebContentsImpl*>(shell()->web_contents())
+                             ->GetFrameTree()
+                             ->root()
+                             ->current_frame_host());
+
+  // Perform a cross-site navigation.
+  {
+    TestNavigationObserver observer(shell()->web_contents());
+    GURL url = embedded_test_server()->GetURL("foo.com", "/title3.html");
+    NavigateToURL(shell(), url);
+    EXPECT_EQ(url, observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+  }
+
+  // The RenderFrameHost should have changed.
+  EXPECT_NE(initial_rfh, static_cast<WebContentsImpl*>(shell()->web_contents())
+                             ->GetFrameTree()
+                             ->root()
+                             ->current_frame_host());
+}
+
+// Ensure that renderer initiated same-site navigations work with browser side
+// navigation.
+// TODO(arthursonzogni): Remove this test once NavigationMojoResponse has
+// launched.
+IN_PROC_BROWSER_TEST_F(NavigationMojoResponseBrowserTest,
+                       RendererInitiatedSameSiteNavigation) {
+  // Perform a navigation with no live renderer.
+  {
+    TestNavigationObserver observer(shell()->web_contents());
+    GURL url(embedded_test_server()->GetURL("/simple_links.html"));
+    NavigateToURL(shell(), url);
+    EXPECT_EQ(url, observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+  }
+
+  RenderFrameHost* initial_rfh =
+      static_cast<WebContentsImpl*>(shell()->web_contents())
+          ->GetFrameTree()
+          ->root()
+          ->current_frame_host();
+
+  // Simulate clicking on a same-site link.
+  {
+    TestNavigationObserver observer(shell()->web_contents());
+    GURL url(embedded_test_server()->GetURL("/title2.html"));
+    bool success = false;
+    EXPECT_TRUE(ExecuteScriptAndExtractBool(
+        shell(), "window.domAutomationController.send(clickSameSiteLink());",
+        &success));
+    EXPECT_TRUE(success);
+    EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+    EXPECT_EQ(url, observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+  }
+
+  // The RenderFrameHost should not have changed.
+  EXPECT_EQ(initial_rfh, static_cast<WebContentsImpl*>(shell()->web_contents())
+                             ->GetFrameTree()
+                             ->root()
+                             ->current_frame_host());
+}
+
+// Ensure that renderer initiated cross-site navigations work with browser side
+// navigation.
+// TODO(arthursonzogni): Remove this test once NavigationMojoResponse has
+// launched.
+IN_PROC_BROWSER_TEST_F(NavigationMojoResponseBrowserTest,
+                       RendererInitiatedCrossSiteNavigation) {
+  // Perform a navigation with no live renderer.
+  {
+    TestNavigationObserver observer(shell()->web_contents());
+    GURL url(embedded_test_server()->GetURL("/simple_links.html"));
+    NavigateToURL(shell(), url);
+    EXPECT_EQ(url, observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+  }
+
+  RenderFrameHost* initial_rfh =
+      static_cast<WebContentsImpl*>(shell()->web_contents())
+          ->GetFrameTree()
+          ->root()
+          ->current_frame_host();
+
+  // Simulate clicking on a cross-site link.
+  {
+    TestNavigationObserver observer(shell()->web_contents());
+    const char kReplacePortNumber[] =
+        "window.domAutomationController.send(setPortNumber(%d));";
+    uint16_t port_number = embedded_test_server()->port();
+    GURL url = embedded_test_server()->GetURL("foo.com", "/title2.html");
+    bool success = false;
+    EXPECT_TRUE(ExecuteScriptAndExtractBool(
+        shell(), base::StringPrintf(kReplacePortNumber, port_number),
+        &success));
+    success = false;
+    EXPECT_TRUE(ExecuteScriptAndExtractBool(
+        shell(), "window.domAutomationController.send(clickCrossSiteLink());",
+        &success));
+    EXPECT_TRUE(success);
+    EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
+    EXPECT_EQ(url, observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+  }
+
+  // The RenderFrameHost should not have changed unless site-per-process is
+  // enabled.
+  if (AreAllSitesIsolatedForTesting()) {
+    EXPECT_NE(initial_rfh,
+              static_cast<WebContentsImpl*>(shell()->web_contents())
+                  ->GetFrameTree()
+                  ->root()
+                  ->current_frame_host());
+  } else {
+    EXPECT_EQ(initial_rfh,
+              static_cast<WebContentsImpl*>(shell()->web_contents())
+                  ->GetFrameTree()
+                  ->root()
+                  ->current_frame_host());
+  }
+}
+
+// Ensure that browser side navigation handles navigation failures.
+// TODO(arthursonzogni): Remove this test once NavigationMojoResponse has
+// launched.
+IN_PROC_BROWSER_TEST_F(NavigationMojoResponseBrowserTest, FailedNavigation) {
+  // Perform a navigation with no live renderer.
+  {
+    TestNavigationObserver observer(shell()->web_contents());
+    GURL url(embedded_test_server()->GetURL("/title1.html"));
+    NavigateToURL(shell(), url);
+    EXPECT_EQ(url, observer.last_navigation_url());
+    EXPECT_TRUE(observer.last_navigation_succeeded());
+  }
+
+  // Now navigate to an unreachable url.
+  {
+    TestNavigationObserver observer(shell()->web_contents());
+    GURL error_url(
+        net::URLRequestFailedJob::GetMockHttpUrl(net::ERR_CONNECTION_RESET));
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::BindOnce(&net::URLRequestFailedJob::AddUrlHandler));
+    NavigateToURL(shell(), error_url);
+    EXPECT_EQ(error_url, observer.last_navigation_url());
+    NavigationEntry* entry =
+        shell()->web_contents()->GetController().GetLastCommittedEntry();
+    EXPECT_EQ(PAGE_TYPE_ERROR, entry->GetPageType());
+  }
+}
+
 }  // namespace content

content/browser/loader/navigation_url_loader.cc

@@ -14,6 +14,7 @@
 #include "content/browser/loader/navigation_url_loader_network_service.h"
 #include "content/browser/loader/url_loader_request_handler.h"
 #include "content/public/browser/navigation_ui_data.h"
+#include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/content_features.h"
 
 namespace content {
@@ -33,7 +34,8 @@ std::unique_ptr<NavigationURLLoader> NavigationURLLoader::Create(
         resource_context, storage_partition, std::move(request_info),
         std::move(navigation_ui_data), service_worker_handle, delegate);
   }
-  if (base::FeatureList::IsEnabled(features::kNetworkService)) {
+  if (base::FeatureList::IsEnabled(features::kNetworkService) ||
+      IsNavigationMojoResponseEnabled()) {
     return std::make_unique<NavigationURLLoaderNetworkService>(
         resource_context, storage_partition, std::move(request_info),
         std::move(navigation_ui_data), service_worker_handle, appcache_handle,

content/browser/loader/navigation_url_loader_impl_core.cc

@@ -54,7 +54,8 @@ void NavigationURLLoaderImplCore::Start(
     ResourceDispatcherHostImpl::Get()->BeginNavigationRequest(
         resource_context, url_request_context_getter->GetURLRequestContext(),
         upload_file_system_context, *request_info,
-        std::move(navigation_ui_data), this, service_worker_handle_core,
+        std::move(navigation_ui_data), this, mojom::URLLoaderClientPtr(),
+        mojom::URLLoaderRequest(), service_worker_handle_core,
         appcache_handle_core);
   }
 

content/browser/loader/navigation_url_loader_network_service.h

@@ -7,6 +7,7 @@
 
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
+#include "base/time/time.h"
 #include "content/browser/loader/navigation_url_loader.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/ssl_status.h"
@@ -57,6 +58,7 @@ class CONTENT_EXPORT NavigationURLLoaderNetworkService
 
  private:
   class URLLoaderRequestController;
+  void OnRequestStarted(base::TimeTicks timestamp);
 
   void BindNonNetworkURLLoaderFactoryRequest(
       const GURL& url,

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -1558,7 +1558,8 @@ ResourceDispatcherHostImpl::AddStandardHandlers(
   handler.reset(new ThrottlingResourceHandler(
       std::move(handler), request, std::move(post_mime_sniffing_throttles)));
 
-  if (IsBrowserSideNavigationEnabled() && IsResourceTypeFrame(resource_type)) {
+  if (IsBrowserSideNavigationEnabled() && IsResourceTypeFrame(resource_type) &&
+      !IsNavigationMojoResponseEnabled()) {
     DCHECK(navigation_loader_core);
     DCHECK(stream_handle);
     // PlzNavigate
@@ -1999,12 +2000,18 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
     const NavigationRequestInfo& info,
     std::unique_ptr<NavigationUIData> navigation_ui_data,
     NavigationURLLoaderImplCore* loader,
+    mojom::URLLoaderClientPtr url_loader_client,
+    mojom::URLLoaderRequest url_loader_request,
     ServiceWorkerNavigationHandleCore* service_worker_handle_core,
     AppCacheNavigationHandleCore* appcache_handle_core) {
   // PlzNavigate: BeginNavigationRequest currently should only be used for the
   // browser-side navigations project.
   CHECK(IsBrowserSideNavigationEnabled());
 
+  DCHECK_EQ(IsNavigationMojoResponseEnabled(), !loader);
+  DCHECK_EQ(IsNavigationMojoResponseEnabled(), url_loader_client.is_bound());
+  DCHECK_EQ(IsNavigationMojoResponseEnabled(), url_loader_request.is_pending());
+
   ResourceType resource_type = info.is_main_frame ?
       RESOURCE_TYPE_MAIN_FRAME : RESOURCE_TYPE_SUB_FRAME;
 
@@ -2029,7 +2036,12 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
           info.common_params.url,
           resource_type,
           resource_context))) {
-    loader->NotifyRequestFailed(false, net::ERR_ABORTED, base::nullopt);
+    if (IsNavigationMojoResponseEnabled()) {
+      url_loader_client->OnComplete(
+          network::URLLoaderCompletionStatus(net::ERR_ABORTED));
+    } else {
+      loader->NotifyRequestFailed(false, net::ERR_ABORTED, base::nullopt);
+    }
     return;
   }
 
@@ -2075,7 +2087,12 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
   if (body) {
     if (!GetBodyBlobDataHandles(body, resource_context, &blob_handles)) {
       new_request->CancelWithError(net::ERR_INSUFFICIENT_RESOURCES);
-      loader->NotifyRequestFailed(false, net::ERR_ABORTED, base::nullopt);
+      if (IsNavigationMojoResponseEnabled()) {
+        url_loader_client->OnComplete(
+            network::URLLoaderCompletionStatus(net::ERR_ABORTED));
+      } else {
+        loader->NotifyRequestFailed(false, net::ERR_ABORTED, base::nullopt);
+      }
       return;
     }
     new_request->set_upload(UploadDataStreamBuilder::Build(
@@ -2153,19 +2170,26 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
         new_request.get(), appcache_handle_core->host(), resource_type, false);
   }
 
-  StreamContext* stream_context =
-      GetStreamContextForResourceContext(resource_context);
-  // Note: the stream should be created with immediate mode set to true to
-  // ensure that data read will be flushed to the reader as soon as it's
-  // available. Otherwise, we risk delaying transmitting the body of the
-  // resource to the renderer, which will delay parsing accordingly.
-  std::unique_ptr<ResourceHandler> handler(
-      new StreamResourceHandler(new_request.get(), stream_context->registry(),
-                                new_request->url().GetOrigin(), true));
-  std::unique_ptr<StreamHandle> stream_handle =
-      static_cast<StreamResourceHandler*>(handler.get())
-          ->stream()
-          ->CreateHandle();
+  std::unique_ptr<ResourceHandler> handler;
+  std::unique_ptr<StreamHandle> stream_handle;
+  if (IsNavigationMojoResponseEnabled()) {
+    handler = std::make_unique<MojoAsyncResourceHandler>(
+        new_request.get(), this, std::move(url_loader_request),
+        std::move(url_loader_client), resource_type);
+  } else {
+    StreamContext* stream_context =
+        GetStreamContextForResourceContext(resource_context);
+    // Note: the stream should be created with immediate mode set to true to
+    // ensure that data read will be flushed to the reader as soon as it's
+    // available. Otherwise, we risk delaying transmitting the body of the
+    // resource to the renderer, which will delay parsing accordingly.
+    handler = std::make_unique<StreamResourceHandler>(
+        new_request.get(), stream_context->registry(),
+        new_request->url().GetOrigin(), true);
+    stream_handle = static_cast<StreamResourceHandler*>(handler.get())
+                        ->stream()
+                        ->CreateHandle();
+  }
 
   // Safe to consider navigations as kNoCORS.
   // TODO(davidben): Fix the dependency on child_id/route_id. Those are used

content/browser/loader/resource_dispatcher_host_impl.h

@@ -270,6 +270,8 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
       const NavigationRequestInfo& info,
       std::unique_ptr<NavigationUIData> navigation_ui_data,
       NavigationURLLoaderImplCore* loader,
+      mojom::URLLoaderClientPtr url_loader_client,
+      mojom::URLLoaderRequest url_loader_request,
       ServiceWorkerNavigationHandleCore* service_worker_handle_core,
       AppCacheNavigationHandleCore* appcache_handle_core);
 

testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

@@ -57,6 +57,7 @@
 -NavigationControllerBrowserTest.ErrorPageReplacement
 -NavigationHandleImplBrowserTest.ErrorCodeOnRedirect
 -NavigationHandleImplBrowserTest.RedirectToRendererDebugUrl
+-NavigationMojoResponseBrowserTest.FailedNavigation
 -PlzNavigateNavigationHandleImplBrowserTest.ErrorPageNetworkError
 -PowerMonitorTest.TestGpuProcess
 -PowerMonitorTest.TestRendererProcess