Fix handling of 'Use system proxy settings' policy

The problem is that using system proxy settings is triggered when the
internal proxy prefs of Chrome are set to their default values.
Unfortunately it is possible that these prefs are set to their defaults but
they are not controlled by the default pref store. This CL fixes this by
checking for the default values and not only for the origin of prefs. A longer term solution is described in
http://crbug.com/65732

BUG=65736
TEST=manual

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@68591 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 54 insertions, 15 deletions

diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index a551857..58b353f 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -584,15 +584,6 @@ void ChromeURLRequestContextGetter::ReleaseURLRequestContext() {
   url_request_context_ = NULL;
 }
 
-void ChromeURLRequestContextGetter::RegisterUserPrefs(
-    PrefService* pref_service) {
-  pref_service->RegisterBooleanPref(prefs::kNoProxyServer, false);
-  pref_service->RegisterBooleanPref(prefs::kProxyAutoDetect, false);
-  pref_service->RegisterStringPref(prefs::kProxyServer, "");
-  pref_service->RegisterStringPref(prefs::kProxyPacUrl, "");
-  pref_service->RegisterStringPref(prefs::kProxyBypassList, "");
-}
-
 net::CookieStore* ChromeURLRequestContextGetter::GetCookieStore() {
   // If we are running on the IO thread this is real easy.
   if (BrowserThread::CurrentlyOn(BrowserThread::IO))
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h
index 1815c01..791805d 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -234,8 +234,6 @@ class ChromeURLRequestContextGetter : public URLRequestContextGetter,
   ChromeURLRequestContextGetter(Profile* profile,
                                 ChromeURLRequestContextFactory* factory);
 
-  static void RegisterUserPrefs(PrefService* user_prefs);
-
   // Note that GetURLRequestContext() can only be called from the IO
   // thread (it will assert otherwise). GetCookieStore() and
   // GetIOMessageLoopProxy however can be called from any thread.
diff --git a/chrome/browser/net/pref_proxy_config_service.cc b/chrome/browser/net/pref_proxy_config_service.cc
index 27155e6..10ae74b 100644
--- a/chrome/browser/net/pref_proxy_config_service.cc
+++ b/chrome/browser/net/pref_proxy_config_service.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/net/pref_proxy_config_service.h"
 
+#include "base/values.h"
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/prefs/pref_set_observer.h"
@@ -12,6 +13,33 @@
 #include "chrome/common/notification_type.h"
 #include "chrome/common/pref_names.h"
 
+namespace {
+
+const bool kProxyPrefDefaultBoolean = false;
+const char kProxyPrefDefaultString[] = "";
+
+// Determines if a value of a proxy pref is set to its default. Default values
+// have a special role in the proxy pref system, because if all of the proxy
+// prefs are set to their defaults, then the system proxy settings are applied.
+// TODO(gfeher): Proxy preferences should be refactored to avoid the need
+// for such solutions. See crbug.com/65732
+bool IsDefaultValue(const Value* value) {
+  bool b = false;
+  std::string s;
+  if (value->IsType(Value::TYPE_BOOLEAN) &&
+      value->GetAsBoolean(&b)) {
+    return b == kProxyPrefDefaultBoolean;
+  } else if (value->IsType(Value::TYPE_STRING) &&
+             value->GetAsString(&s)) {
+    return s == kProxyPrefDefaultString;
+  } else {
+    NOTREACHED() << "Invalid type for a proxy preference.";
+    return false;
+  }
+}
+
+}  // namespace
+
 PrefProxyConfigTracker::PrefProxyConfigTracker(PrefService* pref_service)
     : pref_service_(pref_service) {
   valid_ = ReadPrefConfig(&pref_config_);
@@ -97,12 +125,17 @@ bool PrefProxyConfigTracker::ReadPrefConfig(net::ProxyConfig* config) {
   // are ignored. That's because chrome treats the system settings as the
   // default values, which should apply if there's no explicit value forced by
   // policy or the user.
+  // Preferences that are set to their default values are also ignored,
+  // regardless of their controlling source. This is because 'use system proxy
+  // settings' is currently encoded by all the preferences being set to their
+  // defaults. This will change when crbug.com/65732 is addressed.
   bool found_enable_proxy_pref = false;
   for (size_t i = 0; i < arraysize(proxy_prefs); i++) {
     const PrefService::Preference* pref =
         pref_service_->FindPreference(proxy_prefs[i]);
     DCHECK(pref);
-    if (pref && (!pref->IsUserModifiable() || pref->HasUserSetting())) {
+    if (pref && (!pref->IsUserModifiable() || pref->HasUserSetting()) &&
+        !IsDefaultValue(pref->GetValue())) {
       found_enable_proxy_pref = true;
       break;
     }
@@ -221,3 +254,18 @@ void PrefProxyConfigService::RegisterObservers() {
     registered_observers_ = true;
   }
 }
+
+// static
+void PrefProxyConfigService::RegisterUserPrefs(
+    PrefService* pref_service) {
+  pref_service->RegisterBooleanPref(prefs::kNoProxyServer,
+                                    kProxyPrefDefaultBoolean);
+  pref_service->RegisterBooleanPref(prefs::kProxyAutoDetect,
+                                    kProxyPrefDefaultBoolean);
+  pref_service->RegisterStringPref(prefs::kProxyServer,
+                                   kProxyPrefDefaultString);
+  pref_service->RegisterStringPref(prefs::kProxyPacUrl,
+                                   kProxyPrefDefaultString);
+  pref_service->RegisterStringPref(prefs::kProxyBypassList,
+                                   kProxyPrefDefaultString);
+}
diff --git a/chrome/browser/net/pref_proxy_config_service.h b/chrome/browser/net/pref_proxy_config_service.h
index f8c7061..a0f6cd0 100644
--- a/chrome/browser/net/pref_proxy_config_service.h
+++ b/chrome/browser/net/pref_proxy_config_service.h
@@ -103,6 +103,8 @@ class PrefProxyConfigService
   virtual bool GetLatestProxyConfig(net::ProxyConfig* config);
   virtual void OnLazyPoll();
 
+  static void RegisterUserPrefs(PrefService* user_prefs);
+
  private:
   // ProxyConfigService::Observer implementation:
   virtual void OnProxyConfigChanged(const net::ProxyConfig& config);
diff --git a/chrome/browser/net/pref_proxy_config_service_unittest.cc b/chrome/browser/net/pref_proxy_config_service_unittest.cc
index e7917b1..fd24dd6 100644
--- a/chrome/browser/net/pref_proxy_config_service_unittest.cc
+++ b/chrome/browser/net/pref_proxy_config_service_unittest.cc
@@ -67,7 +67,7 @@ class PrefProxyConfigServiceTestBase : public TESTBASE {
 
   virtual void SetUp() {
     ASSERT_TRUE(pref_service_.get());
-    ChromeURLRequestContextGetter::RegisterUserPrefs(pref_service_.get());
+    PrefProxyConfigService::RegisterUserPrefs(pref_service_.get());
     fixed_config_.set_pac_url(GURL(kFixedPacUrl));
     delegate_service_ = new TestProxyConfigService(fixed_config_);
     proxy_config_tracker_ = new PrefProxyConfigTracker(pref_service_.get());
diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 7497dd6..d79981d 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -29,8 +29,8 @@
 #include "chrome/browser/instant/instant_controller.h"
 #include "chrome/browser/metrics/metrics_log.h"
 #include "chrome/browser/metrics/metrics_service.h"
-#include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/net/predictor_api.h"
+#include "chrome/browser/net/pref_proxy_config_service.h"
 #include "chrome/browser/net/net_pref_observer.h"
 #include "chrome/browser/notifications/desktop_notification_service.h"
 #include "chrome/browser/page_info_model.h"
@@ -135,7 +135,7 @@ void RegisterUserPrefs(PrefService* user_prefs) {
   GeolocationContentSettingsMap::RegisterUserPrefs(user_prefs);
   TranslatePrefs::RegisterUserPrefs(user_prefs);
   DesktopNotificationService::RegisterUserPrefs(user_prefs);
-  ChromeURLRequestContextGetter::RegisterUserPrefs(user_prefs);
+  PrefProxyConfigService::RegisterUserPrefs(user_prefs);
 #if defined(TOOLKIT_VIEWS)
   BrowserActionsContainer::RegisterUserPrefs(user_prefs);
 #elif defined(TOOLKIT_GTK)