Defer DRP start up until after the context getter is created

chrome/browser/profiles/profile_impl_io_data.cc

Index: chrome/browser/profiles/profile_impl_io_data.cc
diff --git a/chrome/browser/profiles/profile_impl_io_data.cc b/chrome/browser/profiles/profile_impl_io_data.cc
index 0ef95436f2099fc191372e468a08daedd2a573af..63a2cbd13bf18b36a4c7a2c689536c72222b215d 100644
--- a/chrome/browser/profiles/profile_impl_io_data.cc
+++ b/chrome/browser/profiles/profile_impl_io_data.cc
@@ -186,46 +186,32 @@ void ProfileImplIOData::Handle::Init(
   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*
@@ -241,20 +227,34 @@ 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());