Fix crash during CertificateReportingService shutdown

chrome/browser/safe_browsing/certificate_reporting_service.cc

 // Copyright 2016 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/safe_browsing/certificate_reporting_service.h"
 
 #include "base/bind_helpers.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/time/clock.h"
 #include "chrome/browser/browser_process.h"
@@ skipping 17 matching lines @@
                        const CertificateReportingService::Report& item2) {
   return item1.creation_time > item2.creation_time;
 }
 
 // Records an UMA histogram of the net errors when certificate reports
 // fail to send.
 void RecordUMAOnFailure(int net_error) {
   UMA_HISTOGRAM_SPARSE_SLOWLY("SSL.CertificateErrorReportFailure", -net_error);
 }
 
+void CleanupOnIOThread(
+    std::unique_ptr<CertificateReportingService::Reporter> reporter) {
+  reporter.reset();
+}
+
 }  // namespace
 
 CertificateReportingService::BoundedReportList::BoundedReportList(
     size_t max_size)
     : max_size_(max_size) {
   CHECK(max_size <= 20)
       << "Current implementation is not efficient for a large list.";
   DCHECK(thread_checker_.CalledOnValidThread());
 }
 
@@ skipping 109 matching lines @@
 CertificateReportingService::CertificateReportingService(
     safe_browsing::SafeBrowsingService* safe_browsing_service,
     scoped_refptr<net::URLRequestContextGetter> url_request_context_getter,
     Profile* profile,
     uint8_t server_public_key[/* 32 */],
     uint32_t server_public_key_version,
     size_t max_queued_report_count,
     base::TimeDelta max_report_age,
     base::Clock* clock)
     : pref_service_(*profile->GetPrefs()),
-      enabled_(true),
       url_request_context_(nullptr),
       max_queued_report_count_(max_queued_report_count),
       max_report_age_(max_report_age),
       clock_(clock),
       server_public_key_(server_public_key),
       server_public_key_version_(server_public_key_version) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   DCHECK(clock_);
   // Subscribe to SafeBrowsing shutdown notifications.
   safe_browsing_service_shutdown_subscription_ =
       safe_browsing_service->RegisterShutdownCallback(base::Bind(
           &CertificateReportingService::Shutdown, base::Unretained(this)));
 
   // Subscribe to SafeBrowsing preference change notifications.
   safe_browsing_state_subscription_ =
       safe_browsing_service->RegisterStateCallback(
           base::Bind(&CertificateReportingService::OnPreferenceChanged,
                      base::Unretained(this)));
 
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&CertificateReportingService::InitializeOnIOThread,
-                 base::Unretained(this), enabled_, url_request_context_getter,
+                 base::Unretained(this), true, url_request_context_getter,
                  max_queued_report_count_, max_report_age_, clock_,
                  server_public_key_, server_public_key_version_));
 }
 
 CertificateReportingService::~CertificateReportingService() {
   DCHECK(!reporter_);
 }
 
 void CertificateReportingService::Shutdown() {
   // Shutdown will be called twice: Once after SafeBrowsing shuts down, and once
   // when all KeyedServices shut down. All calls after the first one are no-op.
-  enabled_ = false;
-  Reset();
+  url_request_context_ = nullptr;
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&CleanupOnIOThread, base::Passed(std::move(reporter_))));
 }
 
 void CertificateReportingService::Send(const std::string& serialized_report) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (!reporter_) {
     return;
   }
   content::BrowserThread::PostTask(
       content::BrowserThread::IO, FROM_HERE,
       base::Bind(&CertificateReportingService::Reporter::Send,
@@ skipping 22 matching lines @@
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   DCHECK(!url_request_context_);
   url_request_context_ = url_request_context_getter->GetURLRequestContext();
   ResetOnIOThread(enabled, url_request_context_, max_queued_report_count,
                   max_report_age, clock, server_public_key,
                   server_public_key_version);
 }
 
 void CertificateReportingService::SetEnabled(bool enabled) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  enabled_ = enabled;
-  Reset();
+  // Don't reset if the service is already shut down.
+  if (!url_request_context_)
+    return;
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&CertificateReportingService::ResetOnIOThread,
+                 base::Unretained(this), enabled, url_request_context_,
+                 max_queued_report_count_, max_report_age_, clock_,
+                 server_public_key_, server_public_key_version_));
 }
 
 CertificateReportingService::Reporter*
 CertificateReportingService::GetReporterForTesting() const {
   return reporter_.get();
 }
 
 // static
 GURL CertificateReportingService::GetReportingURLForTesting() {
   return GURL(kExtendedReportingUploadUrl);
 }
 
-void CertificateReportingService::Reset() {
-  content::BrowserThread::PostTask(
-      content::BrowserThread::IO, FROM_HERE,
-      base::Bind(&CertificateReportingService::ResetOnIOThread,
-                 base::Unretained(this), enabled_, url_request_context_,
-                 max_queued_report_count_, max_report_age_, clock_,
-                 server_public_key_, server_public_key_version_));
-}
-
 void CertificateReportingService::ResetOnIOThread(
     bool enabled,
     net::URLRequestContext* url_request_context,
     size_t max_queued_report_count,
     base::TimeDelta max_report_age,
     base::Clock* clock,
     uint8_t* const server_public_key,
     uint32_t server_public_key_version) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   // url_request_context_ is null during shutdown.
   if (!enabled || !url_request_context) {
-    reporter_.reset(nullptr);
+    reporter_.reset();
     return;
   }
   std::unique_ptr<certificate_reporting::ErrorReporter> error_reporter;
   if (server_public_key) {
     // Only used in tests.
     std::unique_ptr<net::ReportSender> report_sender(new net::ReportSender(
         url_request_context, net::ReportSender::DO_NOT_SEND_COOKIES));
     error_reporter.reset(new certificate_reporting::ErrorReporter(
         GURL(kExtendedReportingUploadUrl), server_public_key,
         server_public_key_version, std::move(report_sender)));
   } else {
     error_reporter.reset(new certificate_reporting::ErrorReporter(
         url_request_context, GURL(kExtendedReportingUploadUrl),
         net::ReportSender::DO_NOT_SEND_COOKIES));
   }
-
   reporter_.reset(
       new Reporter(std::move(error_reporter),
                    std::unique_ptr<BoundedReportList>(
                        new BoundedReportList(max_queued_report_count)),
                    clock, max_report_age, true /* retries_enabled */));
 }
 
 void CertificateReportingService::OnPreferenceChanged() {
   safe_browsing::SafeBrowsingService* safe_browsing_service_ =
       g_browser_process->safe_browsing_service();
   const bool enabled = safe_browsing_service_ &&
                        safe_browsing_service_->enabled_by_prefs() &&
                        safe_browsing::IsExtendedReportingEnabled(pref_service_);
   SetEnabled(enabled);
 }