Domain Reliability: More security review.

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 "base/bind.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/stl_util.h"
 #include "net/base/load_flags.h"
 #include "net/url_request/url_fetcher.h"
 #include "net/url_request/url_fetcher_delegate.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace domain_reliability {
 
 namespace {
 
 class DomainReliabilityUploaderImpl
     : public DomainReliabilityUploader, net::URLFetcherDelegate {

[Ryan Sleevi, 2014/04/26 02:37:39]

"git cl format" should clean this up for you, as it has my style-sense tingling.

[Deprecated (see juliatuttle), 2014/04/28 22:01:13]

Done.

  public:
   DomainReliabilityUploaderImpl(
       scoped_refptr<net::URLRequestContextGetter> url_request_context_getter)

[Ryan Sleevi, 2014/04/26 02:37:39]

const scoped_refptr<...>&

[Deprecated (see juliatuttle), 2014/04/28 22:01:13]

Done.

       : url_request_context_getter_(url_request_context_getter) {}
 
   virtual ~DomainReliabilityUploaderImpl() {
     // Delete any in-flight URLFetchers.
     STLDeleteContainerPairFirstPointers(
         upload_callbacks_.begin(), upload_callbacks_.end());
   }
 
   // DomainReliabilityUploader implementation:
   virtual void UploadReport(
@@ skipping 24 matching lines @@
     DCHECK(callback_it != upload_callbacks_.end());
 
     VLOG(1) << "Upload finished with " << fetcher->GetResponseCode();
 
     UMA_HISTOGRAM_SPARSE_SLOWLY("DomainReliability.UploadResponseCode",
                                 fetcher->GetResponseCode());
 
     bool success = fetcher->GetResponseCode() == 200;
     callback_it->second.Run(success);
 
     delete callback_it->first;

[Ryan Sleevi, 2014/04/26 02:37:39]

You don't have to de-constify this? (See line 74)

[Deprecated (see juliatuttle), 2014/04/28 22:01:13]

Apparently not!  Deleting a const pointer seems to be valid.

     upload_callbacks_.erase(callback_it);
   }
 
  private:
   using DomainReliabilityUploader::UploadCallback;
   typedef std::map<const net::URLFetcher*, UploadCallback> UploadCallbackMap;
 
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
   UploadCallbackMap upload_callbacks_;
 };
 
 }  // namespace
 
 DomainReliabilityUploader::DomainReliabilityUploader() {}
-
 DomainReliabilityUploader::~DomainReliabilityUploader() {}
 
 // static
 scoped_ptr<DomainReliabilityUploader> DomainReliabilityUploader::Create(
     scoped_refptr<net::URLRequestContextGetter> url_request_context_getter) {
   return scoped_ptr<DomainReliabilityUploader>(
       new DomainReliabilityUploaderImpl(url_request_context_getter));
 }
 
 }  // namespace domain_reliability