Evaluate NetLog::Entry callbacks relative to each observers' log level.

Adjust a test to check this case. This doesn't handle cases where log events
only appear at a given log level because of external checks, but it will avoid
the bytes showing up everywhere.

BUG=348156

Review URL: https://codereview.chromium.org/183123006

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

4 files changed, 113 insertions, 60 deletions

diff --git a/net/base/net_log.cc b/net/base/net_log.cc
index d1e9c98..dfd07f8 100644
--- a/net/base/net_log.cc
+++ b/net/base/net_log.cc
@@ -123,21 +123,21 @@ bool NetLog::Source::FromEventParameters(base::Value* event_params,
 base::Value* NetLog::Entry::ToValue() const {
   base::DictionaryValue* entry_dict(new base::DictionaryValue());
 
-  entry_dict->SetString("time", TickCountToString(time_));
+  entry_dict->SetString("time", TickCountToString(data_->time));
 
   // Set the entry source.
   base::DictionaryValue* source_dict = new base::DictionaryValue();
-  source_dict->SetInteger("id", source_.id);
-  source_dict->SetInteger("type", static_cast<int>(source_.type));
+  source_dict->SetInteger("id", data_->source.id);
+  source_dict->SetInteger("type", static_cast<int>(data_->source.type));
   entry_dict->Set("source", source_dict);
 
   // Set the event info.
-  entry_dict->SetInteger("type", static_cast<int>(type_));
-  entry_dict->SetInteger("phase", static_cast<int>(phase_));
+  entry_dict->SetInteger("type", static_cast<int>(data_->type));
+  entry_dict->SetInteger("phase", static_cast<int>(data_->phase));
 
   // Set the event-specific parameters.
-  if (parameters_callback_) {
-    base::Value* value = parameters_callback_->Run(log_level_);
+  if (data_->parameters_callback) {
+    base::Value* value = data_->parameters_callback->Run(log_level_);
     if (value)
       entry_dict->Set("params", value);
   }
@@ -146,25 +146,30 @@ base::Value* NetLog::Entry::ToValue() const {
 }
 
 base::Value* NetLog::Entry::ParametersToValue() const {
-  if (parameters_callback_)
-    return parameters_callback_->Run(log_level_);
+  if (data_->parameters_callback)
+    return data_->parameters_callback->Run(log_level_);
   return NULL;
 }
 
-NetLog::Entry::Entry(
+NetLog::EntryData::EntryData(
     EventType type,
     Source source,
     EventPhase phase,
     base::TimeTicks time,
-    const ParametersCallback* parameters_callback,
-    LogLevel log_level)
-    : type_(type),
-      source_(source),
-      phase_(phase),
-      time_(time),
-      parameters_callback_(parameters_callback),
-      log_level_(log_level) {
-};
+    const ParametersCallback* parameters_callback)
+    : type(type),
+      source(source),
+      phase(phase),
+      time(time),
+      parameters_callback(parameters_callback) {
+}
+
+NetLog::EntryData::~EntryData() {
+}
+
+NetLog::Entry::Entry(const EntryData* data, LogLevel log_level)
+    : data_(data), log_level_(log_level) {
+}
 
 NetLog::Entry::~Entry() {
 }
@@ -189,6 +194,10 @@ NetLog* NetLog::ThreadSafeObserver::net_log() const {
   return net_log_;
 }
 
+void NetLog::ThreadSafeObserver::OnAddEntryData(const EntryData& entry_data) {
+  OnAddEntry(Entry(&entry_data, log_level()));
+}
+
 NetLog::NetLog()
     : last_id_(0),
       base_log_level_(LOG_NONE),
@@ -383,15 +392,14 @@ void NetLog::AddEntry(EventType type,
                       const Source& source,
                       EventPhase phase,
                       const NetLog::ParametersCallback* parameters_callback) {
-  LogLevel log_level = GetLogLevel();
-  if (log_level == LOG_NONE)
+  if (GetLogLevel() == LOG_NONE)
     return;
-  Entry entry(type, source, phase, base::TimeTicks::Now(),
-              parameters_callback, log_level);
+  EntryData entry_data(type, source, phase, base::TimeTicks::Now(),
+                       parameters_callback);
 
   // Notify all of the log observers.
   base::AutoLock lock(lock_);
-  FOR_EACH_OBSERVER(ThreadSafeObserver, observers_, OnAddEntry(entry));
+  FOR_EACH_OBSERVER(ThreadSafeObserver, observers_, OnAddEntryData(entry_data));
 }
 
 void BoundNetLog::AddEntry(NetLog::EventType type,
diff --git a/net/base/net_log.h b/net/base/net_log.h
index 875d785..901362a 100644
--- a/net/base/net_log.h
+++ b/net/base/net_log.h
@@ -120,19 +120,32 @@ class NET_EXPORT NetLog {
     uint32 id;
   };
 
+  struct NET_EXPORT EntryData {
+    EntryData(EventType type,
+              Source source,
+              EventPhase phase,
+              base::TimeTicks time,
+              const ParametersCallback* parameters_callback);
+    ~EntryData();
+
+    const EventType type;
+    const Source source;
+    const EventPhase phase;
+    const base::TimeTicks time;
+    const ParametersCallback* const parameters_callback;
+  };
+
+  // An Entry pre-binds EntryData to a LogLevel, so observers will observe the
+  // output of ToValue() and ParametersToValue() at their log level rather than
+  // current maximum.
   class NET_EXPORT Entry {
    public:
-    Entry(EventType type,
-          Source source,
-          EventPhase phase,
-          base::TimeTicks time,
-          const ParametersCallback* parameters_callback,
-          LogLevel log_level);
+    Entry(const EntryData* data, LogLevel log_level);
     ~Entry();
 
-    EventType type() const { return type_; }
-    Source source() const { return source_; }
-    EventPhase phase() const { return phase_; }
+    EventType type() const { return data_->type; }
+    Source source() const { return data_->source; }
+    EventPhase phase() const { return data_->phase; }
 
     // Serializes the specified event to a Value.  The Value also includes the
     // current time.  Caller takes ownership of returned Value.  Takes in a time
@@ -144,11 +157,7 @@ class NET_EXPORT NetLog {
     base::Value* ParametersToValue() const;
 
    private:
-    const EventType type_;
-    const Source source_;
-    const EventPhase phase_;
-    const base::TimeTicks time_;
-    const ParametersCallback* parameters_callback_;
+    const EntryData* const data_;
 
     // Log level when the event occurred.
     const LogLevel log_level_;
@@ -196,6 +205,8 @@ class NET_EXPORT NetLog {
    private:
     friend class NetLog;
 
+    void OnAddEntryData(const EntryData& entry_data);
+
     // Both of these values are only modified by the NetLog.
     LogLevel log_level_;
     NetLog* net_log_;
diff --git a/net/base/net_log_logger_unittest.cc b/net/base/net_log_logger_unittest.cc
index 05c16c7..17cc771 100644
--- a/net/base/net_log_logger_unittest.cc
+++ b/net/base/net_log_logger_unittest.cc
@@ -57,12 +57,12 @@ TEST_F(NetLogLoggerTest, GeneratesValidJSONWithOneEvent) {
 
     const int kDummyId = 1;
     NetLog::Source source(NetLog::SOURCE_SPDY_SESSION, kDummyId);
-    NetLog::Entry entry(NetLog::TYPE_PROXY_SERVICE,
-                        source,
-                        NetLog::PHASE_BEGIN,
-                        base::TimeTicks::Now(),
-                        NULL,
-                        NetLog::LOG_BASIC);
+    NetLog::EntryData entry_data(NetLog::TYPE_PROXY_SERVICE,
+                                 source,
+                                 NetLog::PHASE_BEGIN,
+                                 base::TimeTicks::Now(),
+                                 NULL);
+    NetLog::Entry entry(&entry_data, NetLog::LOG_BASIC);
     logger.OnAddEntry(entry);
   }
 
@@ -89,12 +89,12 @@ TEST_F(NetLogLoggerTest, GeneratesValidJSONWithMultipleEvents) {
 
     const int kDummyId = 1;
     NetLog::Source source(NetLog::SOURCE_SPDY_SESSION, kDummyId);
-    NetLog::Entry entry(NetLog::TYPE_PROXY_SERVICE,
-                        source,
-                        NetLog::PHASE_BEGIN,
-                        base::TimeTicks::Now(),
-                        NULL,
-                        NetLog::LOG_BASIC);
+    NetLog::EntryData entry_data(NetLog::TYPE_PROXY_SERVICE,
+                                 source,
+                                 NetLog::PHASE_BEGIN,
+                                 base::TimeTicks::Now(),
+                                 NULL);
+    NetLog::Entry entry(&entry_data, NetLog::LOG_BASIC);
 
     // Add the entry multiple times.
     logger.OnAddEntry(entry);
diff --git a/net/base/net_log_unittest.cc b/net/base/net_log_unittest.cc
index 7a98f79..4bd43a5 100644
--- a/net/base/net_log_unittest.cc
+++ b/net/base/net_log_unittest.cc
@@ -5,6 +5,7 @@
 #include "net/base/net_log_unittest.h"
 
 #include "base/bind.h"
+#include "base/memory/scoped_vector.h"
 #include "base/synchronization/waitable_event.h"
 #include "base/threading/simple_thread.h"
 #include "base/values.h"
@@ -94,8 +95,35 @@ class CountingObserver : public NetLog::ThreadSafeObserver {
   int count_;
 };
 
+class LoggingObserver : public NetLog::ThreadSafeObserver {
+ public:
+  LoggingObserver() {}
+
+  virtual ~LoggingObserver() {
+    if (net_log())
+      net_log()->RemoveThreadSafeObserver(this);
+  }
+
+  virtual void OnAddEntry(const NetLog::Entry& entry) OVERRIDE {
+    base::Value* value = entry.ToValue();
+    base::DictionaryValue* dict = NULL;
+    ASSERT_TRUE(value->GetAsDictionary(&dict));
+    values_.push_back(dict);
+  }
+
+  size_t GetNumValues() const { return values_.size(); }
+  base::DictionaryValue* GetValue(size_t index) const { return values_[index]; }
+
+ private:
+  ScopedVector<base::DictionaryValue> values_;
+};
+
+base::Value* LogLevelToValue(NetLog::LogLevel log_level) {
+  return new base::FundamentalValue(log_level);
+}
+
 void AddEvent(NetLog* net_log) {
-  net_log->AddGlobalEntry(NetLog::TYPE_CANCELLED);
+  net_log->AddGlobalEntry(NetLog::TYPE_CANCELLED, base::Bind(LogLevelToValue));
 }
 
 // A thread that waits until an event has been signalled before calling
@@ -272,10 +300,10 @@ TEST(NetLogTest, NetLogAddRemoveObserver) {
   EXPECT_EQ(3, observer.count());
 }
 
-// Test adding and removing two observers.
+// Test adding and removing two observers at different log levels.
 TEST(NetLogTest, NetLogTwoObservers) {
   NetLog net_log;
-  CountingObserver observer[2];
+  LoggingObserver observer[2];
 
   // Add first observer.
   net_log.AddThreadSafeObserver(&observer[0], NetLog::LOG_ALL_BUT_BYTES);
@@ -292,10 +320,16 @@ TEST(NetLogTest, NetLogTwoObservers) {
   EXPECT_EQ(NetLog::LOG_ALL, observer[1].log_level());
   EXPECT_EQ(NetLog::LOG_ALL, net_log.GetLogLevel());
 
-  // Add event and make sure both observers receive it.
+  // Add event and make sure both observers receive it at their respective log
+  // levels.
+  int param;
   AddEvent(&net_log);
-  EXPECT_EQ(1, observer[0].count());
-  EXPECT_EQ(1, observer[1].count());
+  ASSERT_EQ(1U, observer[0].GetNumValues());
+  ASSERT_TRUE(observer[0].GetValue(0)->GetInteger("params", &param));
+  EXPECT_EQ(observer[0].log_level(), param);
+  ASSERT_EQ(1U, observer[1].GetNumValues());
+  ASSERT_TRUE(observer[1].GetValue(0)->GetInteger("params", &param));
+  EXPECT_EQ(observer[1].log_level(), param);
 
   // Remove second observer.
   net_log.RemoveThreadSafeObserver(&observer[1]);
@@ -306,8 +340,8 @@ TEST(NetLogTest, NetLogTwoObservers) {
 
   // Add event and make sure only second observer gets it.
   AddEvent(&net_log);
-  EXPECT_EQ(2, observer[0].count());
-  EXPECT_EQ(1, observer[1].count());
+  EXPECT_EQ(2U, observer[0].GetNumValues());
+  EXPECT_EQ(1U, observer[1].GetNumValues());
 
   // Remove first observer.
   net_log.RemoveThreadSafeObserver(&observer[0]);
@@ -317,8 +351,8 @@ TEST(NetLogTest, NetLogTwoObservers) {
 
   // Add event and make sure neither observer gets it.
   AddEvent(&net_log);
-  EXPECT_EQ(2, observer[0].count());
-  EXPECT_EQ(1, observer[1].count());
+  EXPECT_EQ(2U, observer[0].GetNumValues());
+  EXPECT_EQ(1U, observer[1].GetNumValues());
 }
 
 // Makes sure that adding and removing observers simultaneously on different