ProxyConfig behaved like a struct, but was defined as a class.

Changed it to be a proper class with hidden implementation variables, setters etc.

Also seized this opportunity to move the bypass list from being a member of ProxyConfig, to being a member of ProxyRules. This is a more correct hiearchy, since the bypass rules only apply to the manual settings. Lastly, this makes it possible to have the manual rules evaluation be a method on ProxyRules, and shift some more code out of proxy_service.
Review URL: http://codereview.chromium.org/651070

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

3 files changed, 31 insertions, 35 deletions

diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index bb8cba9..01181bc2 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -934,21 +934,21 @@ net::ProxyConfig* CreateProxyConfig(const CommandLine& command_line) {
   if (command_line.HasSwitch(switches::kProxyServer)) {
     const std::wstring& proxy_server =
         command_line.GetSwitchValue(switches::kProxyServer);
-    proxy_config->proxy_rules.ParseFromString(WideToASCII(proxy_server));
+    proxy_config->proxy_rules().ParseFromString(WideToASCII(proxy_server));
   }
 
   if (command_line.HasSwitch(switches::kProxyPacUrl)) {
-    proxy_config->pac_url =
+    proxy_config->set_pac_url(
         GURL(WideToASCII(command_line.GetSwitchValue(
-            switches::kProxyPacUrl)));
+            switches::kProxyPacUrl))));
   }
 
   if (command_line.HasSwitch(switches::kProxyAutoDetect)) {
-    proxy_config->auto_detect = true;
+    proxy_config->set_auto_detect(true);
   }
 
   if (command_line.HasSwitch(switches::kProxyBypassList)) {
-    proxy_config->bypass_rules.ParseFromString(
+    proxy_config->proxy_rules().bypass_rules.ParseFromString(
         WideToASCII(command_line.GetSwitchValue(
             switches::kProxyBypassList)));
   }
diff --git a/chrome/browser/net/chrome_url_request_context_unittest.cc b/chrome/browser/net/chrome_url_request_context_unittest.cc
index e7d679c..676340e 100644
--- a/chrome/browser/net/chrome_url_request_context_unittest.cc
+++ b/chrome/browser/net/chrome_url_request_context_unittest.cc
@@ -57,8 +57,7 @@ TEST(ChromeUrlRequestContextTest, CreateProxyConfigTest) {
     bool is_null;
     bool auto_detect;
     GURL pac_url;
-    net::ProxyConfig::ProxyRules proxy_rules;
-    const char* proxy_bypass_list;  // newline separated
+    net::ProxyRulesExpectation proxy_rules;
   } tests[] = {
     {
       TEST_DESC("Empty command line"),
@@ -68,8 +67,7 @@ TEST(ChromeUrlRequestContextTest, CreateProxyConfigTest) {
       true,                                               // is_null
       false,                                              // auto_detect
       GURL(),                                             // pac_url
-      net::ProxyConfig::ProxyRules(),                     // proxy_rules
-      "",                                                 // proxy_bypass_list
+      net::ProxyRulesExpectation::Empty(),
     },
     {
       TEST_DESC("No proxy"),
@@ -79,8 +77,7 @@ TEST(ChromeUrlRequestContextTest, CreateProxyConfigTest) {
       false,                                              // is_null
       false,                                              // auto_detect
       GURL(),                                             // pac_url
-      net::ProxyConfig::ProxyRules(),                     // proxy_rules
-      "",                                                 // proxy_bypass_list
+      net::ProxyRulesExpectation::Empty(),
     },
     {
       TEST_DESC("No proxy with extra parameters."),
@@ -90,8 +87,7 @@ TEST(ChromeUrlRequestContextTest, CreateProxyConfigTest) {
       false,                                              // is_null
       false,                                              // auto_detect
       GURL(),                                             // pac_url
-      net::ProxyConfig::ProxyRules(),                     // proxy_rules
-      "",                                                 // proxy_bypass_list
+      net::ProxyRulesExpectation::Empty(),
     },
     {
       TEST_DESC("Single proxy."),
@@ -101,8 +97,9 @@ TEST(ChromeUrlRequestContextTest, CreateProxyConfigTest) {
       false,                                              // is_null
       false,                                              // auto_detect
       GURL(),                                             // pac_url
-      net::MakeSingleProxyRules("http://proxy:8888"),     // proxy_rules
-      "",                                                 // proxy_bypass_list
+      net::ProxyRulesExpectation::Single(
+          "proxy:8888",  // single proxy
+          ""),           // bypass rules
     },
     {
       TEST_DESC("Per scheme proxy."),
@@ -112,10 +109,11 @@ TEST(ChromeUrlRequestContextTest, CreateProxyConfigTest) {
       false,                                              // is_null
       false,                                              // auto_detect
       GURL(),                                             // pac_url
-      net::MakeProxyPerSchemeRules("httpproxy:8888",
-                                   "",
-                                   "ftpproxy:8889"),      // proxy_rules
-      "",                                                 // proxy_bypass_list
+      net::ProxyRulesExpectation::PerScheme(
+          "httpproxy:8888",  // http
+          "",                // https
+          "ftpproxy:8889",   // ftp
+          ""),               // bypass rules
     },
     {
       TEST_DESC("Per scheme proxy with bypass URLs."),
@@ -125,11 +123,12 @@ TEST(ChromeUrlRequestContextTest, CreateProxyConfigTest) {
       false,                                              // is_null
       false,                                              // auto_detect
       GURL(),                                             // pac_url
-      net::MakeProxyPerSchemeRules("httpproxy:8888",
-                                   "",
-                                   "ftpproxy:8889"),      // proxy_rules
-      // TODO(eroman): 127.0.0.1/8 is unsupported, so it was dropped
-      "*.google.com\nfoo.com:99\n1.2.3.4:22\n",
+      net::ProxyRulesExpectation::PerScheme(
+          "httpproxy:8888",  // http
+          "",                // https
+          "ftpproxy:8889",   // ftp
+          // TODO(eroman): 127.0.0.1/8 is unsupported, so it was dropped
+          "*.google.com,foo.com:99,1.2.3.4:22"),
     },
     {
       TEST_DESC("Pac URL with proxy bypass URLs"),
@@ -139,9 +138,9 @@ TEST(ChromeUrlRequestContextTest, CreateProxyConfigTest) {
       false,                                              // is_null
       false,                                              // auto_detect
       GURL("http://wpad/wpad.dat"),                       // pac_url
-      net::ProxyConfig::ProxyRules(),                     // proxy_rules
-      // TODO(eroman): 127.0.0.1/8 is unsupported, so it was dropped
-      "*.google.com\nfoo.com:99\n1.2.3.4:22\n",
+      net::ProxyRulesExpectation::EmptyWithBypass(
+          // TODO(eroman): 127.0.0.1/8 is unsupported, so it was dropped
+          "*.google.com,foo.com:99,1.2.3.4:22"),
     },
     {
       TEST_DESC("Autodetect"),
@@ -151,8 +150,7 @@ TEST(ChromeUrlRequestContextTest, CreateProxyConfigTest) {
       false,                                              // is_null
       true,                                               // auto_detect
       GURL(),                                             // pac_url
-      net::ProxyConfig::ProxyRules(),                     // proxy_rules
-      "",                                                 // proxy_bypass_list
+      net::ProxyRulesExpectation::Empty(),
     }
   };
 
@@ -166,11 +164,9 @@ TEST(ChromeUrlRequestContextTest, CreateProxyConfigTest) {
       EXPECT_TRUE(config == NULL);
     } else {
       EXPECT_TRUE(config != NULL);
-      EXPECT_EQ(tests[i].auto_detect, config->auto_detect);
-      EXPECT_EQ(tests[i].pac_url, config->pac_url);
-      EXPECT_EQ(tests[i].proxy_bypass_list,
-                net::FlattenProxyBypass(config->bypass_rules));
-      EXPECT_EQ(tests[i].proxy_rules, config->proxy_rules);
+      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()));
     }
   }
 }
diff --git a/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc b/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc
index bb4c1b3..4d2eeb8 100644
--- a/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc
+++ b/chrome/browser/net/resolve_proxy_msg_helper_unittest.cc
@@ -14,7 +14,7 @@
 class MockProxyConfigService : public net::ProxyConfigService {
  public:
   virtual int GetProxyConfig(net::ProxyConfig* results) {
-    results->pac_url = GURL("http://pac");
+    results->set_pac_url(GURL("http://pac"));
     return net::OK;
   }
 };