Offload disk accesses to WorkerPool in ExtensionProtocolHandler

chrome/browser/extensions/extension_protocols.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 "chrome/browser/extensions/extension_protocols.h"
 
 #include <algorithm>
 
 #include "base/compiler_specific.h"
 #include "base/file_path.h"
 #include "base/logging.h"
+#include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
+#include "base/threading/worker_pool.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/extension_info_map.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_file_util.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "chrome/common/url_constants.h"
 #include "content/public/browser/resource_request_info.h"
 #include "googleurl/src/url_util.h"
@@ skipping 24 matching lines @@
   }
 
   if (send_cors_header) {
     raw_headers.append(1, '\0');
     raw_headers.append("Access-Control-Allow-Origin: *");
   }
   raw_headers.append(2, '\0');
   return new net::HttpResponseHeaders(raw_headers);
 }
 
+void ReadMimeTypeFromFile(const FilePath& filename,
+                          std::string* mime_type,
+                          bool* result) {
+  *result = net::GetMimeTypeFromFile(filename, mime_type);
+}
+
 class URLRequestResourceBundleJob : public net::URLRequestSimpleJob {
  public:
   URLRequestResourceBundleJob(
       net::URLRequest* request, const FilePath& filename, int resource_id,
       const std::string& content_security_policy, bool send_cors_header)
       : net::URLRequestSimpleJob(request),
         filename_(filename),
-        resource_id_(resource_id) {
+        resource_id_(resource_id),
+        weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
     response_info_.headers = BuildHttpHeaders(content_security_policy,
                                               send_cors_header);
   }
 
   // Overridden from URLRequestSimpleJob:
-  virtual bool GetData(std::string* mime_type,
-                       std::string* charset,
-                       std::string* data) const OVERRIDE {
+  virtual int GetData(std::string* mime_type,
+                      std::string* charset,
+                      std::string* data,
+                      const net::CompletionCallback& callback) const OVERRIDE {
     const ResourceBundle& rb = ResourceBundle::GetSharedInstance();
     *data = rb.GetRawDataResource(
         resource_id_, ui::SCALE_FACTOR_NONE).as_string();
 
-    // Requests should not block on the disk!  On Windows this goes to the
-    // registry.
-    //   http://code.google.com/p/chromium/issues/detail?id=59849
-    bool result;
-    {
-      base::ThreadRestrictions::ScopedAllowIO allow_io;
-      result = net::GetMimeTypeFromFile(filename_, mime_type);
-    }
+    std::string* read_mime_type = new std::string;
+    bool* read_result = new bool;
+    bool posted = base::WorkerPool::PostTaskAndReply(
+        FROM_HERE,
+        base::Bind(&ReadMimeTypeFromFile, filename_,
+                   base::Unretained(read_mime_type),
+                   base::Unretained(read_result)),
+        base::Bind(&URLRequestResourceBundleJob::OnMimeTypeRead,
+                   weak_factory_.GetWeakPtr(),
+                   mime_type, charset, data,
+                   base::Owned(read_mime_type),
+                   base::Owned(read_result),
+                   callback),
+        true /* task is slow */);
+    DCHECK(posted);
 
-    if (StartsWithASCII(*mime_type, "text/", false)) {
-      // All of our HTML files should be UTF-8 and for other resource types
-      // (like images), charset doesn't matter.
-      DCHECK(IsStringUTF8(*data));
-      *charset = "utf-8";
-    }
-    return result;
+    return net::ERR_IO_PENDING;
   }
 
   virtual void GetResponseInfo(net::HttpResponseInfo* info) {
     *info = response_info_;
   }
 
  private:
   virtual ~URLRequestResourceBundleJob() { }
 
+  void OnMimeTypeRead(std::string* out_mime_type,
+                      std::string* charset,
+                      std::string* data,
+                      std::string* read_mime_type,
+                      bool* read_result,
+                      const net::CompletionCallback& callback) {
+    *out_mime_type = *read_mime_type;
+    if (StartsWithASCII(*read_mime_type, "text/", false)) {
+      // All of our HTML files should be UTF-8 and for other resource types
+      // (like images), charset doesn't matter.
+      DCHECK(IsStringUTF8(*data));
+      *charset = "utf-8";
+    }
+    int result = *read_result? net::OK: net::ERR_INVALID_URL;

[wtc, 2012/07/13 22:55:14]

The use of net::ERR_INVALID_URL here was not obvious.  (I
know it was meant to match the error code used in the original
code.)  I would think that the most common reason for
net::GetMimeTypeFromFile to fail is ERR_FILE_NOT_FOUND.
I guess ERR_INVALID_URL can be interpreted to mean the file
URL is invalid?

[wtc, 2012/07/14 00:08:55]

Thank you for the explanation.  I didn't think about this
hard enough.  I saw net::GetMimeTypeFromFile takes a 'filename'
argument and just assumed the most common reason of failure is
"file not found".

I agree that ERR_INVALID_URL is appropriate here.

+    callback.Run(result);
+  }
+
   // We need the filename of the resource to determine the mime type.
   FilePath filename_;
 
   // The resource bundle id to load.
   int resource_id_;
 
   net::HttpResponseInfo response_info_;
+
+  mutable base::WeakPtrFactory<URLRequestResourceBundleJob> weak_factory_;
 };
 
 class GeneratedBackgroundPageJob : public net::URLRequestSimpleJob {
  public:
   GeneratedBackgroundPageJob(net::URLRequest* request,
                              const scoped_refptr<const Extension> extension,
                              const std::string& content_security_policy)
       : net::URLRequestSimpleJob(request),
         extension_(extension) {
     const bool send_cors_headers = false;
     response_info_.headers = BuildHttpHeaders(content_security_policy,
                                               send_cors_headers);
   }
 
   // Overridden from URLRequestSimpleJob:
-  virtual bool GetData(std::string* mime_type,
-                       std::string* charset,
-                       std::string* data) const OVERRIDE {
+  virtual int GetData(std::string* mime_type,
+                      std::string* charset,
+                      std::string* data,
+                      const net::CompletionCallback& callback) const OVERRIDE {
     *mime_type = "text/html";
     *charset = "utf-8";
 
     *data = "<!DOCTYPE html>\n<body>\n";
     for (size_t i = 0; i < extension_->background_scripts().size(); ++i) {
       *data += "<script src=\"";
       *data += extension_->background_scripts()[i];
       *data += "\"></script>\n";
     }
 
-    return true;
+    return net::OK;
   }
 
   virtual void GetResponseInfo(net::HttpResponseInfo* info) {
     *info = response_info_;
   }
 
  private:
   virtual ~GeneratedBackgroundPageJob() {}
 
   scoped_refptr<const Extension> extension_;
   net::HttpResponseInfo response_info_;
 };
 
+void ReadResourceFilePath(const ExtensionResource& resource,
+                          FilePath* file_path) {
+  *file_path = resource.GetFilePath();
+}
+
 class URLRequestExtensionJob : public net::URLRequestFileJob {
  public:
   URLRequestExtensionJob(net::URLRequest* request,
-                         const FilePath& filename,
+                         const std::string& extension_id,
+                         const FilePath& directory_path,
                          const std::string& content_security_policy,
                          bool send_cors_header)
-    : net::URLRequestFileJob(request, filename) {
+    : net::URLRequestFileJob(request, FilePath()),
+      // TODO(tc): Move all of these files into resources.pak so we don't break
+      // when updating on Linux.
+      resource_(extension_id, directory_path,
+                extension_file_util::ExtensionURLToRelativeFilePath(
+                    request->url())),
+      weak_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
       response_info_.headers = BuildHttpHeaders(content_security_policy,
                                                 send_cors_header);
   }
 
   virtual void GetResponseInfo(net::HttpResponseInfo* info) OVERRIDE {
     *info = response_info_;
   }
 
+  virtual void Start() OVERRIDE {
+    FilePath* read_file_path = new FilePath;
+    bool posted = base::WorkerPool::PostTaskAndReply(
+        FROM_HERE,
+        base::Bind(&ReadResourceFilePath, resource_,
+                   base::Unretained(read_file_path)),
+        base::Bind(&URLRequestExtensionJob::OnFilePathRead,
+                   weak_factory_.GetWeakPtr(),
+                   base::Owned(read_file_path)),
+        true /* task is slow */);
+    DCHECK(posted);
+  }
+
  private:
   virtual ~URLRequestExtensionJob() {}
 
+  void OnFilePathRead(FilePath* read_file_path) {
+    file_path_ = *read_file_path;
+    URLRequestFileJob::Start();
+  }
+
   net::HttpResponseInfo response_info_;
+  ExtensionResource resource_;
+  base::WeakPtrFactory<URLRequestExtensionJob> weak_factory_;
 };
 
 bool ExtensionCanLoadInIncognito(const ResourceRequestInfo* info,
                                  const std::string& extension_id,
                                  ExtensionInfoMap* extension_info_map) {
   if (!extension_info_map->IsIncognitoEnabled(extension_id))
     return false;
 
   // Only allow incognito toplevel navigations to extension resources in
   // split mode. In spanning mode, the extension must run in a single process,
@@ skipping 131 matching lines @@
       FilePath bm_resource_path =
           FilePath().AppendASCII(kComponentExtensionResources[i].name);
       bm_resource_path = bm_resource_path.NormalizePathSeparators();
       if (relative_path == bm_resource_path) {
         return new URLRequestResourceBundleJob(request, relative_path,
             kComponentExtensionResources[i].value, content_security_policy,
             send_cors_header);
       }
     }
   }
-  // TODO(tc): Move all of these files into resources.pak so we don't break
-  // when updating on Linux.
-  ExtensionResource resource(extension_id, directory_path,
-      extension_file_util::ExtensionURLToRelativeFilePath(request->url()));
 
-  FilePath resource_file_path;
-  {
-    // Getting the file path will touch the file system.  Fixing
-    // crbug.com/59849 would also fix this.  Suppress the error for now.
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-    resource_file_path = resource.GetFilePath();
-  }
-
-  return new URLRequestExtensionJob(request, resource_file_path,
+  return new URLRequestExtensionJob(request, extension_id, directory_path,
                                     content_security_policy, send_cors_header);
 }
 
 }  // namespace
 
 net::URLRequestJobFactory::ProtocolHandler* CreateExtensionProtocolHandler(
     bool is_incognito,
     ExtensionInfoMap* extension_info_map) {
   return new ExtensionProtocolHandler(is_incognito, extension_info_map);
 }