Fix a memory error in ProxyResolverV8Tracing.

net/proxy/proxy_resolver_v8_tracing_unittest.cc

 // Copyright (c) 2013 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 "net/proxy/proxy_resolver_v8_tracing.h"
 
 #include "base/file_util.h"
 #include "base/message_loop.h"
 #include "base/path_service.h"
 #include "base/stl_util.h"
@@ skipping 743 matching lines @@
   BlockableHostResolver()
       : num_cancelled_requests_(0), waiting_for_resolve_(false) {}
 
   virtual int Resolve(const RequestInfo& info,
                       AddressList* addresses,
                       const CompletionCallback& callback,
                       RequestHandle* out_req,
                       const BoundNetLog& net_log) OVERRIDE {
     EXPECT_FALSE(callback.is_null());
     EXPECT_TRUE(out_req);
-    *out_req = reinterpret_cast<RequestHandle*>(1);  // Magic value.
 
     if (!action_.is_null())
       action_.Run();
 
     // Indicate to the caller that a request was received.
     EXPECT_TRUE(waiting_for_resolve_);
     MessageLoop::current()->Quit();
 
+    // This line is intentionally after action_.Run(), since one of the
+    // tests does a cancellation inside of Resolve(), and it is more
+    // interesting if *out_req hasn't been written yet at that point.
+    *out_req = reinterpret_cast<RequestHandle*>(1);  // Magic value.
+
     // Return ERR_IO_PENDING as this request will NEVER be completed.
     // Expectation is for the caller to later cancel the request.
     return ERR_IO_PENDING;
   }
 
   virtual int ResolveFromCache(const RequestInfo& info,
                                AddressList* addresses,
                                const BoundNetLog& net_log) OVERRIDE {
     NOTREACHED();
     return ERR_DNS_CACHE_MISS;
@@ skipping 59 matching lines @@
   resolver.CancelRequest(request1);
   resolver.CancelRequest(request2);
 
   EXPECT_EQ(2, host_resolver.num_cancelled_requests());
 
   // After leaving this scope, the ProxyResolver is destroyed.
   // This should not cause any problems, as the outstanding work
   // should have been cancelled.
 }
 
+void CancelRequestAndPause(ProxyResolverV8Tracing* resolver,
+                           ProxyResolver::RequestHandle request) {
+  resolver->CancelRequest(request);
+
+  // Sleep for a little bit. This makes it more likely for the worker
+  // thread to have returned from its call, and serves as a regression
+  // test for http://crbug.com/173373.
+  base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(30));
+}
+
 // In non-blocking mode, the worker thread actually does block for
 // a short time to see if the result is in the DNS cache. Test
 // cancellation while the worker thread is waiting on this event.
 TEST_F(ProxyResolverV8TracingTest, CancelWhileBlockedInNonBlockingDns) {

[mmenke, 2013/01/31 21:11:23]

Hmm...The purpose of this test was to cancel before the non blocking DNS hears
about the cache miss.  Too bad there's no
"MessageLoop::PostTaskToRunBeforeTheCurrentlyExecutingTaskWasPosted()".

We could just wait a couple milliseconds after posting the instead.  Not
perfect, but it avoids getting us into this unexpected situation.

[eroman, 2013/01/31 21:29:49]

I think the test works as is (it is testing two important situations now). Are
you suggesting changing it or adding a new one?

   BlockableHostResolver host_resolver;
   MockErrorObserver* error_observer = new MockErrorObserver;
   ProxyResolverV8Tracing resolver(&host_resolver, error_observer, NULL);
 
   InitResolver(&resolver, "dns.js");
 
   ProxyInfo proxy_info;
   ProxyResolver::RequestHandle request;
 
   int rv = resolver.GetProxyForURL(
       GURL("http://foo/"), &proxy_info,
       base::Bind(&CrashCallback), &request, BoundNetLog());
 
   EXPECT_EQ(ERR_IO_PENDING, rv);
 
-  host_resolver.SetAction(base::Bind(&ProxyResolverV8Tracing::CancelRequest,
-                                     base::Unretained(&resolver), request));
+  host_resolver.SetAction(
+      base::Bind(CancelRequestAndPause, &resolver, request));
 
   host_resolver.WaitUntilRequestIsReceived();
 
   // At this point the host resolver ran Resolve(), and should have cancelled
   // the request.
 
   EXPECT_EQ(1, host_resolver.num_cancelled_requests());
 }
 
 // Cancel the request while there is a pending DNS request, however before
@@ skipping 36 matching lines @@
 
   host_resolver.WaitUntilRequestIsReceived();
 
   resolver.CancelSetPacScript();
   EXPECT_EQ(1, host_resolver.num_cancelled_requests());
 }
 
 }  // namespace
 
 }  // namespace net