Introduce a separate preference for 'proxy server mode'

chrome/browser/prefs/proxy_prefs.h

+// Copyright (c) 2010 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.
+
+#ifndef CHROME_BROWSER_PREFS_PROXY_PREFS_H_
+#define CHROME_BROWSER_PREFS_PROXY_PREFS_H_
+#pragma once
+
+#include <string>
+
+namespace ProxyPrefs {
+  // Possible types of specifying proxy settings. Do not change the order of
+  // the constants, because numeric values are exposed to users.
+  // If you add an enum constant, you should also add a string to
+  // kProxyModeNames in the .cc file.
+  // Note that these constants serve for the internal representation in pref
+  // stores. Proxy policies use a different representation, see
+  // configuration_policy_store_interface.h.

[Mattias Nissler (ping if slow), 2010/12/21 15:54:29]

The last 3 lines of the comment feel like they really belong in
configuration_policy_store_interface and not here.

[battre (please use the other), 2010/12/21 20:14:09]

They both cross reference each other. I think it is good having this information
in both places.

[Mattias Nissler (ping if slow), 2010/12/22 10:22:11]

Yeah, but the pref layer should not depend on what policy is doing, so I feel
the comment doesn't belong here. I don't have strong feelings about this though.

[battre, 2010/12/22 14:41:16]

I agree.

Done.

+  enum ProxyMode {

[eroman, 2010/12/21 22:55:21]

nit: I suggest prefixing all of these constants by MODE_*

[battre, 2010/12/22 10:01:27]

Done.

+    // Direct connection to the network, other proxy preferences are ignored.
+    DISABLED = 0,

[eroman, 2010/12/21 22:55:21]

I suggest calling this DIRECT rather than DISABLED.

[battre, 2010/12/22 10:01:27]

Done.

+
+    // Try to retrieve a PAC script from http://wpad/wpad.dat or fall back to
+    // direct connection.
+    AUTO_DETECT = 1,
+
+    // Try to retrieve a PAC script from kProxyPacURL or fall back to direct
+    // connection.
+    PAC_SCRIPT = 2,
+
+    // Use the settings specified in kProxyServer and kProxyBypassList.
+    FIXED_SERVERS = 3,

[eroman, 2010/12/21 22:55:21]

See my comment about storing the proxy servers for each scheme as a separate
preference value.

If you do that, then it would be natural to split this into two modes:

  (1) A mode for using a single proxy for everything (would look to
kProxyForAllSchemes).

  (2) A mode to use specific proxy server based on URL. Would use either
kProxyForHttpUrl, kProxyForHttpsUrl, kProxyForFtpUrl, or kProxyForAllSchemes.
(The latter would be used as a "fallback" when the more appropriate url-specific
proxy preference was not specified, and would be the way for specifying SOCKS
proxies).

This maps pretty directly to how the preferences are stored internally by
net::ProxyConfig

[battre, 2010/12/22 10:01:27]

Noted in http://crbug.com/67779

+
+    // The system's proxy settings are used, other proxy preferences are
+    // ignored.
+    SYSTEM = 4,
+
+    NUM_MODES
+  };
+
+  bool IntToProxyMode(int in_value, ProxyMode* out_value);
+  bool StringToProxyMode(const std::string& in_value,
+                         ProxyMode* out_value);
+};
+
+#endif  // CHROME_BROWSER_PREFS_PROXY_PREFS_H_