Create a URLRequestJobFactory to replace the URLRequest globals.

URLRequest::Interceptor and URLRequest::ProtocolFactory are globally registered. This causes a variety of problems. This provides a method for replacing them.

It used to be the case that we used net::URLRequest::IsHandledProtocol()/net::URLRequest::IsHandledURL() to see if the request would be handled by Chrome, or deferred to an external protocol handler. This required that URLRequest be aware of all protocol handlers.

We instead provide ProfileIOData::IsHandledProtocol(), which checks to see if there are any Chrome registered protocol handlers, and if not, checks the default ones in net::URLRequest.

Note this doesn't work for custom handlers (registerProtocolHandler) because they are dynamic and profile-specific. We would have to add a member function to ProfileIOData and not use a global. This is problematic since we check ProfileIOData::IsHandledProtocol in the RenderViewContextMenu, which runs on the UI thread, whereas ProfileIOData lives on the IO thread. RenderViewContextMenu is using also using it synchronously, which makes it a pain to support.

So, we don't support custom handlers in ProfileIOData::IsHandledProtocol(). This means that "save as" won't work for custom handlers. Seems ok for now.

This also fixes the multiprofile/incognito bugs where if a profile registers a custom handler, and then a different profile / an incognito profile registers the same custom handler and then unregisters it, which globally unregisters it, so the original profile is now broken. By removing the use of the globals, we fix this.

Also fixes a bunch of style guide violations in the ProtocolHandler/ProtocolHandlerRegistry code.

This changelist replaces two existing URLRequest::ProtocolFactory uses: chrome-extension/user-script and custom handlers.

Also improve the tests in ResourceDispatcherHost so we don't have to do as many NULL checks. Change the MockResourceContext to create a TestURLRequestContext.

BUG=81979
TEST=none


Review URL: http://codereview.chromium.org/6960006

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@85376 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 77 insertions, 86 deletions

diff --git a/chrome/browser/custom_handlers/protocol_handler.cc b/chrome/browser/custom_handlers/protocol_handler.cc
index e9ed3cb..1b2bec1 100644
--- a/chrome/browser/custom_handlers/protocol_handler.cc
+++ b/chrome/browser/custom_handlers/protocol_handler.cc
@@ -11,34 +11,36 @@
 ProtocolHandler::ProtocolHandler(const std::string& protocol,
                                  const GURL& url,
                                  const string16& title)
