Revert 89486 - Revert 87047 - Revert 86422 - Make DHCP WPAD on by default.

Revert 89486 - Revert 87047 - Revert 86422 - Make DHCP WPAD on by default.

Reason: Turning back on for trunk to collect data as performance worries are addressed.

BUG=18575, 84047
TEST=Run Chrome on Windows without any flags. Enable auto-detect in proxy configuration. Net log should show attempts to auto-detect via DHCP.

Review URL: http://codereview.chromium.org/7167016

TBR=joi@chromium.org

NOTE: I'll roll it back in if it turns out this wasn't the problem.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=89629

[Jói, 2011-06-19 22:48:39]

LGTM

On Sun, Jun 19, 2011 at 6:36 PM,  <cevans@chromium.org> wrote:
> Reviewers: Jói,
>
> Description:
> Revert 89486 - Revert 87047 - Revert 86422 - Make DHCP WPAD on by default.
>
> Reason: Turning back on for trunk to collect data as performance worries are
> addressed.
>
> BUG=18575,84047
> TEST=Run Chrome on Windows without any flags. Enable auto-detect in proxy
> configuration. Net log should show attempts to auto-detect via DHCP.
>
> Review URL: http://codereview.chromium.org/7167016
>
> TBR=joi@chromium.org
>
> NOTE: I'll roll it back in if it turns out this wasn't the problem.
>
> Please review this at http://codereview.chromium.org/7200025/
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src/
>
> Affected files:
>  M     chrome/browser/net/connection_tester.cc
>  M     chrome/browser/net/proxy_service_factory.cc
>  M     chrome/common/chrome_switches.h
>  M     chrome/common/chrome_switches.cc
>  M     net/proxy/dhcp_proxy_script_fetcher_factory.h
>  M     net/proxy/dhcp_proxy_script_fetcher_factory.cc
>  M     net/proxy/dhcp_proxy_script_fetcher_factory_unittest.cc
>
>
> Index: chrome/browser/net/connection_tester.cc
> ===================================================================
> --- chrome/browser/net/connection_tester.cc     (revision 89628)
> +++ chrome/browser/net/connection_tester.cc     (working copy)
> @@ -187,8 +187,8 @@
>
>     net::DhcpProxyScriptFetcherFactory dhcp_factory;
>     if (CommandLine::ForCurrentProcess()->HasSwitch(
> -        switches::kDisableDhcpWpad)) {
> -      dhcp_factory.set_enabled(false);
> +        switches::kEnableDhcpWpad)) {
> +      dhcp_factory.set_enabled(true);
>     }
>
>     proxy_service->reset(net::ProxyService::CreateUsingV8ProxyResolver(
> Index: chrome/browser/net/proxy_service_factory.cc
> ===================================================================
> --- chrome/browser/net/proxy_service_factory.cc (revision 89628)
> +++ chrome/browser/net/proxy_service_factory.cc (working copy)
> @@ -86,8 +86,8 @@
>   net::ProxyService* proxy_service;
>   if (use_v8) {
>     net::DhcpProxyScriptFetcherFactory dhcp_factory;
> -    if (command_line.HasSwitch(switches::kDisableDhcpWpad)) {
> -      dhcp_factory.set_enabled(false);
> +    if (command_line.HasSwitch(switches::kEnableDhcpWpad)) {
> +      dhcp_factory.set_enabled(true);
>     }
>
>     proxy_service = net::ProxyService::CreateUsingV8ProxyResolver(
> Index: chrome/common/chrome_switches.cc
> ===================================================================
> --- chrome/common/chrome_switches.cc    (revision 89628)
> +++ chrome/common/chrome_switches.cc    (working copy)
> @@ -202,9 +202,6 @@
>  // Disables the custom JumpList on Windows 7.
>  const char kDisableCustomJumpList[]         = "disable-custom-jumplist";
>
> -// Disables retrieval of PAC URLs from DHCP as per the WPAD standard.
> -const char kDisableDhcpWpad[]               = "disable-dhcp-wpad";
> -
>  // Disable extensions.
>  const char kDisableExtensions[]             = "disable-extensions";
>
> @@ -380,6 +377,11 @@
>  // Enables web developers to create apps for Chrome without using crx
> packages.
>  const char kEnableCrxlessWebApps[]          = "enable-crxless-web-apps";
>
> +// Enables retrieval of PAC URLs from DHCP as per the WPAD standard. Note
> +// that this feature is not supported on all platforms, and using the flag
> +// is a no-op on such platforms.
> +const char kEnableDhcpWpad[]                = "enable-dhcp-wpad";
> +
>  // Enable DNS side checking of certificates. Still experimental, should
> only
>  // be used by developers at the current time.
>  const char kEnableDNSCertProvenanceChecking[] =
> Index: chrome/common/chrome_switches.h
> ===================================================================
> --- chrome/common/chrome_switches.h     (revision 89628)
> +++ chrome/common/chrome_switches.h     (working copy)
> @@ -68,7 +68,6 @@
>  extern const char kDisableClientSidePhishingDetection[];
>  extern const char kDisableConnectBackupJobs[];
>  extern const char kDisableCustomJumpList[];
> -extern const char kDisableDhcpWpad[];
>  extern const char kDisableExtensionsFileAccessCheck[];
>  extern const char kDisableExtensions[];
>  extern const char kDisableFlashSandbox[];
> @@ -116,6 +115,7 @@
>  extern const char kEnableCompositeToTexture[];
>  extern const char kEnableConnectBackupJobs[];
>  extern const char kEnableCrxlessWebApps[];
> +extern const char kEnableDhcpWpad[];
>  extern const char kEnableDNSCertProvenanceChecking[];
>  extern const char kEnableExperimentalExtensionApis[];
>  extern const char kEnableExtensionTimelineApi[];
> Index: net/proxy/dhcp_proxy_script_fetcher_factory.cc
> ===================================================================
> --- net/proxy/dhcp_proxy_script_fetcher_factory.cc      (revision 89628)
> +++ net/proxy/dhcp_proxy_script_fetcher_factory.cc      (working copy)
> @@ -15,7 +15,9 @@
>
>  DhcpProxyScriptFetcherFactory::DhcpProxyScriptFetcherFactory()
>     : feature_enabled_(false) {
> -  set_enabled(true);
> +  // TODO(joi): Change this default, and the comment on |set_enabled()|,
> +  // when the time is right.
> +  set_enabled(false);
>  }
>
>  DhcpProxyScriptFetcher* DhcpProxyScriptFetcherFactory::Create(
> Index: net/proxy/dhcp_proxy_script_fetcher_factory.h
> ===================================================================
> --- net/proxy/dhcp_proxy_script_fetcher_factory.h       (revision 89628)
> +++ net/proxy/dhcp_proxy_script_fetcher_factory.h       (working copy)
> @@ -47,7 +47,7 @@
>   // Attempts to enable/disable the DHCP WPAD feature.  Does nothing
>   // if |IsSupported()| returns false.
>   //
> -  // The default is |enabled() == true|.
> +  // The current default is |enabled() == false|.
>   void set_enabled(bool enabled);
>
>   // Returns true if the DHCP WPAD feature is enabled.  Always returns
> Index: net/proxy/dhcp_proxy_script_fetcher_factory_unittest.cc
> ===================================================================
> --- net/proxy/dhcp_proxy_script_fetcher_factory_unittest.cc     (revision
> 89628)
> +++ net/proxy/dhcp_proxy_script_fetcher_factory_unittest.cc     (working
> copy)
> @@ -9,10 +9,8 @@
>
>  namespace net {
>  namespace {
> -
>  TEST(DhcpProxyScriptFetcherFactoryTest, DoNothingWhenDisabled) {
>   DhcpProxyScriptFetcherFactory factory;
> -  factory.set_enabled(false);
>   scoped_ptr<DhcpProxyScriptFetcher> fetcher(factory.Create(NULL));
>   EXPECT_EQ("", fetcher->GetFetcherName());
>  }
> @@ -38,11 +36,7 @@
>
>  TEST(DhcpProxyScriptFetcherFactoryTest, SetEnabled) {
>   DhcpProxyScriptFetcherFactory factory;
> -#if defined(OS_WIN)
> -  EXPECT_TRUE(factory.enabled());
> -#else
>   EXPECT_FALSE(factory.enabled());
> -#endif  // defined(OS_WIN)
>
>   factory.set_enabled(false);
>   EXPECT_FALSE(factory.enabled());
>
>
>