redesign and reimplement proxy config service and tracker, revise proxy ui on cros

chrome/browser/chromeos/proxy_config_service_impl_unittest.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/proxy_config_service_impl.h"
 
 #include <map>
 #include <string>
 #include <vector>
 
 #include "base/format_macros.h"
 #include "base/json/json_value_serializer.h"
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "chrome/browser/chromeos/cros/cros_library.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/testing_pref_service.h"
 #include "content/browser/browser_thread.h"
 #include "net/proxy/proxy_config_service_common_unittest.h"
 #include "testing/gtest/include/gtest/gtest.h"
-#include "testing/platform_test.h"
 
 namespace chromeos {
 
 namespace {
 
 struct Input {  // Fields of chromeos::ProxyConfigServiceImpl::ProxyConfig.
   ProxyConfigServiceImpl::ProxyConfig::Mode mode;
   const char* pac_url;
   const char* single_uri;
   const char* http_uri;
   const char* https_uri;
   const char* ftp_uri;
   const char* socks_uri;
   const char* bypass_rules;
 };
 
 // Builds an identifier for each test in an array.
 #define TEST_DESC(desc) base::StringPrintf("at line %d <%s>", __LINE__, desc)
 
 // Shortcuts to declare enums within chromeos's ProxyConfig.
 #define MK_MODE(mode) ProxyConfigServiceImpl::ProxyConfig::MODE_##mode
-#define MK_SRC(src) ProxyConfigServiceImpl::ProxyConfig::SOURCE_##src
 #define MK_SCHM(scheme) net::ProxyServer::SCHEME_##scheme
+#define MK_AVAIL(avail) net::ProxyConfigService::CONFIG_##avail
 
 // Inspired from net/proxy/proxy_config_service_linux_unittest.cc.
-const struct {
+static const struct TestParams {

[Mattias Nissler (ping if slow), 2011/10/25 12:43:11]

No need for static, we're in a anonymous namespace.

[kuan, 2011/10/25 16:01:50]

Done.

   // Short description to identify the test
   std::string description;
 
   bool is_valid;
-  bool test_read_write_access;
 
   Input input;
 
   // Expected outputs from fields of net::ProxyConfig (via IO).
   bool auto_detect;
   GURL pac_url;
   net::ProxyRulesExpectation proxy_rules;
 } tests[] = {
   {
     TEST_DESC("No proxying"),
 
     true,  // is_valid
-    true,  // test_read_write_access
 
     { // Input.
       MK_MODE(DIRECT),  // mode
     },
 
     // Expected result.
     false,                                   // auto_detect
     GURL(),                                  // pac_url
     net::ProxyRulesExpectation::Empty(),     // proxy_rules
   },
 
   {
     TEST_DESC("Auto detect"),
 
     true,  // is_valid
-    true,  // test_read_write_access
 
     { // Input.
       MK_MODE(AUTO_DETECT),  // mode
     },
 
     // Expected result.
     true,                                    // auto_detect
     GURL(),                                  // pac_url
     net::ProxyRulesExpectation::Empty(),     // proxy_rules
   },
 
   {
     TEST_DESC("Valid PAC URL"),
 
     true,  // is_valid
-    true,  // test_read_write_access
 
     { // Input.
       MK_MODE(PAC_SCRIPT),     // mode
       "http://wpad/wpad.dat",  // pac_url
     },
 
     // Expected result.
     false,                                   // auto_detect
     GURL("http://wpad/wpad.dat"),            // pac_url
     net::ProxyRulesExpectation::Empty(),     // proxy_rules
   },
 
   {
     TEST_DESC("Invalid PAC URL"),
 
     false,  // is_valid
-    false,  // test_read_write_access
 
     { // Input.
       MK_MODE(PAC_SCRIPT),  // mode
       "wpad.dat",           // pac_url
     },
 
     // Expected result.
     false,                                   // auto_detect
     GURL(),                                  // pac_url
     net::ProxyRulesExpectation::Empty(),     // proxy_rules
   },
 
   {
     TEST_DESC("Single-host in proxy list"),
 
     true,  // is_valid
-    true,  // test_read_write_access
 
     { // Input.
       MK_MODE(SINGLE_PROXY),  // mode
       NULL,                   // pac_url
       "www.google.com",       // single_uri
     },
 
     // Expected result.
     false,                                   // auto_detect
     GURL(),                                  // pac_url
     net::ProxyRulesExpectation::Single(      // proxy_rules
         "www.google.com:80",  // single proxy
         ""),                  // bypass rules
   },
 
   {
     TEST_DESC("Single-host, different port"),
 
     true,   // is_valid
-    false,  // test_read_write_access
 
     { // Input.
       MK_MODE(SINGLE_PROXY),  // mode
       NULL,                   // pac_url
       "www.google.com:99",    // single_uri
     },
 
     // Expected result.
     false,                                   // auto_detect
     GURL(),                                  // pac_url
     net::ProxyRulesExpectation::Single(      // proxy_rules
         "www.google.com:99",  // single
         ""),                  // bypass rules
   },
 
   {
     TEST_DESC("Tolerate a scheme"),
 
     true,   // is_valid
-    false,  // test_read_write_access
 
     { // Input.
       MK_MODE(SINGLE_PROXY),       // mode
       NULL,                        // pac_url
       "http://www.google.com:99",  // single_uri
     },
 
     // Expected result.
     false,                                   // auto_detect
     GURL(),                                  // pac_url
     net::ProxyRulesExpectation::Single(      // proxy_rules
         "www.google.com:99",  // single proxy
         ""),                  // bypass rules
   },
 
   {
     TEST_DESC("Per-scheme proxy rules"),
 
     true,  // is_valid
-    true,  // test_read_write_access
 
     { // Input.
       MK_MODE(PROXY_PER_SCHEME),  // mode
       NULL,                       // pac_url
       NULL,                       // single_uri
       "www.google.com:80",        // http_uri
       "www.foo.com:110",          // https_uri
       "ftp.foo.com:121",          // ftp_uri
       "socks.com:888",            // socks_uri
     },
 
     // Expected result.
     false,                          // auto_detect
     GURL(),                         // pac_url
     net::ProxyRulesExpectation::PerSchemeWithSocks(  // proxy_rules
         "www.google.com:80",        // http
         "https://www.foo.com:110",  // https
         "ftp.foo.com:121",          // ftp
         "socks5://socks.com:888",   // fallback proxy
         ""),                        // bypass rules
   },
 
   {
     TEST_DESC("Bypass rules"),
 
     true,  // is_valid
-    true,  // test_read_write_access
 
     { // Input.
       MK_MODE(SINGLE_PROXY),      // mode
       NULL,                       // pac_url
       "www.google.com",           // single_uri
       NULL, NULL, NULL, NULL,     // per-proto
       "*.google.com, *foo.com:99, 1.2.3.4:22, 127.0.0.1/8",  // bypass_rules
     },
 
     // Expected result.
     false,                          // auto_detect
     GURL(),                         // pac_url
     net::ProxyRulesExpectation::Single(                      // proxy_rules
         "www.google.com:80",                                 // single proxy
         "*.google.com,*foo.com:99,1.2.3.4:22,127.0.0.1/8"),  // bypass_rules
   },
 };  // tests
 
-}  // namespace
+template<typename TESTBASE>
+class ProxyConfigServiceImplTestBase : public TESTBASE {
+ protected:
+  ProxyConfigServiceImplTestBase()
+      : ui_thread_(BrowserThread::UI, &loop_),
+        io_thread_(BrowserThread::IO, &loop_) {}
 
-class ProxyConfigServiceImplTest : public PlatformTest {
- protected:
-  ProxyConfigServiceImplTest()
-      : ui_thread_(BrowserThread::UI, &message_loop_),
-        io_thread_(BrowserThread::IO, &message_loop_) {
+  virtual void Init(PrefService* pref_service) {
+    ASSERT_TRUE(pref_service);
+    PrefProxyConfigTracker::RegisterPrefs(pref_service);
+    proxy_config_service_.reset(new ChromeProxyConfigService(NULL));
+    config_service_impl_.reset(new ProxyConfigServiceImpl(pref_service));
+    config_service_impl_->SetChromeProxyConfigService(
+        proxy_config_service_.get());
+    // SetChromeProxyConfigService triggers update of initial prefs proxy
+    // config by tracker to chrome proxy config service, so flush all pending
+    // tasks so that tests start fresh.
+    loop_.RunAllPending();
   }
 
-  virtual ~ProxyConfigServiceImplTest() {
-    config_service_ = NULL;
-    MessageLoop::current()->RunAllPending();
-  }
-
-  void CreateConfigService(
-      const ProxyConfigServiceImpl::ProxyConfig& init_config) {
-    // Instantiate proxy config service with |init_config|.
-    config_service_ = new ProxyConfigServiceImpl(init_config);
-    config_service_->SetTesting();
+  virtual void TearDown() {
+    config_service_impl_->DetachFromPrefService();
+    loop_.RunAllPending();
+    config_service_impl_.reset();
+    proxy_config_service_.reset();
   }
 
   void SetAutomaticProxy(
       ProxyConfigServiceImpl::ProxyConfig::Mode mode,
-      ProxyConfigServiceImpl::ProxyConfig::Source source,
       const char* pac_url,
       ProxyConfigServiceImpl::ProxyConfig* config,
       ProxyConfigServiceImpl::ProxyConfig::AutomaticProxy* automatic_proxy) {
     config->mode = mode;
-    automatic_proxy->source = source;
+    config->state = ProxyPrefs::CONFIG_SYSTEM;
     if (pac_url)
       automatic_proxy->pac_url = GURL(pac_url);
   }
 
   void SetManualProxy(
       ProxyConfigServiceImpl::ProxyConfig::Mode mode,
-      ProxyConfigServiceImpl::ProxyConfig::Source source,
       const char* server_uri,
       net::ProxyServer::Scheme scheme,
       ProxyConfigServiceImpl::ProxyConfig* config,
       ProxyConfigServiceImpl::ProxyConfig::ManualProxy* manual_proxy) {
     if (!server_uri)
       return;
     config->mode = mode;
-    manual_proxy->source = source;
+    config->state = ProxyPrefs::CONFIG_SYSTEM;
     manual_proxy->server = net::ProxyServer::FromURI(server_uri, scheme);
   }
 
   void InitConfigWithTestInput(
-      const Input& input, ProxyConfigServiceImpl::ProxyConfig::Source source,
-      ProxyConfigServiceImpl::ProxyConfig* init_config) {
+      const Input& input, ProxyConfigServiceImpl::ProxyConfig* test_config) {
     switch (input.mode) {
       case MK_MODE(DIRECT):
       case MK_MODE(AUTO_DETECT):
       case MK_MODE(PAC_SCRIPT):
-        SetAutomaticProxy(input.mode, source, input.pac_url, init_config,
-                          &init_config->automatic_proxy);
+        SetAutomaticProxy(input.mode, input.pac_url, test_config,
+                          &test_config->automatic_proxy);
         return;
       case MK_MODE(SINGLE_PROXY):
-        SetManualProxy(input.mode, source, input.single_uri, MK_SCHM(HTTP),
-                       init_config, &init_config->single_proxy);
+        SetManualProxy(input.mode, input.single_uri, MK_SCHM(HTTP),
+                       test_config, &test_config->single_proxy);
         break;
       case MK_MODE(PROXY_PER_SCHEME):
-        SetManualProxy(input.mode, source, input.http_uri, MK_SCHM(HTTP),
-                       init_config, &init_config->http_proxy);
-        SetManualProxy(input.mode, source, input.https_uri, MK_SCHM(HTTPS),
-                       init_config, &init_config->https_proxy);
-        SetManualProxy(input.mode, source, input.ftp_uri, MK_SCHM(HTTP),
-                       init_config, &init_config->ftp_proxy);
-        SetManualProxy(input.mode, source, input.socks_uri, MK_SCHM(SOCKS5),
-                       init_config, &init_config->socks_proxy);
+        SetManualProxy(input.mode, input.http_uri, MK_SCHM(HTTP),
+                       test_config, &test_config->http_proxy);
+        SetManualProxy(input.mode, input.https_uri, MK_SCHM(HTTPS),
+                       test_config, &test_config->https_proxy);
+        SetManualProxy(input.mode, input.ftp_uri, MK_SCHM(HTTP),
+                       test_config, &test_config->ftp_proxy);
+        SetManualProxy(input.mode, input.socks_uri, MK_SCHM(SOCKS5),
+                       test_config, &test_config->socks_proxy);
         break;
     }
-    if (input.bypass_rules) {
-      init_config->bypass_rules.ParseFromString(input.bypass_rules);
-    }
-  }
-
-  void TestReadWriteAccessForMode(const Input& input,
-      ProxyConfigServiceImpl::ProxyConfig::Source source) {
-    // Init config from |source|.
-    ProxyConfigServiceImpl::ProxyConfig init_config;
-    InitConfigWithTestInput(input, source, &init_config);
-    CreateConfigService(init_config);
-
-    ProxyConfigServiceImpl::ProxyConfig config;
-    config_service()->UIGetProxyConfig(&config);
-
-    // For owner, write access to config should be equal CanBeWrittenByOwner().
-    // For non-owner, config is never writeable.
-    bool expected_writeable_by_owner = CanBeWrittenByOwner(source);
-    if (config.mode == MK_MODE(PROXY_PER_SCHEME)) {
-      if (input.http_uri) {
-        EXPECT_EQ(expected_writeable_by_owner,
-                  config.CanBeWrittenByUser(true, "http"));
-        EXPECT_FALSE(config.CanBeWrittenByUser(false, "http"));
-      }
-      if (input.https_uri) {
-        EXPECT_EQ(expected_writeable_by_owner,
-                  config.CanBeWrittenByUser(true, "http"));
-        EXPECT_FALSE(config.CanBeWrittenByUser(false, "https"));
-      }
-      if (input.ftp_uri) {
-        EXPECT_EQ(expected_writeable_by_owner,
-                  config.CanBeWrittenByUser(true, "http"));
-        EXPECT_FALSE(config.CanBeWrittenByUser(false, "ftp"));
-      }
-      if (input.socks_uri) {
-        EXPECT_EQ(expected_writeable_by_owner,
-                  config.CanBeWrittenByUser(true, "http"));
-        EXPECT_FALSE(config.CanBeWrittenByUser(false, "socks"));
-      }
-    } else {
-      EXPECT_EQ(expected_writeable_by_owner,
-                config.CanBeWrittenByUser(true, std::string()));
-      EXPECT_FALSE(config.CanBeWrittenByUser(false, std::string()));
-    }
-  }
-
-  void TestReadWriteAccessForScheme(
-      ProxyConfigServiceImpl::ProxyConfig::Source source,
-      const char* server_uri,
-      const std::string& scheme) {
-    // Init with manual |scheme| proxy.
-    ProxyConfigServiceImpl::ProxyConfig init_config;
-    ProxyConfigServiceImpl::ProxyConfig::ManualProxy* proxy =
-        init_config.MapSchemeToProxy(scheme);
-    net::ProxyServer::Scheme net_scheme = MK_SCHM(HTTP);
-    if (scheme == "http" || scheme == "ftp")
-      net_scheme = MK_SCHM(HTTP);
-    else if (scheme == "https")
-      net_scheme = MK_SCHM(HTTPS);
-    else if (scheme == "socks")
-      net_scheme = MK_SCHM(SOCKS4);
-    SetManualProxy(MK_MODE(PROXY_PER_SCHEME), source, server_uri, net_scheme,
-                   &init_config, proxy);
-    CreateConfigService(init_config);
-
-    ProxyConfigServiceImpl::ProxyConfig config;
-    config_service()->UIGetProxyConfig(&config);
-
-    // For owner, write access to config should be equal CanBeWrittenByOwner().
-    // For non-owner, config is never writeable.
-    bool expected_writeable_by_owner = CanBeWrittenByOwner(source);
-    EXPECT_EQ(expected_writeable_by_owner,
-              config.CanBeWrittenByUser(true, scheme));
-    EXPECT_FALSE(config.CanBeWrittenByUser(false, scheme));
-
-    const char* all_schemes[] = {
-      "http", "https", "ftp", "socks",
-    };
-
-    // Rest of protos should be writeable by owner, but not writeable by
-    // non-owner.
-    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(all_schemes); ++i) {
-      if (scheme == all_schemes[i])
-        continue;
-      EXPECT_TRUE(config.CanBeWrittenByUser(true, all_schemes[i]));
-      EXPECT_FALSE(config.CanBeWrittenByUser(false, all_schemes[i]));
-    }
+    if (input.bypass_rules)
+      test_config->bypass_rules.ParseFromString(input.bypass_rules);
   }
 
   // Synchronously gets the latest proxy config.
-  bool SyncGetLatestProxyConfig(net::ProxyConfig* config) {
+  net::ProxyConfigService::ConfigAvailability SyncGetLatestProxyConfig(
+      net::ProxyConfig* config) {
+    *config = net::ProxyConfig();
     // Let message loop process all messages.
-    MessageLoop::current()->RunAllPending();
-    // Calls IOGetProxyConfig (which is called from
+    loop_.RunAllPending();
+    // Calls ChromeProIOGetProxyConfig (which is called from
     // ProxyConfigService::GetLatestProxyConfig), running on faked IO thread.
-    return config_service_->IOGetProxyConfig(config);
+    return proxy_config_service_->GetLatestProxyConfig(config);
   }
 
-  ProxyConfigServiceImpl* config_service() const {
-    return config_service_;
+  MessageLoop loop_;
+  scoped_ptr<ChromeProxyConfigService> proxy_config_service_;
+  scoped_ptr<ProxyConfigServiceImpl> config_service_impl_;
+
+ private:
+  // Default stub state has ethernet as the active connected network and
+  // PROFILE_SHARED as profile type, which this unittest expects.
+  ScopedStubCrosEnabler stub_cros_enabler_;
+  BrowserThread ui_thread_;
+  BrowserThread io_thread_;
+};
+
+class ProxyConfigServiceImplTest
+    : public ProxyConfigServiceImplTestBase<testing::Test> {
+ protected:
+  virtual void SetUp() {
+    Init(&pref_service_);
   }
 
- private:
-  bool CanBeWrittenByOwner(
-    ProxyConfigServiceImpl::ProxyConfig::Source source) const {
-    return source == MK_SRC(POLICY) ? false : true;
-  }
-
-  ScopedStubCrosEnabler stub_cros_enabler_;
-  MessageLoop message_loop_;
-  BrowserThread ui_thread_;
-  BrowserThread io_thread_;
-
-  scoped_refptr<ProxyConfigServiceImpl> config_service_;
+  TestingPrefService pref_service_;
 };
 
-TEST_F(ProxyConfigServiceImplTest, ChromeosProxyConfigToNetProxyConfig) {
+TEST_F(ProxyConfigServiceImplTest, SerializeAndDeserializeForNetwork) {
+  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
+    if (!tests[i].is_valid)
+      continue;
+
+    SCOPED_TRACE(StringPrintf("Test[%" PRIuS "] %s", i,
+                              tests[i].description.c_str()));
+
+    ProxyConfigServiceImpl::ProxyConfig source_config;
+    InitConfigWithTestInput(tests[i].input, &source_config);
+
+    // Serialize source_config into std::string.
+    std::string serialized_value;
+    EXPECT_TRUE(source_config.SerializeForNetwork(&serialized_value));
+
+    // Deserialize std:string into target_config.
+    ProxyConfigServiceImpl::ProxyConfig target_config;
+    EXPECT_TRUE(target_config.DeserializeForNetwork(serialized_value));
+
+    // Compare the configs after serialization and deserialization.
+    net::ProxyConfig net_src_cfg;
+    net::ProxyConfig net_tgt_cfg;
+    source_config.ToNetProxyConfig(&net_src_cfg);
+    target_config.ToNetProxyConfig(&net_tgt_cfg);
+#if !defined(NDEBUG)
+    if (!net_src_cfg.Equals(net_tgt_cfg)) {
+      std::string src_output, tgt_output;
+      JSONStringValueSerializer src_serializer(&src_output);
+      src_serializer.Serialize(*net_src_cfg.ToValue());
+      JSONStringValueSerializer tgt_serializer(&tgt_output);
+      tgt_serializer.Serialize(*net_tgt_cfg.ToValue());
+      VLOG(1) << "source:\n" << src_output
+              << "\ntarget:\n" << tgt_output;

[Mattias Nissler (ping if slow), 2011/10/24 10:18:28]

It seems like this should really be the message that gets logged if the
EXPECT_TRUE() below fails.

[kuan, 2011/10/25 02:01:54]

test is no longer valid, so it's removed.

+    }
+#endif  // !defined(NDEBUG)
+    EXPECT_TRUE(net_src_cfg.Equals(net_tgt_cfg));
+  }
+}
+
+TEST_F(ProxyConfigServiceImplTest, NetworkProxy) {
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
     SCOPED_TRACE(StringPrintf("Test[%" PRIuS "] %s", i,
                               tests[i].description.c_str()));
 
-    ProxyConfigServiceImpl::ProxyConfig init_config;
-    InitConfigWithTestInput(tests[i].input, MK_SRC(OWNER), &init_config);
-    CreateConfigService(init_config);
+    ProxyConfigServiceImpl::ProxyConfig test_config;
+    InitConfigWithTestInput(tests[i].input, &test_config);
+    config_service_impl_->SetTesting(&test_config);
 
     net::ProxyConfig config;
-    SyncGetLatestProxyConfig(&config);
+    EXPECT_EQ(MK_AVAIL(VALID), SyncGetLatestProxyConfig(&config));
 
     EXPECT_EQ(tests[i].auto_detect, config.auto_detect());
     EXPECT_EQ(tests[i].pac_url, config.pac_url());
     EXPECT_TRUE(tests[i].proxy_rules.Matches(config.proxy_rules()));
   }
 }
 
 TEST_F(ProxyConfigServiceImplTest, ModifyFromUI) {
   for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
     SCOPED_TRACE(StringPrintf("Test[%" PRIuS "] %s", i,
                               tests[i].description.c_str()));
 
     // Init with direct.
-    ProxyConfigServiceImpl::ProxyConfig init_config;
-    SetAutomaticProxy(MK_MODE(DIRECT), MK_SRC(OWNER), NULL, &init_config,
-                      &init_config.automatic_proxy);
-    CreateConfigService(init_config);
+    ProxyConfigServiceImpl::ProxyConfig test_config;
+    SetAutomaticProxy(MK_MODE(DIRECT), NULL, &test_config,
+                      &test_config.automatic_proxy);
+    config_service_impl_->SetTesting(&test_config);
 
     // Set config to tests[i].input via UI.
     net::ProxyBypassRules bypass_rules;
     const Input& input = tests[i].input;
     switch (input.mode) {
       case MK_MODE(DIRECT) :
-        config_service()->UISetProxyConfigToDirect();
+        config_service_impl_->UISetProxyConfigToDirect();
         break;
       case MK_MODE(AUTO_DETECT) :
-        config_service()->UISetProxyConfigToAutoDetect();
+        config_service_impl_->UISetProxyConfigToAutoDetect();
         break;
       case MK_MODE(PAC_SCRIPT) :
-        config_service()->UISetProxyConfigToPACScript(GURL(input.pac_url));
+        config_service_impl_->UISetProxyConfigToPACScript(GURL(input.pac_url));
         break;
       case MK_MODE(SINGLE_PROXY) :
-        config_service()->UISetProxyConfigToSingleProxy(
+        config_service_impl_->UISetProxyConfigToSingleProxy(
             net::ProxyServer::FromURI(input.single_uri, MK_SCHM(HTTP)));
         if (input.bypass_rules) {
           bypass_rules.ParseFromString(input.bypass_rules);
-          config_service()->UISetProxyConfigBypassRules(bypass_rules);
+          config_service_impl_->UISetProxyConfigBypassRules(bypass_rules);
         }
         break;
       case MK_MODE(PROXY_PER_SCHEME) :
         if (input.http_uri) {
-          config_service()->UISetProxyConfigToProxyPerScheme("http",
+          config_service_impl_->UISetProxyConfigToProxyPerScheme("http",
                   net::ProxyServer::FromURI(input.http_uri, MK_SCHM(HTTP)));
         }
         if (input.https_uri) {
-          config_service()->UISetProxyConfigToProxyPerScheme("https",
+          config_service_impl_->UISetProxyConfigToProxyPerScheme("https",
               net::ProxyServer::FromURI(input.https_uri, MK_SCHM(HTTPS)));
         }
         if (input.ftp_uri) {
-          config_service()->UISetProxyConfigToProxyPerScheme("ftp",
+          config_service_impl_->UISetProxyConfigToProxyPerScheme("ftp",
               net::ProxyServer::FromURI(input.ftp_uri, MK_SCHM(HTTP)));
         }
         if (input.socks_uri) {
-          config_service()->UISetProxyConfigToProxyPerScheme("socks",
+          config_service_impl_->UISetProxyConfigToProxyPerScheme("socks",
               net::ProxyServer::FromURI(input.socks_uri, MK_SCHM(SOCKS5)));
         }
         if (input.bypass_rules) {
           bypass_rules.ParseFromString(input.bypass_rules);
-          config_service()->UISetProxyConfigBypassRules(bypass_rules);
+          config_service_impl_->UISetProxyConfigBypassRules(bypass_rules);
         }
         break;
     }
 
     // Retrieve config from IO thread.
     net::ProxyConfig io_config;
-    SyncGetLatestProxyConfig(&io_config);
+    EXPECT_EQ(MK_AVAIL(VALID), SyncGetLatestProxyConfig(&io_config));
     EXPECT_EQ(tests[i].auto_detect, io_config.auto_detect());
     EXPECT_EQ(tests[i].pac_url, io_config.pac_url());
     EXPECT_TRUE(tests[i].proxy_rules.Matches(io_config.proxy_rules()));
 
     // Retrieve config from UI thread.
     ProxyConfigServiceImpl::ProxyConfig ui_config;
-    config_service()->UIGetProxyConfig(&ui_config);
+    config_service_impl_->UIGetProxyConfig(&ui_config);
     EXPECT_EQ(input.mode, ui_config.mode);
     if (tests[i].is_valid) {
       if (input.pac_url)
         EXPECT_EQ(GURL(input.pac_url), ui_config.automatic_proxy.pac_url);
       const net::ProxyRulesExpectation& proxy_rules = tests[i].proxy_rules;
       if (input.single_uri)
         EXPECT_EQ(proxy_rules.single_proxy,
                   ui_config.single_proxy.server.ToURI());
       if (input.http_uri)
         EXPECT_EQ(proxy_rules.proxy_for_http,
                   ui_config.http_proxy.server.ToURI());
       if (input.https_uri)
         EXPECT_EQ(proxy_rules.proxy_for_https,
                   ui_config.https_proxy.server.ToURI());
       if (input.ftp_uri)
         EXPECT_EQ(proxy_rules.proxy_for_ftp,
                   ui_config.ftp_proxy.server.ToURI());
       if (input.socks_uri) {
         EXPECT_EQ(proxy_rules.fallback_proxy,
                   ui_config.socks_proxy.server.ToURI());
       }
       if (input.bypass_rules)
         EXPECT_TRUE(bypass_rules.Equals(ui_config.bypass_rules));
     }
   }
 }
 
-TEST_F(ProxyConfigServiceImplTest, ProxyChangedObserver) {
-  // This is used to observe for OnProxyConfigChanged notification.
-  class ProxyChangedObserver : public net::ProxyConfigService::Observer {
-   public:
-    explicit ProxyChangedObserver(
-        const scoped_refptr<ProxyConfigServiceImpl>& config_service)
-        : config_service_(config_service) {
-      config_service_->AddObserver(this);
+TEST_F(ProxyConfigServiceImplTest, DynamicPrefsOverride) {
+  // Determine groups of 3 test inputs to use for managed, recommended and
+  // network proxies.
+  int test = 0;
+  for (size_t i = 0, j = 0; i < ARRAYSIZE_UNSAFE(tests);) {
+    int managed_idx = -1;
+    int recommended_idx = -1;
+    int network_idx = -1;
+    for (j = i; j < ARRAYSIZE_UNSAFE(tests) && network_idx == -1; ++j) {
+      // Only use valid input that has proxy.
+      if (!tests[j].is_valid || tests[j].input.mode == MK_MODE(DIRECT))
+        continue;
+      if (managed_idx == -1)
+        managed_idx = j;
+      else if (recommended_idx == -1)
+        recommended_idx = j;
+      else if (network_idx == -1)
+        network_idx = j;
     }
-    virtual ~ProxyChangedObserver() {
-      config_service_->RemoveObserver(this);
-    }
-    net::ProxyConfigService::ConfigAvailability availability() const {
-      return availability_;
-    }
-    const net::ProxyConfig& config() const {
-      return config_;
-    }
+    if (network_idx == -1)  // Insufficient inputs for a set of 3 proxies.
+      break;
 
-   private:
-    virtual void OnProxyConfigChanged(
-        const net::ProxyConfig& config,
-        net::ProxyConfigService::ConfigAvailability availability) {
-      config_ = config;
-      availability_ = availability;
-    }
+    // Next group starts after the current managed proxy.
+    i = managed_idx + 1;

[Mattias Nissler (ping if slow), 2011/10/24 10:18:28]

This whole index-searching seems pretty brittle to me. Can't you just put a
static table of the index triplets here? That way, it would also be more obvious
to the reader which TestParams go with each other.

[kuan, 2011/10/25 02:01:54]

i intentionally don't want to hardcode the triple-pairing, 'cos then i change
the original table, i must remember to change the triple-pairings, which i think
it's more error-prone.  i already print out the triple-pairings if a test fails,
so i think that suffices.

[Mattias Nissler (ping if slow), 2011/10/25 12:43:11]

I can see that it might be slightly more work for you in the future, but the way
it's currently done is basically unreadable for everyone else IMHO. I thought we
have the general goal to optimize code for being readable rather than for some
(IMHO minor) shortcuts when updating. I don't feel this discussion is worth more
of my time though, so your call :)

[kuan, 2011/10/25 16:01:50]

Done.

 
-    scoped_refptr<ProxyConfigServiceImpl> config_service_;
-    net::ProxyConfigService::ConfigAvailability availability_;
-    net::ProxyConfig config_;
-  };
+    const TestParams& managed_params = tests[managed_idx];
+    const TestParams& recommended_params = tests[recommended_idx];
+    const TestParams& network_params = tests[network_idx];
 
-  // Init with direct.
-  ProxyConfigServiceImpl::ProxyConfig init_config;
-  SetAutomaticProxy(MK_MODE(DIRECT), MK_SRC(OWNER), NULL, &init_config,
-                    &init_config.automatic_proxy);
-  CreateConfigService(init_config);
+    SCOPED_TRACE(StringPrintf(
+        "Test[%" PRIuS "] managed=[%s], recommended=[%s], network=[%s]", test++,
+        managed_params.description.c_str(),
+        recommended_params.description.c_str(),
+        network_params.description.c_str()));
 
-  ProxyChangedObserver observer(config_service());
+    ProxyConfigServiceImpl::ProxyConfig managed_config;
+    InitConfigWithTestInput(managed_params.input, &managed_config);
+    ProxyConfigServiceImpl::ProxyConfig recommended_config;
+    InitConfigWithTestInput(recommended_params.input, &recommended_config);
+    ProxyConfigServiceImpl::ProxyConfig network_config;
+    InitConfigWithTestInput(network_params.input, &network_config);
 
-  // Set to pac script from UI.
-  EXPECT_TRUE(config_service()->UISetProxyConfigToPACScript(
-      GURL("http://wpad.dat")));
-  // Retrieve config from IO thread.
-  net::ProxyConfig io_config;
-  SyncGetLatestProxyConfig(&io_config);
+    // hManaged proxy pref should take effect over recommended proxy and

[Mattias Nissler (ping if slow), 2011/10/24 10:18:28]

s/hManaged/Managed/

[kuan, 2011/10/25 02:01:54]

Done.

+    // non-existent network proxy.
+    config_service_impl_->SetTesting(NULL);
+    pref_service_.SetManagedPref(prefs::kProxy,
+                                 managed_config.ToPrefProxyConfig());
+    pref_service_.SetRecommendedPref(prefs::kProxy,
+                                     recommended_config.ToPrefProxyConfig());
+    net::ProxyConfig actual_config;
+    EXPECT_EQ(MK_AVAIL(VALID), SyncGetLatestProxyConfig(&actual_config));
+    EXPECT_EQ(managed_params.auto_detect, actual_config.auto_detect());
+    EXPECT_EQ(managed_params.pac_url, actual_config.pac_url());
+    EXPECT_TRUE(managed_params.proxy_rules.Matches(
+        actual_config.proxy_rules()));
 
-  // Observer should have gotten the same new proxy config.
-  EXPECT_EQ(net::ProxyConfigService::CONFIG_VALID, observer.availability());
-  EXPECT_TRUE(io_config.Equals(observer.config()));
-}
+    // Recommended proxy pref should take effect when managed proxy pref is
+    // removed.
+    pref_service_.RemoveManagedPref(prefs::kProxy);
+    EXPECT_EQ(MK_AVAIL(VALID), SyncGetLatestProxyConfig(&actual_config));
+    EXPECT_EQ(recommended_params.auto_detect, actual_config.auto_detect());
+    EXPECT_EQ(recommended_params.pac_url, actual_config.pac_url());
+    EXPECT_TRUE(recommended_params.proxy_rules.Matches(
+        actual_config.proxy_rules()));
 
-TEST_F(ProxyConfigServiceImplTest, SerializeAndDeserializeForNetwork) {
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
-    if (!tests[i].is_valid)
-      continue;
+    // Network proxy should take take effect over recommended proxy pref.
+    config_service_impl_->SetTesting(&network_config);
+    EXPECT_EQ(MK_AVAIL(VALID), SyncGetLatestProxyConfig(&actual_config));
+    EXPECT_EQ(network_params.auto_detect, actual_config.auto_detect());
+    EXPECT_EQ(network_params.pac_url, actual_config.pac_url());
+    EXPECT_TRUE(network_params.proxy_rules.Matches(
+        actual_config.proxy_rules()));
 
-    SCOPED_TRACE(StringPrintf("Test[%" PRIuS "] %s", i,
-                              tests[i].description.c_str()));
+    // Managed proxy pref should take effect over network proxy.
+    pref_service_.SetManagedPref(prefs::kProxy,
+                                 managed_config.ToPrefProxyConfig());
+    EXPECT_EQ(MK_AVAIL(VALID), SyncGetLatestProxyConfig(&actual_config));
+    EXPECT_EQ(managed_params.auto_detect, actual_config.auto_detect());
+    EXPECT_EQ(managed_params.pac_url, actual_config.pac_url());
+    EXPECT_TRUE(managed_params.proxy_rules.Matches(
+        actual_config.proxy_rules()));
 
-    ProxyConfigServiceImpl::ProxyConfig source_config;
-    InitConfigWithTestInput(tests[i].input, MK_SRC(OWNER), &source_config);
+    // Network proxy should take effect over recommended proxy pref when managed
+    // proxy pref is removed.
+    pref_service_.RemoveManagedPref(prefs::kProxy);
+    EXPECT_EQ(MK_AVAIL(VALID), SyncGetLatestProxyConfig(&actual_config));
+    EXPECT_EQ(network_params.auto_detect, actual_config.auto_detect());
+    EXPECT_EQ(network_params.pac_url, actual_config.pac_url());
+    EXPECT_TRUE(network_params.proxy_rules.Matches(
+        actual_config.proxy_rules()));
 
-    // Serialize source_config into std::string.
-    std::string serialized_value;
-    EXPECT_TRUE(source_config.SerializeForNetwork(&serialized_value));
-
-    // Deserialize std:string into target_config.
-    ProxyConfigServiceImpl::ProxyConfig target_config;
-    EXPECT_TRUE(target_config.DeserializeForNetwork(serialized_value));
-
-    // Compare the configs after serialization and deserialization.
-    net::ProxyConfig net_src_cfg;
-    net::ProxyConfig net_tgt_cfg;
-    source_config.ToNetProxyConfig(&net_src_cfg);
-    target_config.ToNetProxyConfig(&net_tgt_cfg);
-#if !defined(NDEBUG)
-    if (!net_src_cfg.Equals(net_tgt_cfg)) {
-      std::string src_output, tgt_output;
-      JSONStringValueSerializer src_serializer(&src_output);
-      src_serializer.Serialize(*net_src_cfg.ToValue());
-      JSONStringValueSerializer tgt_serializer(&tgt_output);
-      tgt_serializer.Serialize(*net_tgt_cfg.ToValue());
-      VLOG(1) << "source:\n" << src_output
-              << "\ntarget:\n" << tgt_output;
-    }
-#endif  // !defined(NDEBUG)
-    EXPECT_TRUE(net_src_cfg.Equals(net_tgt_cfg));
+    // Removing recommended proxy pref should have no effect on network proxy.
+    pref_service_.RemoveRecommendedPref(prefs::kProxy);
+    EXPECT_EQ(MK_AVAIL(VALID), SyncGetLatestProxyConfig(&actual_config));
+    EXPECT_EQ(network_params.auto_detect, actual_config.auto_detect());
+    EXPECT_EQ(network_params.pac_url, actual_config.pac_url());
+    EXPECT_TRUE(network_params.proxy_rules.Matches(
+        actual_config.proxy_rules()));
   }
 }
 
-TEST_F(ProxyConfigServiceImplTest, ReadWriteAccessForPolicySource) {
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
-    if (!tests[i].test_read_write_access)
-      continue;
-    SCOPED_TRACE(StringPrintf("Test[%" PRIuS "] %s", i,
-                              tests[i].description.c_str()));
-    TestReadWriteAccessForMode(tests[i].input, MK_SRC(POLICY));
-  }
-}
-
-TEST_F(ProxyConfigServiceImplTest, ReadWriteAccessForOwnerSource) {
-  for (size_t i = 0; i < ARRAYSIZE_UNSAFE(tests); ++i) {
-    if (!tests[i].test_read_write_access)
-      continue;
-    SCOPED_TRACE(StringPrintf("Test[%" PRIuS "] %s", i,
-                              tests[i].description.c_str()));
-    TestReadWriteAccessForMode(tests[i].input, MK_SRC(OWNER));
-  }
-}
-
-TEST_F(ProxyConfigServiceImplTest, ReadWriteAccessForMixedSchemes) {
-  const char* http_uri = "www.google.com:80";
-  const char* https_uri = "www.foo.com:110";
-  const char* ftp_uri = "ftp.foo.com:121";
-  const char* socks_uri = "socks.com:888";
-
-  // Init with policy source.
-  TestReadWriteAccessForScheme(MK_SRC(POLICY), http_uri, "http");
-  TestReadWriteAccessForScheme(MK_SRC(POLICY), https_uri, "https");
-  TestReadWriteAccessForScheme(MK_SRC(POLICY), ftp_uri, "ftp");
-  TestReadWriteAccessForScheme(MK_SRC(POLICY), socks_uri, "socks");
-
-  // Init with owner source.
-  TestReadWriteAccessForScheme(MK_SRC(OWNER), http_uri, "http");
-  TestReadWriteAccessForScheme(MK_SRC(OWNER), https_uri, "https");
-  TestReadWriteAccessForScheme(MK_SRC(OWNER), ftp_uri, "ftp");
-  TestReadWriteAccessForScheme(MK_SRC(OWNER), socks_uri, "socks");
-}
+}  // namespace
 
 }  // namespace chromeos