Remove stl_util's deletion functions from net/dns/.

net/dns/host_resolver_impl.cc

Index: net/dns/host_resolver_impl.cc
diff --git a/net/dns/host_resolver_impl.cc b/net/dns/host_resolver_impl.cc
index bfff315cb84424a60aaddfe011c09a39b71a689f..dcd9e4e7cf3422aae7b56497c16ac5befa0b65a4 100644
--- a/net/dns/host_resolver_impl.cc
+++ b/net/dns/host_resolver_impl.cc
@@ -30,7 +30,6 @@
 #include "base/metrics/sparse_histogram.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/single_thread_task_runner.h"
-#include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
@@ -1735,9 +1734,7 @@ class HostResolverImpl::Job : public PrioritizedDispatcher::Job,
     // new job with the same key in case one of the OnComplete callbacks decides
     // to spawn one. Consequently, the job deletes itself when CompleteRequests
     // is done.
-    std::unique_ptr<Job> self_deleter(this);
-
-    resolver_->RemoveJob(this);
+    std::unique_ptr<Job> self_deleter = resolver_->RemoveJob(this);
 
     if (is_running()) {
       if (is_proc_running()) {
@@ -1895,9 +1892,9 @@ HostResolverImpl::HostResolverImpl(const Options& options, NetLog* net_log)
 HostResolverImpl::~HostResolverImpl() {
   // Prevent the dispatcher from starting new jobs.
   dispatcher_->SetLimitsToZero();
-  // It's now safe for Jobs to call KillDsnTask on destruction, because
+  // It's now safe for Jobs to call KillDnsTask on destruction, because
   // OnJobComplete will not start any new jobs.
-  base::STLDeleteValues(&jobs_);
+  jobs_.clear();

[Avi (use Gerrit), 2016/11/10 01:16:08]

This is not safe.

This will destroy the map with the Job objects, which, in their destructors will
call back into RemoveJob, which will mess with the map, re-entrantly.

Rethinking this.

 
   NetworkChangeNotifier::RemoveIPAddressObserver(this);
   NetworkChangeNotifier::RemoveConnectionTypeObserver(this);
@@ -1966,9 +1963,9 @@ int HostResolverImpl::Resolve(const RequestInfo& info,
         return rv;
       }
     }
-    jobs_.insert(jobit, std::make_pair(key, job));
+    jobs_.insert(jobit, std::make_pair(key, base::WrapUnique(job)));
   } else {
-    job = jobit->second;
+    job = jobit->second.get();
   }
 
   // Can't complete synchronously. Create and attach request.
@@ -2300,11 +2297,16 @@ void HostResolverImpl::CacheResult(const Key& key,
     cache_->Set(key, entry, base::TimeTicks::Now(), ttl);
 }
 
-void HostResolverImpl::RemoveJob(Job* job) {
+std::unique_ptr<HostResolverImpl::Job> HostResolverImpl::RemoveJob(Job* job) {
   DCHECK(job);
   JobMap::iterator it = jobs_.find(job->key());
-  if (it != jobs_.end() && it->second == job)
+  if (it != jobs_.end() && it->second.get() == job) {
+    std::unique_ptr<Job> job = std::move(it->second);
     jobs_.erase(it);
+    return job;
+  }
+
+  return nullptr;
 }
 
 HostResolverImpl::Key HostResolverImpl::GetEffectiveKeyForRequest(
@@ -2359,9 +2361,9 @@ void HostResolverImpl::AbortAllInProgressJobs() {
   // first collect and remove all running jobs from |jobs_|.
   std::vector<std::unique_ptr<Job>> jobs_to_abort;
   for (JobMap::iterator it = jobs_.begin(); it != jobs_.end(); ) {
-    Job* job = it->second;
+    Job* job = it->second.get();
     if (job->is_running()) {
-      jobs_to_abort.push_back(base::WrapUnique(job));
+      jobs_to_abort.push_back(std::move(it->second));
       jobs_.erase(it++);
     } else {
       DCHECK(job->is_queued());
@@ -2414,7 +2416,7 @@ void HostResolverImpl::TryServingAllJobsFromHosts() {
   base::WeakPtr<HostResolverImpl> self = weak_ptr_factory_.GetWeakPtr();
 
   for (JobMap::iterator it = jobs_.begin(); self.get() && it != jobs_.end();) {
-    Job* job = it->second;
+    Job* job = it->second.get();
     ++it;
     // This could remove |job| from |jobs_|, but iterator will remain valid.
     job->ServeFromHosts();