Emit an event to NetLog whenever the proxy settings change.

Also removes the operator<< on ProxyConfig.

BUG=52004

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

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

8 files changed, 219 insertions, 145 deletions

diff --git a/chrome/browser/dom_ui/net_internals_ui.cc b/chrome/browser/dom_ui/net_internals_ui.cc
index aa5343e..d64e68c 100644
--- a/chrome/browser/dom_ui/net_internals_ui.cc
+++ b/chrome/browser/dom_ui/net_internals_ui.cc
@@ -5,7 +5,6 @@
 #include "chrome/browser/dom_ui/net_internals_ui.h"
 
 #include <algorithm>
-#include <sstream>
 #include <string>
 #include <utility>
 #include <vector>
@@ -601,19 +600,15 @@ void NetInternalsMessageHandler::IOThreadImpl::OnGetProxySettings(
   URLRequestContext* context = context_getter_->GetURLRequestContext();
   net::ProxyService* proxy_service = context->proxy_service();
 
-  // TODO(eroman): send a dictionary rather than a flat string, so client can do
-  //               its own presentation.
-  std::string settings_string;
-
+  Value* settings_value = NULL;
   if (proxy_service->config_has_been_initialized()) {
-    // net::ProxyConfig defines an operator<<.
-    std::ostringstream stream;
-    stream << proxy_service->config();
-    settings_string = stream.str();
+    settings_value = proxy_service->config().ToValue();
+  } else {
+    settings_value = Value::CreateStringValue(std::string());
   }
 
   CallJavascriptFunction(L"g_browser.receivedProxySettings",
-                         Value::CreateStringValue(settings_string));
+                         settings_value);
 }
 
 void NetInternalsMessageHandler::IOThreadImpl::OnReloadProxySettings(
diff --git a/chrome/browser/importer/firefox_proxy_settings_unittest.cc b/chrome/browser/importer/firefox_proxy_settings_unittest.cc
index ed2a66c..a1d84c3 100644
--- a/chrome/browser/importer/firefox_proxy_settings_unittest.cc
+++ b/chrome/browser/importer/firefox_proxy_settings_unittest.cc
@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <sstream>
-
 #include "testing/gtest/include/gtest/gtest.h"
 
 #include "base/file_path.h"
@@ -56,11 +54,6 @@ TEST_F(FirefoxProxySettingsTest, TestParse) {
   net::ProxyConfig config;
   EXPECT_TRUE(settings.ToProxyConfig(&config));
 
-  // Pretty-print |config| to a string (easy way to define the expectations).
-  std::ostringstream stream;
-  stream << config;
-  std::string pretty_printed_config = stream.str();
-
   EXPECT_EQ(
       "Automatic settings:\n"
       "  Auto-detect: No\n"
@@ -75,7 +68,7 @@ TEST_F(FirefoxProxySettingsTest, TestParse) {
       "    *localhost\n"
       "    127.0.0.1\n"
       "    *noproxy.com",
-      pretty_printed_config);
+      config.ToString());
 }
 
 TEST_F(FirefoxProxySettingsTest, TestParseAutoConfigUrl) {
@@ -108,11 +101,6 @@ TEST_F(FirefoxProxySettingsTest, TestParseAutoConfigUrl) {
   net::ProxyConfig config;
   EXPECT_TRUE(settings.ToProxyConfig(&config));
 
-  // Pretty-print |config| to a string (easy way to define the expectations).
-  std::ostringstream stream;
-  stream << config;
-  std::string pretty_printed_config = stream.str();
-
   EXPECT_EQ(
       "Automatic settings:\n"
       "  Auto-detect: No\n"
@@ -120,5 +108,5 @@ TEST_F(FirefoxProxySettingsTest, TestParseAutoConfigUrl) {
       "Manual settings:\n"
       "  Proxy server: [None]\n"
       "  Bypass list: [None]",
-      pretty_printed_config);
+      config.ToString());
 }
diff --git a/chrome/browser/resources/net_internals/logviewpainter.js b/chrome/browser/resources/net_internals/logviewpainter.js
index f8152f2..d7d31dd 100644
--- a/chrome/browser/resources/net_internals/logviewpainter.js
+++ b/chrome/browser/resources/net_internals/logviewpainter.js
@@ -145,6 +145,9 @@ function getTextForExtraParams(entry, doStripCookies) {
     case LogEventType.HTTP_TRANSACTION_READ_TUNNEL_RESPONSE_HEADERS:
       return getTextForResponseHeadersExtraParam(entry, doStripCookies);
 
+    case LogEventType.PROXY_CONFIG_CHANGED:
+      return getTextForProxyConfigChangedExtraParam(entry);
+
     default:
       var out = [];
       for (var k in entry.params) {
@@ -249,6 +252,25 @@ function getTextForResponseHeadersExtraParam(entry, doStripCookies) {
   return indentLines(' --> ', headers);
 }
 
+function getTextForProxyConfigChangedExtraParam(entry) {
+  var params = entry.params;
+  var out = '';
+  var indentation = '        ';
+
+  if (params.old_config) {
+    // The previous configuration may not be present in the case of
+    // the initial proxy settings fetch.
+    out += ' --> old_config =\n' +
+           indentLines(indentation, params.old_config.split('\n'));
+    out += '\n';
+  }
+
+  out += ' --> new_config =\n' +
+         indentLines(indentation, params.new_config.split('\n'));
+
+  return out;
+}
+
 function getTextForEvent(entry) {
   var text = '';
 
diff --git a/net/base/net_log_event_type_list.h b/net/base/net_log_event_type_list.h
index 2a7afeb..f729c2d 100644
--- a/net/base/net_log_event_type_list.h
+++ b/net/base/net_log_event_type_list.h
@@ -91,6 +91,19 @@ EVENT_TYPE(PROXY_SERVICE_WAITING_FOR_INIT_PAC)
 //   }
 EVENT_TYPE(PROXY_SERVICE_RESOLVED_PROXY_LIST)
 
+// This event is emitted whenever the proxy settings used by ProxyService
+// change.
+//
+// It contains these parameters:
+//  {
+//     "old_config": <Dump of the previous proxy settings>,
+//     "new_config": <Dump of the new proxy settings>
+//  }
+//
+// Note that the "old_config" key will be omitted on the first fetch of the
+// proxy settings (since there wasn't a previous value).
+EVENT_TYPE(PROXY_CONFIG_CHANGED)
+
 // ------------------------------------------------------------------------
 // Proxy Resolver
 // ------------------------------------------------------------------------
diff --git a/net/proxy/proxy_config.cc b/net/proxy/proxy_config.cc
index f27fbc4..19fa926 100644
--- a/net/proxy/proxy_config.cc
+++ b/net/proxy/proxy_config.cc
@@ -4,14 +4,122 @@
 
 #include "net/proxy/proxy_config.h"
 
+#include <sstream>
+
 #include "base/logging.h"
 #include "base/string_number_conversions.h"
 #include "base/string_tokenizer.h"
 #include "base/string_util.h"
+#include "base/values.h"
 #include "net/proxy/proxy_info.h"
 
 namespace net {
 
+namespace {
+
+// Helper to stringize a ProxyServer.
+std::ostream& operator<<(std::ostream& out,
+                         const ProxyServer& proxy_server) {
+  if (proxy_server.is_valid())
+    out << proxy_server.ToURI();
+  return out;
+}
+
+const char* BoolToYesNoString(bool b) {
+  return b ? "Yes" : "No";
+}
+
+std::ostream& operator<<(std::ostream& out,
+                         const ProxyConfig::ProxyRules& rules) {
+  // Stringize the type enum.
+  std::string type;
+  switch (rules.type) {
+    case net::ProxyConfig::ProxyRules::TYPE_NO_RULES:
+      type = "TYPE_NO_RULES";
+      break;
+    case net::ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME:
+      type = "TYPE_PROXY_PER_SCHEME";
+      break;
+    case net::ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY:
+      type = "TYPE_SINGLE_PROXY";
+      break;
+    default:
+      type = base::IntToString(rules.type);
+      break;
+  }
+  return out << "  {\n"
+             << "    type: " << type << "\n"
+             << "    single_proxy: " << rules.single_proxy << "\n"
+             << "    proxy_for_http: " << rules.proxy_for_http << "\n"
+             << "    proxy_for_https: " << rules.proxy_for_https << "\n"
+             << "    proxy_for_ftp: " << rules.proxy_for_ftp << "\n"
+             << "    socks_proxy: " << rules.socks_proxy << "\n"
+             << "  }";
+}
+
+std::ostream& operator<<(std::ostream& out, const ProxyConfig& config) {
+  // "Automatic" settings.
+  out << "Automatic settings:\n";
+  out << "  Auto-detect: " << BoolToYesNoString(config.auto_detect()) << "\n";
+  out << "  Custom PAC script: ";
+  if (config.has_pac_url())
+    out << config.pac_url();
+  else
+    out << "[None]";
+  out << "\n";
+
+  // "Manual" settings.
+  out << "Manual settings:\n";
+  out << "  Proxy server: ";
+
+  switch (config.proxy_rules().type) {
+    case net::ProxyConfig::ProxyRules::TYPE_NO_RULES:
+      out << "[None]\n";
+      break;
+    case net::ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY:
+      out << config.proxy_rules().single_proxy;
+      out << "\n";
+      break;
+    case net::ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME:
+      out << "\n";
+      if (config.proxy_rules().proxy_for_http.is_valid())
+        out << "    HTTP: " << config.proxy_rules().proxy_for_http << "\n";
+      if (config.proxy_rules().proxy_for_https.is_valid())
+        out << "    HTTPS: " << config.proxy_rules().proxy_for_https << "\n";
+      if (config.proxy_rules().proxy_for_ftp.is_valid())
+        out << "    FTP: " << config.proxy_rules().proxy_for_ftp << "\n";
+      if (config.proxy_rules().socks_proxy.is_valid())
+        out << "    SOCKS: " << config.proxy_rules().socks_proxy << "\n";
+      break;
+  }
+
+  if (config.proxy_rules().reverse_bypass)
+    out << "  Only use proxy for: ";
+  else
+    out << "  Bypass list: ";
+  if (config.proxy_rules().bypass_rules.rules().empty()) {
+    out << "[None]";
+  } else {
+    const net::ProxyBypassRules& bypass_rules =
+        config.proxy_rules().bypass_rules;
+    net::ProxyBypassRules::RuleList::const_iterator it;
+    for (it = bypass_rules.rules().begin();
+         it != bypass_rules.rules().end(); ++it) {
+      out << "\n    " << (*it)->ToString();
+    }
+  }
+
+  return out;
+}
+
+std::string ProxyConfigToString(const ProxyConfig& proxy_config) {
+  std::ostringstream stream;
+  stream << proxy_config;
+  return stream.str();
+}
+
+}  // namespace
+
 bool ProxyConfig::ProxyRules::Equals(const ProxyRules& other) const {
   return type == other.type &&
          single_proxy == other.single_proxy &&
@@ -145,103 +253,16 @@ bool ProxyConfig::MayRequirePACResolver() const {
   return auto_detect_ || has_pac_url();
 }
 
-}  // namespace net
-
-namespace {
-
-// Helper to stringize a ProxyServer.
-std::ostream& operator<<(std::ostream& out,
-                         const net::ProxyServer& proxy_server) {
-  if (proxy_server.is_valid())
-    out << proxy_server.ToURI();
-  return out;
-}
-
-const char* BoolToYesNoString(bool b) {
-  return b ? "Yes" : "No";
+std::string ProxyConfig::ToString() const {
+  return ProxyConfigToString(*this);
 }
 
-}  // namespace
-
-std::ostream& operator<<(std::ostream& out,
-                         const net::ProxyConfig::ProxyRules& rules) {
-  // Stringize the type enum.
-  std::string type;
-  switch (rules.type) {
-    case net::ProxyConfig::ProxyRules::TYPE_NO_RULES:
-      type = "TYPE_NO_RULES";
-      break;
-    case net::ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME:
-      type = "TYPE_PROXY_PER_SCHEME";
-      break;
-    case net::ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY:
-      type = "TYPE_SINGLE_PROXY";
-      break;
-    default:
-      type = base::IntToString(rules.type);
-      break;
-  }
-  return out << "  {\n"
-             << "    type: " << type << "\n"
-             << "    single_proxy: " << rules.single_proxy << "\n"
-             << "    proxy_for_http: " << rules.proxy_for_http << "\n"
-             << "    proxy_for_https: " << rules.proxy_for_https << "\n"
-             << "    proxy_for_ftp: " << rules.proxy_for_ftp << "\n"
-             << "    socks_proxy: " << rules.socks_proxy << "\n"
-             << "  }";
+Value* ProxyConfig::ToValue() const {
+  // TODO(eroman): send a dictionary rather than a flat string, so the
+  //               javascript client can do prettier formatting.
+  //               crbug.com/52011
+  return Value::CreateStringValue(ToString());
 }
 
-std::ostream& operator<<(std::ostream& out, const net::ProxyConfig& config) {
-  // "Automatic" settings.
-  out << "Automatic settings:\n";
-  out << "  Auto-detect: " << BoolToYesNoString(config.auto_detect()) << "\n";
-  out << "  Custom PAC script: ";
-  if (config.has_pac_url())
-    out << config.pac_url();
-  else
-    out << "[None]";
-  out << "\n";
-
-  // "Manual" settings.
-  out << "Manual settings:\n";
-  out << "  Proxy server: ";
-
-  switch (config.proxy_rules().type) {
-    case net::ProxyConfig::ProxyRules::TYPE_NO_RULES:
-      out << "[None]\n";
-      break;
-    case net::ProxyConfig::ProxyRules::TYPE_SINGLE_PROXY:
-      out << config.proxy_rules().single_proxy;
-      out << "\n";
-      break;
-    case net::ProxyConfig::ProxyRules::TYPE_PROXY_PER_SCHEME:
-      out << "\n";
-      if (config.proxy_rules().proxy_for_http.is_valid())
-        out << "    HTTP: " << config.proxy_rules().proxy_for_http << "\n";
-      if (config.proxy_rules().proxy_for_https.is_valid())
-        out << "    HTTPS: " << config.proxy_rules().proxy_for_https << "\n";
-      if (config.proxy_rules().proxy_for_ftp.is_valid())
-        out << "    FTP: " << config.proxy_rules().proxy_for_ftp << "\n";
-      if (config.proxy_rules().socks_proxy.is_valid())
-        out << "    SOCKS: " << config.proxy_rules().socks_proxy << "\n";
-      break;
-  }
-
-  if (config.proxy_rules().reverse_bypass)
-    out << "  Only use proxy for: ";
-  else
-    out << "  Bypass list: ";
-  if (config.proxy_rules().bypass_rules.rules().empty()) {
-    out << "[None]";
-  } else {
-    const net::ProxyBypassRules& bypass_rules =
-        config.proxy_rules().bypass_rules;
-    net::ProxyBypassRules::RuleList::const_iterator it;
-    for (it = bypass_rules.rules().begin();
-         it != bypass_rules.rules().end(); ++it) {
-      out << "\n    " << (*it)->ToString();
-    }
-  }
+}  // namespace net
 
-  return out;
-}
diff --git a/net/proxy/proxy_config.h b/net/proxy/proxy_config.h
index a4d535da..5254216 100644
--- a/net/proxy/proxy_config.h
+++ b/net/proxy/proxy_config.h
@@ -6,7 +6,6 @@
 #define NET_PROXY_PROXY_CONFIG_H_
 #pragma once
 
-#include <ostream>
 #include <string>
 #include <vector>
 
@@ -14,6 +13,8 @@
 #include "net/proxy/proxy_bypass_rules.h"
 #include "net/proxy/proxy_server.h"
 
+class Value;
+
 namespace net {
 
 class ProxyInfo;
@@ -115,7 +116,7 @@ class ProxyConfig {
   // Used to numerically identify this configuration.
   ID id() const { return id_; }
   void set_id(int id) { id_ = id; }
-  bool is_valid() { return id_ != INVALID_ID; }
+  bool is_valid() const { return id_ != INVALID_ID; }
 
   // Returns true if the given config is equivalent to this config.
   bool Equals(const ProxyConfig& other) const;
@@ -124,6 +125,13 @@ class ProxyConfig {
   // use a PAC resolver.
   bool MayRequirePACResolver() const;
 
+  // Creates a textual dump of the configuration.
+  std::string ToString() const;
+
+  // Creates a Value dump of this configuration. The caller is responsible for
+  // deleting the returned value.
+  Value* ToValue() const;
+
   ProxyRules& proxy_rules() {
     return proxy_rules_;
   }
@@ -185,13 +193,6 @@ class ProxyConfig {
 
 }  // namespace net
 
-// Dumps a human-readable string representation of the configuration to |out|;
-// used when logging the configuration changes.
-std::ostream& operator<<(std::ostream& out, const net::ProxyConfig& config);
 
-// Dumps a human-readable string representation of the |rules| to |out|;
-// used for logging and for better unittest failure output.
-std::ostream& operator<<(std::ostream& out,
-                         const net::ProxyConfig::ProxyRules& rules);
 
 #endif  // NET_PROXY_PROXY_CONFIG_H_
diff --git a/net/proxy/proxy_config_unittest.cc b/net/proxy/proxy_config_unittest.cc
index 5806f30..864b200 100644
--- a/net/proxy/proxy_config_unittest.cc
+++ b/net/proxy/proxy_config_unittest.cc
@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include <ostream>
-
 #include "net/proxy/proxy_config.h"
 #include "net/proxy/proxy_config_service_common_unittest.h"
 #include "net/proxy/proxy_info.h"
@@ -253,12 +251,6 @@ TEST(ProxyConfigTest, ParseProxyRules) {
   }
 }
 
-std::string ProxyConfigToString(const ProxyConfig& config) {
-  std::ostringstream stream;
-  stream << config;
-  return stream.str();
-}
-
 TEST(ProxyConfigTest, ToString) {
   // Manual proxy.
   {
@@ -272,7 +264,7 @@ TEST(ProxyConfigTest, ToString) {
               "Manual settings:\n"
               "  Proxy server: single-proxy:81\n"
               "  Bypass list: [None]",
-              ProxyConfigToString(config));
+              config.ToString());
   }
 
   // Autodetect + custom PAC + manual proxy.
@@ -288,7 +280,7 @@ TEST(ProxyConfigTest, ToString) {
               "Manual settings:\n"
               "  Proxy server: single-proxy:81\n"
               "  Bypass list: [None]",
-              ProxyConfigToString(config));
+              config.ToString());
   }
 
   // Manual proxy with bypass list + bypass local.
@@ -309,7 +301,7 @@ TEST(ProxyConfigTest, ToString) {
               "    google.com\n"
               "    bypass2.net:1730\n"
               "    <local>",
-              ProxyConfigToString(config));
+              config.ToString());
   }
 
   // Proxy-per scheme (HTTP and HTTPS)
@@ -327,7 +319,7 @@ TEST(ProxyConfigTest, ToString) {
               "    HTTP: proxy-for-http:1801\n"
               "    HTTPS: proxy-for-https:1802\n"
               "  Bypass list: [None]",
-              ProxyConfigToString(config));
+              config.ToString());
   }
 
   // Proxy-per scheme (HTTP and SOCKS)
@@ -345,7 +337,7 @@ TEST(ProxyConfigTest, ToString) {
               "    HTTP: proxy-for-http:1801\n"
               "    SOCKS: socks4://socks-server:6083\n"
               "  Bypass list: [None]",
-              ProxyConfigToString(config));
+              config.ToString());
   }
 
   // No proxy.
@@ -359,7 +351,7 @@ TEST(ProxyConfigTest, ToString) {
               "Manual settings:\n"
               "  Proxy server: [None]\n"
               "  Bypass list: [None]",
-              ProxyConfigToString(config));
+              config.ToString());
   }
 
   // Manual proxy with bypass list + bypass local, list reversed.
