Bind weak ptr in PostStartDnsProbeTask, not StartDnsProbe

chrome/browser/net/net_error_tab_helper.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/net/net_error_tab_helper.h"
 
 #include "base/bind.h"
 #include "base/metrics/field_trial.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
@@ skipping 32 matching lines @@
 bool IsDnsError(int net_error) {
   return net_error == net::ERR_NAME_NOT_RESOLVED ||
          net_error == net::ERR_NAME_RESOLUTION_FAILED;
 }
 
 bool GetEnabledByTrial() {
   return (FieldTrialList::FindFullName(kDnsProbeFieldTrialName)
           == kDnsProbeFieldTrialEnableGroupName);
 }
 
 void DnsProbeCallback(

[Ryan Sleevi, 2013/02/12 00:29:31]

nit: It would be helpful to rename this function to something more meaningful
("callback" usually refers to bound functions, not function names themselves)

"OnDnsProbeFinishedOnIOThread" is one more descriptive name for this.

[Deprecated (see juliatuttle), 2013/02/13 03:17:57]

Done.

-    base::WeakPtr<NetErrorTabHelper> tab_helper,
-    base::Callback<void(NetErrorTabHelper*, DnsProbeResult)> callback,
+    base::Callback<void(DnsProbeResult)> callback,

[Ryan Sleevi, 2013/02/12 00:29:31]

nit: const base::Callback<void(DnsProbeResult)>& callback,

Also on line 65

[Deprecated (see juliatuttle), 2013/02/13 03:17:57]

Done.

     DnsProbeResult result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
-      base::Bind(callback, tab_helper, result));
+      base::Bind(callback, result));
 }
 
 void StartDnsProbe(

[Ryan Sleevi, 2013/02/12 00:29:31]

nit: StartDnsProbeOnIOThread ?

[Deprecated (see juliatuttle), 2013/02/13 03:17:57]

Done.

-    base::WeakPtr<NetErrorTabHelper> tab_helper,
-    base::Callback<void(NetErrorTabHelper*, DnsProbeResult)> callback,
+    base::Callback<void(DnsProbeResult)> callback,
     IOThread* io_thread) {

[Ryan Sleevi, 2013/02/12 00:29:31]

Since you're using g_browser_process->io_thread() on line 194, why bother
binding IOThread here for the function?

Why not just grab g_browser_process->io_thread() here (or more aptly, line 68),
while you're executing on the IO thread?

[Deprecated (see juliatuttle), 2013/02/13 03:17:57]

Done.

   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
 
   DnsProbeService* probe_service =
       io_thread->globals()->dns_probe_service.get();
-  probe_service->ProbeDns(base::Bind(&DnsProbeCallback,
-                                     tab_helper,
-                                     callback));
+
+  probe_service->ProbeDns(base::Bind(&DnsProbeCallback, callback));
 }
 
 }  // namespace
 
 NetErrorTabHelper::~NetErrorTabHelper() {
 }
 
 // static
 void NetErrorTabHelper::set_state_for_testing(TestingState state) {
   testing_state_ = state;
@@ skipping 96 matching lines @@
 
   PostStartDnsProbeTask();
 
   dns_probe_state_ = DNS_PROBE_STARTED;
 }
 
 void NetErrorTabHelper::PostStartDnsProbeTask() {
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      base::Bind(&StartDnsProbe,
-                 weak_factory_.GetWeakPtr(),
-                 base::Bind(&NetErrorTabHelper::OnDnsProbeFinished),
-                 g_browser_process->io_thread()));
+      base::Bind(
+          &StartDnsProbe,
+          base::Bind(
+              &NetErrorTabHelper::OnDnsProbeFinished,
+              weak_factory_.GetWeakPtr()),
+          g_browser_process->io_thread()));
 }
 
 void NetErrorTabHelper::OnDnsProbeFinished(DnsProbeResult result) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   DCHECK_EQ(DNS_PROBE_STARTED, dns_probe_state_);
 
   dns_probe_result_ = result;
   dns_probe_state_ = DNS_PROBE_FINISHED;
   MaybeSendInfo();
 }
@@ skipping 33 matching lines @@
   if (testing_state_ != TESTING_DEFAULT)
     return testing_state_ == TESTING_FORCE_ENABLED;
 
   // TODO(ttuttle): Disable on mobile?
   return enabled_by_trial_
          && pref_initialized_
          && *resolve_errors_with_web_service_;
 }
 
 }  // namespace chrome_browser_net