Add a source id to global NetLog entries, which makes them

easier to sort in about:net-internals, making the world a
happier, fuzzier place.

Also slightly simplifies the functions to add NetLog entries,
and makes it impossible to create a BoundNetLog with both an
invalid source and a non-NULL NetLog.

BUG=116597
Review URL: https://chromiumcodereview.appspot.com/9585026

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

13 files changed, 90 insertions, 113 deletions

diff --git a/net/base/capturing_net_log.cc b/net/base/capturing_net_log.cc
index 3d36982..2c6867c 100644
--- a/net/base/capturing_net_log.cc
+++ b/net/base/capturing_net_log.cc
@@ -20,7 +20,7 @@ CapturingNetLog::Entry::Entry(EventType type,
 CapturingNetLog::Entry::~Entry() {}
 
 CapturingNetLog::CapturingNetLog(size_t max_num_entries)
-    : last_id_(-1),
+    : last_id_(0),
       max_num_entries_(max_num_entries),
       log_level_(LOG_ALL_BUT_BYTES) {
 }
@@ -42,13 +42,14 @@ void CapturingNetLog::SetLogLevel(NetLog::LogLevel log_level) {
   log_level_ = log_level;
 }
 
-void CapturingNetLog::AddEntry(EventType type,
-                               const base::TimeTicks& time,
-                               const Source& source,
-                               EventPhase phase,
-                               EventParameters* extra_parameters) {
+void CapturingNetLog::AddEntry(
+    EventType type,
+    const Source& source,
+    EventPhase phase,
+    const scoped_refptr<EventParameters>& extra_parameters) {
+  DCHECK(source.is_valid());
   base::AutoLock lock(lock_);
-  Entry entry(type, time, source, phase, extra_parameters);
+  Entry entry(type, base::TimeTicks::Now(), source, phase, extra_parameters);
   if (entries_.size() + 1 < max_num_entries_)
     entries_.push_back(entry);
 }
@@ -78,27 +79,25 @@ void CapturingNetLog::RemoveThreadSafeObserver(
   NOTIMPLEMENTED() << "Not currently used by net unit tests.";
 }
 
-CapturingBoundNetLog::CapturingBoundNetLog(const NetLog::Source& source,
-                                           CapturingNetLog* net_log)
-    : source_(source), capturing_net_log_(net_log) {
-}
-
 CapturingBoundNetLog::CapturingBoundNetLog(size_t max_num_entries)
-    : capturing_net_log_(new CapturingNetLog(max_num_entries)) {}
+    : capturing_net_log_(max_num_entries),
+      net_log_(BoundNetLog::Make(&capturing_net_log_,
+                                 net::NetLog::SOURCE_NONE)) {
+}
 
 CapturingBoundNetLog::~CapturingBoundNetLog() {}
 
 void CapturingBoundNetLog::GetEntries(
     CapturingNetLog::EntryList* entry_list) const {
-  capturing_net_log_->GetEntries(entry_list);
+  capturing_net_log_.GetEntries(entry_list);
 }
 
 void CapturingBoundNetLog::Clear() {
-  capturing_net_log_->Clear();
+  capturing_net_log_.Clear();
 }
 
 void CapturingBoundNetLog::SetLogLevel(NetLog::LogLevel log_level) {
-  capturing_net_log_->SetLogLevel(log_level);
+  capturing_net_log_.SetLogLevel(log_level);
 }
 
 }  // namespace net
diff --git a/net/base/capturing_net_log.h b/net/base/capturing_net_log.h
index 97b141c..2482d04 100644
--- a/net/base/capturing_net_log.h
+++ b/net/base/capturing_net_log.h
@@ -12,7 +12,6 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
 #include "base/synchronization/lock.h"
 #include "base/time.h"
 #include "net/base/net_export.h"
@@ -57,11 +56,11 @@ class NET_EXPORT CapturingNetLog : public NetLog {
   void SetLogLevel(NetLog::LogLevel log_level);
 
   // NetLog implementation:
-  virtual void AddEntry(EventType type,
-                        const base::TimeTicks& time,
-                        const Source& source,
-                        EventPhase phase,
-                        EventParameters* extra_parameters) OVERRIDE;
+  virtual void AddEntry(
+      EventType type,
+      const Source& source,
+      EventPhase phase,
+      const scoped_refptr<EventParameters>& extra_parameters) OVERRIDE;
   virtual uint32 NextID() OVERRIDE;
   virtual LogLevel GetLogLevel() const OVERRIDE;
   virtual void AddThreadSafeObserver(ThreadSafeObserver* observer,
@@ -92,16 +91,12 @@ class NET_EXPORT CapturingNetLog : public NetLog {
 // bound() method.
 class NET_EXPORT_PRIVATE CapturingBoundNetLog {
  public:
-  CapturingBoundNetLog(const NetLog::Source& source, CapturingNetLog* net_log);
-
   explicit CapturingBoundNetLog(size_t max_num_entries);
 
   ~CapturingBoundNetLog();
 
   // The returned BoundNetLog is only valid while |this| is alive.
-  BoundNetLog bound() const {
-    return BoundNetLog(source_, capturing_net_log_.get());
-  }
+  BoundNetLog bound() const { return net_log_; }
 
   // Fills |entry_list| with all entries in the log.
   void GetEntries(CapturingNetLog::EntryList* entry_list) const;
@@ -112,8 +107,8 @@ class NET_EXPORT_PRIVATE CapturingBoundNetLog {
   void SetLogLevel(NetLog::LogLevel log_level);
 
  private:
-  NetLog::Source source_;
-  scoped_ptr<CapturingNetLog> capturing_net_log_;
+  CapturingNetLog capturing_net_log_;
+  const BoundNetLog net_log_;
 
   DISALLOW_COPY_AND_ASSIGN(CapturingBoundNetLog);
 };
diff --git a/net/base/file_stream_unittest.cc b/net/base/file_stream_unittest.cc
index 9500709..b55e22e 100644
--- a/net/base/file_stream_unittest.cc
+++ b/net/base/file_stream_unittest.cc
@@ -51,11 +51,11 @@ class NetLogForNotifyingFileClosure : public NetLog {
   }
 
   // NetLog overrides:
-  virtual void AddEntry(EventType type,
-                        const base::TimeTicks& time,
-                        const Source& source,
-                        EventPhase phase,
-                        EventParameters* extra_parameters) OVERRIDE {
+  virtual void AddEntry(
+      EventType type,
+      const Source& source,
+      EventPhase phase,
+      const scoped_refptr<EventParameters>& params) OVERRIDE {
     if (type == TYPE_FILE_STREAM_CLOSE) {
       on_closure_.Signal();
     }
diff --git a/net/base/net_log.cc b/net/base/net_log.cc
index 1de670e..0b23907 100644
--- a/net/base/net_log.cc
+++ b/net/base/net_log.cc
@@ -76,6 +76,14 @@ NetLog* NetLog::ThreadSafeObserver::net_log() const {
   return net_log_;
 }
 
+void NetLog::AddGlobalEntry(EventType type,
+                            const scoped_refptr<EventParameters>& params) {
+  AddEntry(type,
+           Source(net::NetLog::SOURCE_NONE, this->NextID()),
+           net::NetLog::PHASE_NONE,
+           params);
+}
+
 // static
 std::string NetLog::TickCountToString(const base::TimeTicks& time) {
   int64 delta_time = (time - base::TimeTicks()).InMilliseconds();
@@ -197,19 +205,8 @@ void BoundNetLog::AddEntry(
     NetLog::EventType type,
     NetLog::EventPhase phase,
     const scoped_refptr<NetLog::EventParameters>& params) const {
-  if (net_log_) {
-    net_log_->AddEntry(type, base::TimeTicks::Now(), source_, phase, params);
-  }
-}
-
-void BoundNetLog::AddEntryWithTime(
-    NetLog::EventType type,
-    const base::TimeTicks& time,
-    NetLog::EventPhase phase,
-    const scoped_refptr<NetLog::EventParameters>& params) const {
-  if (net_log_) {
-    net_log_->AddEntry(type, time, source_, phase, params);
-  }
+  if (net_log_)
+    net_log_->AddEntry(type, source_, phase, params);
 }
 
 void BoundNetLog::AddEvent(
diff --git a/net/base/net_log.h b/net/base/net_log.h
index cfb8081..707c321 100644
--- a/net/base/net_log.h
+++ b/net/base/net_log.h
@@ -25,7 +25,7 @@ namespace net {
 // that generated the message (for example, which URLRequest or which
 // SocketStream).
 //
-// To avoid needing to pass in the "source id" to the logging functions, NetLog
+// To avoid needing to pass in the "source ID" to the logging functions, NetLog
 // is usually accessed through a BoundNetLog, which will always pass in a
 // specific source ID.
 //
@@ -133,6 +133,16 @@ class NET_EXPORT NetLog {
     //
     // It is illegal for an Observer to call any NetLog or
     // NetLog::Observer functions in response to a call to OnAddEntry.
+    //
+    // |type| - The type of the event.
+    // |time| - The time when the event occurred.
+    // |source| - The source that generated the event.
+    // |phase| - An optional parameter indicating whether this is the start/end
+    //           of an action.
+    // |params| - Optional (may be NULL) parameters for this event.
+    //            The specific subclass of EventParameters is defined
+    //            by the contract for events of this |type|.
+    // TODO(eroman): Take a scoped_refptr<EventParameters> instead.
     virtual void OnAddEntry(EventType type,
                             const base::TimeTicks& time,
                             const Source& source,
@@ -152,21 +162,9 @@ class NET_EXPORT NetLog {
   NetLog() {}
   virtual ~NetLog() {}
 
-  // Emits an event to the log stream.
-  //  |type| - The type of the event.
-  //  |time| - The time when the event occurred.
-  //  |source| - The source that generated the event.
-  //  |phase| - An optional parameter indicating whether this is the start/end
-  //            of an action.
-  //  |params| - Optional (may be NULL) parameters for this event.
-  //             The specific subclass of EventParameters is defined
-  //             by the contract for events of this |type|.
-  //             TODO(eroman): Take a scoped_refptr<> instead.
-  virtual void AddEntry(EventType type,
-                        const base::TimeTicks& time,
-                        const Source& source,
-                        EventPhase phase,
-                        EventParameters* params) = 0;
+  // Emits a global event to the log stream, with its own unique source ID.
+  void AddGlobalEntry(EventType type,
+                      const scoped_refptr<EventParameters>& params);
 
   // Returns a unique ID which can be used as a source ID.
   virtual uint32 NextID() = 0;
@@ -232,6 +230,13 @@ class NET_EXPORT NetLog {
                                              bool use_strings);
 
  protected:
+  // This is the internal function used by AddGlobalEntry and BoundNetLogs.
+  virtual void AddEntry(
+      EventType type,
+      const Source& source,
+      EventPhase phase,
+      const scoped_refptr<NetLog::EventParameters>& params) = 0;
+
   // Subclasses must call these in the corresponding functions to set an
   // observer's |net_log_| and |log_level_| values.
   void OnAddObserver(ThreadSafeObserver* observer, LogLevel log_level);
@@ -240,6 +245,8 @@ class NET_EXPORT NetLog {
   void OnRemoveObserver(ThreadSafeObserver* observer);
 
  private:
+  friend class BoundNetLog;
+
   DISALLOW_COPY_AND_ASSIGN(NetLog);
 };
 
@@ -249,22 +256,12 @@ class NET_EXPORT BoundNetLog {
  public:
   BoundNetLog() : net_log_(NULL) {}
 
-  BoundNetLog(const NetLog::Source& source, NetLog* net_log)
-      : source_(source), net_log_(net_log) {
-  }
-
   // Convenience methods that call through to the NetLog, passing in the
   // currently bound source.
   void AddEntry(NetLog::EventType type,
                 NetLog::EventPhase phase,
                 const scoped_refptr<NetLog::EventParameters>& params) const;
 
-  void AddEntryWithTime(
-      NetLog::EventType type,
-      const base::TimeTicks& time,
-      NetLog::EventPhase phase,
-      const scoped_refptr<NetLog::EventParameters>& params) const;
-
   // Convenience methods that call through to the NetLog, passing in the
   // currently bound source, current time, and a fixed "capture phase"
   // (begin, end, or none).
@@ -310,6 +307,10 @@ class NET_EXPORT BoundNetLog {
   NetLog* net_log() const { return net_log_; }
 
  private:
+  BoundNetLog(const NetLog::Source& source, NetLog* net_log)
+      : source_(source), net_log_(net_log) {
+  }
+
   NetLog::Source source_;
   NetLog* net_log_;
 };
diff --git a/net/base/net_log_unittest.cc b/net/base/net_log_unittest.cc
index 8523955..c9258a9 100644
--- a/net/base/net_log_unittest.cc
+++ b/net/base/net_log_unittest.cc
@@ -1,7 +1,8 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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 "base/memory/scoped_ptr.h"
 #include "net/base/capturing_net_log.h"
 #include "net/base/net_log.h"
 #include "net/base/net_log_unittest.h"
diff --git a/net/http/http_auth_handler_unittest.cc b/net/http/http_auth_handler_unittest.cc
index 84809e1..79aa2bc 100644
--- a/net/http/http_auth_handler_unittest.cc
+++ b/net/http/http_auth_handler_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -17,7 +17,6 @@
 namespace net {
 
 TEST(HttpAuthHandlerTest, NetLog) {
-  NetLog::Source source;
   GURL origin("http://www.example.com");
   std::string challenge = "Mock asdf";
   AuthCredentials credentials(ASCIIToUTF16("user"), ASCIIToUTF16("pass"));
@@ -38,7 +37,8 @@ TEST(HttpAuthHandlerTest, NetLog) {
             challenge.begin(), challenge.end());
         HttpAuthHandlerMock mock_handler;
         CapturingNetLog capturing_net_log(CapturingNetLog::kUnbounded);
-        BoundNetLog bound_net_log(source, &capturing_net_log);
+        BoundNetLog bound_net_log(BoundNetLog::Make(&capturing_net_log,
+                                                    net::NetLog::SOURCE_NONE));
 
         mock_handler.InitFromChallenge(&tokenizer, target,
                                        origin, bound_net_log);
diff --git a/net/proxy/proxy_resolver_js_bindings.cc b/net/proxy/proxy_resolver_js_bindings.cc
index 89d7164..08b84ac 100644
--- a/net/proxy/proxy_resolver_js_bindings.cc
+++ b/net/proxy/proxy_resolver_js_bindings.cc
@@ -299,14 +299,8 @@ class DefaultJSBindings : public ProxyResolverJSBindings {
     LogEventToCurrentRequest(NetLog::PHASE_NONE, type, params);
 
     // Emit to the global NetLog event stream.
-    if (net_log_) {
-      net_log_->AddEntry(
-          type,
-          base::TimeTicks::Now(),
-          NetLog::Source(),
-          NetLog::PHASE_NONE,
-          params);
-    }
+    if (net_log_)
+      net_log_->AddGlobalEntry(type, params);
   }
 
   scoped_ptr<SyncHostResolver> host_resolver_;
diff --git a/net/proxy/proxy_resolver_js_bindings_unittest.cc b/net/proxy/proxy_resolver_js_bindings_unittest.cc
index f4b273a..a1377a8 100644
--- a/net/proxy/proxy_resolver_js_bindings_unittest.cc
+++ b/net/proxy/proxy_resolver_js_bindings_unittest.cc
@@ -275,7 +275,7 @@ TEST(ProxyResolverJSBindingsTest, NetLog) {
 
   // Attach a capturing NetLog as the current request's log stream.
   CapturingNetLog log(CapturingNetLog::kUnbounded);
-  BoundNetLog bound_log(NetLog::Source(NetLog::SOURCE_NONE, 0), &log);
+  BoundNetLog bound_log(BoundNetLog::Make(&log, NetLog::SOURCE_NONE));
   ProxyResolverRequestContext context(&bound_log, NULL);
   bindings->set_current_request_context(&context);
 
diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index a18805b..39638e2 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -1265,12 +1265,10 @@ void ProxyService::ReportSuccess(const ProxyInfo& result) {
       existing->second.bad_until = iter->second.bad_until;
   }
   if (net_log_) {
-    net_log_->AddEntry(NetLog::TYPE_BAD_PROXY_LIST_REPORTED,
-                       TimeTicks::Now(),
-                       NetLog::Source(),
-                       NetLog::PHASE_NONE,
-                       make_scoped_refptr(
-                           new BadProxyListNetLogParam(new_retry_info)));
+    net_log_->AddGlobalEntry(
+        NetLog::TYPE_BAD_PROXY_LIST_REPORTED,
+        make_scoped_refptr(
+            new BadProxyListNetLogParam(new_retry_info)));
   }
 }
 
@@ -1473,11 +1471,8 @@ void ProxyService::OnProxyConfigChanged(
   if (net_log_) {
     scoped_refptr<NetLog::EventParameters> params(
         new ProxyConfigChangedNetLogParam(fetched_config_, effective_config));
-    net_log_->AddEntry(net::NetLog::TYPE_PROXY_CONFIG_CHANGED,
-                       TimeTicks::Now(),
-                       NetLog::Source(),
-                       NetLog::PHASE_NONE,
-                       params);
+    net_log_->AddGlobalEntry(net::NetLog::TYPE_PROXY_CONFIG_CHANGED,
+                                params);
   }
 
   // Set the new configuration as the most recently fetched one.
diff --git a/net/socket/socket_test_util.cc b/net/socket/socket_test_util.cc
index e199307..ef5b0db 100644
--- a/net/socket/socket_test_util.cc
+++ b/net/socket/socket_test_util.cc
@@ -636,7 +636,7 @@ void MockClientSocketFactory::ClearSSLSessionCache() {
 MockClientSocket::MockClientSocket(net::NetLog* net_log)
     : ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
       connected_(false),
-      net_log_(net::NetLog::Source(), net_log) {
+      net_log_(BoundNetLog::Make(net_log, net::NetLog::SOURCE_NONE)) {
 }
 
 bool MockClientSocket::SetReceiveBufferSize(int32 size) {
@@ -1208,7 +1208,7 @@ MockUDPClientSocket::MockUDPClientSocket(SocketDataProvider* data,
       need_read_data_(true),
       pending_buf_(NULL),
       pending_buf_len_(0),
-      net_log_(net::NetLog::Source(), net_log),
+      net_log_(BoundNetLog::Make(net_log, net::NetLog::SOURCE_NONE)),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
   DCHECK(data_);
   data_->Reset();
diff --git a/net/url_request/url_request_throttler_manager.cc b/net/url_request/url_request_throttler_manager.cc
index 8e634e8..e29f5c2 100644
--- a/net/url_request/url_request_throttler_manager.cc
+++ b/net/url_request/url_request_throttler_manager.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -65,7 +65,7 @@ scoped_refptr<URLRequestThrottlerEntryInterface>
         IsLocalhost(host)) {
       if (!logged_for_localhost_disabled_ && IsLocalhost(host)) {
         logged_for_localhost_disabled_ = true;
-        net_log_->AddEvent(
+        net_log_.AddEvent(
             NetLog::TYPE_THROTTLING_DISABLED_FOR_HOST,
             make_scoped_refptr(new NetLogStringParameter("host", host)));
       }
@@ -90,7 +90,7 @@ void URLRequestThrottlerManager::AddToOptOutList(const std::string& host) {
   if (opt_out_hosts_.find(host) == opt_out_hosts_.end()) {
     UMA_HISTOGRAM_COUNTS("Throttling.SiteOptedOut", 1);
 
-    net_log_->EndEvent(
+    net_log_.EndEvent(
         NetLog::TYPE_THROTTLING_DISABLED_FOR_HOST,
         make_scoped_refptr(new NetLogStringParameter("host", host)));
     opt_out_hosts_.insert(host);
@@ -133,13 +133,12 @@ bool URLRequestThrottlerManager::enforce_throttling() {
 
 void URLRequestThrottlerManager::set_net_log(NetLog* net_log) {
   DCHECK(net_log);
-  NetLog::Source source(NetLog::SOURCE_EXPONENTIAL_BACKOFF_THROTTLING,
-                        net_log->NextID());
-  net_log_.reset(new BoundNetLog(source, net_log));
+  net_log_ = BoundNetLog::Make(net_log,
+                               NetLog::SOURCE_EXPONENTIAL_BACKOFF_THROTTLING);
 }
 
 NetLog* URLRequestThrottlerManager::net_log() const {
-  return net_log_->net_log();
+  return net_log_.net_log();
 }
 
 void URLRequestThrottlerManager::OnIPAddressChanged() {
@@ -166,10 +165,6 @@ URLRequestThrottlerManager::URLRequestThrottlerManager()
   url_id_replacements_.ClearUsername();
   url_id_replacements_.ClearQuery();
   url_id_replacements_.ClearRef();
-
-  // Make sure there is always a net_log_ instance, even if it logs to
-  // nowhere.
-  net_log_.reset(new BoundNetLog());
 }
 
 URLRequestThrottlerManager::~URLRequestThrottlerManager() {
diff --git a/net/url_request/url_request_throttler_manager.h b/net/url_request/url_request_throttler_manager.h
index d97e0b9..778c695 100644
--- a/net/url_request/url_request_throttler_manager.h
+++ b/net/url_request/url_request_throttler_manager.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 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.
 
@@ -166,8 +166,8 @@ class NET_EXPORT URLRequestThrottlerManager
   // being disabled for localhost.
   bool logged_for_localhost_disabled_;
 
-  // NetLog to use, or NULL if none configured.
-  scoped_ptr<BoundNetLog> net_log_;
+  // NetLog to use, if configured.
+  BoundNetLog net_log_;
 
   // Valid once we've registered for network notifications.
   base::PlatformThreadId registered_from_thread_;