@@ -381,7 +373,7 @@ TEST(ProxyConfigTest, ToString) {
               "    google.com\n"
               "    bypass2.net:1730\n"
               "    <local>",
-              ProxyConfigToString(config));
+              config.ToString());
   }
 }
 
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index 7358f1c..e5d3167 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -8,6 +8,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/logging.h"
+#include "base/values.h"
 #include "base/histogram.h"
 #include "base/message_loop.h"
 #include "base/string_util.h"
@@ -40,8 +41,10 @@ using base::TimeTicks;
 
 namespace net {
 
-static const size_t kMaxNumNetLogEntries = 100;
-static const size_t kDefaultNumPacThreads = 4;
+namespace {
+
+const size_t kMaxNumNetLogEntries = 100;
+const size_t kDefaultNumPacThreads = 4;
 
 // Config getter that always returns direct settings.
 class ProxyConfigServiceDirect : public ProxyConfigService {
@@ -139,6 +142,33 @@ class ProxyResolverFactoryForNonV8 : public ProxyResolverFactory {
   }
 };
 
+// NetLog parameter to describe a proxy configuration change.
+class ProxyConfigChangedNetLogParam : public NetLog::EventParameters {
+ public:
+  ProxyConfigChangedNetLogParam(const ProxyConfig& old_config,
+                                const ProxyConfig& new_config)
+      : old_config_(old_config),
+        new_config_(new_config) {
+  }
+
+  virtual Value* ToValue() const {
+    DictionaryValue* dict = new DictionaryValue();
+    // The "old_config" is optional -- the first notification will not have
+    // any "previous" configuration.
+    if (old_config_.is_valid())
+      dict->Set("old_config", old_config_.ToValue());
+    dict->Set("new_config", new_config_.ToValue());
+    return dict;
+  }
+
+ private:
+  const ProxyConfig old_config_;
+  const ProxyConfig new_config_;
+  DISALLOW_COPY_AND_ASSIGN(ProxyConfigChangedNetLogParam);
+};
+
+}  // namespace
+
 // ProxyService::PacRequest ---------------------------------------------------
 
 class ProxyService::PacRequest
@@ -643,12 +673,24 @@ ProxyConfigService* ProxyService::CreateSystemProxyConfigService(
 }
 
 void ProxyService::OnProxyConfigChanged(const ProxyConfig& config) {
+  ProxyConfig old_config = config_;
   ResetProxyConfig();
 
   // Increment the ID to reflect that the config has changed.
   config_ = config;
   config_.set_id(next_config_id_++);
 
+  // Emit the proxy settings change to the NetLog stream.
+  if (net_log_) {
+    scoped_refptr<NetLog::EventParameters> params =
+        new ProxyConfigChangedNetLogParam(old_config, config);
+    net_log_->AddEntry(net::NetLog::TYPE_PROXY_CONFIG_CHANGED,
+                       base::TimeTicks::Now(),
+                       NetLog::Source(),
+                       NetLog::PHASE_NONE,
+                       params);
+  }
+
   if (!config_.MayRequirePACResolver()) {
     SetReady();
     return;