Rename CapturingNetLog::Entry to CapturingNetLog::CaptureEntry,

and have it keep its own copy of a NetLog Entry's parameters.

This is in preparation for a NetLog refactoring that will result
in NetLog parameters lasting no longer than the function call,
to avoid always having to copy all parameters.

This does make CapturingNetLogs much less efficient themselves,
but they should not be used in production code, anyways.

TEST=NONE
BUG=126243

Review URL: https://chromiumcodereview.appspot.com/10442085

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

net/base/capturing_net_log.cc

@@ -5,19 +5,58 @@
 #include "net/base/capturing_net_log.h"
 
 #include "base/logging.h"
+#include "base/values.h"
 
 namespace net {
 
-CapturingNetLog::Entry::Entry(EventType type,
-                              const base::TimeTicks& time,
-                              Source source,
-                              EventPhase phase,
-                              EventParameters* extra_parameters)
-    : type(type), time(time), source(source), phase(phase),
-      extra_parameters(extra_parameters) {
+CapturingNetLog::CapturedEntry::CapturedEntry(
+    EventType type,
+    const base::TimeTicks& time,
+    Source source,
+    EventPhase phase,
+    scoped_ptr<DictionaryValue> params)
+    : type(type),
+      time(time),
+      source(source),
+      phase(phase),
+      params(params.Pass()) {
+}
+
+CapturingNetLog::CapturedEntry::CapturedEntry(const CapturedEntry& entry) {
+  *this = entry;
+}
+
+CapturingNetLog::CapturedEntry::~CapturedEntry() {}
+
+CapturingNetLog::CapturedEntry&
+CapturingNetLog::CapturedEntry::operator=(const CapturedEntry& entry) {
+  type = entry.type;
+  time = entry.time;
+  source = entry.source;
+  phase = entry.phase;
+  params.reset(entry.params.get() ? entry.params->DeepCopy() : NULL);
+  return *this;
+}
+
+bool CapturingNetLog::CapturedEntry::GetStringValue(
+    const std::string& name,
+    std::string* value) const {
+  if (!params.get())
+    return false;
+  return params->GetString(name, value);
 }
 
-CapturingNetLog::Entry::~Entry() {}
+bool CapturingNetLog::CapturedEntry::GetIntegerValue(
+    const std::string& name,
+    int* value) const {
+  if (!params.get())
+    return false;
+  return params->GetInteger(name, value);
+}
+
+bool CapturingNetLog::CapturedEntry::GetNetErrorCode(int* value) const {
+  return GetIntegerValue("net_error", value);
+}
 
 CapturingNetLog::CapturingNetLog(size_t max_num_entries)
     : last_id_(0),
@@ -27,14 +66,14 @@ CapturingNetLog::CapturingNetLog(size_t max_num_entries)
 
 CapturingNetLog::~CapturingNetLog() {}
 
-void CapturingNetLog::GetEntries(EntryList* entry_list) const {
+void CapturingNetLog::GetEntries(CapturedEntryList* entry_list) const {
   base::AutoLock lock(lock_);
-  *entry_list = entries_;
+  *entry_list = captured_entries_;
 }
 
 void CapturingNetLog::Clear() {
   base::AutoLock lock(lock_);
-  entries_.clear();
+  captured_entries_.clear();
 }
 
 void CapturingNetLog::SetLogLevel(NetLog::LogLevel log_level) {
@@ -49,9 +88,22 @@ void CapturingNetLog::AddEntry(
     const scoped_refptr<EventParameters>& extra_parameters) {
   DCHECK(source.is_valid());
   base::AutoLock lock(lock_);
-  Entry entry(type, base::TimeTicks::Now(), source, phase, extra_parameters);
-  if (entries_.size() + 1 < max_num_entries_)
-    entries_.push_back(entry);
+  if (captured_entries_.size() >= max_num_entries_)
+    return;
+  // Using Dictionaries instead of Values makes checking values a little
+  // simpler.
+  DictionaryValue* param_dict = NULL;
+  if (extra_parameters) {
+    Value* param_value = extra_parameters->ToValue();
+    if (param_value && !param_value->GetAsDictionary(&param_dict))
+      delete param_value;
+  }
+  captured_entries_.push_back(
+      CapturedEntry(type,
+                    base::TimeTicks::Now(),
+                    source,
+                    phase,
+                    scoped_ptr<DictionaryValue>(param_dict)));
 }
 
 uint32 CapturingNetLog::NextID() {
@@ -88,7 +140,7 @@ CapturingBoundNetLog::CapturingBoundNetLog(size_t max_num_entries)
 CapturingBoundNetLog::~CapturingBoundNetLog() {}
 
 void CapturingBoundNetLog::GetEntries(
-    CapturingNetLog::EntryList* entry_list) const {
+    CapturingNetLog::CapturedEntryList* entry_list) const {
   capturing_net_log_.GetEntries(entry_list);
 }
 

net/base/capturing_net_log.h

@@ -6,40 +6,67 @@
 #define NET_BASE_CAPTURING_NET_LOG_H_
 #pragma once
 
+#include <string>
 #include <vector>
 
 #include "base/atomicops.h"
 #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"
 #include "net/base/net_log.h"
 
+namespace base {
+class DictionaryValue;
+}
+
 namespace net {
 
 // CapturingNetLog is an implementation of NetLog that saves messages to a
-// bounded buffer.
+// bounded buffer.  CapturingNetLogs should only be used in unit tests, never
+// in production code.
+// TODO(mmenke):  Move CapturingNetLog to net_unittests project, once the CL
+//                to remove the use of it in the Jingle unit tests has landed.
 class NET_EXPORT CapturingNetLog : public NetLog {
  public:
-  struct NET_EXPORT Entry {
-    Entry(EventType type,
-          const base::TimeTicks& time,
-          Source source,
-          EventPhase phase,
-          EventParameters* extra_parameters);
-    ~Entry();
+  struct NET_EXPORT_PRIVATE CapturedEntry {
+    CapturedEntry(EventType type,
+                  const base::TimeTicks& time,
+                  Source source,
+                  EventPhase phase,
+                  scoped_ptr<base::DictionaryValue> params);
+    // Copy constructor needed to store in a std::vector because of the
+    // scoped_ptr.
+    CapturedEntry(const CapturedEntry& entry);
+
+    ~CapturedEntry();
+
+    // Equality operator needed to store in a std::vector because of the
+    // scoped_ptr.
+    CapturedEntry& operator=(const CapturedEntry& entry);
+
+    // Attempt to retrieve an value of the specified type with the given name
+    // from |params|.  Returns true on success, false on failure.  Does not
+    // modify |value| on failure.
+    bool GetStringValue(const std::string& name, std::string* value) const;
+    bool GetIntegerValue(const std::string& name, int* value) const;
+
+    // Same as GetIntegerValue, but returns the error code associated with a
+    // log entry.
+    bool GetNetErrorCode(int* value) const;
 
     EventType type;
     base::TimeTicks time;
     Source source;
     EventPhase phase;
-    scoped_refptr<EventParameters> extra_parameters;
+    scoped_ptr<base::DictionaryValue> params;
   };
 
   // Ordered set of entries that were logged.
-  typedef std::vector<Entry> EntryList;
+  typedef std::vector<CapturedEntry> CapturedEntryList;
 
   enum { kUnbounded = -1 };
 
@@ -49,7 +76,7 @@ class NET_EXPORT CapturingNetLog : public NetLog {
   virtual ~CapturingNetLog();
 
   // Returns the list of all entries in the log.
-  void GetEntries(EntryList* entry_list) const;
+  void GetEntries(CapturedEntryList* entry_list) const;
 
   void Clear();
 
@@ -77,7 +104,7 @@ class NET_EXPORT CapturingNetLog : public NetLog {
   base::subtle::Atomic32 last_id_;
 
   size_t max_num_entries_;
-  EntryList entries_;
+  CapturedEntryList captured_entries_;
 
   NetLog::LogLevel log_level_;
 
@@ -99,7 +126,7 @@ class NET_EXPORT_PRIVATE CapturingBoundNetLog {
   BoundNetLog bound() const { return net_log_; }
 
   // Fills |entry_list| with all entries in the log.
-  void GetEntries(CapturingNetLog::EntryList* entry_list) const;
+  void GetEntries(CapturingNetLog::CapturedEntryList* entry_list) const;
 
   void Clear();
 

net/base/net_log_event_type_list.h

@@ -1039,7 +1039,7 @@ EVENT_TYPE(SPDY_SESSION_SENT_WINDOW_UPDATE)
 // Sending of a SPDY CREDENTIAL frame (which sends a certificate or
 // certificate chain to the server).
 //   {
-//     "slot"     : <The slot that this certificate should be stored in >,
+//     "slot"     : <The slot that this certificate should be stored in>,
 //     "origin"   : <The origin this certificate should be used for>,
 //   }
 EVENT_TYPE(SPDY_SESSION_SEND_CREDENTIAL)
@@ -1065,7 +1065,7 @@ EVENT_TYPE(SPDY_SESSION_STALLED_ON_SEND_WINDOW)
 
 // Session is closing
 //   {
-//     "status"     : <The error status of the closure>,
+//     "net_error"  : <The error status of the closure>,
 //     "description": <The textual description for the closure>,
 //   }
 EVENT_TYPE(SPDY_SESSION_CLOSE)

net/base/net_log_unittest.cc

@@ -19,7 +19,7 @@ TEST(NetLog, ScopedNetLogEventTest) {
   scoped_ptr<ScopedNetLogEvent> net_log_event(
       new ScopedNetLogEvent(net_log, NetLog::TYPE_REQUEST_ALIVE, NULL));
 
-  CapturingNetLog::EntryList entries;
+  CapturingNetLog::CapturedEntryList entries;
   log.GetEntries(&entries);
   EXPECT_EQ(1u, entries.size());
   EXPECT_TRUE(LogContainsBeginEvent(entries, 0, NetLog::TYPE_REQUEST_ALIVE));

net/base/net_log_unittest.h

@@ -1,4 +1,4 @@
-// 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.
 
@@ -21,7 +21,7 @@ inline base::TimeTicks MakeTime(int t) {
 }
 
 inline ::testing::AssertionResult LogContainsEventHelper(
-    const CapturingNetLog::EntryList& entries,
+    const CapturingNetLog::CapturedEntryList& entries,
     int i,  // Negative indices are reverse indices.
     const base::TimeTicks& expected_time,
     bool check_time,
@@ -33,7 +33,7 @@ inline ::testing::AssertionResult LogContainsEventHelper(
       static_cast<size_t>(i);
   if (j >= entries.size())
     return ::testing::AssertionFailure() << j << " is out of bounds.";
-  const CapturingNetLog::Entry& entry = entries[j];
+  const CapturingNetLog::CapturedEntry& entry = entries[j];
   if (expected_event != entry.type) {
     return ::testing::AssertionFailure()
         << "Actual event: " << NetLog::EventTypeToString(entry.type)
@@ -57,7 +57,7 @@ inline ::testing::AssertionResult LogContainsEventHelper(
 }
 
 inline ::testing::AssertionResult LogContainsEventAtTime(
-    const CapturingNetLog::EntryList& log,
+    const CapturingNetLog::CapturedEntryList& log,
     int i,  // Negative indices are reverse indices.
     const base::TimeTicks& expected_time,
     NetLog::EventType expected_event,
@@ -68,7 +68,7 @@ inline ::testing::AssertionResult LogContainsEventAtTime(
 
 // Version without timestamp.
 inline ::testing::AssertionResult LogContainsEvent(
-    const CapturingNetLog::EntryList& log,
+    const CapturingNetLog::CapturedEntryList& log,
     int i,  // Negative indices are reverse indices.
     NetLog::EventType expected_event,
     NetLog::EventPhase expected_phase) {
@@ -78,22 +78,22 @@ inline ::testing::AssertionResult LogContainsEvent(
 
 // Version for PHASE_BEGIN (and no timestamp).
 inline ::testing::AssertionResult LogContainsBeginEvent(
-    const CapturingNetLog::EntryList& log,
+    const CapturingNetLog::CapturedEntryList& log,
     int i,  // Negative indices are reverse indices.
     NetLog::EventType expected_event) {
   return LogContainsEvent(log, i, expected_event, NetLog::PHASE_BEGIN);
 }
 
 // Version for PHASE_END (and no timestamp).
 inline ::testing::AssertionResult LogContainsEndEvent(
-    const CapturingNetLog::EntryList& log,
+    const CapturingNetLog::CapturedEntryList& log,
     int i,  // Negative indices are reverse indices.
     NetLog::EventType expected_event) {
   return LogContainsEvent(log, i, expected_event, NetLog::PHASE_END);
 }
 
 inline ::testing::AssertionResult LogContainsEntryWithType(
-    const CapturingNetLog::EntryList& entries,
+    const CapturingNetLog::CapturedEntryList& entries,
     int i, // Negative indices are reverse indices.
     NetLog::EventType type) {
   // Negative indices are reverse indices.
@@ -102,7 +102,7 @@ inline ::testing::AssertionResult LogContainsEntryWithType(
       static_cast<size_t>(i);
   if (j >= entries.size())
     return ::testing::AssertionFailure() << j << " is out of bounds.";
-  const CapturingNetLog::Entry& entry = entries[j];
+  const CapturingNetLog::CapturedEntry& entry = entries[j];
   if (entry.type != type)
     return ::testing::AssertionFailure() << "Type does not match.";
   return ::testing::AssertionSuccess();
@@ -113,13 +113,13 @@ inline ::testing::AssertionResult LogContainsEntryWithType(
 // as long as the first index where it is found is at least |min_index|.
 // Returns the position where the event was found.
 inline size_t ExpectLogContainsSomewhere(
-    const CapturingNetLog::EntryList& entries,
+    const CapturingNetLog::CapturedEntryList& entries,
     size_t min_index,
     NetLog::EventType expected_event,
     NetLog::EventPhase expected_phase) {
   size_t i = 0;
   for (; i < entries.size(); ++i) {
-    const CapturingNetLog::Entry& entry = entries[i];
+    const CapturingNetLog::CapturedEntry& entry = entries[i];
     if (entry.type == expected_event &&
         entry.phase == expected_phase)
       break;
@@ -133,13 +133,13 @@ inline size_t ExpectLogContainsSomewhere(
 // as long as one index where it is found is at least |min_index|.
 // Returns the first such position where the event was found.
 inline size_t ExpectLogContainsSomewhereAfter(
-    const CapturingNetLog::EntryList& entries,
+    const CapturingNetLog::CapturedEntryList& entries,
     size_t min_index,
     NetLog::EventType expected_event,
     NetLog::EventPhase expected_phase) {
   size_t i = min_index;
   for (; i < entries.size(); ++i) {
-    const CapturingNetLog::Entry& entry = entries[i];
+    const CapturingNetLog::CapturedEntry& entry = entries[i];
     if (entry.type == expected_event &&
         entry.phase == expected_phase)
       break;

net/http/http_auth_handler_unittest.cc

@@ -48,7 +48,7 @@ TEST(HttpAuthHandlerTest, NetLog) {
         if (async)
           test_callback.WaitForResult();
 
-        net::CapturingNetLog::EntryList entries;
+        net::CapturingNetLog::CapturedEntryList entries;
         capturing_net_log.GetEntries(&entries);
 
         EXPECT_EQ(2u, entries.size());

net/http/http_cache_unittest.cc

@@ -436,7 +436,7 @@ TEST(HttpCache, SimpleGETNoDiskCache) {
 
   // Check that the NetLog was filled as expected.
   // (We attempted to both Open and Create entries, but both failed).
-  net::CapturingNetLog::EntryList entries;
+  net::CapturingNetLog::CapturedEntryList entries;
   log.GetEntries(&entries);
 
   EXPECT_EQ(6u, entries.size());
@@ -579,7 +579,7 @@ TEST(HttpCache, SimpleGET_LoadOnlyFromCache_Hit) {
                             log.bound());
 
   // Check that the NetLog was filled as expected.
-  net::CapturingNetLog::EntryList entries;
+  net::CapturingNetLog::CapturedEntryList entries;
   log.GetEntries(&entries);
 
   EXPECT_EQ(8u, entries.size());
@@ -710,7 +710,7 @@ TEST(HttpCache, SimpleGET_LoadBypassCache) {
   RunTransactionTestWithLog(cache.http_cache(), transaction, log.bound());
 
   // Check that the NetLog was filled as expected.
-  net::CapturingNetLog::EntryList entries;
+  net::CapturingNetLog::CapturedEntryList entries;
   log.GetEntries(&entries);
 
   EXPECT_EQ(8u, entries.size());