Remove stl_util's STLDeleteContainerPairPointers.

chrome/browser/safe_browsing/client_side_detection_service.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/safe_browsing/client_side_detection_service.h"
 
 #include <algorithm>
 #include <memory>
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/single_thread_task_runner.h"
 #include "base/stl_util.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/safe_browsing/client_model.pb.h"
@@ skipping 44 matching lines @@
 const int ClientSideDetectionService::kReportsIntervalDays = 1;
 const int ClientSideDetectionService::kMaxReportsPerInterval = 3;
 const int ClientSideDetectionService::kNegativeCacheIntervalDays = 1;
 const int ClientSideDetectionService::kPositiveCacheIntervalMinutes = 30;
 
 const char ClientSideDetectionService::kClientReportPhishingUrl[] =
     "https://sb-ssl.google.com/safebrowsing/clientreport/phishing";
 const char ClientSideDetectionService::kClientReportMalwareUrl[] =
     "https://sb-ssl.google.com/safebrowsing/clientreport/malware-check";
 
-struct ClientSideDetectionService::ClientReportInfo {
+struct ClientSideDetectionService::ClientPhishingReportInfo {
+  std::unique_ptr<net::URLFetcher> fetcher;
   ClientReportPhishingRequestCallback callback;
   GURL phishing_url;
 };
 
 struct ClientSideDetectionService::ClientMalwareReportInfo {
+  std::unique_ptr<net::URLFetcher> fetcher;
   ClientReportMalwareRequestCallback callback;
   // This is the original landing url, may not be the malware url.
   GURL original_url;
 };
 
 ClientSideDetectionService::CacheState::CacheState(bool phish, base::Time time)
     : is_phishing(phish),
       timestamp(time) {}
 
 ClientSideDetectionService::ClientSideDetectionService(
     net::URLRequestContextGetter* request_context_getter)
     : enabled_(false),
       request_context_getter_(request_context_getter),
       weak_factory_(this) {
   base::Closure update_renderers =
       base::Bind(&ClientSideDetectionService::SendModelToRenderers,
                  base::Unretained(this));
   model_loader_standard_.reset(
       new ModelLoader(update_renderers, request_context_getter, false));
   model_loader_extended_.reset(
       new ModelLoader(update_renderers, request_context_getter, true));
 
   registrar_.Add(this, content::NOTIFICATION_RENDERER_PROCESS_CREATED,
                  content::NotificationService::AllBrowserContextsAndSources());
 }
 
 ClientSideDetectionService::~ClientSideDetectionService() {
   weak_factory_.InvalidateWeakPtrs();
-  base::STLDeleteContainerPairPointers(client_phishing_reports_.begin(),
-                                       client_phishing_reports_.end());
-  client_phishing_reports_.clear();
-  base::STLDeleteContainerPairPointers(client_malware_reports_.begin(),
-                                       client_malware_reports_.end());
-  client_malware_reports_.clear();
 }
 
 // static
 ClientSideDetectionService* ClientSideDetectionService::Create(
     net::URLRequestContextGetter* request_context_getter) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   return new ClientSideDetectionService(request_context_getter);
 }
 
 void ClientSideDetectionService::SetEnabledAndRefreshState(bool enabled) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   SendModelToRenderers();  // always refresh the renderer state
   if (enabled == enabled_)
     return;
   enabled_ = enabled;
   if (enabled_) {
     // Refresh the models when the service is enabled.  This can happen when
     // either of the preferences are toggled, or early during startup if
     // safe browsing is already enabled. In a lot of cases the model will be
     // in the cache so it  won't actually be fetched from the network.
     // We delay the first model fetches to avoid slowing down browser startup.
     model_loader_standard_->ScheduleFetch(kInitialClientModelFetchDelayMs);
     model_loader_extended_->ScheduleFetch(kInitialClientModelFetchDelayMs);
   } else {
     // Cancel model loads in progress.
     model_loader_standard_->CancelFetcher();
     model_loader_extended_->CancelFetcher();
     // Invoke pending callbacks with a false verdict.
-    for (std::map<const net::URLFetcher*, ClientReportInfo*>::iterator it =
-             client_phishing_reports_.begin();
+    for (auto it = client_phishing_reports_.begin();
          it != client_phishing_reports_.end(); ++it) {
-      ClientReportInfo* info = it->second;
+      ClientPhishingReportInfo* info = it->second.get();
       if (!info->callback.is_null())
         info->callback.Run(info->phishing_url, false);
     }
-    base::STLDeleteContainerPairPointers(client_phishing_reports_.begin(),
-                                         client_phishing_reports_.end());
     client_phishing_reports_.clear();
-    for (std::map<const net::URLFetcher*, ClientMalwareReportInfo*>::iterator it
-             = client_malware_reports_.begin();
+    for (auto it = client_malware_reports_.begin();
          it != client_malware_reports_.end(); ++it) {
-      ClientMalwareReportInfo* info = it->second;
+      ClientMalwareReportInfo* info = it->second.get();
       if (!info->callback.is_null())
         info->callback.Run(info->original_url, info->original_url, false);
     }
-    base::STLDeleteContainerPairPointers(client_malware_reports_.begin(),
-                                         client_malware_reports_.end());
     client_malware_reports_.clear();
     cache_.clear();
   }
 }
 
 void ClientSideDetectionService::SendClientReportPhishingRequest(
     ClientPhishingRequest* verdict,
     bool is_extended_reporting,
     const ClientReportPhishingRequestCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ skipping 24 matching lines @@
   }
 
   return address.IsReserved();
 }
 
 void ClientSideDetectionService::OnURLFetchComplete(
     const net::URLFetcher* source) {
   std::string data;
   source->GetResponseAsString(&data);
 
-  if (client_phishing_reports_.find(source) != client_phishing_reports_.end()) {
+  if (base::ContainsKey(client_phishing_reports_, source)) {
     HandlePhishingVerdict(source, source->GetURL(), source->GetStatus(),
                           source->GetResponseCode(), data);
-  } else if (client_malware_reports_.find(source) !=
-             client_malware_reports_.end()) {
+  } else if (base::ContainsKey(client_malware_reports_, source)) {
     HandleMalwareVerdict(source, source->GetURL(), source->GetStatus(),
                          source->GetResponseCode(), data);
   } else {
     NOTREACHED();
   }
 }
 
 void ClientSideDetectionService::Observe(
     int type,
     const content::NotificationSource& source,
@@ skipping 65 matching lines @@
 
   std::string request_data;
   if (!request->SerializeToString(&request_data)) {
     UMA_HISTOGRAM_COUNTS("SBClientPhishing.RequestNotSerialized", 1);
     DVLOG(1) << "Unable to serialize the CSD request. Proto file changed?";
     if (!callback.is_null())
       callback.Run(GURL(request->url()), false);
     return;
   }
 
   net::URLFetcher* fetcher =

[mattm, 2016/08/16 20:41:22]

Would be cleaner to store the temporaries as unique_ptrs instead of bare
pointers that later get wrapped.

[Avi (use Gerrit), 2016/08/17 18:25:37]

Hmmm. Once we move them, then we can't use the temporaries. I'll rewrite but I'm
not sure how much cleaner it is.

       net::URLFetcher::Create(0 /* ID used for testing */,
                               GetClientReportUrl(kClientReportPhishingUrl),
                               net::URLFetcher::POST, this).release();
 
   // Remember which callback and URL correspond to the current fetcher object.
-  ClientReportInfo* info = new ClientReportInfo;
+  ClientPhishingReportInfo* info = new ClientPhishingReportInfo;
+  info->fetcher = base::WrapUnique(fetcher);
   info->callback = callback;
   info->phishing_url = GURL(request->url());
-  client_phishing_reports_[fetcher] = info;
+  client_phishing_reports_[fetcher] = base::WrapUnique(info);
 
   fetcher->SetLoadFlags(net::LOAD_DISABLE_CACHE);
   fetcher->SetRequestContext(request_context_getter_.get());
   fetcher->SetUploadData("application/octet-stream", request_data);
   fetcher->Start();
 
   // Record that we made a request
   phishing_report_times_.push(base::Time::Now());
 }
 
@@ skipping 18 matching lines @@
     return;
   }
 
   net::URLFetcher* fetcher =
       net::URLFetcher::Create(0 /* ID used for testing */,
                               GetClientReportUrl(kClientReportMalwareUrl),
                               net::URLFetcher::POST, this).release();
 
   // Remember which callback and URL correspond to the current fetcher object.
   ClientMalwareReportInfo* info = new ClientMalwareReportInfo;
