From e7c6aaf0b8813bd67d4087200d549f8b5f1895fc Mon Sep 17 00:00:00 2001 From: "kouhei@chromium.org" Date: Fri, 7 Jun 2013 21:59:07 +0000 Subject: Refactor net::NetLog to provide implementation of observer pattern, not just the interface. This would make use of net::NetLog::ThreadSafeObserver available from base/net, so custom NetLog implementations can focus on implementing OnAddEntry() without re-implementing all NetLog methods. The implementation of observer pattern was previously provided in ChromeNetLog. The contents of chrome_net_log_unittest are merged to net_log_unittest. TESTS=net_log_unittest BUG=None Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=204861 Review URL: https://chromiumcodereview.appspot.com/16137008 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@204946 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/net/chrome_net_log.cc | 69 +------ chrome/browser/net/chrome_net_log.h | 32 --- chrome/browser/net/chrome_net_log_unittest.cc | 271 -------------------------- chrome/chrome_tests_unit.gypi | 1 - net/base/capturing_net_log.cc | 62 +++--- net/base/capturing_net_log.h | 63 +++--- net/base/net_log.cc | 98 ++++++++-- net/base/net_log.h | 51 +++-- net/base/net_log_unittest.cc | 265 ++++++++++++++++++++++++- net/tools/gdig/file_net_log.cc | 40 +--- net/tools/gdig/file_net_log.h | 26 +-- net/tools/gdig/gdig.cc | 13 +- net/tools/get_server_time/get_server_time.cc | 67 +++---- remoting/host/vlog_net_log.cc | 42 ++-- remoting/host/vlog_net_log.h | 14 +- 15 files changed, 517 insertions(+), 597 deletions(-) delete mode 100644 chrome/browser/net/chrome_net_log_unittest.cc diff --git a/chrome/browser/net/chrome_net_log.cc b/chrome/browser/net/chrome_net_log.cc index b1594dc..7e9a2fc 100644 --- a/chrome/browser/net/chrome_net_log.cc +++ b/chrome/browser/net/chrome_net_log.cc @@ -16,10 +16,7 @@ #include "chrome/common/chrome_switches.h" ChromeNetLog::ChromeNetLog() - : last_id_(0), - base_log_level_(LOG_NONE), - effective_log_level_(LOG_NONE), - net_log_temp_file_(new NetLogTempFile(this)) { + : net_log_temp_file_(new NetLogTempFile(this)) { const CommandLine* command_line = CommandLine::ForCurrentProcess(); // Adjust base log level based on command line switch, if present. // This is done before adding any observers so the call to UpdateLogLevel when @@ -31,7 +28,7 @@ ChromeNetLog::ChromeNetLog() if (base::StringToInt(log_level_string, &command_line_log_level) && command_line_log_level >= LOG_ALL && command_line_log_level <= LOG_NONE) { - base_log_level_ = static_cast(command_line_log_level); + SetBaseLogLevel(static_cast(command_line_log_level)); } } @@ -68,65 +65,3 @@ ChromeNetLog::~ChromeNetLog() { RemoveThreadSafeObserver(net_log_logger_.get()); } -void ChromeNetLog::OnAddEntry(const net::NetLog::Entry& entry) { - base::AutoLock lock(lock_); - - // Notify all of the log observers. - FOR_EACH_OBSERVER(ThreadSafeObserver, observers_, OnAddEntry(entry)); -} - -uint32 ChromeNetLog::NextID() { - return base::subtle::NoBarrier_AtomicIncrement(&last_id_, 1); -} - -net::NetLog::LogLevel ChromeNetLog::GetLogLevel() const { - base::subtle::Atomic32 log_level = - base::subtle::NoBarrier_Load(&effective_log_level_); - return static_cast(log_level); -} - -void ChromeNetLog::AddThreadSafeObserver( - net::NetLog::ThreadSafeObserver* observer, - LogLevel log_level) { - base::AutoLock lock(lock_); - - observers_.AddObserver(observer); - OnAddObserver(observer, log_level); - UpdateLogLevel(); -} - -void ChromeNetLog::SetObserverLogLevel( - net::NetLog::ThreadSafeObserver* observer, - LogLevel log_level) { - base::AutoLock lock(lock_); - - DCHECK(observers_.HasObserver(observer)); - OnSetObserverLogLevel(observer, log_level); - UpdateLogLevel(); -} - -void ChromeNetLog::RemoveThreadSafeObserver( - net::NetLog::ThreadSafeObserver* observer) { - base::AutoLock lock(lock_); - - DCHECK(observers_.HasObserver(observer)); - observers_.RemoveObserver(observer); - OnRemoveObserver(observer); - UpdateLogLevel(); -} - -void ChromeNetLog::UpdateLogLevel() { - lock_.AssertAcquired(); - - // Look through all the observers and find the finest granularity - // log level (higher values of the enum imply *lower* log levels). - LogLevel new_effective_log_level = base_log_level_; - ObserverListBase::Iterator it(observers_); - ThreadSafeObserver* observer; - while ((observer = it.GetNext()) != NULL) { - new_effective_log_level = - std::min(new_effective_log_level, observer->log_level()); - } - base::subtle::NoBarrier_Store(&effective_log_level_, - new_effective_log_level); -} diff --git a/chrome/browser/net/chrome_net_log.h b/chrome/browser/net/chrome_net_log.h index 9bbf152..10e45fb 100644 --- a/chrome/browser/net/chrome_net_log.h +++ b/chrome/browser/net/chrome_net_log.h @@ -25,46 +25,14 @@ class ChromeNetLog : public net::NetLog { ChromeNetLog(); virtual ~ChromeNetLog(); - // NetLog implementation: - virtual uint32 NextID() OVERRIDE; - virtual LogLevel GetLogLevel() const OVERRIDE; - virtual void AddThreadSafeObserver(ThreadSafeObserver* observer, - LogLevel log_level) OVERRIDE; - virtual void SetObserverLogLevel(ThreadSafeObserver* observer, - LogLevel log_level) OVERRIDE; - virtual void RemoveThreadSafeObserver(ThreadSafeObserver* observer) OVERRIDE; - NetLogTempFile* net_log_temp_file() { return net_log_temp_file_.get(); } private: - // NetLog implementation: - virtual void OnAddEntry(const net::NetLog::Entry& entry) OVERRIDE; - - // Called whenever an observer is added or removed, or has its log level - // changed. Must have acquired |lock_| prior to calling. - void UpdateLogLevel(); - - // |lock_| protects access to |observers_|. - base::Lock lock_; - - // Last assigned source ID. Incremented to get the next one. - base::subtle::Atomic32 last_id_; - - // The lowest allowed log level, regardless of any ChromeNetLogObservers. - // Normally defaults to LOG_BASIC, but can be changed with command line flags. - LogLevel base_log_level_; - - // The current log level. - base::subtle::Atomic32 effective_log_level_; - scoped_ptr net_log_logger_; scoped_ptr net_log_temp_file_; - // |lock_| must be acquired whenever reading or writing to this. - ObserverList observers_; - DISALLOW_COPY_AND_ASSIGN(ChromeNetLog); }; diff --git a/chrome/browser/net/chrome_net_log_unittest.cc b/chrome/browser/net/chrome_net_log_unittest.cc deleted file mode 100644 index 6eb0912..0000000 --- a/chrome/browser/net/chrome_net_log_unittest.cc +++ /dev/null @@ -1,271 +0,0 @@ -// 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 "chrome/browser/net/chrome_net_log.h" - -#include "base/synchronization/waitable_event.h" -#include "base/threading/simple_thread.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace { - -const int kThreads = 10; -const int kEvents = 100; - -class CountingObserver : public net::NetLog::ThreadSafeObserver { - public: - CountingObserver() : count_(0) {} - - virtual ~CountingObserver() { - if (net_log()) - net_log()->RemoveThreadSafeObserver(this); - } - - virtual void OnAddEntry(const net::NetLog::Entry& entry) OVERRIDE { - ++count_; - } - - int count() const { return count_; } - - private: - int count_; -}; - -void AddEvent(ChromeNetLog* net_log) { - net_log->AddGlobalEntry(net::NetLog::TYPE_CANCELLED); -} - -// A thread that waits until an event has been signalled before calling -// RunTestThread. -class ChromeNetLogTestThread : public base::SimpleThread { - public: - ChromeNetLogTestThread() : base::SimpleThread("ChromeNetLogTest"), - net_log_(NULL), - start_event_(NULL) { - } - - // We'll wait for |start_event| to be triggered before calling a subclass's - // subclass's RunTestThread() function. - void Init(ChromeNetLog* 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: - ChromeNetLog* 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(ChromeNetLogTestThread); -}; - -// A thread that adds a bunch of events to the NetLog. -class AddEventsTestThread : public ChromeNetLogTestThread { - 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 ChromeNetLogTestThread { - 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()); - } - } - - CountingObserver observer_; - - DISALLOW_COPY_AND_ASSIGN(AddRemoveObserverTestThread); -}; - -// Creates |kThreads| threads of type |ThreadType| and then runs them all -// to completion. -template -void RunTestThreads(ChromeNetLog* 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(ChromeNetLogTest, NetLogEventThreads) { - ChromeNetLog net_log; - - // Attach some observers. Since they're created after |net_log|, they'll - // safely detach themselves on destruction. - CountingObserver observers[3]; - 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(&net_log); - - // Check that each observer saw the emitted events. - const int kTotalEvents = kThreads * kEvents; - for (size_t i = 0; i < arraysize(observers); ++i) - EXPECT_EQ(kTotalEvents, observers[i].count()); -} - -// Test adding and removing a single observer. -TEST(ChromeNetLogTest, NetLogAddRemoveObserver) { - ChromeNetLog net_log; - CountingObserver observer; - - AddEvent(&net_log); - EXPECT_EQ(0, observer.count()); - 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(1, observer.count()); - - // 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(2, observer.count()); - - // 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(2, observer.count()); - - // 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(3, observer.count()); -} - -// Test adding and removing two observers. -TEST(ChromeNetLogTest, NetLogTwoObservers) { - ChromeNetLog net_log; - CountingObserver 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(1, observer[0].count()); - EXPECT_EQ(1, observer[1].count()); - - // 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(2, observer[0].count()); - EXPECT_EQ(1, observer[1].count()); - - // 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(2, observer[0].count()); - EXPECT_EQ(1, observer[1].count()); -} - -// Makes sure that adding and removing observers simultaneously on different -// threads works. -TEST(ChromeNetLogTest, NetLogAddRemoveObserverThreads) { - ChromeNetLog 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(&net_log); -} - -} // namespace diff --git a/chrome/chrome_tests_unit.gypi b/chrome/chrome_tests_unit.gypi index 4ab38eb..6d45397 100644 --- a/chrome/chrome_tests_unit.gypi +++ b/chrome/chrome_tests_unit.gypi @@ -940,7 +940,6 @@ 'browser/nacl_host/nacl_validation_cache_unittest.cc', 'browser/nacl_host/pnacl_translation_cache_unittest.cc', 'browser/net/chrome_fraudulent_certificate_reporter_unittest.cc', - 'browser/net/chrome_net_log_unittest.cc', 'browser/net/chrome_network_delegate_unittest.cc', 'browser/net/connection_tester_unittest.cc', 'browser/net/dns_probe_job_unittest.cc', diff --git a/net/base/capturing_net_log.cc b/net/base/capturing_net_log.cc index b90dcae..f938931 100644 --- a/net/base/capturing_net_log.cc +++ b/net/base/capturing_net_log.cc @@ -67,20 +67,19 @@ std::string CapturingNetLog::CapturedEntry::GetParamsJson() const { return json; } -CapturingNetLog::CapturingNetLog() - : last_id_(0), - log_level_(LOG_ALL_BUT_BYTES) { -} +CapturingNetLog::Observer::Observer() {} -CapturingNetLog::~CapturingNetLog() {} +CapturingNetLog::Observer::~Observer() {} -void CapturingNetLog::GetEntries(CapturedEntryList* entry_list) const { +void CapturingNetLog::Observer::GetEntries( + CapturedEntryList* entry_list) const { base::AutoLock lock(lock_); *entry_list = captured_entries_; } -void CapturingNetLog::GetEntriesForSource(NetLog::Source source, - CapturedEntryList* entry_list) const { +void CapturingNetLog::Observer::GetEntriesForSource( + NetLog::Source source, + CapturedEntryList* entry_list) const { base::AutoLock lock(lock_); entry_list->clear(); for (CapturedEntryList::const_iterator entry = captured_entries_.begin(); @@ -90,22 +89,17 @@ void CapturingNetLog::GetEntriesForSource(NetLog::Source source, } } -size_t CapturingNetLog::GetSize() const { +size_t CapturingNetLog::Observer::GetSize() const { base::AutoLock lock(lock_); return captured_entries_.size(); } -void CapturingNetLog::Clear() { +void CapturingNetLog::Observer::Clear() { base::AutoLock lock(lock_); captured_entries_.clear(); } -void CapturingNetLog::SetLogLevel(NetLog::LogLevel log_level) { - base::AutoLock lock(lock_); - log_level_ = log_level; -} - -void CapturingNetLog::OnAddEntry(const net::NetLog::Entry& entry) { +void CapturingNetLog::Observer::OnAddEntry(const net::NetLog::Entry& entry) { // Only BoundNetLogs without a NetLog should have an invalid source. CHECK(entry.source().IsValid()); @@ -126,29 +120,35 @@ void CapturingNetLog::OnAddEntry(const net::NetLog::Entry& entry) { scoped_ptr(param_dict))); } -uint32 CapturingNetLog::NextID() { - return base::subtle::NoBarrier_AtomicIncrement(&last_id_, 1); +CapturingNetLog::CapturingNetLog() { + AddThreadSafeObserver(&capturing_net_log_observer_, LOG_ALL_BUT_BYTES); } -NetLog::LogLevel CapturingNetLog::GetLogLevel() const { - base::AutoLock lock(lock_); - return log_level_; +CapturingNetLog::~CapturingNetLog() { + RemoveThreadSafeObserver(&capturing_net_log_observer_); } -void CapturingNetLog::AddThreadSafeObserver( - NetLog::ThreadSafeObserver* observer, - NetLog::LogLevel log_level) { - NOTIMPLEMENTED() << "Not currently used by net unit tests."; +void CapturingNetLog::SetLogLevel(NetLog::LogLevel log_level) { + SetObserverLogLevel(&capturing_net_log_observer_, log_level); } -void CapturingNetLog::SetObserverLogLevel(ThreadSafeObserver* observer, - LogLevel log_level) { - NOTIMPLEMENTED() << "Not currently used by net unit tests."; +void CapturingNetLog::GetEntries( + CapturingNetLog::CapturedEntryList* entry_list) const { + capturing_net_log_observer_.GetEntries(entry_list); +} + +void CapturingNetLog::GetEntriesForSource( + NetLog::Source source, + CapturedEntryList* entry_list) const { + capturing_net_log_observer_.GetEntriesForSource(source, entry_list); } -void CapturingNetLog::RemoveThreadSafeObserver( - NetLog::ThreadSafeObserver* observer) { - NOTIMPLEMENTED() << "Not currently used by net unit tests."; +size_t CapturingNetLog::GetSize() const { + return capturing_net_log_observer_.GetSize(); +} + +void CapturingNetLog::Clear() { + capturing_net_log_observer_.Clear(); } CapturingBoundNetLog::CapturingBoundNetLog() diff --git a/net/base/capturing_net_log.h b/net/base/capturing_net_log.h index d3586d1..d23beb9 100644 --- a/net/base/capturing_net_log.h +++ b/net/base/capturing_net_log.h @@ -23,9 +23,10 @@ class DictionaryValue; namespace net { -// CapturingNetLog is an implementation of NetLog that saves messages to a -// bounded buffer. It is intended for testing only, and is part of the -// net_test_support project. +// CapturingNetLog is a NetLog which instantiates Observer that saves messages +// to a bounded buffer. It is intended for testing only, and is part of the +// net_test_support project. This is provided for convinience and compatilbility +// with the old unittests. class CapturingNetLog : public NetLog { public: struct CapturedEntry { @@ -71,40 +72,48 @@ class CapturingNetLog : public NetLog { CapturingNetLog(); virtual ~CapturingNetLog(); - // Returns the list of all entries in the log. + void SetLogLevel(LogLevel log_level); + + // Below methods are forwarded to capturing_net_log_observer_. void GetEntries(CapturedEntryList* entry_list) const; + void GetEntriesForSource(Source source, CapturedEntryList* entry_list) const; + size_t GetSize() const; + void Clear(); - // Fills |entry_list| with all entries in the log from the specified Source. - void GetEntriesForSource(NetLog::Source source, - CapturedEntryList* entry_list) const; + private: + // Observer is an implementation of NetLog::ThreadSafeObserver + // that saves messages to a bounded buffer. It is intended for testing only, + // and is part of the net_test_support project. + class Observer : public NetLog::ThreadSafeObserver { + public: + Observer(); + virtual ~Observer(); - // Returns number of entries in the log. - size_t GetSize() const; + // Returns the list of all entries in the log. + void GetEntries(CapturedEntryList* entry_list) const; - void Clear(); + // Fills |entry_list| with all entries in the log from the specified Source. + void GetEntriesForSource(Source source, + CapturedEntryList* entry_list) const; - void SetLogLevel(NetLog::LogLevel log_level); + // Returns number of entries in the log. + size_t GetSize() const; - // NetLog implementation: - virtual void OnAddEntry(const net::NetLog::Entry& entry) OVERRIDE; - virtual uint32 NextID() OVERRIDE; - virtual LogLevel GetLogLevel() const OVERRIDE; - virtual void AddThreadSafeObserver(ThreadSafeObserver* observer, - LogLevel log_level) OVERRIDE; - virtual void SetObserverLogLevel(ThreadSafeObserver* observer, - LogLevel log_level) OVERRIDE; - virtual void RemoveThreadSafeObserver(ThreadSafeObserver* observer) OVERRIDE; + void Clear(); - private: - // Needs to be "mutable" so can use it in GetEntries(). - mutable base::Lock lock_; + private: + // ThreadSafeObserver implementation: + virtual void OnAddEntry(const Entry& entry) OVERRIDE; + + // Needs to be "mutable" so can use it in GetEntries(). + mutable base::Lock lock_; - // Last assigned source ID. Incremented to get the next one. - base::subtle::Atomic32 last_id_; + CapturedEntryList captured_entries_; - CapturedEntryList captured_entries_; + DISALLOW_COPY_AND_ASSIGN(Observer); + }; - NetLog::LogLevel log_level_; + Observer capturing_net_log_observer_; DISALLOW_COPY_AND_ASSIGN(CapturingNetLog); }; diff --git a/net/base/net_log.cc b/net/base/net_log.cc index 0ca551e..1b61698 100644 --- a/net/base/net_log.cc +++ b/net/base/net_log.cc @@ -187,6 +187,15 @@ NetLog* NetLog::ThreadSafeObserver::net_log() const { return net_log_; } +NetLog::NetLog() + : last_id_(0), + base_log_level_(LOG_NONE), + effective_log_level_(LOG_NONE) { +} + +NetLog::~NetLog() { +} + void NetLog::AddGlobalEntry(EventType type) { AddEntry(type, Source(net::NetLog::SOURCE_NONE, NextID()), @@ -203,6 +212,73 @@ void NetLog::AddGlobalEntry( ¶meters_callback); } +uint32 NetLog::NextID() { + return base::subtle::NoBarrier_AtomicIncrement(&last_id_, 1); +} + +void NetLog::SetBaseLogLevel(LogLevel log_level) { + base::AutoLock lock(lock_); + base_log_level_ = log_level; + + UpdateLogLevel(); +} + +NetLog::LogLevel NetLog::GetLogLevel() const { + base::subtle::Atomic32 log_level = + base::subtle::NoBarrier_Load(&effective_log_level_); + return static_cast(log_level); +} + +void NetLog::AddThreadSafeObserver( + net::NetLog::ThreadSafeObserver* observer, + LogLevel log_level) { + base::AutoLock lock(lock_); + + DCHECK(!observer->net_log_); + observers_.AddObserver(observer); + observer->net_log_ = this; + observer->log_level_ = log_level; + UpdateLogLevel(); +} + +void NetLog::SetObserverLogLevel( + net::NetLog::ThreadSafeObserver* observer, + LogLevel log_level) { + base::AutoLock lock(lock_); + + DCHECK(observers_.HasObserver(observer)); + DCHECK_EQ(this, observer->net_log_); + observer->log_level_ = log_level; + UpdateLogLevel(); +} + +void NetLog::RemoveThreadSafeObserver( + net::NetLog::ThreadSafeObserver* observer) { + base::AutoLock lock(lock_); + + DCHECK(observers_.HasObserver(observer)); + DCHECK_EQ(this, observer->net_log_); + observers_.RemoveObserver(observer); + observer->net_log_ = NULL; + UpdateLogLevel(); +} + +void NetLog::UpdateLogLevel() { + lock_.AssertAcquired(); + + // Look through all the observers and find the finest granularity + // log level (higher values of the enum imply *lower* log levels). + LogLevel new_effective_log_level = base_log_level_; + ObserverListBase::Iterator it(observers_); + ThreadSafeObserver* observer; + while ((observer = it.GetNext()) != NULL) { + new_effective_log_level = + std::min(new_effective_log_level, observer->log_level()); + } + base::subtle::NoBarrier_Store(&effective_log_level_, + new_effective_log_level); +} + // static std::string NetLog::TickCountToString(const base::TimeTicks& time) { int64 delta_time = (time - base::TimeTicks()).InMilliseconds(); @@ -301,23 +377,6 @@ NetLog::ParametersCallback NetLog::StringCallback(const char* name, return base::Bind(&NetLogString16Callback, name, value); } -void NetLog::OnAddObserver(ThreadSafeObserver* observer, LogLevel log_level) { - DCHECK(!observer->net_log_); - observer->net_log_ = this; - observer->log_level_ = log_level; -} - -void NetLog::OnSetObserverLogLevel(ThreadSafeObserver* observer, - LogLevel log_level) { - DCHECK_EQ(this, observer->net_log_); - observer->log_level_ = log_level; -} - -void NetLog::OnRemoveObserver(ThreadSafeObserver* observer) { - DCHECK_EQ(this, observer->net_log_); - observer->net_log_ = NULL; -} - void NetLog::AddEntry(EventType type, const Source& source, EventPhase phase, @@ -327,7 +386,10 @@ void NetLog::AddEntry(EventType type, return; Entry entry(type, source, phase, base::TimeTicks::Now(), parameters_callback, log_level); - OnAddEntry(entry); + + // Notify all of the log observers. + base::AutoLock lock(lock_); + FOR_EACH_OBSERVER(ThreadSafeObserver, observers_, OnAddEntry(entry)); } void BoundNetLog::AddEntry(NetLog::EventType type, diff --git a/net/base/net_log.h b/net/base/net_log.h index 8ae33e9..fb5637d 100644 --- a/net/base/net_log.h +++ b/net/base/net_log.h @@ -7,10 +7,13 @@ #include +#include "base/atomicops.h" #include "base/basictypes.h" #include "base/callback_forward.h" #include "base/compiler_specific.h" +#include "base/observer_list.h" #include "base/string16.h" +#include "base/synchronization/lock.h" #include "base/time.h" #include "net/base/net_export.h" @@ -198,8 +201,8 @@ class NET_EXPORT NetLog { DISALLOW_COPY_AND_ASSIGN(ThreadSafeObserver); }; - NetLog() {} - virtual ~NetLog() {} + NetLog(); + virtual ~NetLog(); // Emits a global event to the log stream, with its own unique source ID. void AddGlobalEntry(EventType type); @@ -208,11 +211,11 @@ class NET_EXPORT NetLog { // Returns a unique ID which can be used as a source ID. All returned IDs // will be unique and greater than 0. - virtual uint32 NextID() = 0; + uint32 NextID(); // Returns the logging level for this NetLog. This is used to avoid computing // and saving expensive log entries. - virtual LogLevel GetLogLevel() const = 0; + LogLevel GetLogLevel() const; // Adds an observer and sets its log level. The observer must not be // watching any NetLog, including this one, when this is called. @@ -224,21 +227,19 @@ class NET_EXPORT NetLog { // // NetLog implementations must call NetLog::OnAddObserver to update the // observer's internal state. - virtual void AddThreadSafeObserver(ThreadSafeObserver* observer, - LogLevel log_level) = 0; + void AddThreadSafeObserver(ThreadSafeObserver* observer, LogLevel log_level); // Sets the log level of |observer| to |log_level|. |observer| must be // watching |this|. NetLog implementations must call // NetLog::OnSetObserverLogLevel to update the observer's internal state. - virtual void SetObserverLogLevel(ThreadSafeObserver* observer, - LogLevel log_level) = 0; + void SetObserverLogLevel(ThreadSafeObserver* observer, LogLevel log_level); // Removes an observer. NetLog implementations must call // NetLog::OnAddObserver to update the observer's internal state. // // For thread safety reasons, it is recommended that this not be called in // an object's destructor. - virtual void RemoveThreadSafeObserver(ThreadSafeObserver* observer) = 0; + void RemoveThreadSafeObserver(ThreadSafeObserver* observer); // Converts a time to the string format that the NetLog uses to represent // times. Strings are used since integers may overflow. @@ -293,16 +294,8 @@ class NET_EXPORT NetLog { const base::string16* value); protected: - // Child classes should respond to the new entry here. This includes - // creating the Entry object and alerting their observers. - virtual void OnAddEntry(const Entry& entry) = 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); - void OnSetObserverLogLevel(ThreadSafeObserver* observer, - LogLevel log_level); - void OnRemoveObserver(ThreadSafeObserver* observer); + // Set the lowest allowed log level, regardless of any Observers. + void SetBaseLogLevel(LogLevel log_level); private: friend class BoundNetLog; @@ -312,6 +305,26 @@ class NET_EXPORT NetLog { EventPhase phase, const NetLog::ParametersCallback* parameters_callback); + // Called whenever an observer is added or removed, or has its log level + // changed. Must have acquired |lock_| prior to calling. + void UpdateLogLevel(); + + // |lock_| protects access to |observers_|. + base::Lock lock_; + + // Last assigned source ID. Incremented to get the next one. + base::subtle::Atomic32 last_id_; + + // The lowest allowed log level, regardless of any Observers. + // Normally defaults to LOG_NONE, but can be changed with SetBaseLogLevel + LogLevel base_log_level_; + + // The current log level. + base::subtle::Atomic32 effective_log_level_; + + // |lock_| must be acquired whenever reading or writing to this. + ObserverList observers_; + DISALLOW_COPY_AND_ASSIGN(NetLog); }; diff --git a/net/base/net_log_unittest.cc b/net/base/net_log_unittest.cc index d52f3b7..7a98f79 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); @@ -20,7 +25,7 @@ base::Value* NetLogLevelCallback(NetLog::LogLevel log_level) { TEST(NetLogTest, Basic) { CapturingNetLog net_log; - net::CapturingNetLog::CapturedEntryList entries; + CapturingNetLog::CapturedEntryList entries; net_log.GetEntries(&entries); EXPECT_EQ(0u, entries.size()); @@ -48,7 +53,7 @@ TEST(NetLogTest, LogLevels) { net_log.AddGlobalEntry(NetLog::TYPE_SOCKET_ALIVE, base::Bind(NetLogLevelCallback)); - net::CapturingNetLog::CapturedEntryList entries; + CapturingNetLog::CapturedEntryList entries; net_log.GetEntries(&entries); if (log_level == NetLog::LOG_NONE) { @@ -70,6 +75,262 @@ TEST(NetLogTest, LogLevels) { } } +class CountingObserver : public NetLog::ThreadSafeObserver { + public: + CountingObserver() : count_(0) {} + + virtual ~CountingObserver() { + if (net_log()) + net_log()->RemoveThreadSafeObserver(this); + } + + virtual void OnAddEntry(const NetLog::Entry& entry) OVERRIDE { + ++count_; + } + + int count() const { return count_; } + + private: + int count_; +}; + +void AddEvent(NetLog* net_log) { + net_log->AddGlobalEntry(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) { + } + + // 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_, NetLog::LOG_BASIC); + ASSERT_EQ(net_log_, observer_.net_log()); + ASSERT_EQ(NetLog::LOG_BASIC, observer_.log_level()); + + net_log_->SetObserverLogLevel(&observer_, NetLog::LOG_ALL_BUT_BYTES); + ASSERT_EQ(net_log_, observer_.net_log()); + ASSERT_EQ(NetLog::LOG_ALL_BUT_BYTES, observer_.log_level()); + ASSERT_LE(net_log_->GetLogLevel(), NetLog::LOG_ALL_BUT_BYTES); + + net_log_->SetObserverLogLevel(&observer_, NetLog::LOG_ALL); + ASSERT_EQ(net_log_, observer_.net_log()); + ASSERT_EQ(NetLog::LOG_ALL, observer_.log_level()); + ASSERT_LE(net_log_->GetLogLevel(), NetLog::LOG_ALL); + + net_log_->RemoveThreadSafeObserver(&observer_); + ASSERT_TRUE(!observer_.net_log()); + } + } + + CountingObserver observer_; + + DISALLOW_COPY_AND_ASSIGN(AddRemoveObserverTestThread); +}; + +// Creates |kThreads| threads of type |ThreadType| and then runs them all +// to completion. +template +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. + CountingObserver observers[3]; + for (size_t i = 0; i < arraysize(observers); ++i) + net_log.AddThreadSafeObserver(&observers[i], NetLog::LOG_BASIC); + + // Run a bunch of threads to completion, each of which will emit events to + // |net_log|. + RunTestThreads(&net_log); + + // Check that each observer saw the emitted events. + const int kTotalEvents = kThreads * kEvents; + for (size_t i = 0; i < arraysize(observers); ++i) + EXPECT_EQ(kTotalEvents, observers[i].count()); +} + +// Test adding and removing a single observer. +TEST(NetLogTest, NetLogAddRemoveObserver) { + NetLog net_log; + CountingObserver observer; + + AddEvent(&net_log); + EXPECT_EQ(0, observer.count()); + EXPECT_EQ(NULL, observer.net_log()); + EXPECT_EQ(NetLog::LOG_NONE, net_log.GetLogLevel()); + + // Add the observer and add an event. + net_log.AddThreadSafeObserver(&observer, NetLog::LOG_BASIC); + EXPECT_EQ(&net_log, observer.net_log()); + EXPECT_EQ(NetLog::LOG_BASIC, observer.log_level()); + EXPECT_EQ(NetLog::LOG_BASIC, net_log.GetLogLevel()); + + AddEvent(&net_log); + EXPECT_EQ(1, observer.count()); + + // Change the observer's logging level and add an event. + net_log.SetObserverLogLevel(&observer, NetLog::LOG_ALL); + EXPECT_EQ(&net_log, observer.net_log()); + EXPECT_EQ(NetLog::LOG_ALL, observer.log_level()); + EXPECT_EQ(NetLog::LOG_ALL, net_log.GetLogLevel()); + + AddEvent(&net_log); + EXPECT_EQ(2, observer.count()); + + // Remove observer and add an event. + net_log.RemoveThreadSafeObserver(&observer); + EXPECT_EQ(NULL, observer.net_log()); + EXPECT_EQ(NetLog::LOG_NONE, net_log.GetLogLevel()); + + AddEvent(&net_log); + EXPECT_EQ(2, observer.count()); + + // Add the observer a final time, and add an event. + net_log.AddThreadSafeObserver(&observer, NetLog::LOG_ALL); + EXPECT_EQ(&net_log, observer.net_log()); + EXPECT_EQ(NetLog::LOG_ALL, observer.log_level()); + EXPECT_EQ(NetLog::LOG_ALL, net_log.GetLogLevel()); + + AddEvent(&net_log); + EXPECT_EQ(3, observer.count()); +} + +// Test adding and removing two observers. +TEST(NetLogTest, NetLogTwoObservers) { + NetLog net_log; + CountingObserver observer[2]; + + // Add first observer. + net_log.AddThreadSafeObserver(&observer[0], NetLog::LOG_ALL_BUT_BYTES); + EXPECT_EQ(&net_log, observer[0].net_log()); + EXPECT_EQ(NULL, observer[1].net_log()); + EXPECT_EQ(NetLog::LOG_ALL_BUT_BYTES, observer[0].log_level()); + EXPECT_EQ(NetLog::LOG_ALL_BUT_BYTES, net_log.GetLogLevel()); + + // Add second observer observer. + net_log.AddThreadSafeObserver(&observer[1], NetLog::LOG_ALL); + EXPECT_EQ(&net_log, observer[0].net_log()); + EXPECT_EQ(&net_log, observer[1].net_log()); + EXPECT_EQ(NetLog::LOG_ALL_BUT_BYTES, observer[0].log_level()); + 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. + AddEvent(&net_log); + EXPECT_EQ(1, observer[0].count()); + EXPECT_EQ(1, observer[1].count()); + + // 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(NetLog::LOG_ALL_BUT_BYTES, observer[0].log_level()); + EXPECT_EQ(NetLog::LOG_ALL_BUT_BYTES, net_log.GetLogLevel()); + + // 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()); + + // Remove first observer. + net_log.RemoveThreadSafeObserver(&observer[0]); + EXPECT_EQ(NULL, observer[0].net_log()); + EXPECT_EQ(NULL, observer[1].net_log()); + EXPECT_EQ(NetLog::LOG_NONE, net_log.GetLogLevel()); + + // Add event and make sure neither observer gets it. + AddEvent(&net_log); + EXPECT_EQ(2, observer[0].count()); + EXPECT_EQ(1, observer[1].count()); +} + +// 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(&net_log); +} + } // namespace } // namespace net diff --git a/net/tools/gdig/file_net_log.cc b/net/tools/gdig/file_net_log.cc index 51cf78d..7c755c8 100644 --- a/net/tools/gdig/file_net_log.cc +++ b/net/tools/gdig/file_net_log.cc @@ -12,24 +12,20 @@ namespace net { -FileNetLog::FileNetLog(FILE* destination, LogLevel level) - : log_level_(level), - destination_(destination) { +FileNetLogObserver::FileNetLogObserver(FILE* destination) + : destination_(destination) { DCHECK(destination != NULL); - // Without calling GetNext() once here, the first GetNext will return 0 - // that is not a valid id. - sequence_number_.GetNext(); } -FileNetLog::~FileNetLog() { +FileNetLogObserver::~FileNetLogObserver() { } -void FileNetLog::OnAddEntry(const net::NetLog::Entry& entry) { +void FileNetLogObserver::OnAddEntry(const net::NetLog::Entry& entry) { // Only BoundNetLogs without a NetLog should have an invalid source. DCHECK(entry.source().IsValid()); - const char* source = SourceTypeToString(entry.source().type); - const char* type = EventTypeToString(entry.type()); + const char* source = NetLog::SourceTypeToString(entry.source().type); + const char* type = NetLog::EventTypeToString(entry.type()); scoped_ptr param_value(entry.ParametersToValue()); std::string params; @@ -49,28 +45,4 @@ void FileNetLog::OnAddEntry(const net::NetLog::Entry& entry) { entry.source().id, source, type, params.c_str()); } -uint32 FileNetLog::NextID() { - return sequence_number_.GetNext(); -} - -NetLog::LogLevel FileNetLog::GetLogLevel() const { - return log_level_; -} - -void FileNetLog::AddThreadSafeObserver( - NetLog::ThreadSafeObserver* observer, - NetLog::LogLevel log_level) { - NOTIMPLEMENTED() << "Not currently used by gdig."; -} - -void FileNetLog::SetObserverLogLevel(ThreadSafeObserver* observer, - LogLevel log_level) { - NOTIMPLEMENTED() << "Not currently used by gdig."; -} - -void FileNetLog::RemoveThreadSafeObserver( - NetLog::ThreadSafeObserver* observer) { - NOTIMPLEMENTED() << "Not currently used by gdig."; -} - } // namespace net diff --git a/net/tools/gdig/file_net_log.h b/net/tools/gdig/file_net_log.h index cb80380..1ad78f1 100644 --- a/net/tools/gdig/file_net_log.h +++ b/net/tools/gdig/file_net_log.h @@ -7,7 +7,6 @@ #include -#include "base/atomic_sequence_num.h" #include "base/basictypes.h" #include "base/synchronization/lock.h" #include "base/time.h" @@ -15,27 +14,18 @@ namespace net { -// FileNetLog is a simple implementation of NetLog that prints out all -// the events received into the stream passed to the constructor. -class FileNetLog : public NetLog { +// FileNetLogObserver is a simple implementation of NetLog::ThreadSafeObserver +// that prints out all the events received into the stream passed +// to the constructor. +class FileNetLogObserver : public NetLog::ThreadSafeObserver { public: - explicit FileNetLog(FILE* destination, LogLevel level); - virtual ~FileNetLog(); + explicit FileNetLogObserver(FILE* destination); + virtual ~FileNetLogObserver(); - private: - // NetLog implementation: + // NetLog::ThreadSafeObserver implementation: virtual void OnAddEntry(const net::NetLog::Entry& entry) OVERRIDE; - virtual uint32 NextID() OVERRIDE; - virtual LogLevel GetLogLevel() const OVERRIDE; - virtual void AddThreadSafeObserver(ThreadSafeObserver* observer, - LogLevel log_level) OVERRIDE; - virtual void SetObserverLogLevel(ThreadSafeObserver* observer, - LogLevel log_level) OVERRIDE; - virtual void RemoveThreadSafeObserver(ThreadSafeObserver* observer) OVERRIDE; - - base::AtomicSequenceNumber sequence_number_; - const NetLog::LogLevel log_level_; + private: FILE* const destination_; base::Lock lock_; diff --git a/net/tools/gdig/gdig.cc b/net/tools/gdig/gdig.cc index 1640408..7cbb9a4 100644 --- a/net/tools/gdig/gdig.cc +++ b/net/tools/gdig/gdig.cc @@ -182,6 +182,7 @@ bool LoadReplayLog(const base::FilePath& file_path, ReplayLog* replay_log) { class GDig { public: GDig(); + ~GDig(); enum Result { RESULT_NO_RESOLVE = -3, @@ -219,7 +220,8 @@ class GDig { base::CancelableClosure timeout_closure_; scoped_ptr dns_config_service_; - scoped_ptr log_; + scoped_ptr log_observer_; + scoped_ptr log_; scoped_ptr resolver_; }; @@ -232,6 +234,11 @@ GDig::GDig() active_resolves_(0) { } +GDig::~GDig() { + if (log_) + log_->RemoveThreadSafeObserver(log_observer_.get()); +} + GDig::Result GDig::Main(int argc, const char* argv[]) { if (!ParseCommandLine(argc, argv)) { fprintf(stderr, @@ -299,7 +306,9 @@ bool GDig::ParseCommandLine(int argc, const char* argv[]) { return false; } } - log_.reset(new FileNetLog(stderr, level)); + log_.reset(new NetLog); + log_observer_.reset(new FileNetLogObserver(stderr)); + log_->AddThreadSafeObserver(log_observer_.get(), level); } print_config_ = parsed_command_line.HasSwitch("print_config"); diff --git a/net/tools/get_server_time/get_server_time.cc b/net/tools/get_server_time/get_server_time.cc index df9e3fc..1fca24e 100644 --- a/net/tools/get_server_time/get_server_time.cc +++ b/net/tools/get_server_time/get_server_time.cc @@ -103,14 +103,18 @@ class QuitDelegate : public net::URLFetcherDelegate { DISALLOW_COPY_AND_ASSIGN(QuitDelegate); }; -// NetLog implementation that simply prints events to the logs. -class PrintingLog : public net::NetLog { +// NetLog::ThreadSafeObserver implementation that simply prints events +// to the logs. +class PrintingLogObserver : public net::NetLog::ThreadSafeObserver { public: - PrintingLog() : next_id_(1) {} + PrintingLogObserver() {} - virtual ~PrintingLog() {} + virtual ~PrintingLogObserver() { + // This is guaranteed to be safe as this program is single threaded. + net_log()->RemoveThreadSafeObserver(this); + } - // NetLog implementation: + // NetLog::ThreadSafeObserver implementation: virtual void OnAddEntry(const net::NetLog::Entry& entry) OVERRIDE { // The log level of the entry is unknown, so just assume it maps // to VLOG(1). @@ -134,43 +138,13 @@ class PrintingLog : public net::NetLog { << event_type << ": " << event_phase << params_str; } - virtual uint32 NextID() OVERRIDE { - return next_id_++; - } - - virtual LogLevel GetLogLevel() const OVERRIDE { - const int vlog_level = logging::GetVlogLevel(__FILE__); - if (vlog_level <= 0) { - return LOG_BASIC; - } - if (vlog_level == 1) { - return LOG_ALL_BUT_BYTES; - } - return LOG_ALL; - } - - virtual void AddThreadSafeObserver(ThreadSafeObserver* observer, - LogLevel log_level) OVERRIDE { - NOTIMPLEMENTED(); - } - - virtual void SetObserverLogLevel(ThreadSafeObserver* observer, - LogLevel log_level) OVERRIDE { - NOTIMPLEMENTED(); - } - - virtual void RemoveThreadSafeObserver(ThreadSafeObserver* observer) OVERRIDE { - NOTIMPLEMENTED(); - } - private: - uint32 next_id_; - - DISALLOW_COPY_AND_ASSIGN(PrintingLog); + DISALLOW_COPY_AND_ASSIGN(PrintingLogObserver); }; // Builds a URLRequestContext assuming there's only a single loop. -scoped_ptr BuildURLRequestContext() { +scoped_ptr +BuildURLRequestContext(net::NetLog* net_log) { net::URLRequestContextBuilder builder; #if defined(OS_LINUX) // On Linux, use a fixed ProxyConfigService, since the default one @@ -181,7 +155,7 @@ scoped_ptr BuildURLRequestContext() { new net::ProxyConfigServiceFixed(net::ProxyConfig())); #endif scoped_ptr context(builder.Build()); - context->set_net_log(new PrintingLog()); + context->set_net_log(net_log); return context.Pass(); } @@ -189,9 +163,10 @@ class SingleThreadRequestContextGetter : public net::URLRequestContextGetter { public: // Since there's only a single thread, there's no need to worry // about when |context_| gets created. - explicit SingleThreadRequestContextGetter( + SingleThreadRequestContextGetter( + net::NetLog* net_log, const scoped_refptr& main_task_runner) - : context_(BuildURLRequestContext()), + : context_(BuildURLRequestContext(net_log)), main_task_runner_(main_task_runner) {} virtual net::URLRequestContext* GetURLRequestContext() OVERRIDE { @@ -281,8 +256,16 @@ int main(int argc, char* argv[]) { // which causes the DNS resolution to abort. It's simpler to just // not instantiate one, since only a single request is sent anyway. + // The declaration order for net_log and printing_log_observer is + // important. The destructor of PrintingLogObserver removes itself + // from net_log, so net_log must be available for entire lifetime of + // printing_log_observer. + net::NetLog net_log; + PrintingLogObserver printing_log_observer; + net_log.AddThreadSafeObserver(&printing_log_observer, net::NetLog::LOG_ALL); scoped_refptr context_getter( - new SingleThreadRequestContextGetter(main_loop.message_loop_proxy())); + new SingleThreadRequestContextGetter(&net_log, + main_loop.message_loop_proxy())); QuitDelegate delegate; scoped_ptr fetcher( diff --git a/remoting/host/vlog_net_log.cc b/remoting/host/vlog_net_log.cc index 05e9a35..2e2e436 100644 --- a/remoting/host/vlog_net_log.cc +++ b/remoting/host/vlog_net_log.cc @@ -13,13 +13,25 @@ namespace remoting { -VlogNetLog::VlogNetLog() : id_(0) { +class VlogNetLog::Observer : public net::NetLog::ThreadSafeObserver { + public: + Observer(); + virtual ~Observer(); + + // NetLog::ThreadSafeObserver overrides: + virtual void OnAddEntry(const net::NetLog::Entry& entry) OVERRIDE; + + private: + DISALLOW_COPY_AND_ASSIGN(Observer); +}; + +VlogNetLog::Observer::Observer() { } -VlogNetLog::~VlogNetLog() { +VlogNetLog::Observer::~Observer() { } -void VlogNetLog::OnAddEntry(const NetLog::Entry& entry) { +void VlogNetLog::Observer::OnAddEntry(const net::NetLog::Entry& entry) { if (VLOG_IS_ON(4)) { scoped_ptr value(entry.ToValue()); std::string json; @@ -28,27 +40,13 @@ void VlogNetLog::OnAddEntry(const NetLog::Entry& entry) { } } -uint32 VlogNetLog::NextID() { - // TODO(mmenke): Make this threadsafe and start with 1 instead of 0. - return id_++; -} - -net::NetLog::LogLevel VlogNetLog::GetLogLevel() const { - return LOG_ALL_BUT_BYTES; +VlogNetLog::VlogNetLog() + : observer_(new Observer()) { + AddThreadSafeObserver(observer_.get(), LOG_ALL_BUT_BYTES); } -void VlogNetLog::AddThreadSafeObserver(ThreadSafeObserver* observer, - net::NetLog::LogLevel log_level) { - NOTIMPLEMENTED(); -} - -void VlogNetLog::SetObserverLogLevel(ThreadSafeObserver* observer, - net::NetLog::LogLevel log_level) { - NOTIMPLEMENTED(); -} - -void VlogNetLog::RemoveThreadSafeObserver(ThreadSafeObserver* observer) { - NOTIMPLEMENTED(); +VlogNetLog::~VlogNetLog() { + RemoveThreadSafeObserver(observer_.get()); } } // namespace remoting diff --git a/remoting/host/vlog_net_log.h b/remoting/host/vlog_net_log.h index d056a3a..393ab1c 100644 --- a/remoting/host/vlog_net_log.h +++ b/remoting/host/vlog_net_log.h @@ -6,6 +6,7 @@ #define REMOTING_HOST_VLOG_NET_LOG_H_ #include "base/memory/scoped_handle.h" +#include "base/memory/scoped_ptr.h" #include "net/base/net_log.h" namespace remoting { @@ -19,18 +20,9 @@ class VlogNetLog : public net::NetLog { VlogNetLog(); virtual ~VlogNetLog(); - // NetLog overrides: - virtual void OnAddEntry(const NetLog::Entry& entry) OVERRIDE; - virtual uint32 NextID() OVERRIDE; - virtual LogLevel GetLogLevel() const OVERRIDE; - virtual void AddThreadSafeObserver(ThreadSafeObserver* observer, - LogLevel log_level) OVERRIDE; - virtual void SetObserverLogLevel(ThreadSafeObserver* observer, - LogLevel log_level) OVERRIDE; - virtual void RemoveThreadSafeObserver(ThreadSafeObserver* observer) OVERRIDE; - private: - uint32 id_; + class Observer; + scoped_ptr observer_; DISALLOW_COPY_AND_ASSIGN(VlogNetLog); }; -- cgit v1.1