Revert 188912 "Removed static factories for data, ftp, file, and..."

Broke layout tests userscripts/user-script-plugin-document.html and plugins/plugin-document-back-forward.html on all platforms.

> Removed static factories for data, ftp, file, and about jobs.
> Instead add corresponding ProtocolHandlers as needed.
> Remove URLRequestContext members used by these static 
> factories.  Bake FtpAuthCache into FtpProtocolHandler as it
> was already unique per FtpProtocolHandler.
> This is a revived version of http://crrev.com/10836206
> 
> BUG=142945
> 
> 
> Review URL: https://chromiumcodereview.appspot.com/11931024

TBR=pauljensen@chromium.org
Review URL: https://codereview.chromium.org/12605011

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

29 files changed, 175 insertions, 149 deletions

diff --git a/net/url_request/file_protocol_handler.cc b/net/url_request/file_protocol_handler.cc
index c58c56b..13a772e 100644
--- a/net/url_request/file_protocol_handler.cc
+++ b/net/url_request/file_protocol_handler.cc
@@ -44,8 +44,4 @@ URLRequestJob* FileProtocolHandler::MaybeCreateJob(
   return new URLRequestFileJob(request, network_delegate, file_path);
 }
 
-bool FileProtocolHandler::IsSafeRedirectTarget(const GURL& location) const {
-  return false;
-}
-
 }  // namespace net
diff --git a/net/url_request/file_protocol_handler.h b/net/url_request/file_protocol_handler.h
index 8087a6e..619227b 100644
--- a/net/url_request/file_protocol_handler.h
+++ b/net/url_request/file_protocol_handler.h
@@ -9,8 +9,6 @@
 #include "base/compiler_specific.h"
 #include "net/url_request/url_request_job_factory.h"
 