+  info->fetcher = base::WrapUnique(fetcher);
   info->callback = callback;
   info->original_url = GURL(request->url());
-  client_malware_reports_[fetcher] = info;
+  client_malware_reports_[fetcher] = base::WrapUnique(info);
 
   fetcher->SetLoadFlags(net::LOAD_DISABLE_CACHE);
   fetcher->SetRequestContext(request_context_getter_.get());
   fetcher->SetUploadData("application/octet-stream", request_data);
   fetcher->Start();
 
   UMA_HISTOGRAM_ENUMERATION("SBClientMalware.SentReports",
                             REPORT_SENT, REPORT_RESULT_MAX);
 
   UMA_HISTOGRAM_COUNTS("SBClientMalware.IPBlacklistRequestPayloadSize",
                        request_data.size());
 
   // Record that we made a malware request
   malware_report_times_.push(base::Time::Now());
 }
 
 
 void ClientSideDetectionService::HandlePhishingVerdict(
     const net::URLFetcher* source,
     const GURL& url,
     const net::URLRequestStatus& status,
     int response_code,
     const std::string& data) {
   ClientPhishingResponse response;
-  std::unique_ptr<ClientReportInfo> info(client_phishing_reports_[source]);
+  std::unique_ptr<ClientPhishingReportInfo> info =
+      std::move(client_phishing_reports_[source]);
+  client_phishing_reports_.erase(source);
+
   bool is_phishing = false;
   if (status.is_success() && net::HTTP_OK == response_code &&
       response.ParseFromString(data)) {
     // Cache response, possibly flushing an old one.
     cache_[info->phishing_url] =
-        make_linked_ptr(new CacheState(response.phishy(), base::Time::Now()));
+        base::WrapUnique(new CacheState(response.phishy(), base::Time::Now()));
     is_phishing = response.phishy();
   } else {
     DLOG(ERROR) << "Unable to get the server verdict for URL: "
                 << info->phishing_url << " status: " << status.status() << " "
                 << "response_code:" << response_code;
   }
   if (!info->callback.is_null())
     info->callback.Run(info->phishing_url, is_phishing);
-  client_phishing_reports_.erase(source);
-  delete source;
 }
 
 void ClientSideDetectionService::HandleMalwareVerdict(
     const net::URLFetcher* source,
     const GURL& url,
     const net::URLRequestStatus& status,
     int response_code,
     const std::string& data) {
   if (status.is_success()) {
     UMA_HISTOGRAM_SPARSE_SLOWLY(
         "SBClientMalware.IPBlacklistRequestResponseCode", response_code);
   }
   // status error is negative, so we put - in front of it.
   UMA_HISTOGRAM_SPARSE_SLOWLY(
       "SBClientMalware.IPBlacklistRequestNetError", -status.error());
 
   ClientMalwareResponse response;
-  std::unique_ptr<ClientMalwareReportInfo> info(
-      client_malware_reports_[source]);
+  std::unique_ptr<ClientMalwareReportInfo> info =
+      std::move(client_malware_reports_[source]);
+  client_malware_reports_.erase(source);
+
   bool should_blacklist = false;
   if (status.is_success() && net::HTTP_OK == response_code &&
       response.ParseFromString(data)) {
     should_blacklist = response.blacklist();
   } else {
     DLOG(ERROR) << "Unable to get the server verdict for URL: "
                 << info->original_url << " status: " << status.status() << " "
                 << "response_code:" << response_code;
   }
 
   if (!info->callback.is_null()) {
     if (response.has_bad_url())
       info->callback.Run(info->original_url, GURL(response.bad_url()),
                          should_blacklist);
     else
       info->callback.Run(info->original_url, info->original_url, false);
   }
 
-  client_malware_reports_.erase(source);
-  delete source;
 }
 
 bool ClientSideDetectionService::IsInCache(const GURL& url) {
   UpdateCache();
 
   return cache_.find(url) != cache_.end();
 }
 
 bool ClientSideDetectionService::GetValidCachedResult(const GURL& url,
                                                       bool* is_phishing) {
   UpdateCache();
 
-  PhishingCache::iterator it = cache_.find(url);
+  auto it = cache_.find(url);
   if (it == cache_.end()) {
     return false;
   }
 
   // We still need to check if the result is valid.
   const CacheState& cache_state = *it->second;
   if (cache_state.is_phishing ?
       cache_state.timestamp > base::Time::Now() -
           base::TimeDelta::FromMinutes(kPositiveCacheIntervalMinutes) :
       cache_state.timestamp > base::Time::Now() -
@@ skipping 10 matching lines @@
   // could be used for this purpose even if we will not use the entry to
   // satisfy the request from the cache.
   base::TimeDelta positive_cache_interval =
       std::max(base::TimeDelta::FromMinutes(kPositiveCacheIntervalMinutes),
                base::TimeDelta::FromDays(kReportsIntervalDays));
   base::TimeDelta negative_cache_interval =
       std::max(base::TimeDelta::FromDays(kNegativeCacheIntervalDays),
                base::TimeDelta::FromDays(kReportsIntervalDays));
 
   // Remove elements from the cache that will no longer be used.
-  for (PhishingCache::iterator it = cache_.begin(); it != cache_.end();) {
+  for (auto it = cache_.begin(); it != cache_.end();) {
     const CacheState& cache_state = *it->second;
     if (cache_state.is_phishing ?
         cache_state.timestamp > base::Time::Now() - positive_cache_interval :
         cache_state.timestamp > base::Time::Now() - negative_cache_interval) {
       ++it;
     } else {
       cache_.erase(it++);
     }
   }
 }
@@ skipping 34 matching lines @@
     const std::string& report_url) {
   GURL url(report_url);
   std::string api_key = google_apis::GetAPIKey();
   if (!api_key.empty())
     url = url.Resolve("?key=" + net::EscapeQueryParamValue(api_key, true));
 
   return url;
 }
 
 }  // namespace safe_browsing