Move IOThread initialization to happen asynchronously.

chrome/browser/io_thread.cc

Index: chrome/browser/io_thread.cc
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index dde18801b1567adb1d4b0d8e1ed17feb9776afd4..27e0c829561f75c4d626674f80f7c3933c70a94b 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -463,33 +463,28 @@ net::URLRequestContextGetter* IOThread::system_url_request_context_getter() {
   return system_url_request_context_getter_.get();
 }
 
-void IOThread::Init() {
-  TRACE_EVENT0("startup", "IOThread::Init");
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
+void IOThread::DoInitTasks() {
 #if defined(USE_NSS) || defined(OS_IOS)
   net::SetMessageLoopForNSSHttpIO();
 #endif
 
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-
   DCHECK(!globals_);
   globals_ = new Globals;
 
+  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
+
   // Add an observer that will emit network change events to the ChromeNetLog.
   // Assuming NetworkChangeNotifier dispatches in FIFO order, we should be
   // logging the network change before other IO thread consumers respond to it.
-  network_change_observer_.reset(
-      new LoggingNetworkChangeObserver(net_log_));
+  network_change_observer_.reset(new LoggingNetworkChangeObserver(net_log_));
 
   // Setup the HistogramWatcher to run on the IO thread.
   net::NetworkChangeNotifier::InitHistogramWatcher();
 
   globals_->extension_event_router_forwarder =
       extension_event_router_forwarder_;
-  ChromeNetworkDelegate* network_delegate =
-      new ChromeNetworkDelegate(extension_event_router_forwarder_,
-                                &system_enable_referrers_);
+  ChromeNetworkDelegate* network_delegate = new ChromeNetworkDelegate(
+      extension_event_router_forwarder_, &system_enable_referrers_);
   if (command_line.HasSwitch(switches::kDisableExtensionsHttpThrottling))
     network_delegate->NeverThrottleRequests();
   globals_->system_network_delegate.reset(network_delegate);
@@ -506,24 +501,24 @@ void IOThread::Init() {
     spdyproxy_auth_origin_ = SPDY_PROXY_AUTH_ORIGIN;
 #endif
   }
-  globals_->http_auth_handler_factory.reset(CreateDefaultAuthHandlerFactory(
-      globals_->host_resolver.get()));
+  globals_->http_auth_handler_factory
+      .reset(CreateDefaultAuthHandlerFactory(globals_->host_resolver.get()));

[mmenke, 2013/07/10 18:48:24]

nit:  For all of these, it's much more common across the chrome code base to put
the method on the same name as the class.  I don't think I've ever seen this
indentation style used in chrome, actually, except perhaps where it was
necessary due to variable name length.

[mmenke, 2013/07/10 18:51:04]

"On the same line as the object name", rather.

   globals_->http_server_properties.reset(new net::HttpServerPropertiesImpl);
   // For the ProxyScriptFetcher, we use a direct ProxyService.
-  globals_->proxy_script_fetcher_proxy_service.reset(
-      net::ProxyService::CreateDirectWithNetLog(net_log_));
+  globals_->proxy_script_fetcher_proxy_service
+      .reset(net::ProxyService::CreateDirectWithNetLog(net_log_));
   // In-memory cookie store.
   globals_->system_cookie_store = new net::CookieMonster(NULL, NULL);
   // In-memory server bound cert store.
-  globals_->system_server_bound_cert_service.reset(
-      new net::ServerBoundCertService(
-          new net::DefaultServerBoundCertStore(NULL),
-          base::WorkerPool::GetTaskRunner(true)));
+  globals_->system_server_bound_cert_service
+      .reset(new net::ServerBoundCertService(
+           new net::DefaultServerBoundCertStore(NULL),
+           base::WorkerPool::GetTaskRunner(true)));
   globals_->dns_probe_service.reset(new chrome_browser_net::DnsProbeService());
   globals_->load_time_stats.reset(new chrome_browser_net::LoadTimeStats());
   globals_->host_mapping_rules.reset(new net::HostMappingRules());
-  globals_->http_user_agent_settings.reset(
-      new BasicHttpUserAgentSettings(std::string()));
+  globals_->http_user_agent_settings
+      .reset(new BasicHttpUserAgentSettings(std::string()));
   if (command_line.HasSwitch(switches::kHostRules)) {
     TRACE_EVENT_BEGIN0("startup", "IOThread::Init:SetRulesFromString");
     globals_->host_mapping_rules->SetRulesFromString(
@@ -542,15 +537,13 @@ void IOThread::Init() {
   }
   globals_->enable_quic.set(ShouldEnableQuic(command_line));
   if (command_line.HasSwitch(switches::kOriginToForceQuicOn)) {
-    net::HostPortPair quic_origin =
-        net::HostPortPair::FromString(
-            command_line.GetSwitchValueASCII(switches::kOriginToForceQuicOn));
+    net::HostPortPair quic_origin = net::HostPortPair::FromString(
+        command_line.GetSwitchValueASCII(switches::kOriginToForceQuicOn));
     if (!quic_origin.IsEmpty()) {
       globals_->origin_to_force_quic_on.set(quic_origin);
     }
   }
-  if (command_line.HasSwitch(
-          switches::kEnableUserAlternateProtocolPorts)) {
+  if (command_line.HasSwitch(switches::kEnableUserAlternateProtocolPorts)) {
     globals_->enable_user_alternate_protocol_ports = true;
   }
   InitializeNetworkOptions(command_line);
@@ -574,8 +567,8 @@ void IOThread::Init() {
   job_factory->SetProtocolHandler(chrome::kFileScheme,
                                   new net::FileProtocolHandler());
 #if !defined(DISABLE_FTP_SUPPORT)
-  globals_->proxy_script_fetcher_ftp_transaction_factory.reset(
-      new net::FtpNetworkLayer(globals_->host_resolver.get()));
+  globals_->proxy_script_fetcher_ftp_transaction_factory
+      .reset(new net::FtpNetworkLayer(globals_->host_resolver.get()));
   job_factory->SetProtocolHandler(
       chrome::kFtpScheme,
       new net::FtpProtocolHandler(
@@ -589,21 +582,19 @@ void IOThread::Init() {
   // Always done in production, disabled only for unit tests.
   globals_->throttler_manager->set_enable_thread_checks(true);
 
-  globals_->proxy_script_fetcher_context.reset(
-      ConstructProxyScriptFetcherContext(globals_, net_log_));
+  globals_->proxy_script_fetcher_context
+      .reset(ConstructProxyScriptFetcherContext(globals_, net_log_));
 
-  globals_->network_time_notifier.reset(
-      new net::NetworkTimeNotifier(
-          scoped_ptr<base::TickClock>(new base::DefaultTickClock())));
+  globals_->network_time_notifier.reset(new net::NetworkTimeNotifier(
+      scoped_ptr<base::TickClock>(new base::DefaultTickClock())));
 
   sdch_manager_ = new net::SdchManager();
 
 #if defined(OS_MACOSX) && !defined(OS_IOS)
   // Start observing Keychain events. This needs to be done on the UI thread,
   // as Keychain services requires a CFRunLoop.
-  BrowserThread::PostTask(BrowserThread::UI,
-                          FROM_HERE,
-                          base::Bind(&ObserveKeychainEvents));
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE, base::Bind(&ObserveKeychainEvents));
 #endif
 
   // InitSystemRequestContext turns right around and posts a task back
@@ -622,6 +613,16 @@ void IOThread::Init() {
                                      weak_factory_.GetWeakPtr()));
 }
 
+void IOThread::Init() {
+  TRACE_EVENT0("startup", "IOThread::Init");
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));

[awong, 2013/07/10 18:45:47]

This confuses me...

My understanding of the original code was:

(1) UI BrowserThread does "new IOThread()"
(2) UI BrowserThread does effectively "PostTask(UI, FROM_HERE,
Bind(&IOThread::Init)" via BrowserThreadDelegate.
(3) IO BrowserThread runs IOThread::Init.

Now we know that we can post tasks to the IO BrowserThread that assume the
global IOThread object is initialized.

In the modified code, we haven't changed the behavior of the UI thread at all.

What we do change is in (3) we now delay the initialization of the IOThread
object on the IO BrowserThread such that there is a gap in time where IOThread
is not initialized.  This seems wrong.

+
+  BrowserThread::PostTask(
+      BrowserThread::IO,
+      FROM_HERE,
+      base::Bind(&IOThread::DoInitTasks, base::Unretained(this)));
+}
+
 void IOThread::CleanUp() {
   base::debug::LeakTracker<SafeBrowsingURLRequestContext>::CheckForLeaks();