-  : protocol_(protocol),
-    url_(url),
-    title_(title) {
+    : protocol_(protocol),
+      url_(url),
+      title_(title) {
 }
 
 ProtocolHandler ProtocolHandler::CreateProtocolHandler(
     const std::string& protocol,
     const GURL& url,
     const string16& title) {
-  std::string lower_protocol(protocol);
-  lower_protocol = StringToLowerASCII(protocol);
+  std::string lower_protocol = StringToLowerASCII(protocol);
   return ProtocolHandler(lower_protocol, url, title);
 }
 
 ProtocolHandler::ProtocolHandler() {
 }
 
-const ProtocolHandler ProtocolHandler::kEmpty;
-
 bool ProtocolHandler::IsValidDict(const DictionaryValue* value) {
   return value->HasKey("protocol") && value->HasKey("url") &&
     value->HasKey("title");
 }
 
+const ProtocolHandler& ProtocolHandler::EmptyProtocolHandler() {
+  static const ProtocolHandler* const kEmpty = new ProtocolHandler();
+  return *kEmpty;
+}
+
 ProtocolHandler ProtocolHandler::CreateProtocolHandler(
     const DictionaryValue* value) {
   if (!IsValidDict(value)) {
-    return kEmpty;
+    return EmptyProtocolHandler();
   }
   std::string protocol, url;
   string16 title;
@@ -48,14 +50,14 @@ ProtocolHandler ProtocolHandler::CreateProtocolHandler(
   return ProtocolHandler::CreateProtocolHandler(protocol, GURL(url), title);
 }
 
-GURL ProtocolHandler::TranslateUrl(const GURL& url) {
+GURL ProtocolHandler::TranslateUrl(const GURL& url) const {
   std::string translatedUrlSpec(url_.spec());
   ReplaceSubstringsAfterOffset(&translatedUrlSpec, 0, "%s",
       EscapeQueryParamValue(url.spec(), true));
   return GURL(translatedUrlSpec);
 }
 
-DictionaryValue* ProtocolHandler::Encode() {
+DictionaryValue* ProtocolHandler::Encode() const {
   DictionaryValue* d = new DictionaryValue();
   d->Set("protocol", Value::CreateStringValue(protocol_));
   d->Set("url", Value::CreateStringValue(url_.spec()));
@@ -63,9 +65,8 @@ DictionaryValue* ProtocolHandler::Encode() {
   return d;
 }
 
-bool ProtocolHandler::operator==(const ProtocolHandler &other) const {
+bool ProtocolHandler::operator==(const ProtocolHandler& other) const {
   return protocol_ == other.protocol_ &&
     url_ == other.url_ &&
     title_ == other.title_;
 }
-
diff --git a/chrome/browser/custom_handlers/protocol_handler.h b/chrome/browser/custom_handlers/protocol_handler.h
index 36387b5..e145222 100644
--- a/chrome/browser/custom_handlers/protocol_handler.h
+++ b/chrome/browser/custom_handlers/protocol_handler.h
@@ -29,23 +29,21 @@ class ProtocolHandler {
   static bool IsValidDict(const DictionaryValue* value);
 
   // Canonical empty ProtocolHandler.
-  static const ProtocolHandler kEmpty;
-
-  explicit ProtocolHandler();
+  static const ProtocolHandler& EmptyProtocolHandler();
 
   // Interpolates the given URL into the URL template of this handler.
-  GURL TranslateUrl(const GURL& url);
+  GURL TranslateUrl(const GURL& url) const;
 
   // Encodes this protocol handler as a DictionaryValue. The caller is
   // responsible for deleting the returned value.
-  DictionaryValue* Encode();
+  DictionaryValue* Encode() const;
 
-  std::string protocol() const { return protocol_; }
-  GURL url() const { return url_;}
-  string16 title() const { return title_; }
+  const std::string& protocol() const { return protocol_; }
+  const GURL& url() const { return url_;}
+  const string16& title() const { return title_; }
 
   bool IsEmpty() const {
-    return protocol_ == "";
+    return protocol_.empty();
   }
 
   bool operator==(const ProtocolHandler &other) const;
@@ -54,10 +52,11 @@ class ProtocolHandler {
   ProtocolHandler(const std::string& protocol,
                   const GURL& url,
                   const string16& title);
+  ProtocolHandler();
+
   std::string protocol_;
   GURL url_;
   string16 title_;
 };
 
 #endif  // CHROME_BROWSER_CUSTOM_HANDLERS_PROTOCOL_HANDLER_H_
-
diff --git a/chrome/browser/custom_handlers/protocol_handler_registry.cc b/chrome/browser/custom_handlers/protocol_handler_registry.cc
index 7edb280..b4991e2 100644
--- a/chrome/browser/custom_handlers/protocol_handler_registry.cc
+++ b/chrome/browser/custom_handlers/protocol_handler_registry.cc
@@ -4,11 +4,14 @@
 
 #include "chrome/browser/custom_handlers/protocol_handler_registry.h"
 
+#include <utility>
+
 #include "base/memory/scoped_ptr.h"
 #include "chrome/browser/custom_handlers/protocol_handler.h"
 #include "chrome/browser/custom_handlers/register_protocol_handler_infobar_delegate.h"
 #include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/profiles/profile_io_data.h"
 #include "chrome/common/pref_names.h"
 #include "content/browser/child_process_security_policy.h"
 #include "net/base/network_delegate.h"
@@ -19,16 +22,16 @@
 
 ProtocolHandlerRegistry::ProtocolHandlerRegistry(Profile* profile,
     Delegate* delegate)
-  : profile_(profile),
-    delegate_(delegate),
-    enabled_(true) {
+    : profile_(profile),
+      delegate_(delegate),
+      enabled_(true) {
 }
 
 ProtocolHandlerRegistry::~ProtocolHandlerRegistry() {
 }
 
-ProtocolHandlerList& ProtocolHandlerRegistry::GetHandlerListFor(
-    const std::string& scheme) {
+ProtocolHandlerRegistry::ProtocolHandlerList&
+ProtocolHandlerRegistry::GetHandlerListFor(const std::string& scheme) {
   ProtocolHandlerMultiMap::iterator p = protocol_handlers_.find(scheme);
   if (p == protocol_handlers_.end()) {
     protocol_handlers_[scheme] = ProtocolHandlerList();
@@ -60,7 +63,7 @@ void ProtocolHandlerRegistry::Enable() {
   }
   enabled_ = true;
   for (ProtocolHandlerMultiMap::const_iterator p = protocol_handlers_.begin();
-      p != protocol_handlers_.end(); p++) {
+       p != protocol_handlers_.end(); ++p) {
     delegate_->RegisterExternalHandler(p->first);
   }
 }
@@ -71,13 +74,13 @@ void ProtocolHandlerRegistry::Disable() {
   }
   enabled_ = false;
   for (ProtocolHandlerMultiMap::const_iterator p = protocol_handlers_.begin();
-      p != protocol_handlers_.end(); p++) {
+       p != protocol_handlers_.end(); ++p) {
     delegate_->DeregisterExternalHandler(p->first);
   }
 }
 
 std::vector<const DictionaryValue*>
-ProtocolHandlerRegistry::GetHandlersFromPref(const char* pref_name) {
+ProtocolHandlerRegistry::GetHandlersFromPref(const char* pref_name) const {
   std::vector<const DictionaryValue*> result;
   PrefService* prefs = profile_->GetPrefs();
   if (!prefs->HasPrefPath(pref_name)) {
@@ -86,7 +89,7 @@ ProtocolHandlerRegistry::GetHandlersFromPref(const char* pref_name) {
 
   const ListValue* handlers = prefs->GetList(pref_name);
   if (handlers) {
-    for (size_t i = 0; i < handlers->GetSize(); i++) {
+    for (size_t i = 0; i < handlers->GetSize(); ++i) {
       DictionaryValue* dict;
       handlers->GetDictionary(i, &dict);
       if (ProtocolHandler::IsValidDict(dict)) {
@@ -107,7 +110,7 @@ void ProtocolHandlerRegistry::Load() {
     GetHandlersFromPref(prefs::kRegisteredProtocolHandlers);
   for (std::vector<const DictionaryValue*>::const_iterator p =
       registered_handlers.begin();
-      p != registered_handlers.end(); p++) {
+      p != registered_handlers.end(); ++p) {
     ProtocolHandler handler = ProtocolHandler::CreateProtocolHandler(*p);
     RegisterProtocolHandler(handler);
     bool is_default = false;
@@ -118,8 +121,8 @@ void ProtocolHandlerRegistry::Load() {
   std::vector<const DictionaryValue*> ignored_handlers =
     GetHandlersFromPref(prefs::kIgnoredProtocolHandlers);
   for (std::vector<const DictionaryValue*>::const_iterator p =
-      ignored_handlers.begin();
-      p != ignored_handlers.end(); p++) {
+       ignored_handlers.begin();
+       p != ignored_handlers.end(); ++p) {
     IgnoreProtocolHandler(ProtocolHandler::CreateProtocolHandler(*p));
   }
   is_loading_ = false;
@@ -157,7 +160,7 @@ bool ProtocolHandlerRegistry::CanSchemeBeOverridden(
 void ProtocolHandlerRegistry::GetHandledProtocols(
     std::vector<std::string>* output) const {
   ProtocolHandlerMultiMap::const_iterator p;
-  for (p = protocol_handlers_.begin(); p != protocol_handlers_.end(); p++) {
+  for (p = protocol_handlers_.begin(); p != protocol_handlers_.end(); ++p) {
     output->push_back(p->first);
   }
 }
@@ -165,7 +168,7 @@ void ProtocolHandlerRegistry::GetHandledProtocols(
 void ProtocolHandlerRegistry::RemoveIgnoredHandler(
     const ProtocolHandler& handler) {
   for (ProtocolHandlerList::iterator p = ignored_protocol_handlers_.begin();
-      p != ignored_protocol_handlers_.end(); p++) {
+       p != ignored_protocol_handlers_.end(); ++p) {
     if (handler == *p) {
       ignored_protocol_handlers_.erase(p);
       break;
@@ -181,7 +184,7 @@ bool ProtocolHandlerRegistry::IsRegistered(const ProtocolHandler& handler) {
 bool ProtocolHandlerRegistry::IsIgnored(const ProtocolHandler& handler) const {
   ProtocolHandlerList::const_iterator i;
   for (i = ignored_protocol_handlers_.begin();
-      i != ignored_protocol_handlers_.end(); i++) {
+      i != ignored_protocol_handlers_.end(); ++i) {
     if (*i == handler) {
       return true;
     }
@@ -202,17 +205,12 @@ void ProtocolHandlerRegistry::RemoveHandler(const ProtocolHandler& handler) {
     handlers.erase(p);
   }
 
-  if (default_handlers_[handler.protocol()] == handler) {
-    default_handlers_.erase(handler.protocol());
+  ProtocolHandlerMap::iterator it = default_handlers_.find(handler.protocol());
+  if (it != default_handlers_.end() && it->second == handler) {
+    default_handlers_.erase(it);
   }
 }
 
-net::URLRequestJob* ProtocolHandlerRegistry::Factory(net::URLRequest* request,
-    const std::string& scheme) {
-  return request->context()->network_delegate()->MaybeCreateURLRequestJob(
-      request);
-}
-
 net::URLRequestJob* ProtocolHandlerRegistry::MaybeCreateJob(
     net::URLRequest* request) const {
   ProtocolHandler handler = GetHandlerFor(request->url().scheme());
@@ -234,9 +232,9 @@ Value* ProtocolHandlerRegistry::EncodeRegisteredHandlers() {
   ListValue* protocol_handlers = new ListValue();
 
   for (ProtocolHandlerMultiMap::iterator i = protocol_handlers_.begin();
-      i != protocol_handlers_.end(); ++i) {
+       i != protocol_handlers_.end(); ++i) {
     for (ProtocolHandlerList::iterator j = i->second.begin();
-        j != i->second.end(); j++) {
+         j != i->second.end(); ++j) {
       DictionaryValue* encoded = j->Encode();
       if (IsDefault(*j)) {
         encoded->Set("default", Value::CreateBooleanValue(true));
@@ -251,7 +249,7 @@ Value* ProtocolHandlerRegistry::EncodeIgnoredHandlers() {
   ListValue* handlers = new ListValue();
 
   for (ProtocolHandlerList::iterator i = ignored_protocol_handlers_.begin();
-      i != ignored_protocol_handlers_.end(); ++i) {
+       i != ignored_protocol_handlers_.end(); ++i) {
     handlers->Append(i->Encode());
   }
   return handlers;
@@ -273,17 +271,18 @@ void ProtocolHandlerRegistry::OnIgnoreRegisterProtocolHandler(
   Save();
 }
 
-void ProtocolHandlerRegistry::RegisterPrefs(PrefService* prefService) {
-  prefService->RegisterListPref(prefs::kRegisteredProtocolHandlers,
-                                PrefService::UNSYNCABLE_PREF);
-  prefService->RegisterListPref(prefs::kIgnoredProtocolHandlers,
-                                PrefService::UNSYNCABLE_PREF);
-  prefService->RegisterBooleanPref(prefs::kCustomHandlersEnabled, true,
-                                   PrefService::UNSYNCABLE_PREF);
+void ProtocolHandlerRegistry::RegisterPrefs(PrefService* pref_service) {
+  pref_service->RegisterListPref(prefs::kRegisteredProtocolHandlers,
+                                 PrefService::UNSYNCABLE_PREF);
+  pref_service->RegisterListPref(prefs::kIgnoredProtocolHandlers,
+                                 PrefService::UNSYNCABLE_PREF);
+  pref_service->RegisterBooleanPref(prefs::kCustomHandlersEnabled, true,
+                                    PrefService::UNSYNCABLE_PREF);
 }
 
 void ProtocolHandlerRegistry::SetDefault(const ProtocolHandler& handler) {
-  default_handlers_[handler.protocol()] = handler;
+  default_handlers_.erase(handler.protocol());
+  default_handlers_.insert(std::make_pair(handler.protocol(), handler));
   Save();
 }
 
@@ -307,7 +306,7 @@ const ProtocolHandler& ProtocolHandlerRegistry::GetHandlerFor(
   if (q != protocol_handlers_.end() && q->second.size() == 1) {
     return q->second[0];
   }
-  return ProtocolHandler::kEmpty;
+  return ProtocolHandler::EmptyProtocolHandler();
 }
 
 bool ProtocolHandlerRegistry::HasDefault(
@@ -331,16 +330,13 @@ void ProtocolHandlerRegistry::Delegate::RegisterExternalHandler(
   if (!policy->IsWebSafeScheme(protocol)) {
     policy->RegisterWebSafeScheme(protocol);
   }
-  net::URLRequest::RegisterProtocolFactory(protocol,
-      &ProtocolHandlerRegistry::Factory);
 }
 
 void ProtocolHandlerRegistry::Delegate::DeregisterExternalHandler(
     const std::string& protocol) {
-  net::URLRequest::RegisterProtocolFactory(protocol, NULL);
 }
 
 bool ProtocolHandlerRegistry::Delegate::IsExternalHandlerRegistered(
     const std::string& protocol) {
-  return net::URLRequest::IsHandledProtocol(protocol);
+  return ProfileIOData::IsHandledProtocol(protocol);
 }
diff --git a/chrome/browser/custom_handlers/protocol_handler_registry.h b/chrome/browser/custom_handlers/protocol_handler_registry.h
index 8ce29eed..eaeb0c6 100644
--- a/chrome/browser/custom_handlers/protocol_handler_registry.h
+++ b/chrome/browser/custom_handlers/protocol_handler_registry.h
@@ -18,10 +18,6 @@
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_job.h"
 
-typedef std::map<std::string, ProtocolHandler> ProtocolHandlerMap;
-typedef std::vector<ProtocolHandler> ProtocolHandlerList;
-typedef std::map<std::string, ProtocolHandlerList> ProtocolHandlerMultiMap;
-
 // This is where handlers for protocols registered with
 // navigator.registerProtocolHandler() are registered. Each Profile owns an
 // instance of this class, which is initialized on browser start through
@@ -31,7 +27,16 @@ typedef std::map<std::string, ProtocolHandlerList> ProtocolHandlerMultiMap;
 class ProtocolHandlerRegistry
     : public base::RefCountedThreadSafe<ProtocolHandlerRegistry> {
  public:
-  class Delegate;
+  // TODO(koz): Refactor this to eliminate the unnecessary virtuals. All that
+  // should be needed is a way to ensure that the list of websafe protocols is
+  // updated.
+  class Delegate {
+   public:
+    virtual ~Delegate();
+    virtual void RegisterExternalHandler(const std::string& protocol);
+    virtual void DeregisterExternalHandler(const std::string& protocol);
+    virtual bool IsExternalHandlerRegistered(const std::string& protocol);
+  };
 
   ProtocolHandlerRegistry(Profile* profile, Delegate* delegate);
   ~ProtocolHandlerRegistry();
@@ -94,11 +99,6 @@ class ProtocolHandlerRegistry
   // Causes the given protocol handler to not be ignored anymore.
   void RemoveIgnoredHandler(const ProtocolHandler& handler);
 
-  // URLRequestFactory for use with URLRequest::RegisterProtocolFactory().
-  // Redirects any URLRequests for which there is a matching protocol handler.
-  static net::URLRequestJob* Factory(net::URLRequest* request,
-                                     const std::string& scheme);
-
   // Registers the preferences that we store registered protocol handlers in.
   static void RegisterPrefs(PrefService* prefService);
 
@@ -114,20 +114,15 @@ class ProtocolHandlerRegistry
   // will not handle requests.
   void Disable();
 
-  bool enabled() { return enabled_; }
-
-  class Delegate {
-   public:
-    virtual ~Delegate();
-    virtual void RegisterExternalHandler(const std::string& protocol);
-    virtual void DeregisterExternalHandler(const std::string& protocol);
-    virtual bool IsExternalHandlerRegistered(const std::string& protocol);
-  };
+  bool enabled() const { return enabled_; }
 
  private:
-
   friend class base::RefCountedThreadSafe<ProtocolHandlerRegistry>;
 
+  typedef std::map<std::string, ProtocolHandler> ProtocolHandlerMap;
+  typedef std::vector<ProtocolHandler> ProtocolHandlerList;
+  typedef std::map<std::string, ProtocolHandlerList> ProtocolHandlerMultiMap;
+
   // Returns a JSON list of protocol handlers. The caller is responsible for
   // deleting this Value.
   Value* EncodeRegisteredHandlers();
@@ -142,7 +137,7 @@ class ProtocolHandlerRegistry
   // Get the DictionaryValues stored under the given pref name that are valid
   // ProtocolHandler values.
   std::vector<const DictionaryValue*> GetHandlersFromPref(
-      const char* pref_name);
+      const char* pref_name) const;
 
   // Ignores future requests to register the given protocol handler.
   void IgnoreProtocolHandler(const ProtocolHandler& handler);
@@ -180,4 +175,3 @@ class ProtocolHandlerRegistry
   DISALLOW_COPY_AND_ASSIGN(ProtocolHandlerRegistry);
 };
 #endif  // CHROME_BROWSER_CUSTOM_HANDLERS_PROTOCOL_HANDLER_REGISTRY_H_
-
diff --git a/chrome/browser/custom_handlers/protocol_handler_registry_unittest.cc b/chrome/browser/custom_handlers/protocol_handler_registry_unittest.cc
index 982d451..5613be7 100644
--- a/chrome/browser/custom_handlers/protocol_handler_registry_unittest.cc
+++ b/chrome/browser/custom_handlers/protocol_handler_registry_unittest.cc
@@ -42,7 +42,10 @@ class FakeDelegate : public ProtocolHandlerRegistry::Delegate {
 
 
 class ProtocolHandlerRegistryTest : public testing::Test {
- public:
+ protected:
+  ProtocolHandlerRegistryTest()
+      : test_protocol_handler_(CreateProtocolHandler("test", "test")) {}
+
   FakeDelegate* delegate() const { return delegate_; }
   TestingProfile* profile() const { return profile_.get(); }
   PrefService* pref_service() const { return profile_->GetPrefs(); }
@@ -70,9 +73,7 @@ class ProtocolHandlerRegistryTest : public testing::Test {
     registry_->Load();
   }
 
- private:
   virtual void SetUp() {
-    test_protocol_handler_ = CreateProtocolHandler("test", "test");
     profile_.reset(new TestingProfile());
     profile_->SetPrefService(new TestingPrefService());
     delegate_ = new FakeDelegate();