PlzNavigate: make NavigationResourceHandler a LayeredResourceHandler

content/browser/loader/resource_dispatcher_host_impl.cc

Index: content/browser/loader/resource_dispatcher_host_impl.cc
diff --git a/content/browser/loader/resource_dispatcher_host_impl.cc b/content/browser/loader/resource_dispatcher_host_impl.cc
index 7271a3ac1ced5a4e7514c3296c9c5c942d01eef3..01d6b4418e3b999e54661f6e6a2aec49d5c89cc5 100644
--- a/content/browser/loader/resource_dispatcher_host_impl.cc
+++ b/content/browser/loader/resource_dispatcher_host_impl.cc
@@ -524,8 +524,8 @@ ResourceDispatcherHostImpl::MaybeInterceptAsStream(
   StreamContext* stream_context =
       GetStreamContextForResourceContext(info->GetContext());
 
-  std::unique_ptr<StreamResourceHandler> handler(
-      new StreamResourceHandler(request, stream_context->registry(), origin));
+  std::unique_ptr<StreamResourceHandler> handler(new StreamResourceHandler(
+      request, stream_context->registry(), origin, false));
 
   info->set_is_stream(true);
   std::unique_ptr<StreamInfo> stream_info(new StreamInfo);
@@ -1477,7 +1477,7 @@ ResourceDispatcherHostImpl::CreateResourceHandler(
                              request_data.fetch_request_context_type,
                              request_data.fetch_mixed_content_context_type,
                              requester_info->appcache_service(), child_id,
-                             route_id, std::move(handler));
+                             route_id, std::move(handler), nullptr, nullptr);
 }
 
 std::unique_ptr<ResourceHandler>
@@ -1490,7 +1490,9 @@ ResourceDispatcherHostImpl::AddStandardHandlers(
     AppCacheService* appcache_service,
     int child_id,
     int route_id,
-    std::unique_ptr<ResourceHandler> handler) {
+    std::unique_ptr<ResourceHandler> handler,
+    NavigationURLLoaderImplCore* loader,

[mmenke, 2017/04/28 16:02:21]

Can we give this a name that makes it clearer this is not a resource_loader? 
navigation_loader_core?

[clamy, 2017/05/02 15:05:44]

Done.

+    std::unique_ptr<StreamHandle> stream_handle) {
   // The InterceptingResourceHandler will replace its next handler with an
   // appropriate one based on the MIME type of the response if needed. It
   // should be placed at the end of the chain, just before |handler|.
@@ -1502,7 +1504,7 @@ ResourceDispatcherHostImpl::AddStandardHandlers(
 
   // Add a NavigationResourceThrottle for navigations.
   // PlzNavigate: the throttle is unnecessary as communication with the UI
-  // thread is handled by the NavigationURLloader.
+  // thread is handled by the NavigationResourceHandler below.
   if (!IsBrowserSideNavigationEnabled() && IsResourceTypeFrame(resource_type)) {
     throttles.push_back(base::MakeUnique<NavigationResourceThrottle>(
         request, delegate_, fetch_request_context_type,
@@ -1546,6 +1548,14 @@ ResourceDispatcherHostImpl::AddStandardHandlers(
   handler.reset(new ThrottlingResourceHandler(
       std::move(handler), request, std::move(post_mime_sniffing_throttles)));
 
+  if (IsBrowserSideNavigationEnabled() && IsResourceTypeFrame(resource_type)) {

[mmenke, 2017/04/28 16:02:21]

DCHECK(loader);
DCHECK(stream_handle);

Alternatively, could use loader on the branch, and then DCHECK on
IsBrowserSideNavigationEnabled() && IsResourceTypeFrame(resource_type) here (And
the inverse below).

[clamy, 2017/05/02 15:05:44]

Done.

+    // PlzNavigate
+    // Add a NavigationResourceHandler that will control the flow of navigation.
+    handler.reset(new NavigationResourceHandler(request, std::move(handler),
+                                                loader, delegate(),
+                                                std::move(stream_handle)));
+  }

[mmenke, 2017/04/28 16:02:21]

} else {
  DCHECK(!loader);
  DCHECK(!stream_handle);
}

[clamy, 2017/05/02 15:05:44]

Done.

+
   PluginService* plugin_service = nullptr;
 #if BUILDFLAG(ENABLE_PLUGINS)
   plugin_service = PluginService::GetInstance();
@@ -2072,8 +2082,15 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
         new_request.get(), appcache_handle_core->host(), resource_type, false);
   }
 
+  StreamContext* stream_context =
+      GetStreamContextForResourceContext(extra_info->GetContext());

[mmenke, 2017/04/28 16:02:21]

extra_info->GetContext() is just resource_context.

[clamy, 2017/05/02 15:05:44]

Done.

   std::unique_ptr<ResourceHandler> handler(
-      new NavigationResourceHandler(new_request.get(), loader, delegate()));
+      new StreamResourceHandler(new_request.get(), stream_context->registry(),
+                                new_request->url().GetOrigin(), true));

[mmenke, 2017/04/28 16:02:21]

Maybe a comment on the use of immediate mode?

[clamy, 2017/05/02 15:05:44]

Done.

+  std::unique_ptr<StreamHandle> stream_handle =
+      static_cast<StreamResourceHandler*>(handler.get())

[mmenke, 2017/04/28 16:02:21]

Remove this cast, and just make handler a unqiue_ptr<StreamResourceHandler>

[clamy, 2017/05/02 15:05:44]

This require creating another std::unique_ptr<ResourceHandler> in line 2097
(handler = AddStandarHandlers... won't work). Which do you prefer?

+          ->stream()
+          ->CreateHandle();
 
   // TODO(davidben): Fix the dependency on child_id/route_id. Those are used
   // by the ResourceScheduler. currently it's a no-op.
@@ -2085,7 +2102,7 @@ void ResourceDispatcherHostImpl::BeginNavigationRequest(
                            : nullptr,
       -1,  // child_id
       -1,  // route_id
-      std::move(handler));
+      std::move(handler), loader, std::move(stream_handle));
 
   BeginRequestInternal(std::move(new_request), std::move(handler));
 }