Remove interface probing for IPv6 support from HostResolverImpl

Remove interface probing for IPv6 support from HostResolverImpl

Since r195406 the probe is not used. It is now safe to remove this code completely.

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

[mmenke, 2013-07-16 17:48:04]

https://codereview.chromium.org/19286002/diff/1/net/dns/host_resolver_impl.cc
File net/dns/host_resolver_impl.cc (right):

https://codereview.chromium.org/19286002/diff/1/net/dns/host_resolver_impl.cc...
net/dns/host_resolver_impl.cc:1890: void HostResolverImpl::ProbeIPv6Support() {
Leaving this in for now?  Seems like we could just make ipv6_probe_monitoring_
default to true and get rid of the function

https://codereview.chromium.org/19286002/diff/1/net/dns/host_resolver_impl.h
File net/dns/host_resolver_impl.h (right):

https://codereview.chromium.org/19286002/diff/1/net/dns/host_resolver_impl.h#...
net/dns/host_resolver_impl.h:144: virtual void ProbeIPv6Support() OVERRIDE;
Documentation of this function in host_resolver could be updated.

https://codereview.chromium.org/19286002/diff/1/net/dns/host_resolver_impl.h#...
net/dns/host_resolver_impl.h:269: bool ipv6_probe_monitoring_;
This variable name is wrong, as is the comment.  Suggest renaming the variable
to something like "probe_ipv6_support_" or "ipv6_probes_".

[szym, 2013-07-16 20:08:31]

https://codereview.chromium.org/19286002/diff/1/net/dns/host_resolver_impl.cc
File net/dns/host_resolver_impl.cc (right):

https://codereview.chromium.org/19286002/diff/1/net/dns/host_resolver_impl.cc...
net/dns/host_resolver_impl.cc:1890: void HostResolverImpl::ProbeIPv6Support() {
On 2013/07/16 17:48:04, mmenke wrote:
> Leaving this in for now?  Seems like we could just make ipv6_probe_monitoring_
> default to true and get rid of the function

--enable-ipv6  => SetDefaultAddressFamily(UNSPEC)
--disable-ipv6 => SetDefaultAddressFamily(IPV4)
otherwise      => ProbeIPv6Support

The difference between the first and last situations is that the first one
disables probing and forces IPv6 resolutions even if connect(IPv6) fails.

Defaulting ipv6_probe_monitoring_ = true, making this call obsolete. I will do
that for now.

Looking forward, I don't think anyone depends on --enable-ipv6, so we could also
remove that in which case we would always probe except then family is restricted
to IPv4.

https://codereview.chromium.org/19286002/diff/1/net/dns/host_resolver_impl.h
File net/dns/host_resolver_impl.h (right):

https://codereview.chromium.org/19286002/diff/1/net/dns/host_resolver_impl.h#...
net/dns/host_resolver_impl.h:144: virtual void ProbeIPv6Support() OVERRIDE;
On 2013/07/16 17:48:04, mmenke wrote:
> Documentation of this function in host_resolver could be updated.

Removed.

https://codereview.chromium.org/19286002/diff/1/net/dns/host_resolver_impl.h#...
net/dns/host_resolver_impl.h:269: bool ipv6_probe_monitoring_;
On 2013/07/16 17:48:04, mmenke wrote:
> This variable name is wrong, as is the comment.  Suggest renaming the variable
> to something like "probe_ipv6_support_" or "ipv6_probes_".

Done.

[mmenke, 2013-07-16 20:17:20]

https://codereview.chromium.org/19286002/diff/11001/net/dns/host_resolver_imp...
File net/dns/host_resolver_impl.cc (left):

https://codereview.chromium.org/19286002/diff/11001/net/dns/host_resolver_imp...
net/dns/host_resolver_impl.cc:932: net_log_(BoundNetLog::Make(net_log,
NetLog::SOURCE_IPV6_PROBE_JOB)),
Should remove this source type.

https://codereview.chromium.org/19286002/diff/11001/net/dns/host_resolver_imp...
net/dns/host_resolver_impl.cc:935:
net_log_.BeginEvent(NetLog::TYPE_IPV6_PROBE_RUNNING);
And this event type.

https://codereview.chromium.org/19286002/diff/11001/net/dns/host_resolver_imp...
net/dns/host_resolver_impl.cc:949: result_ = TestIPv6Support();
Think we should remove TestIPv6Support and IPv6SupportResult in net_util as
well.  I don't think any other code uses it, and if it does, should presumably
switch to the new method (Though if that's the case, the change probably doesn't
belong in this CL).

[mmenke, 2013-07-17 14:25:09]

https://codereview.chromium.org/19286002/diff/17001/chrome/browser/resources/...
File chrome/browser/resources/net_internals/dns_view.js (left):

https://codereview.chromium.org/19286002/diff/17001/chrome/browser/resources/...
chrome/browser/resources/net_internals/dns_view.js:91: },
This is still needed.

https://codereview.chromium.org/19286002/diff/17001/net/base/net_util.h
File net/base/net_util.h (right):

https://codereview.chromium.org/19286002/diff/17001/net/base/net_util.h#newco...
net/base/net_util.h:423: enum IPv6SupportStatus {
This isn't needed.

https://codereview.chromium.org/19286002/diff/17001/net/base/net_util.h#newco...
net/base/net_util.h:435: struct NET_EXPORT IPv6SupportResult {
Nor is this.

[szym, 2013-07-17 16:32:27]

I'm still trying to diagnose the failure in
HostResolverImplDnsTest.DnsTaskUnspec on the bots (I cannot reproduce on any of
my workstations).

https://codereview.chromium.org/19286002/diff/17001/chrome/browser/resources/...
File chrome/browser/resources/net_internals/dns_view.js (left):

https://codereview.chromium.org/19286002/diff/17001/chrome/browser/resources/...
chrome/browser/resources/net_internals/dns_view.js:91: },
On 2013/07/17 14:25:09, mmenke wrote:
> This is still needed.

I thought this is part of LogEntryObserver interface, but now I see this is the
View base class.

https://codereview.chromium.org/19286002/diff/17001/net/base/net_util.h
File net/base/net_util.h (right):

https://codereview.chromium.org/19286002/diff/17001/net/base/net_util.h#newco...
net/base/net_util.h:423: enum IPv6SupportStatus {
On 2013/07/17 14:25:09, mmenke wrote:
> This isn't needed.

Done.

https://codereview.chromium.org/19286002/diff/17001/net/base/net_util.h#newco...
net/base/net_util.h:435: struct NET_EXPORT IPv6SupportResult {
On 2013/07/17 14:25:09, mmenke wrote:
> Nor is this.

Done.

[mmenke, 2013-07-17 16:37:22]

On 2013/07/17 16:32:27, szym wrote:
> I'm still trying to diagnose the failure in
> HostResolverImplDnsTest.DnsTaskUnspec on the bots (I cannot reproduce on any
of
> my workstations).

Do the bots all have IPv6 support?  Seem to recall somewhere we were forcing
something to assume local IPv6 and bypass probes for test, and after this CL, we
aren't.

[mmenke, 2013-07-17 16:38:01]

On 2013/07/17 16:37:22, mmenke wrote:
> On 2013/07/17 16:32:27, szym wrote:
> > I'm still trying to diagnose the failure in
> > HostResolverImplDnsTest.DnsTaskUnspec on the bots (I cannot reproduce on any
> of
> > my workstations).
> 
> Do the bots all have IPv6 support?  Seem to recall somewhere we were forcing
> something to assume local IPv6 and bypass probes for test, and after this CL,
we
> aren't.

IPv6 support == succeeding IPv6 probe.

[szym, 2013-07-17 18:04:27]

You were right. Disabling probing (by setting default address family to
unspecified) was necessary.

ptal

[mmenke, 2013-07-17 18:16:37]

LGTM!  Deleting lots of code is cool!

Should we get rid of switches::kEnableIPv6?  Don't think it does anything any
more, other than causes us to ignore switches::kDisableIPv6.  Alternatively,
could just make it call
global_host_resolver->SetDefaultAddressFamily(net::ADDRESS_FAMILY_UNSPECIFIED),
to maintain the old behavior.  I don't have any particularly strong feelings
about this.

[szym, 2013-07-17 18:46:55]

On 2013/07/17 18:16:37, mmenke wrote:
> LGTM!  Deleting lots of code is cool!
> 
> Should we get rid of switches::kEnableIPv6?  Don't think it does anything any
> more, other than causes us to ignore switches::kDisableIPv6.  Alternatively,
> could just make it call
>
global_host_resolver->SetDefaultAddressFamily(net::ADDRESS_FAMILY_UNSPECIFIED),
> to maintain the old behavior.  I don't have any particularly strong feelings
> about this.

Thanks. I updated --enable-ipv6 to disable probing. I think the switches are
candidates for removal in the next milestone.

[mmenke, 2013-07-17 18:58:00]

On 2013/07/17 18:46:55, szym wrote:
> On 2013/07/17 18:16:37, mmenke wrote:
> > LGTM!  Deleting lots of code is cool!
> > 
> > Should we get rid of switches::kEnableIPv6?  Don't think it does anything
any
> > more, other than causes us to ignore switches::kDisableIPv6.  Alternatively,
> > could just make it call
> >
>
global_host_resolver->SetDefaultAddressFamily(net::ADDRESS_FAMILY_UNSPECIFIED),
> > to maintain the old behavior.  I don't have any particularly strong feelings
> > about this.
> 
> Thanks. I updated --enable-ipv6 to disable probing. I think the switches are
> candidates for removal in the next milestone.

Still LGTM

[commit-bot: I haz the power, 2013-07-17 19:10:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/szym@chromium.org/19286002/59001

[commit-bot: I haz the power, 2013-07-17 21:07:26]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[szym, 2013-07-17 21:15:45]

jhawkins: PTAL changes to chrome/browser/io_thread.cc

[James Hawkins, 2013-07-17 21:18:00]

lgtm

[commit-bot: I haz the power, 2013-07-17 21:23:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/szym@chromium.org/19286002/59001

[commit-bot: I haz the power, 2013-07-18 03:32:15]

Change committed as 212224