Change HostResolver::Resolve() to take an std::unique_ptr<Request>* rather than a RequestHandle*

extensions/browser/api/dns/dns_api.cc

Index: extensions/browser/api/dns/dns_api.cc
diff --git a/extensions/browser/api/dns/dns_api.cc b/extensions/browser/api/dns/dns_api.cc
index 671254ba4026dc861652631cc4563b53484984f0..fd61ab74a215728c00e71a17b7828608fb8a7518 100644
--- a/extensions/browser/api/dns/dns_api.cc
+++ b/extensions/browser/api/dns/dns_api.cc
@@ -25,7 +25,7 @@ namespace extensions {
 DnsResolveFunction::DnsResolveFunction()
     : resource_context_(NULL),
       response_(false),
-      request_handle_(new net::HostResolver::RequestHandle()),
+      request_handle_(nullptr),

[Devlin, 2016/07/12 15:04:51]

unnecessary (this is the default construction of std::unique_ptr)

[maksims (do not use this acc), 2016/07/19 15:00:25]

Done.

       addresses_(new net::AddressList) {}
 
 DnsResolveFunction::~DnsResolveFunction() {}
@@ -57,14 +57,10 @@ void DnsResolveFunction::WorkOnIOThread() {
   // hostname you'd like to resolve, even though it doesn't use that value in
   // determining its answer.
   net::HostPortPair host_port_pair(hostname_, 0);
-
   net::HostResolver::RequestInfo request_info(host_port_pair);
   int resolve_result = host_resolver->Resolve(
-      request_info,
-      net::DEFAULT_PRIORITY,
-      addresses_.get(),
-      base::Bind(&DnsResolveFunction::OnLookupFinished, this),
-      request_handle_.get(),
+      request_info, net::DEFAULT_PRIORITY, addresses_.get(),
+      base::Bind(&DnsResolveFunction::OnLookupFinished, this), &request_handle_,
       net::BoundNetLog());
 
   // Balanced in OnLookupFinished.
@@ -91,6 +87,10 @@ void DnsResolveFunction::OnLookupFinished(int resolve_result) {
   results_ = Resolve::Results::Create(*resolve_info);
   response_ = true;
 
+  // TODO(maksims): investigate why std::unique_ptr<HostResolver::Request>'s
+  // destructor is not called automatically.

[Devlin, 2016/07/12 15:04:51]

I don't quite follow this comment.  Automatically when?  Is it not called here
after reset()?  I'd find that very surprising (and worrying).

[maksims (do not use this acc), 2016/07/14 12:02:47]

No, I mean that DnsResolveFunction's destructor is not called until request_ is
reseted for some reason. And there is a memory leak then. I cannot find what
exactly causes the trouble. It's a temp workaround. 

Matt, would you please take a look into that? I cannot find the root cause.
Log is here.
https://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium...

[mmenke, 2016/07/14 18:25:31]

Without digging into it, my guess is that the test is leaking the
DnsResolveFunction, and there's some suppression that leak.

In the old code, DnsResolveFunction::WorkOnIOThread doesn't allocate any memory
of its own - it's all owned by the HostResolver, which used to destroy requests
on completion itself.  The new code allocates memory owned by the
DnsResolveFunction there, so if it leaks, the handle is, too.  

The old suppression wouldn't cover this because it only covers stuff allocated
in DnsResolveFunction's constructor.

Anyhow, just a theory.  I'm not at all familiar with this code.

[maksims (do not use this acc), 2016/07/19 15:00:25]

Done.

[maksims (do not use this acc), 2016/07/19 15:00:25]

Done.

[maksims (do not use this acc), 2016/07/19 15:00:25]

I just the implementation of ::RequestImpl and it solved the issue.

[maksims (do not use this acc), 2016/07/19 15:00:25]

Done.

+  request_handle_.reset();
+
   bool post_task_result = BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,