predictors: Return HostResolver::Request from content::PreresolveUrl()

This CL changes the signature of content::PreresolveUrl() so that the
method returns a net::HostResolver::Request object to a caller instead
of holding it inside of the callback state. It is done to guarantee
that there won't be hanging requests after a caller is shut down.

There are only two callers of this method:
chrome_browser_net::Predictor and predictors::PreconnectManager.
They become responsible for keeping Request objects alive.

Bug: 845648
Change-Id: Ie057cde65288341b14813d4df4d7d90a02f588b6
Reviewed-on: https://chromium-review.googlesource.com/1080818
Commit-Queue: Alexandr Ilin <alexilin@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Benoit L <lizeb@chromium.org>
Reviewed-by: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#564866}

chrome/browser/net/predictor.cc

@@ -416,21 +416,21 @@ void Predictor::DiscardAllResults() {
   // Step through result_, recording names of all hosts that can't be erased.
   // We can't erase anything being worked on.
   Results assignees;
-  for (Results::iterator it = results_.begin(); results_.end() != it; ++it) {
-    GURL url(it->first);
-    UrlInfo* info = &it->second;
-    DCHECK(info->HasUrl(url));
-    if (info->is_assigned()) {
-      info->SetPendingDeleteState();
-      assignees[url] = *info;
+  for (auto& result : results_) {
+    const GURL& url = result.first;
+    UrlInfo& info = result.second;
+    DCHECK(info.HasUrl(url));
+    if (info.is_assigned()) {
+      info.SetPendingDeleteState();
+      assignees[url] = std::move(info);
     }
   }
   DCHECK_LE(assignees.size(), max_concurrent_dns_lookups_);
   results_.clear();
   // Put back in the names being worked on.
-  for (Results::iterator it = assignees.begin(); assignees.end() != it; ++it) {
-    DCHECK(it->second.is_marked_to_delete());
-    results_[it->first] = it->second;
+  for (auto& assignee : assignees) {
+    DCHECK(assignee.second.is_marked_to_delete());
+    results_[assignee.first] = std::move(assignee.second);
   }
 }
 
@@ -920,10 +920,13 @@ void Predictor::StartSomeQueuedResolutions() {
     }
 
     info->SetPendingDeleteState();
+    std::unique_ptr<net::HostResolver::Request> request;
     int status =
         content::PreresolveUrl(url_request_context_getter_.get(), url,
                                base::Bind(&Predictor::OnLookupFinished,
-                                          io_weak_factory_->GetWeakPtr(), url));
+                                          io_weak_factory_->GetWeakPtr(), url),
+                               &request);
+    info->set_request(std::move(request));
     if (status == net::ERR_IO_PENDING) {
       // Will complete asynchronously.
       num_pending_lookups_++;

chrome/browser/net/url_info.cc

@@ -79,7 +79,8 @@ UrlInfo::UrlInfo()
       was_linked_(false) {
 }
 
-UrlInfo::UrlInfo(const UrlInfo& other) = default;
+UrlInfo::UrlInfo(UrlInfo&& other) = default;
+UrlInfo& UrlInfo::operator=(UrlInfo&& other) = default;
 
 UrlInfo::~UrlInfo() {}
 

chrome/browser/net/url_info.h

@@ -22,6 +22,7 @@
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "net/base/host_port_pair.h"
+#include "net/dns/host_resolver.h"
 #include "url/gurl.h"
 
 namespace chrome_browser_net {
@@ -72,7 +73,8 @@ class UrlInfo {
   // initializing of the Predictor's map (of info for Hostnames).
   UrlInfo();
 
-  UrlInfo(const UrlInfo& other);
+  UrlInfo(UrlInfo&& other);
+  UrlInfo& operator=(UrlInfo&& other);
 
   ~UrlInfo();
 
@@ -120,6 +122,10 @@ class UrlInfo {
   // transition, and this call is not made.
   void set_time(const base::TimeTicks& time) { time_ = time; }
 
+  void set_request(std::unique_ptr<net::HostResolver::Request> request) {
+    request_ = std::move(request);
+  }
+
  private:
   base::TimeDelta GetDuration() {
     base::TimeTicks old_time = time_;
@@ -159,9 +165,10 @@ class UrlInfo {
   // Record if the motivation for prefetching was ever a page-link-scan.
   bool was_linked_;
 
-  // We put these objects into a std::map, and hence we
-  // need some "evil" constructors.
-  // DISALLOW_COPY_AND_ASSIGN(UrlInfo);
+  // Request object cancels the request to the host resolver on deletion.
+  std::unique_ptr<net::HostResolver::Request> request_;
+
+  DISALLOW_COPY_AND_ASSIGN(UrlInfo);
 };
 
 }  // namespace chrome_browser_net

chrome/browser/predictors/preconnect_manager.cc

@@ -55,7 +55,7 @@ PreresolveJob::PreresolveJob(const GURL& url,
   DCHECK_GE(num_sockets, 0);
 }
 
-PreresolveJob::PreresolveJob(const PreresolveJob& other) = default;
+PreresolveJob::PreresolveJob(PreresolveJob&& other) = default;
 PreresolveJob::~PreresolveJob() = default;
 
 PreconnectManager::PreconnectManager(
@@ -86,8 +86,10 @@ void PreconnectManager::Start(const GURL& url,
 
   for (const auto& request : requests) {
     DCHECK(request.origin.GetOrigin() == request.origin);
-    queued_jobs_.emplace_back(request.origin, request.num_sockets,
-                              request.allow_credentials, info);
+    PreresolveJobId job_id = preresolve_jobs_.Add(
+        std::make_unique<PreresolveJob>(request.origin, request.num_sockets,
+                                        request.allow_credentials, info));
+    queued_jobs_.push_back(job_id);
   }
 
   TryToLaunchPreresolveJobs();
@@ -97,8 +99,9 @@ void PreconnectManager::StartPreresolveHost(const GURL& url) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   if (!url.SchemeIsHTTPOrHTTPS())
     return;
-  queued_jobs_.emplace_front(url.GetOrigin(), 0,
-                             kAllowCredentialsOnPreconnectByDefault, nullptr);
+  PreresolveJobId job_id = preresolve_jobs_.Add(std::make_unique<PreresolveJob>(
+      url.GetOrigin(), 0, kAllowCredentialsOnPreconnectByDefault, nullptr));
+  queued_jobs_.push_front(job_id);
 
   TryToLaunchPreresolveJobs();
 }
@@ -108,8 +111,11 @@ void PreconnectManager::StartPreresolveHosts(
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   // Push jobs in front of the queue due to higher priority.
   for (auto it = hostnames.rbegin(); it != hostnames.rend(); ++it) {
-    queued_jobs_.emplace_front(GURL("http://" + *it), 0,
-                               kAllowCredentialsOnPreconnectByDefault, nullptr);
+    PreresolveJobId job_id =
+        preresolve_jobs_.Add(std::make_unique<PreresolveJob>(
+            GURL("http://" + *it), 0, kAllowCredentialsOnPreconnectByDefault,
+            nullptr));
+    queued_jobs_.push_front(job_id);
   }
 
   TryToLaunchPreresolveJobs();
@@ -120,7 +126,9 @@ void PreconnectManager::StartPreconnectUrl(const GURL& url,
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
   if (!url.SchemeIsHTTPOrHTTPS())
     return;
-  queued_jobs_.emplace_front(url.GetOrigin(), 1, allow_credentials, nullptr);
+  PreresolveJobId job_id = preresolve_jobs_.Add(std::make_unique<PreresolveJob>(
+      url.GetOrigin(), 1, allow_credentials, nullptr));
+  queued_jobs_.push_front(job_id);
 
   TryToLaunchPreresolveJobs();
 }
@@ -145,12 +153,16 @@ void PreconnectManager::PreconnectUrl(const GURL& url,
                          num_sockets, allow_credentials);
 }
 
-int PreconnectManager::PreresolveUrl(
+std::pair<int, std::unique_ptr<net::HostResolver::Request>>
+PreconnectManager::PreresolveUrl(
     const GURL& url,
     const net::CompletionCallback& callback) const {
   DCHECK(url.GetOrigin() == url);
   DCHECK(url.SchemeIsHTTPOrHTTPS());
-  return content::PreresolveUrl(context_getter_.get(), url, callback);
+  std::unique_ptr<net::HostResolver::Request> request;
+  int status =
+      content::PreresolveUrl(context_getter_.get(), url, callback, &request);
+  return {status, std::move(request)};
 }
 
 void PreconnectManager::TryToLaunchPreresolveJobs() {
@@ -158,18 +170,21 @@ void PreconnectManager::TryToLaunchPreresolveJobs() {
 
   while (!queued_jobs_.empty() &&
          inflight_preresolves_count_ < kMaxInflightPreresolves) {
-    auto& job = queued_jobs_.front();
-    PreresolveInfo* info = job.info;
+    auto job_id = queued_jobs_.front();
+    queued_jobs_.pop_front();
+    PreresolveJob* job = preresolve_jobs_.Lookup(job_id);
+    DCHECK(job);
+    PreresolveInfo* info = job->info;
 
     if (!info || !info->was_canceled) {
       int status;
-      if (WouldLikelyProxyURL(job.url)) {
+      if (WouldLikelyProxyURL(job->url)) {
         // Skip preresolve and go straight to preconnect if a proxy is enabled.
         status = net::OK;
       } else {
-        status = PreresolveUrl(
-            job.url, base::Bind(&PreconnectManager::OnPreresolveFinished,
-                                weak_factory_.GetWeakPtr(), job));
+        std::tie(status, job->request) = PreresolveUrl(
+            job->url, base::Bind(&PreconnectManager::OnPreresolveFinished,
+                                 weak_factory_.GetWeakPtr(), job_id));
       }
 
       if (status == net::ERR_IO_PENDING) {
@@ -181,11 +196,12 @@ void PreconnectManager::TryToLaunchPreresolveJobs() {
         // Completed synchronously (was already cached by HostResolver), or else
         // there was (equivalently) some network error that prevents us from
         // finding the name. Status net::OK means it was "found."
-        FinishPreresolve(job, status == net::OK, true);
+        FinishPreresolve(job_id, status == net::OK, true);
       }
+    } else {
+      preresolve_jobs_.Remove(job_id);
     }
 
-    queued_jobs_.pop_front();
     if (info)
       --info->queued_count;
     if (info && info->is_done())
@@ -193,11 +209,14 @@ void PreconnectManager::TryToLaunchPreresolveJobs() {
   }
 }
 
-void PreconnectManager::OnPreresolveFinished(const PreresolveJob& job,
+void PreconnectManager::OnPreresolveFinished(PreresolveJobId job_id,
                                              int result) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  FinishPreresolve(job, result == net::OK, false);
-  PreresolveInfo* info = job.info;
+  PreresolveJob* job = preresolve_jobs_.Lookup(job_id);
+  DCHECK(job);
+  PreresolveInfo* info = job->info;
+
+  FinishPreresolve(job_id, result == net::OK, false);
   --inflight_preresolves_count_;
   if (info)
     --info->inflight_count;
@@ -206,19 +225,23 @@ void PreconnectManager::OnPreresolveFinished(const PreresolveJob& job,
   TryToLaunchPreresolveJobs();
 }
 
-void PreconnectManager::FinishPreresolve(const PreresolveJob& job,
+void PreconnectManager::FinishPreresolve(PreresolveJobId job_id,
                                          bool found,
                                          bool cached) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
-  PreresolveInfo* info = job.info;
+  PreresolveJob* job = preresolve_jobs_.Lookup(job_id);
+  DCHECK(job);
+  PreresolveInfo* info = job->info;
+
   bool need_preconnect =
-      found && job.need_preconnect() && (!info || !info->was_canceled);
+      found && job->need_preconnect() && (!info || !info->was_canceled);
   if (need_preconnect) {
-    PreconnectUrl(GetHSTSRedirect(job.url), info ? info->url : GURL(),
-                  job.num_sockets, job.allow_credentials);
+    PreconnectUrl(GetHSTSRedirect(job->url), info ? info->url : GURL(),
+                  job->num_sockets, job->allow_credentials);
   }
   if (info && found)
-    info->stats->requests_stats.emplace_back(job.url, cached, need_preconnect);
+    info->stats->requests_stats.emplace_back(job->url, cached, need_preconnect);
+  preresolve_jobs_.Remove(job_id);
 }
 
 void PreconnectManager::AllPreresolvesForUrlFinished(PreresolveInfo* info) {

chrome/browser/predictors/preconnect_manager.h

@@ -11,9 +11,11 @@
 #include <string>
 #include <vector>
 
+#include "base/containers/id_map.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "net/base/completion_callback.h"
+#include "net/dns/host_resolver.h"
 #include "net/http/http_request_info.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "url/gurl.h"
@@ -69,7 +71,7 @@ struct PreresolveJob {
                 int num_sockets,
                 bool allow_credentials,
                 PreresolveInfo* info);
-  PreresolveJob(const PreresolveJob& other);
+  PreresolveJob(PreresolveJob&& other);
   ~PreresolveJob();
   bool need_preconnect() const { return num_sockets > 0; }
 
@@ -81,6 +83,9 @@ struct PreresolveJob {
   // context and PreresolveInfo lifetime is tied to PreconnectManager.
   // May be equal to nullptr in case of detached job.
   PreresolveInfo* info;
+  std::unique_ptr<net::HostResolver::Request> request;
+
+  DISALLOW_COPY_AND_ASSIGN(PreresolveJob);
 };
 
 // PreconnectManager is responsible for preresolving and preconnecting to
@@ -132,20 +137,25 @@ class PreconnectManager {
                              const GURL& site_for_cookies,
                              int num_sockets,
                              bool allow_credentials) const;
-  virtual int PreresolveUrl(const GURL& url,
-                            const net::CompletionCallback& callback) const;
+  virtual std::pair<int, std::unique_ptr<net::HostResolver::Request>>
+  PreresolveUrl(const GURL& url, const net::CompletionCallback& callback) const;
 
  private:
+  using PreresolveJobMap = base::IDMap<std::unique_ptr<PreresolveJob>>;
+  using PreresolveJobId = PreresolveJobMap::KeyType;
+  friend class PreconnectManagerTest;
+
   void TryToLaunchPreresolveJobs();
-  void OnPreresolveFinished(const PreresolveJob& job, int result);
-  void FinishPreresolve(const PreresolveJob& job, bool found, bool cached);
+  void OnPreresolveFinished(PreresolveJobId job_id, int result);
+  void FinishPreresolve(PreresolveJobId job_id, bool found, bool cached);
   void AllPreresolvesForUrlFinished(PreresolveInfo* info);
   GURL GetHSTSRedirect(const GURL& url) const;
   bool WouldLikelyProxyURL(const GURL& url) const;
 
   base::WeakPtr<Delegate> delegate_;
   scoped_refptr<net::URLRequestContextGetter> context_getter_;
-  std::list<PreresolveJob> queued_jobs_;
+  std::list<PreresolveJobId> queued_jobs_;
+  PreresolveJobMap preresolve_jobs_;
   std::map<std::string, std::unique_ptr<PreresolveInfo>> preresolve_info_;
   size_t inflight_preresolves_count_ = 0;
 

chrome/browser/predictors/preconnect_manager_unittest.cc

@@ -48,12 +48,18 @@ class MockPreconnectManager : public PreconnectManager {
       base::WeakPtr<Delegate> delegate,
       scoped_refptr<net::URLRequestContextGetter> context_getter);
 
+  std::pair<int, std::unique_ptr<net::HostResolver::Request>> PreresolveUrl(
+      const GURL& url,
+      const net::CompletionCallback& callback) const override {
+    return {PreresolveUrlProxy(url, callback), nullptr};
+  }
+
   MOCK_CONST_METHOD4(PreconnectUrl,
                      void(const GURL& url,
                           const GURL& site_for_cookies,
                           int num_sockets,
                           bool allow_credentials));
-  MOCK_CONST_METHOD2(PreresolveUrl,
+  MOCK_CONST_METHOD2(PreresolveUrlProxy,
                      int(const GURL& url,
                          const net::CompletionCallback& callback));
 };
@@ -93,7 +99,7 @@ TEST_F(PreconnectManagerTest, TestStartOneUrlPreresolve) {
   GURL main_frame_url("http://google.com");
   GURL url_to_preresolve("http://cdn.google.com");
 
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(url_to_preresolve, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(url_to_preresolve, _))
       .WillOnce(Return(net::OK));
   EXPECT_CALL(*mock_delegate_, PreconnectFinishedProxy(main_frame_url));
   preconnect_manager_->Start(main_frame_url,
@@ -106,7 +112,7 @@ TEST_F(PreconnectManagerTest, TestStartOneUrlPreconnect) {
   GURL main_frame_url("http://google.com");
   GURL url_to_preconnect("http://cdn.google.com");
 
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(url_to_preconnect, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(url_to_preconnect, _))
       .WillOnce(Return(net::OK));
   EXPECT_CALL(*preconnect_manager_,
               PreconnectUrl(url_to_preconnect, main_frame_url, 1, true));
@@ -122,7 +128,7 @@ TEST_F(PreconnectManagerTest, TestStopOneUrlBeforePreconnect) {
   net::CompletionCallback callback;
 
   // Preconnect job isn't started before preresolve is completed asynchronously.
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(url_to_preconnect, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(url_to_preconnect, _))
       .WillOnce(DoAll(SaveArg<1>(&callback), Return(net::ERR_IO_PENDING)));
   preconnect_manager_->Start(main_frame_url,
                              {PreconnectRequest(url_to_preconnect, 1)});
@@ -138,7 +144,7 @@ TEST_F(PreconnectManagerTest, TestGetCallbackAfterDestruction) {
   GURL main_frame_url("http://google.com");
   GURL url_to_preconnect("http://cdn.google.com");
   net::CompletionCallback callback;
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(url_to_preconnect, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(url_to_preconnect, _))
       .WillOnce(DoAll(SaveArg<1>(&callback), Return(net::ERR_IO_PENDING)));
   preconnect_manager_->Start(main_frame_url,
                              {PreconnectRequest(url_to_preconnect, 1)});
@@ -159,7 +165,8 @@ TEST_F(PreconnectManagerTest, TestUnqueuedPreresolvesCanceled) {
     // Exactly PreconnectManager::kMaxInflightPreresolves should be preresolved.
     requests.emplace_back(
         GURL(base::StringPrintf("http://cdn%" PRIuS ".google.com", i)), 1);
-    EXPECT_CALL(*preconnect_manager_, PreresolveUrl(requests.back().origin, _))
+    EXPECT_CALL(*preconnect_manager_,
+                PreresolveUrlProxy(requests.back().origin, _))
         .WillOnce(
             DoAll(SaveArg<1>(&callbacks[i]), Return(net::ERR_IO_PENDING)));
   }
@@ -182,9 +189,9 @@ TEST_F(PreconnectManagerTest, TestTwoConcurrentMainFrameUrls) {
   GURL url_to_preconnect2("http://cdn.facebook.com");
   net::CompletionCallback callback2;
 
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(url_to_preconnect1, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(url_to_preconnect1, _))
       .WillOnce(DoAll(SaveArg<1>(&callback1), Return(net::ERR_IO_PENDING)));
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(url_to_preconnect2, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(url_to_preconnect2, _))
       .WillOnce(DoAll(SaveArg<1>(&callback2), Return(net::ERR_IO_PENDING)));
   preconnect_manager_->Start(main_frame_url1,
                              {PreconnectRequest(url_to_preconnect1, 1)});
@@ -214,7 +221,7 @@ TEST_F(PreconnectManagerTest, TestTwoConcurrentSameHostMainFrameUrls) {
   GURL main_frame_url2("http://google.com/search?query=dogs");
   GURL url_to_preconnect2("http://dogs.google.com");
 
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(url_to_preconnect1, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(url_to_preconnect1, _))
       .WillOnce(DoAll(SaveArg<1>(&callback1), Return(net::ERR_IO_PENDING)));
   preconnect_manager_->Start(main_frame_url1,
                              {PreconnectRequest(url_to_preconnect1, 1)});
@@ -234,7 +241,7 @@ TEST_F(PreconnectManagerTest, TestStartPreresolveHost) {
   GURL url("http://cdn.google.com/script.js");
   GURL origin("http://cdn.google.com");
 
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(origin, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(origin, _))
       .WillOnce(Return(net::OK));
   preconnect_manager_->StartPreresolveHost(url);
   // PreconnectFinished shouldn't be called.
@@ -250,9 +257,9 @@ TEST_F(PreconnectManagerTest, TestStartPreresolveHosts) {
   GURL cdn("http://cdn.google.com");
   GURL fonts("http://fonts.google.com");
 
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(cdn, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(cdn, _))
       .WillOnce(Return(net::OK));
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(fonts, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(fonts, _))
       .WillOnce(Return(net::OK));
   preconnect_manager_->StartPreresolveHosts({cdn.host(), fonts.host()});
   base::RunLoop().RunUntilIdle();
@@ -263,7 +270,7 @@ TEST_F(PreconnectManagerTest, TestStartPreconnectUrl) {
   GURL origin("http://cdn.google.com");
   bool allow_credentials = false;
 
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(origin, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(origin, _))
       .WillOnce(Return(net::OK));
   EXPECT_CALL(*preconnect_manager_,
               PreconnectUrl(origin, GURL(), 1, allow_credentials));
@@ -285,7 +292,8 @@ TEST_F(PreconnectManagerTest, TestDetachedRequestHasHigherPriority) {
   for (size_t i = 0; i < count; ++i) {
     requests.emplace_back(
         GURL(base::StringPrintf("http://cdn%" PRIuS ".google.com", i)), 0);
-    EXPECT_CALL(*preconnect_manager_, PreresolveUrl(requests.back().origin, _))
+    EXPECT_CALL(*preconnect_manager_,
+                PreresolveUrlProxy(requests.back().origin, _))
         .WillOnce(
             DoAll(SaveArg<1>(&callbacks[i]), Return(net::ERR_IO_PENDING)));
   }
@@ -300,13 +308,13 @@ TEST_F(PreconnectManagerTest, TestDetachedRequestHasHigherPriority) {
   Mock::VerifyAndClearExpectations(preconnect_manager_.get());
 
   net::CompletionCallback detached_callback;
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(detached_preresolve, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(detached_preresolve, _))
       .WillOnce(
           DoAll(SaveArg<1>(&detached_callback), Return(net::ERR_IO_PENDING)));
   callbacks[0].Run(net::OK);
 
   Mock::VerifyAndClearExpectations(preconnect_manager_.get());
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(queued_url, _))
+  EXPECT_CALL(*preconnect_manager_, PreresolveUrlProxy(queued_url, _))
       .WillOnce(Return(net::OK));
   detached_callback.Run(net::OK);
 
@@ -326,7 +334,8 @@ TEST_F(PreconnectManagerTest, TestHSTSRedirectRespectedForPreconnect) {
   GURL url("http://google.com/search");
   bool allow_credentials = false;
 
-  EXPECT_CALL(*preconnect_manager_, PreresolveUrl(GURL("http://google.com"), _))
+  EXPECT_CALL(*preconnect_manager_,
+              PreresolveUrlProxy(GURL("http://google.com"), _))
       .WillOnce(Return(net::OK));
   EXPECT_CALL(
       *preconnect_manager_,

content/browser/loader/resource_hints_impl.cc

@@ -10,7 +10,6 @@
 #include "content/public/browser/resource_hints.h"
 #include "net/base/address_list.h"
 #include "net/base/load_flags.h"
-#include "net/dns/host_resolver.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_stream_factory.h"
@@ -24,18 +23,7 @@ namespace content {
 
 namespace {
 
-class RequestHolder {
- public:
-  std::unique_ptr<net::HostResolver::Request>* GetRequest() {
-    return &request_;
-  }
-
- private:
-  std::unique_ptr<net::HostResolver::Request> request_;
-};
-
-void OnResolveComplete(std::unique_ptr<RequestHolder> request_holder,
-                       std::unique_ptr<net::AddressList> addresses,
+void OnResolveComplete(std::unique_ptr<net::AddressList> addresses,
                        const net::CompletionCallback& callback,
                        int result) {
   // Plumb the resolution result into the callback if future consumers want
@@ -93,7 +81,8 @@ void PreconnectUrl(net::URLRequestContextGetter* getter,
 
 int PreresolveUrl(net::URLRequestContextGetter* getter,
                   const GURL& url,
-                  const net::CompletionCallback& callback) {
+                  const net::CompletionCallback& callback,
+                  std::unique_ptr<net::HostResolver::Request>* out_request) {
   DCHECK(ResourceDispatcherHostImpl::Get()
              ->io_thread_task_runner()
              ->BelongsToCurrentThread());
@@ -104,21 +93,17 @@ int PreresolveUrl(net::URLRequestContextGetter* getter,
   if (!request_context)
     return net::ERR_CONTEXT_SHUT_DOWN;
 
-  auto request_holder = std::make_unique<RequestHolder>();
   auto addresses = std::make_unique<net::AddressList>();
 
-  // Save raw pointers before the unique_ptr is invalidated by base::Passed.
+  // Save raw pointers before the unique_ptr is invalidated by base::Passed().
   net::AddressList* raw_addresses = addresses.get();
-  std::unique_ptr<net::HostResolver::Request>* out_request =
-      request_holder->GetRequest();
 
   net::HostResolver* resolver = request_context->host_resolver();
   net::HostResolver::RequestInfo resolve_info(net::HostPortPair::FromURL(url));
   resolve_info.set_is_speculative(true);
   return resolver->Resolve(
       resolve_info, net::IDLE, raw_addresses,
-      base::Bind(&OnResolveComplete, base::Passed(&request_holder),
-                 base::Passed(&addresses), callback),
+      base::Bind(&OnResolveComplete, base::Passed(&addresses), callback),
       out_request, net::NetLogWithSource());
 }
 

content/public/browser/resource_hints.h

@@ -7,6 +7,7 @@
 
 #include "content/common/content_export.h"
 #include "net/base/completion_callback.h"
+#include "net/dns/host_resolver.h"
 #include "net/http/http_request_info.h"
 #include "url/gurl.h"
 
@@ -16,6 +17,9 @@ namespace net {
 class URLRequestContextGetter;
 }
 
+// TODO(https://crbug.com/565719): Resource hints aren't used nor implemented in
+// content/. Either the preconnect/preresolve code should be moved to content/
+// or this should be moved outside of content/.
 namespace content {
 
 // A Preconnect instance maintains state while a TCP/IP connection is made, and
@@ -35,9 +39,11 @@ CONTENT_EXPORT void PreconnectUrl(net::URLRequestContextGetter* getter,
 
 // Issues a DNS request to |url|. Note that these requests are sent to the host
 // resolver with priority net::IDLE.
-CONTENT_EXPORT int PreresolveUrl(net::URLRequestContextGetter* getter,
-                                 const GURL& url,
-                                 const net::CompletionCallback& callback);
+CONTENT_EXPORT int PreresolveUrl(
+    net::URLRequestContextGetter* getter,
+    const GURL& url,
+    const net::CompletionCallback& callback,
+    std::unique_ptr<net::HostResolver::Request>* out_req);
 
 }  // namespace content