WebUI: shuffle less between threads when responding to requests

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 39 matching lines @@
 
 namespace content {
 
 namespace {
 
 const char kChromeURLContentSecurityPolicyHeaderBase[] =
     "Content-Security-Policy: script-src chrome://resources 'self'";
 
 const char kChromeURLXFrameOptionsHeader[] = "X-Frame-Options: DENY";
 static const char kNetworkErrorKey[] = "netError";
-const int kNoRenderProcessId = -1;
 
 bool SchemeIsInSchemes(const std::string& scheme,
                        const std::vector<std::string>& schemes) {
   return std::find(schemes.begin(), schemes.end(), scheme) != schemes.end();
 }
 
 // Returns whether |url| passes some sanity checks and is a valid GURL.
 bool CheckURLIsValid(const GURL& url) {
   std::vector<std::string> additional_schemes;
   DCHECK(url.SchemeIs(kChromeDevToolsScheme) || url.SchemeIs(kChromeUIScheme) ||
@@ skipping 186 matching lines @@
       backend_(backend),
       weak_factory_(this) {
   DCHECK(backend);
 }
 
 URLRequestChromeJob::~URLRequestChromeJob() {
   CHECK(!backend_->HasPendingJob(this));
 }
 
 void URLRequestChromeJob::Start() {
+  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)
-    render_process_id = kNoRenderProcessId;
+
+  if (!is_renderer_request) {
+    StartAsync(true);
+    return;
+  }
+
+  const GURL url = request_->url();
+
+  if (url.SchemeIs(kChromeUIScheme)) {

[Charlie Reis, 2016/05/19 20:35:50]

Can we add a TODO here to remove the whole CheckStoragePartitionMatches check if
we can determine it's safe to do so (given that it was added for iframe-based
signin, which no longer exists)?

[Dan Beam, 2016/05/19 23:46:46]

Done.

+    std::vector<std::string> hosts;
+    hosts.push_back(content::kChromeUIResourcesHost);
+    GetContentClient()->
+        browser()->GetAdditionalWebUIHostsToIgnoreParititionCheck(&hosts);
+    if (std::find(hosts.begin(), hosts.end(), url.host()) != hosts.end()) {
+      StartAsync(true);
+      return;
+    }
+  }
+
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(&URLRequestChromeJob::CheckStoragePartitionMatches,
-                 render_process_id, request_->url(),
+                 render_process_id, url,
                  weak_factory_.GetWeakPtr()));
-  TRACE_EVENT_ASYNC_BEGIN1("browser", "DataManager:Request", this, "URL",
-      request_->url().possibly_invalid_spec());
 }
 
 void URLRequestChromeJob::Kill() {
   weak_factory_.InvalidateWeakPtrs();
   backend_->RemoveRequest(this);
   URLRequestJob::Kill();
 }
 
 bool URLRequestChromeJob::GetMimeType(std::string* mime_type) const {
   *mime_type = mime_type_;
@@ skipping 99 matching lines @@
 void URLRequestChromeJob::CheckStoragePartitionMatches(
     int render_process_id,
     const GURL& url,
     const base::WeakPtr<URLRequestChromeJob>& job) {
   // The embedder could put some webui pages in separate storage partition.
   // RenderProcessHostImpl::IsSuitableHost would guard against top level pages
   // being in the same process. We do an extra check to guard against an
   // exploited renderer pretending to add them as a subframe. We skip this check
   // for resources.
   bool allowed = false;
-  std::vector<std::string> hosts;
-  GetContentClient()->
-      browser()->GetAdditionalWebUIHostsToIgnoreParititionCheck(&hosts);
-  if (url.SchemeIs(kChromeUIScheme) &&
-      (url.SchemeIs(kChromeUIScheme) ||
-       std::find(hosts.begin(), hosts.end(), url.host()) != hosts.end())) {
-    allowed = true;
-  } else if (render_process_id == kNoRenderProcessId) {
-    // Request was not issued by renderer.
-    allowed = true;
-  } else {
-    RenderProcessHost* process = RenderProcessHost::FromID(render_process_id);
-    if (process) {
-      StoragePartition* partition = BrowserContext::GetStoragePartitionForSite(
-          process->GetBrowserContext(), url);
-      allowed = partition == process->GetStoragePartition();
-    }
+  RenderProcessHost* process = RenderProcessHost::FromID(render_process_id);
+  if (process) {
+    StoragePartition* partition = BrowserContext::GetStoragePartitionForSite(
+        process->GetBrowserContext(), url);
+    allowed = partition == process->GetStoragePartition();
   }
-
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
       base::Bind(&URLRequestChromeJob::StartAsync, job, allowed));
 }
 
 void URLRequestChromeJob::StartAsync(bool allowed) {
   if (!request_)
     return;
 
@@ skipping 368 matching lines @@
 
 }  // namespace
 
 net::URLRequestJobFactory::ProtocolHandler*
 CreateDevToolsProtocolHandler(content::ResourceContext* resource_context,
                               bool is_incognito) {
   return new DevToolsJobFactory(resource_context, is_incognito);
 }
 
 }  // namespace content