Domain Reliability: Don't crash on shutdown with uploads pending

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 162 matching lines @@
 
   io_data_->lazy_params_.reset(lazy_params);
 
   // Keep track of profile path and cache sizes separately so we can use them
   // on demand when creating storage isolated URLRequestContextGetters.
   io_data_->profile_path_ = profile_path;
   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_->domain_reliability_monitor_ = domain_reliability_monitor.release();

[davidben, 2016/09/26 21:14:48]

This part and the WrapUnique below seems kind of off. It seems there's a window
where the monitor is unowned and then you convert a raw pointer into an owned
pointer.

mmenke probably should confirm this is sane, but perhaps we should put a
std::unique_ptr<Monitor> in the LazyParams, have that own it, and then
InitializeInternal below std::moves it?

[mmenke, 2016/09/26 21:30:09]

Yea, this seems like a problem, and adding it to LazyParams does seem like the
best solution here.

Hrm...  We're also not hooking up DomainReliabilityMonitor to isolated app
URLRequestContexts.  Should we be?

[Julia Tuttle, 2016/09/27 16:49:59]

Done.

[Julia Tuttle, 2016/09/27 16:49:59]

Cool, done.
Not sure, but that'd be a separate CL.

 
   io_data_->InitializeMetricsEnabledStateOnUIThread();
-  if (io_data_->domain_reliability_monitor_)
-    io_data_->domain_reliability_monitor_->MoveToNetworkThread();
+  if (domain_reliability_monitor)
+    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(
@@ skipping 220 matching lines @@
     : cache_max_size(0),
       media_cache_max_size(0),
       session_cookie_mode(
           content::CookieStoreConfig::EPHEMERAL_SESSION_COOKIES) {}
 
 ProfileImplIOData::LazyParams::~LazyParams() {}
 
 ProfileImplIOData::ProfileImplIOData()
     : ProfileIOData(Profile::REGULAR_PROFILE),
       http_server_properties_manager_(NULL),
+      domain_reliability_monitor_(nullptr),
       app_cache_max_size_(0),
       app_media_cache_max_size_(0) {
 }
 
 ProfileImplIOData::~ProfileImplIOData() {
+  if (domain_reliability_monitor_)
+    domain_reliability_monitor_->Shutdown();
+
   DestroyResourceContext();
 
   if (media_request_context_)
     media_request_context_->AssertNoURLRequests();
 }
 
 void ProfileImplIOData::InitializeInternal(
     std::unique_ptr<ChromeNetworkDelegate> chrome_network_delegate,
     ProfileParams* profile_params,
     content::ProtocolHandlerMap* protocol_handlers,
     content::URLRequestInterceptorScopedVector request_interceptors) const {
   // Set up a persistent store for use by the network stack on the IO thread.
   base::FilePath network_json_store_filepath(
       profile_path_.Append(chrome::kNetworkPersistentStateFilename));
   network_json_store_ = new JsonPrefStore(
       network_json_store_filepath,
       JsonPrefStore::GetTaskRunnerForFile(network_json_store_filepath,
                                           BrowserThread::GetBlockingPool()),
       std::unique_ptr<PrefFilter>());
   network_json_store_->ReadPrefsAsync(nullptr);
 
   net::URLRequestContext* main_context = main_request_context();
 
   IOThread* const io_thread = profile_params->io_thread;
   IOThread::Globals* const io_thread_globals = io_thread->globals();
 
   if (domain_reliability_monitor_) {
     domain_reliability::DomainReliabilityMonitor* monitor =
-        domain_reliability_monitor_.get();
+        domain_reliability_monitor_;
     monitor->InitURLRequestContext(main_context);
     monitor->AddBakedInConfigs();
     monitor->SetDiscardUploads(!GetMetricsEnabledStateOnIOThread());
-    chrome_network_delegate->set_domain_reliability_monitor(monitor);
+    chrome_network_delegate->set_domain_reliability_monitor(
+        base::WrapUnique(monitor));
   }
 
   ApplyProfileParamsToContext(main_context);
 
   if (http_server_properties_manager_)
     http_server_properties_manager_->InitializeOnNetworkThread();
 
   main_context->set_transport_security_state(transport_security_state());
   main_context->set_ct_policy_enforcer(
       io_thread_globals->ct_policy_enforcer.get());
@@ skipping 328 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);
 }