Bind weak ptr in PostStartDnsProbeTask, not StartDnsProbe

Bind weak ptr in PostStartDnsProbeTask, not StartDnsProbe

A Callback decides when it is constructed whether it needs to do a weak pointer
check when called.  Therefore, pass the weak pointer of NetErrorTabHelper to
base::Bind when we create the callback instead of binding it later in
StartDnsProbe.

BUG=174720
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=182301

[Deprecated (see juliatuttle), 2013-02-12 00:00:24]

PTAL, mmenke and sleevi.

(dharani, this is hopefully a fix for the bug you pointed out to me.)

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

nits, but as discussed on IM, the functional "don't crash" part LGTM.

https://chromiumcodereview.appspot.com/12223080/diff/1/chrome/browser/net/net...
File chrome/browser/net/net_error_tab_helper.cc (right):

https://chromiumcodereview.appspot.com/12223080/diff/1/chrome/browser/net/net...
chrome/browser/net/net_error_tab_helper.cc:53: void DnsProbeCallback(
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.

https://chromiumcodereview.appspot.com/12223080/diff/1/chrome/browser/net/net...
chrome/browser/net/net_error_tab_helper.cc:54:
base::Callback<void(DnsProbeResult)> callback,
nit: const base::Callback<void(DnsProbeResult)>& callback,

Also on line 65

https://chromiumcodereview.appspot.com/12223080/diff/1/chrome/browser/net/net...
chrome/browser/net/net_error_tab_helper.cc:64: void StartDnsProbe(
nit: StartDnsProbeOnIOThread ?

https://chromiumcodereview.appspot.com/12223080/diff/1/chrome/browser/net/net...
chrome/browser/net/net_error_tab_helper.cc:66: IOThread* io_thread) {
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]

Fixed sleevi's nits.

https://chromiumcodereview.appspot.com/12223080/diff/1/chrome/browser/net/net...
File chrome/browser/net/net_error_tab_helper.cc (right):

https://chromiumcodereview.appspot.com/12223080/diff/1/chrome/browser/net/net...
chrome/browser/net/net_error_tab_helper.cc:53: void DnsProbeCallback(
On 2013/02/12 00:29:31, Ryan Sleevi wrote:
> 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.

Done.

https://chromiumcodereview.appspot.com/12223080/diff/1/chrome/browser/net/net...
chrome/browser/net/net_error_tab_helper.cc:54:
base::Callback<void(DnsProbeResult)> callback,
On 2013/02/12 00:29:31, Ryan Sleevi wrote:
> nit: const base::Callback<void(DnsProbeResult)>& callback,
> 
> Also on line 65

Done.

https://chromiumcodereview.appspot.com/12223080/diff/1/chrome/browser/net/net...
chrome/browser/net/net_error_tab_helper.cc:64: void StartDnsProbe(
On 2013/02/12 00:29:31, Ryan Sleevi wrote:
> nit: StartDnsProbeOnIOThread ?

Done.

https://chromiumcodereview.appspot.com/12223080/diff/1/chrome/browser/net/net...
chrome/browser/net/net_error_tab_helper.cc:66: IOThread* io_thread) {
On 2013/02/12 00:29:31, Ryan Sleevi wrote:
> 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?

Done.

[mmenke, 2013-02-13 15:20:56]

LGTM

[commit-bot: I haz the power, 2013-02-13 16:49:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ttuttle@chromium.org/12223080/3002

[commit-bot: I haz the power, 2013-02-13 19:36:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ttuttle@chromium.org/12223080/3002

[commit-bot: I haz the power, 2013-02-13 20:47:10]

Change committed as 182301