PlzNavigate: fix issue preventing navigations to WebUIs

content/browser/webui/url_data_manager_backend.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/webui/url_data_manager_backend.h"
 
 #include <set>
 
 #include "base/bind.h"
 #include "base/command_line.h"
@@ skipping 17 matching lines @@
 #include "content/browser/net/view_blob_internals_job_factory.h"
 #include "content/browser/net/view_http_cache_job_factory.h"
 #include "content/browser/resource_context_impl.h"
 #include "content/browser/webui/shared_resources_data_source.h"
 #include "content/browser/webui/url_data_source_impl.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/resource_request_info.h"
+#include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/url_constants.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/http/http_response_headers.h"
 #include "net/http/http_status_code.h"
 #include "net/log/net_log_util.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_error_job.h"
 #include "net/url_request/url_request_job.h"
@@ skipping 217 matching lines @@
 void URLRequestChromeJob::Start() {
   const GURL url = request_->url();
 
   TRACE_EVENT_ASYNC_BEGIN1("browser", "DataManager:Request", this, "URL",
       url.possibly_invalid_spec());
 
   int render_process_id, unused;
   bool is_renderer_request = ResourceRequestInfo::GetRenderFrameForRequest(
       request_, &render_process_id, &unused);
 
-  if (!is_renderer_request) {
+  if (!is_renderer_request ||
+      (render_process_id == -1 && IsBrowserSideNavigationEnabled())) {
+    // PlzNavigate: no navigation to a WebUI is coming from a renderer.

[Charlie Reis, 2016/05/20 18:50:32]

Hmm.  This is only a safe change for PlzNavigate if the
CheckStoragePartitionMatches code isn't needed at all, right?

It looks like dbeam's bug fix in https://codereview.chromium.org/1985983002
(i.e., actually checking against the host whitelist and not allowing all
chrome:// URLs) exposed that StoragePartition check doesn't work for
PlzNavigate.

Is there some way to find out what StoragePartition a PlzNavigate request is
being made from?  Or can we show that CheckStoragePartitionMatches is
unnecessary (per dbeam's TODO below) and remove it for everyone?  I'm a bit
hesitant to turn it off just for PlzNavigate if it might matter in some cases.

(Worst case, we can temporarily do something like this to skip the check for
PlzNavigate, but that's a potential security bug until we determine that the
check is unnecessary.)

[clamy, 2016/05/23 14:21:21]

There's one place in the NavigationRequest where we do get a StoragePartition to
initialize ServiceWorkers for the navigation. I've cleaned up the code to get
it, and made it available to check in 
URLRequestChromeJob::CheckStoragePartitionMatches on the UI thread.

     StartAsync(true);
     return;
   }
 
   if (url.SchemeIs(kChromeUIScheme)) {
     // TODO(dbeam): it's not clear that partition checking is needed or used. It
     // was added for iframe-based signin (http://crbug.com/338127), which has
     // since changed. We should remove if no longer necessary.
     std::vector<std::string> hosts;
     hosts.push_back(content::kChromeUIResourcesHost);
@@ skipping 517 matching lines @@
 
 }  // namespace
 
 net::URLRequestJobFactory::ProtocolHandler*
 CreateDevToolsProtocolHandler(content::ResourceContext* resource_context,
                               bool is_incognito) {
   return new DevToolsJobFactory(resource_context, is_incognito);
 }
 
 }  // namespace content