Convert ProtocolHandlerRegistry::Interceptor to a net::URLRequestJobFactory.

chrome/browser/profiles/profile_io_data.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/profiles/profile_io_data.h"
 
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
@@ skipping 154 matching lines @@
 #if defined(ENABLE_NOTIFICATIONS)
   params->notification_service =
       DesktopNotificationServiceFactory::GetForProfile(profile);
 #endif
 
   ProtocolHandlerRegistry* protocol_handler_registry =
       ProtocolHandlerRegistryFactory::GetForProfile(profile);
   DCHECK(protocol_handler_registry);
 
   // The profile instance is only available here in the InitializeOnUIThread
-  // method, so we create the url interceptor here, then save it for
+  // method, so we create the url job factory here, then save it for
   // later delivery to the job factory in LazyInitialize.
-  params->protocol_handler_interceptor.reset(
-      protocol_handler_registry->CreateURLInterceptor());
+  params->protocol_handler_interceptor =
+      protocol_handler_registry->CreateURLRequestJobFactory();
 
   ChromeProxyConfigService* proxy_config_service =
       ProxyServiceFactory::CreateProxyConfigService(true);
   params->proxy_config_service.reset(proxy_config_service);
   profile->GetProxyConfigTracker()->SetChromeProxyConfigService(
       proxy_config_service);
   params->profile = profile;
   profile_params_.reset(params.release());
 
   ChromeNetworkDelegate::InitializePrefsOnUIThread(
@@ skipping 198 matching lines @@
 ChromeURLRequestContext*
 ProfileIOData::GetExtensionsRequestContext() const {
   LazyInitialize();
   return extensions_request_context_.get();
 }
 
 ChromeURLRequestContext*
 ProfileIOData::GetIsolatedAppRequestContext(
     ChromeURLRequestContext* main_context,
     const StoragePartitionDescriptor& partition_descriptor,
-    scoped_ptr<net::URLRequestJobFactory::Interceptor>
+    scoped_ptr<ProtocolHandlerRegistry::JobFactory>
         protocol_handler_interceptor) const {
   LazyInitialize();
   ChromeURLRequestContext* context = NULL;
   if (ContainsKey(app_request_context_map_, partition_descriptor)) {
     context = app_request_context_map_[partition_descriptor];
   } else {
     context = AcquireIsolatedAppRequestContext(
         main_context, partition_descriptor,
         protocol_handler_interceptor.Pass());
     app_request_context_map_[partition_descriptor] = context;
@@ skipping 189 matching lines @@
 }
 
 void ProfileIOData::ApplyProfileParamsToContext(
     ChromeURLRequestContext* context) const {
   context->set_is_incognito(is_incognito());
   context->set_http_user_agent_settings(
       chrome_http_user_agent_settings_.get());
   context->set_ssl_config_service(profile_params_->ssl_config_service);
 }
 
-void ProfileIOData::SetUpJobFactoryDefaults(
-    net::URLRequestJobFactoryImpl* job_factory,
-    scoped_ptr<net::URLRequestJobFactory::Interceptor>
+scoped_ptr<net::URLRequestJobFactory> ProfileIOData::SetUpJobFactoryDefaults(
+    scoped_ptr<net::URLRequestJobFactoryImpl> job_factory,
+    scoped_ptr<ProtocolHandlerRegistry::JobFactory>
         protocol_handler_interceptor,
     net::NetworkDelegate* network_delegate,
     net::FtpTransactionFactory* ftp_transaction_factory,
     net::FtpAuthCache* ftp_auth_cache) const {
   // NOTE(willchan): Keep these protocol handlers in sync with
   // ProfileIOData::IsHandledProtocol().
   bool set_protocol = job_factory->SetProtocolHandler(
       chrome::kFileScheme, new net::FileProtocolHandler());
   DCHECK(set_protocol);
 
   set_protocol = job_factory->SetProtocolHandler(
       chrome::kChromeDevToolsScheme,
       CreateDevToolsProtocolHandler(chrome_url_data_manager_backend(),
                                     network_delegate));
   DCHECK(set_protocol);
-
-  if (protocol_handler_interceptor.get()) {
-    job_factory->AddInterceptor(protocol_handler_interceptor.release());
-  }
-
   set_protocol = job_factory->SetProtocolHandler(
       extensions::kExtensionScheme,
       CreateExtensionProtocolHandler(is_incognito(), GetExtensionInfoMap()));
   DCHECK(set_protocol);
   set_protocol = job_factory->SetProtocolHandler(
       chrome::kExtensionResourceScheme,
       CreateExtensionResourceProtocolHandler());
   DCHECK(set_protocol);
   set_protocol = job_factory->SetProtocolHandler(
       chrome::kChromeUIScheme,
@@ skipping 14 matching lines @@
   job_factory->SetProtocolHandler(
       chrome::kAboutScheme,
       new chrome_browser_net::AboutProtocolHandler());
 #if !defined(DISABLE_FTP_SUPPORT)
   DCHECK(ftp_transaction_factory);
   job_factory->SetProtocolHandler(
       chrome::kFtpScheme,
       new net::FtpProtocolHandler(ftp_transaction_factory,
                                   ftp_auth_cache));
 #endif  // !defined(DISABLE_FTP_SUPPORT)
+
+  if (protocol_handler_interceptor.get()) {
+    protocol_handler_interceptor->Chain(

[erikwright (departed), 2013/01/02 16:29:36]

Passing a custom type around, and configuring the wrapped factory as a deferred
step is possibly the right thing to do here, but if this pattern recurs you
could end up with a lot of boiler-plate code.

If that were the case, this pattern could be simplified as follows:

1) Make the protocol handler interceptor not delegate to another factory.
2) Pass the protocol handler interceptor as a generic URLRequestJobFactory
3) Define a simple URLRequestJobFactory implementation that delegates to two
child factories, in order.
4) Here, create the delegating factory and pass it the interceptor as its first
child and job_factory as its second child.

Again, I wouldn't necessarily go to the trouble just for this case, but if this
pattern recurs it would seem to be worthwhile to centralize it. The other
advantages are:

a) One less custom type exposed.
b) Simpler life-time for the protocol handler interceptor (it doesn't have to
check in every method for a chained factory).

[pauljensen, 2013/01/02 22:13:35]

I agree, I think if this code occurs again we should centralize it, but I think
this one occurrence doesn't warrant it.

+        job_factory.PassAs<net::URLRequestJobFactory>());
+    return protocol_handler_interceptor.PassAs<net::URLRequestJobFactory>();
+  } else {
+    return job_factory.PassAs<net::URLRequestJobFactory>();
+  }
 }
 
 void ProfileIOData::ShutdownOnUIThread() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (signin_names_)
     signin_names_->ReleaseResourcesOnUIThread();
 
   google_services_username_.Destroy();
   google_services_username_pattern_.Destroy();
@@ skipping 54 matching lines @@
 void ProfileIOData::SetCookieSettingsForTesting(
     CookieSettings* cookie_settings) {
   DCHECK(!cookie_settings_.get());
   cookie_settings_ = cookie_settings;
 }
 
 void ProfileIOData::set_signin_names_for_testing(
     SigninNamesOnIOThread* signin_names) {
   signin_names_.reset(signin_names);
 }