Refactor net::NetLog to provide implementation of observer pattern, not just the interface.

net/base/net_log_unittest.cc

Index: net/base/net_log_unittest.cc
diff --git a/net/base/net_log_unittest.cc b/net/base/net_log_unittest.cc
index d52f3b7a1226a3536b80c94f5af94ac716c9b2f0..70af138da004eadf6313f467c6584cab5b5f5266 100644
--- a/net/base/net_log_unittest.cc
+++ b/net/base/net_log_unittest.cc
@@ -5,6 +5,8 @@
 #include "net/base/net_log_unittest.h"
 
 #include "base/bind.h"
+#include "base/synchronization/waitable_event.h"
+#include "base/threading/simple_thread.h"
 #include "base/values.h"
 #include "net/base/net_errors.h"
 
@@ -12,6 +14,9 @@ namespace net {
 
 namespace {
 
+const int kThreads = 10;
+const int kEvents = 100;
+
 base::Value* NetLogLevelCallback(NetLog::LogLevel log_level) {
   base::DictionaryValue* dict = new base::DictionaryValue();
   dict->SetInteger("log_level", log_level);
@@ -19,14 +24,18 @@ base::Value* NetLogLevelCallback(NetLog::LogLevel log_level) {
 }
 
 TEST(NetLogTest, Basic) {
-  CapturingNetLog net_log;
-  net::CapturingNetLog::CapturedEntryList entries;
-  net_log.GetEntries(&entries);
+  NetLog net_log;
+
+  CapturingNetLogObserver capturing_net_log_observer;
+  net_log.AddThreadSafeObserver(&capturing_net_log_observer, NetLog::LOG_BASIC);
+
+  net::CapturingNetLogObserver::CapturedEntryList entries;
+  capturing_net_log_observer.GetEntries(&entries);
   EXPECT_EQ(0u, entries.size());
 
   net_log.AddGlobalEntry(NetLog::TYPE_CANCELLED);
 
-  net_log.GetEntries(&entries);
+  capturing_net_log_observer.GetEntries(&entries);
   ASSERT_EQ(1u, entries.size());
   EXPECT_EQ(NetLog::TYPE_CANCELLED, entries[0].type);
   EXPECT_EQ(NetLog::SOURCE_NONE, entries[0].source.type);
@@ -34,22 +43,29 @@ TEST(NetLogTest, Basic) {
   EXPECT_EQ(NetLog::PHASE_NONE, entries[0].phase);
   EXPECT_GE(base::TimeTicks::Now(), entries[0].time);
   EXPECT_FALSE(entries[0].params);
+
+  net_log.RemoveThreadSafeObserver(&capturing_net_log_observer);
 }
 
 // Check that the correct LogLevel is sent to NetLog Value callbacks, and that
 // LOG_NONE logs no events.
 TEST(NetLogTest, LogLevels) {
-  CapturingNetLog net_log;
+  NetLog net_log;
+
+  CapturingNetLogObserver capturing_net_log_observer;
+  net_log.AddThreadSafeObserver(&capturing_net_log_observer, NetLog::LOG_BASIC);
+
   for (int log_level = NetLog::LOG_ALL; log_level <= NetLog::LOG_NONE;
        ++log_level) {
-    net_log.SetLogLevel(static_cast<NetLog::LogLevel>(log_level));
-    EXPECT_EQ(log_level, net_log.GetLogLevel());
+    net_log.SetObserverLogLevel(&capturing_net_log_observer,
+                                static_cast<NetLog::LogLevel>(log_level));
+    EXPECT_EQ(log_level, capturing_net_log_observer.log_level());
 
     net_log.AddGlobalEntry(NetLog::TYPE_SOCKET_ALIVE,
                            base::Bind(NetLogLevelCallback));
 
-    net::CapturingNetLog::CapturedEntryList entries;
-    net_log.GetEntries(&entries);
+    net::CapturingNetLogObserver::CapturedEntryList entries;
+    capturing_net_log_observer.GetEntries(&entries);
 
     if (log_level == NetLog::LOG_NONE) {
       EXPECT_EQ(0u, entries.size());
@@ -66,8 +82,253 @@ TEST(NetLogTest, LogLevels) {
       EXPECT_EQ(log_level, logged_log_level);
     }
 
-    net_log.Clear();
+    capturing_net_log_observer.Clear();
+  }
+
+  net_log.RemoveThreadSafeObserver(&capturing_net_log_observer);
+}
+
+void AddEvent(NetLog* net_log) {
+  net_log->AddGlobalEntry(net::NetLog::TYPE_CANCELLED);
+}
+
+// A thread that waits until an event has been signalled before calling
+// RunTestThread.
+class NetLogTestThread : public base::SimpleThread {
+ public:
+  NetLogTestThread() : base::SimpleThread("NetLogTest"),
+                             net_log_(NULL),
+                             start_event_(NULL) {

[mmenke, 2013/06/03 14:27:09]

nit: fix indent.

[kouhei (in TOK), 2013/06/04 15:41:44]

Done.

+  }
+
+  // We'll wait for |start_event| to be triggered before calling a subclass's
+  // subclass's RunTestThread() function.
+  void Init(NetLog* net_log, base::WaitableEvent* start_event) {
+    start_event_ = start_event;
+    net_log_ = net_log;
+  }
+
+  virtual void Run() OVERRIDE {
+    start_event_->Wait();
+    RunTestThread();
+  }
+
+  // Subclasses must override this with the code they want to run on their
+  // thread.
+  virtual void RunTestThread() = 0;
+
+ protected:
+  NetLog* net_log_;
+
+ private:
+  // Only triggered once all threads have been created, to make it less likely
+  // each thread completes before the next one starts.
+  base::WaitableEvent* start_event_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetLogTestThread);
+};
+
+// A thread that adds a bunch of events to the NetLog.
+class AddEventsTestThread : public NetLogTestThread {
+ public:
+  AddEventsTestThread() {}
+  virtual ~AddEventsTestThread() {}
+
+ private:
+  virtual void RunTestThread() OVERRIDE {
+    for (int i = 0; i < kEvents; ++i)
+      AddEvent(net_log_);
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(AddEventsTestThread);
+};
+
+// A thread that adds and removes an observer from the NetLog repeatedly.
+class AddRemoveObserverTestThread : public NetLogTestThread {
+ public:
+  AddRemoveObserverTestThread() {}
+
+  virtual ~AddRemoveObserverTestThread() {
+    EXPECT_TRUE(!observer_.net_log());
   }
+
+ private:
+  virtual void RunTestThread() OVERRIDE {
+    for (int i = 0; i < kEvents; ++i) {
+      ASSERT_FALSE(observer_.net_log());
+
+      net_log_->AddThreadSafeObserver(&observer_, net::NetLog::LOG_BASIC);
+      ASSERT_EQ(net_log_, observer_.net_log());
+      ASSERT_EQ(net::NetLog::LOG_BASIC, observer_.log_level());
+
+      net_log_->SetObserverLogLevel(&observer_, net::NetLog::LOG_ALL_BUT_BYTES);
+      ASSERT_EQ(net_log_, observer_.net_log());
+      ASSERT_EQ(net::NetLog::LOG_ALL_BUT_BYTES, observer_.log_level());
+      ASSERT_LE(net_log_->GetLogLevel(), net::NetLog::LOG_ALL_BUT_BYTES);
+
+      net_log_->SetObserverLogLevel(&observer_, net::NetLog::LOG_ALL);
+      ASSERT_EQ(net_log_, observer_.net_log());
+      ASSERT_EQ(net::NetLog::LOG_ALL, observer_.log_level());
+      ASSERT_LE(net_log_->GetLogLevel(), net::NetLog::LOG_ALL);
+
+      net_log_->RemoveThreadSafeObserver(&observer_);
+      ASSERT_TRUE(!observer_.net_log());
+    }
+  }
+
+  CapturingNetLogObserver observer_;
+
+  DISALLOW_COPY_AND_ASSIGN(AddRemoveObserverTestThread);
+};
+
+// Creates |kThreads| threads of type |ThreadType| and then runs them all
+// to completion.
+template<class ThreadType>
+void RunTestThreads(NetLog* net_log) {
+  ThreadType threads[kThreads];
+  base::WaitableEvent start_event(true, false);
+
+  for (size_t i = 0; i < arraysize(threads); ++i) {
+    threads[i].Init(net_log, &start_event);
+    threads[i].Start();
+  }
+
+  start_event.Signal();
+
+  for (size_t i = 0; i < arraysize(threads); ++i)
+    threads[i].Join();
+}
+
+// Makes sure that events on multiple threads are dispatched to all observers.
+TEST(NetLogTest, NetLogEventThreads) {
+  NetLog net_log;
+
+  // Attach some observers.  Since they're created after |net_log|, they'll
+  // safely detach themselves on destruction.
+  CapturingNetLogObserver observers[3];

[mmenke, 2013/06/03 14:27:09]

I'm sorry - turns out that since CapturingNetLogObservers use their own mutex,
we have to use the Observer class that I told you was unnecessary.  Sorry to
make you run back and forth here.

[kouhei (in TOK), 2013/06/04 15:41:44]

ok

+  for (size_t i = 0; i < arraysize(observers); ++i)
+    net_log.AddThreadSafeObserver(&observers[i], net::NetLog::LOG_BASIC);
+
+  // Run a bunch of threads to completion, each of which will emit events to
+  // |net_log|.
+  RunTestThreads<AddEventsTestThread>(&net_log);
+
+  // Check that each observer saw the emitted events.
+  const size_t kTotalEvents = kThreads * kEvents;
+  for (size_t i = 0; i < arraysize(observers); ++i)
+    EXPECT_EQ(kTotalEvents, observers[i].GetSize());
+
+  // Remove all observers.
+  for (size_t i = 0; i < arraysize(observers); ++i)
+    net_log.RemoveThreadSafeObserver(&observers[i]);
+}
+
+// Test adding and removing a single observer.
+TEST(NetLogTest, NetLogAddRemoveObserver) {
+  NetLog net_log;
+  CapturingNetLogObserver observer;
+
+  AddEvent(&net_log);
+  EXPECT_EQ(0U, observer.GetSize());
+  EXPECT_EQ(NULL, observer.net_log());
+  EXPECT_EQ(net::NetLog::LOG_NONE, net_log.GetLogLevel());
+
+  // Add the observer and add an event.
+  net_log.AddThreadSafeObserver(&observer, net::NetLog::LOG_BASIC);
+  EXPECT_EQ(&net_log, observer.net_log());
+  EXPECT_EQ(net::NetLog::LOG_BASIC, observer.log_level());
+  EXPECT_EQ(net::NetLog::LOG_BASIC, net_log.GetLogLevel());
+
+  AddEvent(&net_log);
+  EXPECT_EQ(1U, observer.GetSize());
+
+  // Change the observer's logging level and add an event.
+  net_log.SetObserverLogLevel(&observer, net::NetLog::LOG_ALL);
+  EXPECT_EQ(&net_log, observer.net_log());
+  EXPECT_EQ(net::NetLog::LOG_ALL, observer.log_level());
+  EXPECT_EQ(net::NetLog::LOG_ALL, net_log.GetLogLevel());
+
+  AddEvent(&net_log);
+  EXPECT_EQ(2U, observer.GetSize());
+
+  // Remove observer and add an event.
+  net_log.RemoveThreadSafeObserver(&observer);
+  EXPECT_EQ(NULL, observer.net_log());
+  EXPECT_EQ(net::NetLog::LOG_NONE, net_log.GetLogLevel());
+
+  AddEvent(&net_log);
+  EXPECT_EQ(2U, observer.GetSize());
+
+  // Add the observer a final time, and add an event.
+  net_log.AddThreadSafeObserver(&observer, net::NetLog::LOG_ALL);
+  EXPECT_EQ(&net_log, observer.net_log());
+  EXPECT_EQ(net::NetLog::LOG_ALL, observer.log_level());
+  EXPECT_EQ(net::NetLog::LOG_ALL, net_log.GetLogLevel());
+
+  AddEvent(&net_log);
+  EXPECT_EQ(3U, observer.GetSize());
+
+  // Remove observer.
+  net_log.RemoveThreadSafeObserver(&observer);
+}
+
+// Test adding and removing two observers.
+TEST(NetLogTest, NetLogTwoObservers) {
+  NetLog net_log;
+  CapturingNetLogObserver observer[2];
+
+  // Add first observer.
+  net_log.AddThreadSafeObserver(&observer[0], net::NetLog::LOG_ALL_BUT_BYTES);
+  EXPECT_EQ(&net_log, observer[0].net_log());
+  EXPECT_EQ(NULL, observer[1].net_log());
+  EXPECT_EQ(net::NetLog::LOG_ALL_BUT_BYTES, observer[0].log_level());
+  EXPECT_EQ(net::NetLog::LOG_ALL_BUT_BYTES, net_log.GetLogLevel());
+
+  // Add second observer observer.
+  net_log.AddThreadSafeObserver(&observer[1], net::NetLog::LOG_ALL);
+  EXPECT_EQ(&net_log, observer[0].net_log());
+  EXPECT_EQ(&net_log, observer[1].net_log());
+  EXPECT_EQ(net::NetLog::LOG_ALL_BUT_BYTES, observer[0].log_level());
+  EXPECT_EQ(net::NetLog::LOG_ALL, observer[1].log_level());
+  EXPECT_EQ(net::NetLog::LOG_ALL, net_log.GetLogLevel());
+
+  // Add event and make sure both observers receive it.
+  AddEvent(&net_log);
+  EXPECT_EQ(1U, observer[0].GetSize());
+  EXPECT_EQ(1U, observer[1].GetSize());
+
+  // Remove second observer.
+  net_log.RemoveThreadSafeObserver(&observer[1]);
+  EXPECT_EQ(&net_log, observer[0].net_log());
+  EXPECT_EQ(NULL, observer[1].net_log());
+  EXPECT_EQ(net::NetLog::LOG_ALL_BUT_BYTES, observer[0].log_level());
+  EXPECT_EQ(net::NetLog::LOG_ALL_BUT_BYTES, net_log.GetLogLevel());
+
+  // Add event and make sure only second observer gets it.
+  AddEvent(&net_log);
+  EXPECT_EQ(2U, observer[0].GetSize());
+  EXPECT_EQ(1U, observer[1].GetSize());
+
+  // Remove first observer.
+  net_log.RemoveThreadSafeObserver(&observer[0]);
+  EXPECT_EQ(NULL, observer[0].net_log());
+  EXPECT_EQ(NULL, observer[1].net_log());
+  EXPECT_EQ(net::NetLog::LOG_NONE, net_log.GetLogLevel());
+
+  // Add event and make sure neither observer gets it.
+  AddEvent(&net_log);
+  EXPECT_EQ(2U, observer[0].GetSize());
+  EXPECT_EQ(1U, observer[1].GetSize());
+}
+
+// Makes sure that adding and removing observers simultaneously on different
+// threads works.
+TEST(NetLogTest, NetLogAddRemoveObserverThreads) {
+  NetLog net_log;
+
+  // Run a bunch of threads to completion, each of which will repeatedly add
+  // and remove an observer, and set its logging level.
+  RunTestThreads<AddRemoveObserverTestThread>(&net_log);
 }
 
 }  // namespace