Defer DRP start up until after the context getter is created

chrome/browser/profiles/profile_impl_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_impl_io_data.h"
 
 #include <memory>
 #include <set>
 #include <string>
 #include <utility>
@@ skipping 175 matching lines @@
   io_data_->app_cache_max_size_ = cache_max_size;
   io_data_->app_media_cache_max_size_ = media_cache_max_size;
 
   io_data_->predictor_.reset(predictor);
   io_data_->domain_reliability_monitor_ = std::move(domain_reliability_monitor);
 
   io_data_->InitializeMetricsEnabledStateOnUIThread();
   if (io_data_->domain_reliability_monitor_)
     io_data_->domain_reliability_monitor_->MoveToNetworkThread();
 
-  io_data_->set_data_reduction_proxy_io_data(
-      CreateDataReductionProxyChromeIOData(
-          g_browser_process->io_thread()->net_log(), profile_->GetPrefs(),
-          BrowserThread::GetTaskRunnerForThread(BrowserThread::IO),
-          BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)));
-
-  base::SequencedWorkerPool* pool = BrowserThread::GetBlockingPool();
-  scoped_refptr<base::SequencedTaskRunner> db_task_runner =
-      pool->GetSequencedTaskRunnerWithShutdownBehavior(
-          pool->GetSequenceToken(),
-          base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
-  std::unique_ptr<data_reduction_proxy::DataStore> store(
-      new data_reduction_proxy::DataStoreImpl(profile_path));
-  DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile_)
-      ->InitDataReductionProxySettings(
-          io_data_->data_reduction_proxy_io_data(), profile_->GetPrefs(),
-          profile_->GetRequestContext(), std::move(store),
-          BrowserThread::GetTaskRunnerForThread(BrowserThread::UI),
-          db_task_runner);
-
   io_data_->previews_io_data_ = base::MakeUnique<previews::PreviewsIOData>(
       BrowserThread::GetTaskRunnerForThread(BrowserThread::UI),
       BrowserThread::GetTaskRunnerForThread(BrowserThread::IO));
   PreviewsServiceFactory::GetForProfile(profile_)->Initialize(
       io_data_->previews_io_data_.get(),
       BrowserThread::GetTaskRunnerForThread(BrowserThread::IO), profile_path);
+
+  io_data_->set_data_reduction_proxy_io_data(
+      CreateDataReductionProxyChromeIOData(
+          g_browser_process->io_thread()->net_log(), profile_->GetPrefs(),
+          BrowserThread::GetTaskRunnerForThread(BrowserThread::IO),
+          BrowserThread::GetTaskRunnerForThread(BrowserThread::UI)));

[mmenke, 2016/11/04 19:02:44]

I'm not objecting, but is there a reason you're moving this?

[bengr, 2016/11/07 18:13:40]

Likewise, I'm curious why you're moving this.

[RyanSturm, 2016/11/07 18:21:53]

I talked to mmenke about this, and he's actually asking a slightly different
question then you. He is asking why the previews line moved above DRP, and you
are asking why half of DRP initialization moved down. The basic idea is that
calling profile_->GetRequestContext() here causes IO thread tasks to potentially
start before this method finishes. And the Offline Page URL Request interceptor
was sometimes getting a nullptr due to io_data_->previews_io_data_ not being set
yet.

To answer mmenke's question, there's not a strong reason to move previews above
DRP here, but I'd prefer previews be created first here to prevent possible
similar problems if DRP code changes again.

 }
 
 content::ResourceContext*
     ProfileImplIOData::Handle::GetResourceContext() const {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   LazyInitialize();
   return GetResourceContextNoInit();
 }
 
 content::ResourceContext*
 ProfileImplIOData::Handle::GetResourceContextNoInit() const {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   // Don't call LazyInitialize here, since the resource context is created at
   // the beginning of initalization and is used by some members while they're
   // being initialized (i.e. AppCacheService).
   return io_data_->GetResourceContext();
 }
 
 scoped_refptr<ChromeURLRequestContextGetter>
 ProfileImplIOData::Handle::CreateMainRequestContextGetter(
     content::ProtocolHandlerMap* protocol_handlers,
     content::URLRequestInterceptorScopedVector request_interceptors,
     IOThread* io_thread) const {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   LazyInitialize();
   DCHECK(!main_request_context_getter_.get());
   main_request_context_getter_ = ChromeURLRequestContextGetter::Create(
       profile_, io_data_, protocol_handlers, std::move(request_interceptors));
 
+  base::SequencedWorkerPool* pool = BrowserThread::GetBlockingPool();
+  scoped_refptr<base::SequencedTaskRunner> db_task_runner =
+      pool->GetSequencedTaskRunnerWithShutdownBehavior(
+          pool->GetSequenceToken(),
+          base::SequencedWorkerPool::SKIP_ON_SHUTDOWN);
+  std::unique_ptr<data_reduction_proxy::DataStore> store(
+      new data_reduction_proxy::DataStoreImpl(io_data_->profile_path_));
+  DataReductionProxyChromeSettingsFactory::GetForBrowserContext(profile_)
+      ->InitDataReductionProxySettings(
+          io_data_->data_reduction_proxy_io_data(), profile_->GetPrefs(),
+          main_request_context_getter_.get(), std::move(store),
+          BrowserThread::GetTaskRunnerForThread(BrowserThread::UI),
+          db_task_runner);

[mmenke, 2016/11/04 17:25:10]

What happens if we start making requests on the IO thread before this is done?

[RyanSturm, 2016/11/04 18:46:39]

We still wait for config to be set up in DRP, which takes a couple thread hops
or a fetch of a new config either way. Delaying this by a slight amount should
be handled.

+
   io_data_->predictor_
       ->InitNetworkPredictor(profile_->GetPrefs(),
                              io_thread,
                              main_request_context_getter_.get(),
                              io_data_);
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_PROFILE_URL_REQUEST_CONTEXT_GETTER_INITIALIZED,
       content::Source<Profile>(profile_),
       content::NotificationService::NoDetails());
@@ skipping 543 matching lines @@
     const base::Closure& completion) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   DCHECK(initialized());
 
   DCHECK(transport_security_state());
   // Completes synchronously.
   transport_security_state()->DeleteAllDynamicDataSince(time);
   DCHECK(http_server_properties_manager_);
   http_server_properties_manager_->Clear(completion);
 }