Do GetAdaptersAddresses on a worker thread.

Do GetAdaptersAddresses on a worker thread.

BUG=84047
TEST=net_unittests

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=90258

[Jói, 2011-06-16 18:51:18]

Hi Eric,

This is to unblock the network thread, since UMA shows that GetAdaptersAddresses
can take several hundred milliseconds or more.

After this I'm going for doing DHCP and DNS auto-detect in parallel.  My plan,
as discussed, is to do it by merging the class hierarchies and extracting the
"do parallel lookup on several fetchers with an order of preference for results"
logic from DhcpProxyScriptFecherWin into something like
ParallelProxyScriptFetcher and reusing that in InitProxyResolver.

Cheers,
Jói

[Jói, 2011-06-20 22:23:11]

gentle ping (you were right, I had forgotten about this change from last week)

On Thu, Jun 16, 2011 at 2:51 PM,  <joi@chromium.org> wrote:
> Reviewers: eroman,
>
> Message:
> Hi Eric,
>
> This is to unblock the network thread, since UMA shows that
> GetAdaptersAddresses
> can take several hundred milliseconds or more.
>
> After this I'm going for doing DHCP and DNS auto-detect in parallel.  My
> plan,
> as discussed, is to do it by merging the class hierarchies and extracting
> the
> "do parallel lookup on several fetchers with an order of preference for
> results"
> logic from DhcpProxyScriptFecherWin into something like
> ParallelProxyScriptFetcher and reusing that in InitProxyResolver.
>
> Cheers,
> Jói
>
>
> Description:
> Do GetAdaptersAddresses on a worker thread.
>
> BUG=84047
> TEST=net_unittests
>
>
> Please review this at http://codereview.chromium.org/7189016/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M net/proxy/dhcp_proxy_script_adapter_fetcher_win.h
>  M net/proxy/dhcp_proxy_script_fetcher_win.h
>  M net/proxy/dhcp_proxy_script_fetcher_win.cc
>  M net/proxy/dhcp_proxy_script_fetcher_win_unittest.cc
>
>
>

[eroman, 2011-06-21 03:02:54]

LGTM once my concern regarding cancellation has been addressed.

http://codereview.chromium.org/7189016/diff/1/net/proxy/dhcp_proxy_script_fet...
File net/proxy/dhcp_proxy_script_fetcher_win.cc (right):

http://codereview.chromium.org/7189016/diff/1/net/proxy/dhcp_proxy_script_fet...
net/proxy/dhcp_proxy_script_fetcher_win.cc:353:
owner_->OnGetCandidateAdapterNamesDone(adapter_names_);
How is cancellation handled?

I understand that if |owner_| is deleted, then the weakpointer will be marked as
such.

However what if |owner_| is still alive, but DhcpProxyScriptFetcherWin::Cancel()
was called on it.

In that case, I believe we will go ahead and start fetching for each adapter,
when really we should not do anything.

[Jói, 2011-06-23 01:21:22]

PTAL

Thanks for catching the cancellation issue.  I uploaded a new version that fixes
it and adds a unit test that would have caught the problem.

Cheers,
Jói

[eroman, 2011-06-23 02:11:22]

lgtm

On Wed, Jun 22, 2011 at 6:21 PM,  <joi@chromium.org> wrote:
> PTAL
>
> Thanks for catching the cancellation issue.  I uploaded a new version that
> fixes
> it and adds a unit test that would have caught the problem.
>
> Cheers,
> Jói
>
>
> http://codereview.chromium.org/7189016/
>