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

components/domain_reliability/uploader.cc

 // Copyright 2014 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 "components/domain_reliability/uploader.h"
 
 #include <utility>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 40 matching lines @@
 
 class DomainReliabilityUploaderImpl
     : public DomainReliabilityUploader, net::URLFetcherDelegate {
  public:
   DomainReliabilityUploaderImpl(
       MockableTime* time,
       const scoped_refptr<
           net::URLRequestContextGetter>& url_request_context_getter)
       : time_(time),
         url_request_context_getter_(url_request_context_getter),
-        discard_uploads_(true) {}
+        discard_uploads_(true),
+        shutdown_(false) {}
 
-  ~DomainReliabilityUploaderImpl() override {}
+  ~DomainReliabilityUploaderImpl() override {
+    DCHECK(shutdown_);

[mmenke, 2016/12/14 19:12:17]

include base/logging.h

[Julia Tuttle, 2016/12/15 21:53:00]

Done.

+  }
 
   // DomainReliabilityUploader implementation:
   void UploadReport(
       const std::string& report_json,
       int max_upload_depth,
       const GURL& upload_url,
       const DomainReliabilityUploader::UploadCallback& callback) override {
     VLOG(1) << "Uploading report to " << upload_url;
     VLOG(2) << "Report JSON: " << report_json;
 
-    if (discard_uploads_) {
+    if (discard_uploads_ || shutdown_) {
       VLOG(1) << "Discarding report instead of uploading.";
       UploadResult result;
       result.status = UploadResult::SUCCESS;
       callback.Run(result);
       return;
     }
 
     std::unique_ptr<net::URLFetcher> owned_fetcher =
         net::URLFetcher::Create(0, upload_url, net::URLFetcher::POST, this);
     net::URLFetcher* fetcher = owned_fetcher.get();
@@ skipping 15 matching lines @@
                                now - last_upload_start_time_);
     }
     last_upload_start_time_ = now;
   }
 
   void set_discard_uploads(bool discard_uploads) override {
     discard_uploads_ = discard_uploads;
     VLOG(1) << "Setting discard_uploads to " << discard_uploads;
   }
 
+  void Shutdown() override {
+    DCHECK(!shutdown_);
+    shutdown_ = true;
+    upload_callbacks_.clear();
+  }
+
   // net::URLFetcherDelegate implementation:
   void OnURLFetchComplete(const net::URLFetcher* fetcher) override {
     DCHECK(fetcher);
 
     auto callback_it = upload_callbacks_.find(fetcher);
     DCHECK(callback_it != upload_callbacks_.end());
 
     int net_error = GetNetErrorFromURLRequestStatus(fetcher->GetStatus());
     int http_response_code = fetcher->GetResponseCode();
     base::TimeDelta retry_after;
@@ skipping 28 matching lines @@
     upload_callbacks_.erase(callback_it);
   }
 
  private:
   using DomainReliabilityUploader::UploadCallback;
 
   MockableTime* time_;
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
   std::map<const net::URLFetcher*,
            std::pair<std::unique_ptr<net::URLFetcher>, UploadCallback>>
       upload_callbacks_;

[mmenke, 2016/12/14 19:12:17]

A bit afield from this CL, but do you mind rename this map to upload_tasks_ or
something?  upload_callback_ doesn't like something that has the implication
"This owns URLFetchers."

[Julia Tuttle, 2016/12/15 21:53:00]

Sure, I'll just call it uploads_.

   bool discard_uploads_;
   base::TimeTicks last_upload_start_time_;
+  bool shutdown_;
 };
 
 }  // namespace
 
 DomainReliabilityUploader::DomainReliabilityUploader() {}
 DomainReliabilityUploader::~DomainReliabilityUploader() {}
 
 // static
 std::unique_ptr<DomainReliabilityUploader> DomainReliabilityUploader::Create(
     MockableTime* time,
@@ skipping 12 matching lines @@
 // static
 int DomainReliabilityUploader::GetURLRequestUploadDepth(
     const net::URLRequest& request) {
   UploadUserData* data = static_cast<UploadUserData*>(
       request.GetUserData(UploadUserData::kUserDataKey));
   if (!data)
     return 0;
   return data->depth();
 }
 
+void DomainReliabilityUploader::Shutdown() {}
+
 }  // namespace domain_reliability