-class GURL;
-
 namespace net {
 
 class NetworkDelegate;
@@ -24,7 +22,6 @@ class NET_EXPORT FileProtocolHandler :
   FileProtocolHandler();
   virtual URLRequestJob* MaybeCreateJob(
       URLRequest* request, NetworkDelegate* network_delegate) const OVERRIDE;
-  virtual bool IsSafeRedirectTarget(const GURL& location) const OVERRIDE;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(FileProtocolHandler);
diff --git a/net/url_request/ftp_protocol_handler.cc b/net/url_request/ftp_protocol_handler.cc
index 08071f3..7d9ba88 100644
--- a/net/url_request/ftp_protocol_handler.cc
+++ b/net/url_request/ftp_protocol_handler.cc
@@ -15,9 +15,12 @@
 namespace net {
 
 FtpProtocolHandler::FtpProtocolHandler(
-    FtpTransactionFactory* ftp_transaction_factory)
-    : ftp_transaction_factory_(ftp_transaction_factory) {
+    FtpTransactionFactory* ftp_transaction_factory,
+    FtpAuthCache* ftp_auth_cache)
+    : ftp_transaction_factory_(ftp_transaction_factory),
+      ftp_auth_cache_(ftp_auth_cache) {
   DCHECK(ftp_transaction_factory_);
+  DCHECK(ftp_auth_cache_);
 }
 
 URLRequestJob* FtpProtocolHandler::MaybeCreateJob(
@@ -31,7 +34,7 @@ URLRequestJob* FtpProtocolHandler::MaybeCreateJob(
   return new URLRequestFtpJob(request,
                               network_delegate,
                               ftp_transaction_factory_,
-                              &ftp_auth_cache_);
+                              ftp_auth_cache_);
 }
 
 }  // namespace net
diff --git a/net/url_request/ftp_protocol_handler.h b/net/url_request/ftp_protocol_handler.h
index 52aea7c..871f422 100644
--- a/net/url_request/ftp_protocol_handler.h
+++ b/net/url_request/ftp_protocol_handler.h
@@ -7,11 +7,11 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "net/ftp/ftp_auth_cache.h"
 #include "net/url_request/url_request_job_factory.h"
 
 namespace net {
 
+class FtpAuthCache;
 class FtpTransactionFactory;
 class NetworkDelegate;
 class URLRequestJob;
@@ -20,13 +20,14 @@ class URLRequestJob;
 class NET_EXPORT FtpProtocolHandler :
     public URLRequestJobFactory::ProtocolHandler {
  public:
-  explicit FtpProtocolHandler(FtpTransactionFactory* ftp_transaction_factory);
+  FtpProtocolHandler(FtpTransactionFactory* ftp_transaction_factory,
+                     FtpAuthCache* ftp_auth_cache);
   virtual URLRequestJob* MaybeCreateJob(
       URLRequest* request, NetworkDelegate* network_delegate) const OVERRIDE;
 
  private:
   FtpTransactionFactory* ftp_transaction_factory_;
-  mutable FtpAuthCache ftp_auth_cache_;
+  FtpAuthCache* ftp_auth_cache_;
 
   DISALLOW_COPY_AND_ASSIGN(FtpProtocolHandler);
 };
diff --git a/net/url_request/protocol_intercept_job_factory.cc b/net/url_request/protocol_intercept_job_factory.cc
index d0f92f4..f72fbf9 100644
--- a/net/url_request/protocol_intercept_job_factory.cc
+++ b/net/url_request/protocol_intercept_job_factory.cc
@@ -39,9 +39,4 @@ bool ProtocolInterceptJobFactory::IsHandledURL(const GURL& url) const {
   return job_factory_->IsHandledURL(url);
 }
 
-bool ProtocolInterceptJobFactory::IsSafeRedirectTarget(
-    const GURL& location) const {
-  return job_factory_->IsSafeRedirectTarget(location);
-}
-
 }  // namespace net
diff --git a/net/url_request/protocol_intercept_job_factory.h b/net/url_request/protocol_intercept_job_factory.h
index abc2fb8..7deeb30 100644
--- a/net/url_request/protocol_intercept_job_factory.h
+++ b/net/url_request/protocol_intercept_job_factory.h
@@ -22,8 +22,6 @@ class URLRequestJob;
 // given the option of creating a URLRequestJob for each potential URLRequest.
 // If |protocol_handler_| does not create a job (i.e. MaybeCreateJob() returns
 // NULL) the URLRequest is forwarded to the |job_factory_| to be handled there.
-// Only the MaybeCreateJob() member of |protocol_handler_| is called; the
-// IsSafeRedirectTarget() member is not used.
 class NET_EXPORT ProtocolInterceptJobFactory : public URLRequestJobFactory {
  public:
   ProtocolInterceptJobFactory(scoped_ptr<URLRequestJobFactory> job_factory,
@@ -37,7 +35,6 @@ class NET_EXPORT ProtocolInterceptJobFactory : public URLRequestJobFactory {
       NetworkDelegate* network_delegate) const OVERRIDE;
   virtual bool IsHandledProtocol(const std::string& scheme) const OVERRIDE;
   virtual bool IsHandledURL(const GURL& url) const OVERRIDE;
-  virtual bool IsSafeRedirectTarget(const GURL& location) const OVERRIDE;
 
  private:
   scoped_ptr<URLRequestJobFactory> job_factory_;
diff --git a/net/url_request/url_request_about_job.cc b/net/url_request/url_request_about_job.cc
index 04ad6d8..242a735 100644
--- a/net/url_request/url_request_about_job.cc
+++ b/net/url_request/url_request_about_job.cc
@@ -20,6 +20,13 @@ URLRequestAboutJob::URLRequestAboutJob(URLRequest* request,
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
 }
 
+// static
+URLRequestJob* URLRequestAboutJob::Factory(URLRequest* request,
+                                           NetworkDelegate* network_delegate,
+                                           const std::string& scheme) {
+  return new URLRequestAboutJob(request, network_delegate);
+}
+
 void URLRequestAboutJob::Start() {
   // Start reading asynchronously so that all error reporting and data
   // callbacks happen as they would for network requests.
diff --git a/net/url_request/url_request_about_job.h b/net/url_request/url_request_about_job.h
index 66a888a..ea2b83f 100644
--- a/net/url_request/url_request_about_job.h
+++ b/net/url_request/url_request_about_job.h
@@ -17,6 +17,8 @@ class NET_EXPORT URLRequestAboutJob : public URLRequestJob {
  public:
   URLRequestAboutJob(URLRequest* request, NetworkDelegate* network_delegate);
 
+  static URLRequest::ProtocolFactory Factory;
+
   // URLRequestJob:
   virtual void Start() OVERRIDE;
   virtual bool GetMimeType(std::string* mime_type) const OVERRIDE;
diff --git a/net/url_request/url_request_context.cc b/net/url_request/url_request_context.cc
index ae3a938..e88750b 100644
--- a/net/url_request/url_request_context.cc
+++ b/net/url_request/url_request_context.cc
@@ -10,6 +10,7 @@
 #include "base/string_util.h"
 #include "net/base/host_resolver.h"
 #include "net/cookies/cookie_store.h"
+#include "net/ftp/ftp_transaction_factory.h"
 #include "net/http/http_transaction_factory.h"
 #include "net/url_request/http_user_agent_settings.h"
 #include "net/url_request/url_request.h"
@@ -28,7 +29,11 @@ URLRequestContext::URLRequestContext()
       http_server_properties_(NULL),
       http_user_agent_settings_(NULL),
       transport_security_state_(NULL),
+#if !defined(DISABLE_FTP_SUPPORT)
+      ftp_auth_cache_(new FtpAuthCache),
+#endif
       http_transaction_factory_(NULL),
+      ftp_transaction_factory_(NULL),
       job_factory_(NULL),
       throttler_manager_(NULL),
       url_requests_(new std::set<const URLRequest*>) {
@@ -52,7 +57,9 @@ void URLRequestContext::CopyFrom(const URLRequestContext* other) {
   set_http_server_properties(other->http_server_properties_);
   set_cookie_store(other->cookie_store_);
   set_transport_security_state(other->transport_security_state_);
+  // FTPAuthCache is unique per context.
   set_http_transaction_factory(other->http_transaction_factory_);
+  set_ftp_transaction_factory(other->ftp_transaction_factory_);
   set_job_factory(other->job_factory_);
   set_throttler_manager(other->throttler_manager_);
   set_http_user_agent_settings(other->http_user_agent_settings_);
diff --git a/net/url_request/url_request_context.h b/net/url_request/url_request_context.h
index bb26897..f5dc635 100644
--- a/net/url_request/url_request_context.h
+++ b/net/url_request/url_request_context.h
@@ -19,6 +19,7 @@
 #include "base/threading/non_thread_safe.h"
 #include "net/base/net_export.h"
 #include "net/base/net_log.h"
+#include "net/ftp/ftp_auth_cache.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_server_properties.h"
 #include "net/http/transport_security_state.h"
@@ -29,6 +30,7 @@ namespace net {
 class CertVerifier;
 class CookieStore;
 class FraudulentCertificateReporter;
+class FtpTransactionFactory;
 class HostResolver;
 class HttpAuthHandlerFactory;
 class HttpTransactionFactory;
@@ -129,6 +131,14 @@ class NET_EXPORT URLRequestContext
     http_transaction_factory_ = factory;
   }
 
+  // Gets the ftp transaction factory for this context.
+  FtpTransactionFactory* ftp_transaction_factory() const {
+    return ftp_transaction_factory_;
+  }
+  void set_ftp_transaction_factory(FtpTransactionFactory* factory) {
+    ftp_transaction_factory_ = factory;
+  }
+
   void set_network_delegate(NetworkDelegate* network_delegate) {
     network_delegate_ = network_delegate;
   }
@@ -155,6 +165,15 @@ class NET_EXPORT URLRequestContext
     transport_security_state_ = state;
   }
 
+  // Gets the FTP authentication cache for this context.
+  FtpAuthCache* ftp_auth_cache() const {
+#if !defined(DISABLE_FTP_SUPPORT)
+    return ftp_auth_cache_.get();
+#else
+    return NULL;
+#endif
+  }
+
   // ---------------------------------------------------------------------------
   // Legacy accessors that delegate to http_user_agent_settings_.
   // TODO(pauljensen): Remove after all clients are updated to directly access
@@ -218,7 +237,11 @@ class NET_EXPORT URLRequestContext
   HttpUserAgentSettings* http_user_agent_settings_;
   scoped_refptr<CookieStore> cookie_store_;
   TransportSecurityState* transport_security_state_;
+#if !defined(DISABLE_FTP_SUPPORT)
+  scoped_ptr<FtpAuthCache> ftp_auth_cache_;
+#endif
   HttpTransactionFactory* http_transaction_factory_;
+  FtpTransactionFactory* ftp_transaction_factory_;
   const URLRequestJobFactory* job_factory_;
   URLRequestThrottlerManager* throttler_manager_;
 
diff --git a/net/url_request/url_request_context_builder.cc b/net/url_request/url_request_context_builder.cc
index 4fd9bd9..498e0ca 100644
--- a/net/url_request/url_request_context_builder.cc
+++ b/net/url_request/url_request_context_builder.cc
@@ -26,13 +26,9 @@
 #include "net/http/transport_security_state.h"
 #include "net/proxy/proxy_service.h"
 #include "net/ssl/ssl_config_service_defaults.h"
-#include "net/url_request/data_protocol_handler.h"
-#include "net/url_request/file_protocol_handler.h"
-#include "net/url_request/ftp_protocol_handler.h"
 #include "net/url_request/static_http_user_agent_settings.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_storage.h"
-#include "net/url_request/url_request_job_factory_impl.h"
 
 namespace net {
 
@@ -184,11 +180,7 @@ URLRequestContextBuilder::HttpNetworkSessionParams::~HttpNetworkSessionParams()
 {}
 
 URLRequestContextBuilder::URLRequestContextBuilder()
-    : data_enabled_(false),
-      file_enabled_(false),
-#if !defined(DISABLE_FTP_SUPPORT)
-      ftp_enabled_(false),
-#endif
+    : ftp_enabled_(false),
       http_cache_enabled_(true) {}
 URLRequestContextBuilder::~URLRequestContextBuilder() {}
 
@@ -213,6 +205,11 @@ URLRequestContext* URLRequestContextBuilder::Build() {
 
   storage->set_host_resolver(net::HostResolver::CreateDefaultResolver(NULL));
 
+  if (ftp_enabled_) {
+    storage->set_ftp_transaction_factory(
+        new FtpNetworkLayer(context->host_resolver()));
+  }
+
   context->StartFileThread();
 
   // TODO(willchan): Switch to using this code when
@@ -293,21 +290,6 @@ URLRequestContext* URLRequestContextBuilder::Build() {
   }
   storage->set_http_transaction_factory(http_transaction_factory);
 
-  URLRequestJobFactoryImpl* job_factory = new URLRequestJobFactoryImpl;
-  if (data_enabled_)
-    job_factory->SetProtocolHandler("data", new DataProtocolHandler);
-  if (file_enabled_)
-    job_factory->SetProtocolHandler("file", new FileProtocolHandler);
-#if !defined(DISABLE_FTP_SUPPORT)
-  if (ftp_enabled_) {
-    ftp_transaction_factory_.reset(
-        new FtpNetworkLayer(context->host_resolver()));
-    job_factory->SetProtocolHandler("ftp",
-        new FtpProtocolHandler(ftp_transaction_factory_.get()));
-  }
-#endif
-  storage->set_job_factory(job_factory);
-
   // TODO(willchan): Support sdch.
 
   return context;
diff --git a/net/url_request/url_request_context_builder.h b/net/url_request/url_request_context_builder.h
index 20fc30e..2163ed5 100644
--- a/net/url_request/url_request_context_builder.h
+++ b/net/url_request/url_request_context_builder.h
@@ -25,7 +25,6 @@
 
 namespace net {
 
-class FtpTransactionFactory;
 class HostMappingRules;
 class ProxyConfigService;
 class URLRequestContext;
@@ -83,22 +82,10 @@ class NET_EXPORT URLRequestContextBuilder {
     user_agent_ = user_agent;
   }
 
-  // Control support for data:// requests. By default it's disabled.
-  void set_data_enabled(bool enable) {
-    data_enabled_ = enable;
-  }
-
-  // Control support for file:// requests. By default it's disabled.
-  void set_file_enabled(bool enable) {
-    file_enabled_ = enable;
-  }
-
-#if !defined(DISABLE_FTP_SUPPORT)
-  // Control support for ftp:// requests. By default it's disabled.
+  // By default it's disabled.
   void set_ftp_enabled(bool enable) {
     ftp_enabled_ = enable;
   }
-#endif
 
   // Uses BasicNetworkDelegate by default. Note that calling Build will unset
   // any custom delegate in builder, so this must be called each time before
@@ -127,14 +114,7 @@ class NET_EXPORT URLRequestContextBuilder {
  private:
   std::string accept_language_;
   std::string user_agent_;
-  // Include support for data:// requests.
-  bool data_enabled_;
-  // Include support for file:// requests.
-  bool file_enabled_;
-#if !defined(DISABLE_FTP_SUPPORT)
-  // Include support for ftp:// requests.
   bool ftp_enabled_;
-#endif
   bool http_cache_enabled_;
   HttpCacheParams http_cache_params_;
   HttpNetworkSessionParams http_network_session_params_;
@@ -142,7 +122,6 @@ class NET_EXPORT URLRequestContextBuilder {
   scoped_ptr<ProxyConfigService> proxy_config_service_;
 #endif  // defined(OS_LINUX) || defined(OS_ANDROID)
   scoped_ptr<NetworkDelegate> network_delegate_;
-  scoped_ptr<FtpTransactionFactory> ftp_transaction_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(URLRequestContextBuilder);
 };
diff --git a/net/url_request/url_request_context_storage.cc b/net/url_request/url_request_context_storage.cc
index 7104ab8..e201b8e 100644
--- a/net/url_request/url_request_context_storage.cc
+++ b/net/url_request/url_request_context_storage.cc
@@ -106,6 +106,12 @@ void URLRequestContextStorage::set_http_transaction_factory(
   http_transaction_factory_.reset(http_transaction_factory);
 }
 
+void URLRequestContextStorage::set_ftp_transaction_factory(
+    FtpTransactionFactory* ftp_transaction_factory) {
+  context_->set_ftp_transaction_factory(ftp_transaction_factory);
+  ftp_transaction_factory_.reset(ftp_transaction_factory);
+}
+
 void URLRequestContextStorage::set_job_factory(
     URLRequestJobFactory* job_factory) {
   context_->set_job_factory(job_factory);
diff --git a/net/url_request/url_request_context_storage.h b/net/url_request/url_request_context_storage.h
index 0a9d066..e08b3c8 100644
--- a/net/url_request/url_request_context_storage.h
+++ b/net/url_request/url_request_context_storage.h
@@ -62,6 +62,8 @@ class NET_EXPORT URLRequestContextStorage {
       TransportSecurityState* transport_security_state);
   void set_http_transaction_factory(
       HttpTransactionFactory* http_transaction_factory);
+  void set_ftp_transaction_factory(
+      FtpTransactionFactory* ftp_transaction_factory);
   void set_job_factory(URLRequestJobFactory* job_factory);
   void set_throttler_manager(URLRequestThrottlerManager* throttler_manager);
   void set_http_user_agent_settings(
@@ -91,6 +93,7 @@ class NET_EXPORT URLRequestContextStorage {
   scoped_ptr<TransportSecurityState> transport_security_state_;
 
   scoped_ptr<HttpTransactionFactory> http_transaction_factory_;
+  scoped_ptr<FtpTransactionFactory> ftp_transaction_factory_;
   scoped_ptr<URLRequestJobFactory> job_factory_;
   scoped_ptr<URLRequestThrottlerManager> throttler_manager_;
 
diff --git a/net/url_request/url_request_data_job.cc b/net/url_request/url_request_data_job.cc
index fd248c7..5dffe32 100644
--- a/net/url_request/url_request_data_job.cc
+++ b/net/url_request/url_request_data_job.cc
@@ -16,6 +16,13 @@ URLRequestDataJob::URLRequestDataJob(
     : URLRequestSimpleJob(request, network_delegate) {
 }
 
+// static
+URLRequestJob* URLRequestDataJob::Factory(URLRequest* request,
+                                          NetworkDelegate* network_delegate,
+                                          const std::string& scheme) {
+  return new URLRequestDataJob(request, network_delegate);
+}
+
 int URLRequestDataJob::GetData(std::string* mime_type,
                                std::string* charset,
                                std::string* data,
diff --git a/net/url_request/url_request_data_job.h b/net/url_request/url_request_data_job.h
index d9b2621..7ab6561 100644
--- a/net/url_request/url_request_data_job.h
+++ b/net/url_request/url_request_data_job.h
@@ -18,6 +18,8 @@ class URLRequestDataJob : public URLRequestSimpleJob {
  public:
   URLRequestDataJob(URLRequest* request, NetworkDelegate* network_delegate);
 
+  static URLRequest::ProtocolFactory Factory;
+
   // URLRequestSimpleJob
   virtual int GetData(std::string* mime_type,
                       std::string* charset,
diff --git a/net/url_request/url_request_file_job.cc b/net/url_request/url_request_file_job.cc
index 28dbcd5..6b2798b 100644
--- a/net/url_request/url_request_file_job.cc
+++ b/net/url_request/url_request_file_job.cc
@@ -62,6 +62,34 @@ URLRequestFileJob::URLRequestFileJob(URLRequest* request,
       weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
 
+// static
+URLRequestJob* URLRequestFileJob::Factory(URLRequest* request,
+                                          NetworkDelegate* network_delegate,
+                                          const std::string& scheme) {
+  base::FilePath file_path;
+  const bool is_file = FileURLToFilePath(request->url(), &file_path);
+
+  // Check file access permissions.
+  if (!network_delegate ||
+      !network_delegate->CanAccessFile(*request, file_path)) {
+    return new URLRequestErrorJob(request, network_delegate, ERR_ACCESS_DENIED);
+  }
+  // We need to decide whether to create URLRequestFileJob for file access or
+  // URLRequestFileDirJob for directory access. To avoid accessing the
+  // filesystem, we only look at the path string here.
+  // The code in the URLRequestFileJob::Start() method discovers that a path,
+  // which doesn't end with a slash, should really be treated as a directory,
+  // and it then redirects to the URLRequestFileDirJob.
+  if (is_file &&
+      file_util::EndsWithSeparator(file_path) &&
+      file_path.IsAbsolute())
+    return new URLRequestFileDirJob(request, network_delegate, file_path);
+
+  // Use a regular file request job for all non-directories (including invalid
+  // file names).
+  return new URLRequestFileJob(request, network_delegate, file_path);
+}
+
 void URLRequestFileJob::Start() {
   FileMetaInfo* meta_info = new FileMetaInfo();
   base::WorkerPool::PostTaskAndReply(
diff --git a/net/url_request/url_request_file_job.h b/net/url_request/url_request_file_job.h
index 6fc7fb9..0d9e192 100644
--- a/net/url_request/url_request_file_job.h
+++ b/net/url_request/url_request_file_job.h
@@ -33,6 +33,8 @@ class NET_EXPORT URLRequestFileJob : public URLRequestJob {
                     NetworkDelegate* network_delegate,
                     const base::FilePath& file_path);
 
+  static URLRequest::ProtocolFactory Factory;
+
   // URLRequestJob:
   virtual void Start() OVERRIDE;
   virtual void Kill() OVERRIDE;
diff --git a/net/url_request/url_request_ftp_job.cc b/net/url_request/url_request_ftp_job.cc
index 62124e1..343c4d7 100644
--- a/net/url_request/url_request_ftp_job.cc
+++ b/net/url_request/url_request_ftp_job.cc
@@ -12,7 +12,6 @@
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
-#include "net/ftp/ftp_auth_cache.h"
 #include "net/ftp/ftp_response_info.h"
 #include "net/ftp/ftp_transaction_factory.h"
 #include "net/http/http_response_headers.h"
@@ -39,6 +38,26 @@ URLRequestFtpJob::URLRequestFtpJob(
   DCHECK(ftp_auth_cache);
 }
 
+// static
+URLRequestJob* URLRequestFtpJob::Factory(URLRequest* request,
+                                         NetworkDelegate* network_delegate,
+                                         const std::string& scheme) {
+  DCHECK_EQ(scheme, "ftp");
+
+  int port = request->url().IntPort();
+  if (request->url().has_port() &&
+      !IsPortAllowedByFtp(port) && !IsPortAllowedByOverride(port)) {
+    return new URLRequestErrorJob(request,
+                                  network_delegate,
+                                  ERR_UNSAFE_PORT);
+  }
+
+  return new URLRequestFtpJob(request,
+                              network_delegate,
+                              request->context()->ftp_transaction_factory(),
+                              request->context()->ftp_auth_cache());
+}
+
 bool URLRequestFtpJob::IsSafeRedirect(const GURL& location) {
   // Disallow all redirects.
   return false;
diff --git a/net/url_request/url_request_ftp_job.h b/net/url_request/url_request_ftp_job.h
index dd16b9a..3b053c1 100644
--- a/net/url_request/url_request_ftp_job.h
+++ b/net/url_request/url_request_ftp_job.h
@@ -32,6 +32,11 @@ class URLRequestFtpJob : public URLRequestJob {
                    FtpTransactionFactory* ftp_transaction_factory,
                    FtpAuthCache* ftp_auth_cache);
 
+  // TODO(shalev): get rid of this function in favor of FtpProtocolHandler.
+  static URLRequestJob* Factory(URLRequest* request,
+                                NetworkDelegate* network_delegate,
+                                const std::string& scheme);
+
   // Overridden from URLRequestJob:
   virtual bool IsSafeRedirect(const GURL& location) OVERRIDE;
   virtual bool GetMimeType(std::string* mime_type) const OVERRIDE;
diff --git a/net/url_request/url_request_ftp_job_unittest.cc b/net/url_request/url_request_ftp_job_unittest.cc
index 50f5d05..61dde753 100644
--- a/net/url_request/url_request_ftp_job_unittest.cc
+++ b/net/url_request/url_request_ftp_job_unittest.cc
@@ -6,13 +6,10 @@
 #include "base/run_loop.h"
 #include "net/proxy/proxy_config_service.h"
 #include "net/socket/socket_test_util.h"
-#include "net/url_request/ftp_protocol_handler.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
-#include "net/url_request/url_request_job_factory_impl.h"
 #include "net/url_request/url_request_status.h"
 #include "net/url_request/url_request_test_util.h"
-#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
@@ -52,26 +49,15 @@ class SimpleProxyConfigService : public ProxyConfigService {
   Observer* observer_;
 };
 
-class MockFtpTransactionFactory : public FtpTransactionFactory {
- public:
-  MOCK_METHOD0(CreateTransaction, FtpTransaction*());
-  MOCK_METHOD1(Suspend, void(bool suspend));
-};
-
 class FtpTestURLRequestContext : public TestURLRequestContext {
  public:
   FtpTestURLRequestContext(ClientSocketFactory* socket_factory,
                            ProxyService* proxy_service,
-                           NetworkDelegate* network_delegate,
-                           FtpTransactionFactory* ftp_transaction_factory)
+                           NetworkDelegate* network_delegate)
       : TestURLRequestContext(true) {
     set_client_socket_factory(socket_factory);
     context_storage_.set_proxy_service(proxy_service);
     set_network_delegate(network_delegate);
-    URLRequestJobFactoryImpl* job_factory = new URLRequestJobFactoryImpl;
-    job_factory->SetProtocolHandler(
-        "ftp", new FtpProtocolHandler(ftp_transaction_factory));
-    context_storage_.set_job_factory(job_factory);
     Init();
   }
 };
@@ -83,8 +69,7 @@ class URLRequestFtpJobTest : public testing::Test {
                            new SimpleProxyConfigService, NULL, NULL)),
         request_context_(&socket_factory_,
                          proxy_service_,
-                         &network_delegate_,
-                         &ftp_transaction_factory_) {
+                         &network_delegate_) {
   }
 
   virtual ~URLRequestFtpJobTest() {
@@ -114,7 +99,6 @@ class URLRequestFtpJobTest : public testing::Test {
   ScopedVector<DeterministicSocketData> socket_data_;
   DeterministicMockClientSocketFactory socket_factory_;
   TestNetworkDelegate network_delegate_;
-  ::testing::StrictMock<MockFtpTransactionFactory> ftp_transaction_factory_;
 
   // Owned by |request_context_|:
   ProxyService* proxy_service_;
diff --git a/net/url_request/url_request_http_job.cc b/net/url_request/url_request_http_job.cc
index 5b116b9..716fe4c 100644
--- a/net/url_request/url_request_http_job.cc
+++ b/net/url_request/url_request_http_job.cc
@@ -43,7 +43,6 @@
 #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_factory.h"
 #include "net/url_request/url_request_redirect_job.h"
 #include "net/url_request/url_request_throttler_header_adapter.h"
 #include "net/url_request/url_request_throttler_manager.h"
@@ -1014,16 +1013,25 @@ Filter* URLRequestHttpJob::SetupFilter() const {
 }
 
 bool URLRequestHttpJob::IsSafeRedirect(const GURL& location) {
-  // HTTP is always safe.
-  // TODO(pauljensen): Remove once crbug.com/146591 is fixed.
-  if (location.is_valid() &&
-      (location.scheme() == "http" || location.scheme() == "https")) {
+  // We only allow redirects to certain "safe" protocols.  This does not
+  // restrict redirects to externally handled protocols.  Our consumer would
+  // need to take care of those.
+
+  if (!URLRequest::IsHandledURL(location))
     return true;
+
+  static const char* kSafeSchemes[] = {
+    "http",
+    "https",
+    "ftp"
+  };
+
+  for (size_t i = 0; i < arraysize(kSafeSchemes); ++i) {
+    if (location.SchemeIs(kSafeSchemes[i]))
+      return true;
   }
-  // Query URLRequestJobFactory as to whether |location| would be safe to
-  // redirect to.
-  return request_->context()->job_factory() &&
-      request_->context()->job_factory()->IsSafeRedirectTarget(location);
+
+  return false;
 }
 
 bool URLRequestHttpJob::NeedsAuth() {
diff --git a/net/url_request/url_request_job_factory.cc b/net/url_request/url_request_job_factory.cc
index e203cb1..c1f070b 100644
--- a/net/url_request/url_request_job_factory.cc
+++ b/net/url_request/url_request_job_factory.cc
@@ -8,11 +8,6 @@ namespace net {
 
 URLRequestJobFactory::ProtocolHandler::~ProtocolHandler() {}
 
-bool URLRequestJobFactory::ProtocolHandler::IsSafeRedirectTarget(
-    const GURL& location) const {
-  return true;
-}
-
 URLRequestJobFactory::URLRequestJobFactory() {}
 
 URLRequestJobFactory::~URLRequestJobFactory() {}
diff --git a/net/url_request/url_request_job_factory.h b/net/url_request/url_request_job_factory.h
index d4cc49e..bbd1d09 100644
--- a/net/url_request/url_request_job_factory.h
+++ b/net/url_request/url_request_job_factory.h
@@ -30,13 +30,6 @@ class NET_EXPORT URLRequestJobFactory
 
     virtual URLRequestJob* MaybeCreateJob(
         URLRequest* request, NetworkDelegate* network_delegate) const = 0;
-
-    // Indicates if it should be safe to redirect to |location|. Should handle
-    // protocols handled by MaybeCreateJob().  Only called when registered with
-    // URLRequestJobFactoryImpl::SetProtocolHandler() not called when used with
-    // ProtocolInterceptJobFactory.
-    // NOTE(pauljensen): Default implementation returns true.
-    virtual bool IsSafeRedirectTarget(const GURL& location) const;
   };
 
   URLRequestJobFactory();
@@ -51,8 +44,6 @@ class NET_EXPORT URLRequestJobFactory
 
   virtual bool IsHandledURL(const GURL& url) const = 0;
 
-  virtual bool IsSafeRedirectTarget(const GURL& location) const = 0;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(URLRequestJobFactory);
 };
diff --git a/net/url_request/url_request_job_factory_impl.cc b/net/url_request/url_request_job_factory_impl.cc
index 84fb951..f4c1d01 100644
--- a/net/url_request/url_request_job_factory_impl.cc
+++ b/net/url_request/url_request_job_factory_impl.cc
@@ -64,20 +64,4 @@ bool URLRequestJobFactoryImpl::IsHandledURL(const GURL& url) const {
   return IsHandledProtocol(url.scheme());
 }
 
-bool URLRequestJobFactoryImpl::IsSafeRedirectTarget(
-    const GURL& location) const {
-  DCHECK(CalledOnValidThread());
-  if (!location.is_valid()) {
-    // Error cases are safely handled.
-    return true;
-  }
-  ProtocolHandlerMap::const_iterator it = protocol_handler_map_.find(
-      location.scheme());
-  if (it == protocol_handler_map_.end()) {
-    // Unhandled cases are safely handled.
-    return true;
-  }
-  return it->second->IsSafeRedirectTarget(location);
-}
-
 }  // namespace net
diff --git a/net/url_request/url_request_job_factory_impl.h b/net/url_request/url_request_job_factory_impl.h
index 4f03fb0..8f394e2 100644
--- a/net/url_request/url_request_job_factory_impl.h
+++ b/net/url_request/url_request_job_factory_impl.h
@@ -33,7 +33,6 @@ class NET_EXPORT URLRequestJobFactoryImpl : public URLRequestJobFactory {
       NetworkDelegate* network_delegate) const OVERRIDE;
   virtual bool IsHandledProtocol(const std::string& scheme) const OVERRIDE;
   virtual bool IsHandledURL(const GURL& url) const OVERRIDE;
-  virtual bool IsSafeRedirectTarget(const GURL& location) const OVERRIDE;
 
  private:
   typedef std::map<std::string, ProtocolHandler*> ProtocolHandlerMap;
diff --git a/net/url_request/url_request_job_manager.cc b/net/url_request/url_request_job_manager.cc
index 5a64b41..283ee0d 100644
--- a/net/url_request/url_request_job_manager.cc
+++ b/net/url_request/url_request_job_manager.cc
@@ -12,8 +12,12 @@
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_delegate.h"
+#include "net/url_request/url_request_about_job.h"
 #include "net/url_request/url_request_context.h"
+#include "net/url_request/url_request_data_job.h"
 #include "net/url_request/url_request_error_job.h"
+#include "net/url_request/url_request_file_job.h"
+#include "net/url_request/url_request_ftp_job.h"
 #include "net/url_request/url_request_http_job.h"
 #include "net/url_request/url_request_job_factory.h"
 
@@ -32,6 +36,12 @@ struct SchemeToFactory {
 static const SchemeToFactory kBuiltinFactories[] = {
   { "http", URLRequestHttpJob::Factory },
   { "https", URLRequestHttpJob::Factory },
+  { "file", URLRequestFileJob::Factory },
+#if !defined(DISABLE_FTP_SUPPORT)
+  { "ftp", URLRequestFtpJob::Factory },
+#endif
+  { "about", URLRequestAboutJob::Factory },
+  { "data", URLRequestDataJob::Factory },
 };
 
 // static
diff --git a/net/url_request/url_request_test_util.cc b/net/url_request/url_request_test_util.cc
index 5f00999..e5c0051 100644
--- a/net/url_request/url_request_test_util.cc
+++ b/net/url_request/url_request_test_util.cc
@@ -72,6 +72,14 @@ void TestURLRequestContext::Init() {
     context_storage_.set_proxy_service(ProxyService::CreateDirect());
   if (!cert_verifier())
     context_storage_.set_cert_verifier(CertVerifier::CreateDefault());
+  if (!ftp_transaction_factory()) {
+#if !defined(DISABLE_FTP_SUPPORT)
+    context_storage_.set_ftp_transaction_factory(
+        new FtpNetworkLayer(host_resolver()));
+#else
+    context_storage_.set_ftp_transaction_factory(NULL);
+#endif  // !defined(DISABLE_FTP_SUPPORT)
+  }
   if (!ssl_config_service())
     context_storage_.set_ssl_config_service(new SSLConfigServiceDefaults);
   if (!http_auth_handler_factory()) {
diff --git a/net/url_request/url_request_unittest.cc b/net/url_request/url_request_unittest.cc
index c7e535a..32b99ac 100644
--- a/net/url_request/url_request_unittest.cc
+++ b/net/url_request/url_request_unittest.cc
@@ -58,8 +58,6 @@
 #include "net/socket/ssl_client_socket.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "net/test/test_server.h"
-#include "net/url_request/data_protocol_handler.h"
-#include "net/url_request/file_protocol_handler.h"
 #include "net/url_request/ftp_protocol_handler.h"
 #include "net/url_request/static_http_user_agent_settings.h"
 #include "net/url_request/url_request.h"
@@ -516,9 +514,6 @@ class URLRequestTest : public PlatformTest {
   URLRequestTest() : default_context_(true) {
     default_context_.set_network_delegate(&default_network_delegate_);
     default_context_.set_net_log(&net_log_);
-    job_factory_.SetProtocolHandler("data", new DataProtocolHandler);
-    job_factory_.SetProtocolHandler("file", new FileProtocolHandler);
-    default_context_.set_job_factory(&job_factory_);
     default_context_.Init();
   }
   virtual ~URLRequestTest() {}
@@ -526,14 +521,16 @@ class URLRequestTest : public PlatformTest {
   // Adds the TestJobInterceptor to the default context.
   TestJobInterceptor* AddTestInterceptor() {
     TestJobInterceptor* protocol_handler_ = new TestJobInterceptor();
-    job_factory_.SetProtocolHandler("http", protocol_handler_);
+    job_factory_.reset(new URLRequestJobFactoryImpl);
+    job_factory_->SetProtocolHandler("http", protocol_handler_);
+    default_context_.set_job_factory(job_factory_.get());
     return protocol_handler_;
   }
 
  protected:
   CapturingNetLog net_log_;
   TestNetworkDelegate default_network_delegate_;  // Must outlive URLRequest.
-  URLRequestJobFactoryImpl job_factory_;
+  scoped_ptr<URLRequestJobFactoryImpl> job_factory_;
   TestURLRequestContext default_context_;
 };
 
@@ -3467,20 +3464,6 @@ TEST_F(URLRequestTestHTTP, ContentTypeNormalizationTest) {
   req.Cancel();
 }
 
-TEST_F(URLRequestTestHTTP, ProtocolHandlerAndFactoryRestrictRedirects) {
-  // Test URLRequestJobFactory::ProtocolHandler::IsSafeRedirectTarget().
-  GURL file_url("file:///foo.txt");
-  GURL data_url("data:,foo");
-  FileProtocolHandler file_protocol_handler;
-  EXPECT_FALSE(file_protocol_handler.IsSafeRedirectTarget(file_url));
-  DataProtocolHandler data_protocol_handler;
-  EXPECT_TRUE(data_protocol_handler.IsSafeRedirectTarget(data_url));
-
-  // Test URLRequestJobFactoryImpl::IsSafeRedirectTarget().
-  EXPECT_FALSE(job_factory_.IsSafeRedirectTarget(file_url));
-  EXPECT_TRUE(job_factory_.IsSafeRedirectTarget(data_url));
-}
-
 TEST_F(URLRequestTestHTTP, RestrictRedirects) {
   ASSERT_TRUE(test_server_.Start());
 
@@ -5000,12 +4983,15 @@ TEST_F(URLRequestTestFTP, UnsafePort) {
   ASSERT_TRUE(test_server_.Start());
 
   URLRequestJobFactoryImpl job_factory;
-  FtpNetworkLayer ftp_transaction_factory(default_context_.host_resolver());
 
   GURL url("ftp://127.0.0.1:7");
+  FtpProtocolHandler ftp_protocol_handler(
+      default_context_.ftp_transaction_factory(),
+      default_context_.ftp_auth_cache());
   job_factory.SetProtocolHandler(
       "ftp",
-      new FtpProtocolHandler(&ftp_transaction_factory));
+      new FtpProtocolHandler(default_context_.ftp_transaction_factory(),
+                             default_context_.ftp_auth_cache()));
   default_context_.set_job_factory(&job_factory);
 
   TestDelegate d;