Remove ChromeNetLog dependency from content/browser/debugger.

Add Observer concept to net::NetLog.

Use net::NetLog::Observer in ChromeNetLog.

Remove ChromeNetLog dependency in content/browser/debugger.

Forked from http://codereview.chromium.org/7310029/ to finish Jói's
patch.

BUG=84078
TEST=existing

Review URL: http://codereview.chromium.org/7468019

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

chrome/browser/chrome_content_browser_client.cc

@@ -21,6 +21,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_web_ui.h"
 #include "chrome/browser/google/google_util.h"
+#include "chrome/browser/net/chrome_net_log.h"
 #include "chrome/browser/notifications/desktop_notification_service.h"
 #include "chrome/browser/notifications/desktop_notification_service_factory.h"
 #include "chrome/browser/platform_util.h"
@@ -689,6 +690,10 @@ DevToolsManager* ChromeContentBrowserClient::GetDevToolsManager() {
   return g_browser_process->devtools_manager();
 }
 
+net::NetLog* ChromeContentBrowserClient::GetNetLog() {
+  return g_browser_process->net_log();
+}
+
 bool ChromeContentBrowserClient::IsFastShutdownPossible() {
   const CommandLine& browser_command_line = *CommandLine::ForCurrentProcess();
   return !browser_command_line.HasSwitch(switches::kChromeFrame);

chrome/browser/chrome_content_browser_client.h

@@ -94,6 +94,7 @@ class ChromeContentBrowserClient : public content::ContentBrowserClient {
   virtual ui::Clipboard* GetClipboard() OVERRIDE;
   virtual MHTMLGenerationManager* GetMHTMLGenerationManager() OVERRIDE;
   virtual DevToolsManager* GetDevToolsManager() OVERRIDE;
+  virtual net::NetLog* GetNetLog() OVERRIDE;
   virtual bool IsFastShutdownPossible() OVERRIDE;
   virtual WebPreferences GetWebkitPrefs(Profile* profile,
                                         bool is_web_ui) OVERRIDE;

chrome/browser/net/chrome_net_log.cc

@@ -16,32 +16,72 @@
 #include "chrome/browser/net/passive_log_collector.h"
 #include "chrome/common/chrome_switches.h"
 
-ChromeNetLog::ThreadSafeObserver::ThreadSafeObserver(LogLevel log_level)
+ChromeNetLog::ThreadSafeObserverImpl::ThreadSafeObserverImpl(LogLevel log_level)
     : net_log_(NULL),
-      log_level_(log_level) {
+      ALLOW_THIS_IN_INITIALIZER_LIST(internal_observer_(this, log_level)) {
 }
 
-ChromeNetLog::ThreadSafeObserver::~ThreadSafeObserver() {
+ChromeNetLog::ThreadSafeObserverImpl::~ThreadSafeObserverImpl() {
   DCHECK(!net_log_);
 }
 
-net::NetLog::LogLevel ChromeNetLog::ThreadSafeObserver::log_level() const {
-  return log_level_;
+void ChromeNetLog::ThreadSafeObserverImpl::AddAsObserver(
+    ChromeNetLog* net_log) {
+  DCHECK(!net_log_);
+  net_log_ = net_log;
+  net_log_->AddThreadSafeObserver(&internal_observer_);
 }
 
-void ChromeNetLog::ThreadSafeObserver::AssertNetLogLockAcquired() const {
-  if (net_log_)
-    net_log_->lock_.AssertAcquired();
+void ChromeNetLog::ThreadSafeObserverImpl::RemoveAsObserver() {
+  DCHECK(net_log_);
+  net_log_->RemoveThreadSafeObserver(&internal_observer_);
+  net_log_ = NULL;
 }
 
-void ChromeNetLog::ThreadSafeObserver::SetLogLevel(
+void ChromeNetLog::ThreadSafeObserverImpl::SetLogLevel(
     net::NetLog::LogLevel log_level) {
   DCHECK(net_log_);
   base::AutoLock lock(net_log_->lock_);
-  log_level_ = log_level;
+  internal_observer_.SetLogLevel(log_level);
   net_log_->UpdateLogLevel();
 }
 
+void ChromeNetLog::ThreadSafeObserverImpl::
+AddAsObserverAndGetAllPassivelyCapturedEvents(
+    ChromeNetLog* net_log,
+    EntryList* entries) {
+  DCHECK(!net_log_);
+  net_log_ = net_log;
+  net_log_->AddObserverAndGetAllPassivelyCapturedEvents(&internal_observer_,
+                                                        entries);
+}
+
+void ChromeNetLog::ThreadSafeObserverImpl::AssertNetLogLockAcquired() const {
+  if (net_log_)
+    net_log_->lock_.AssertAcquired();
+}
+
+ChromeNetLog::ThreadSafeObserverImpl::PassThroughObserver::PassThroughObserver(
+    ChromeNetLog::ThreadSafeObserverImpl* owner,
+    net::NetLog::LogLevel log_level)
+    : net::NetLog::ThreadSafeObserver(log_level),
+      ALLOW_THIS_IN_INITIALIZER_LIST(owner_(owner)) {
+}
+
+void ChromeNetLog::ThreadSafeObserverImpl::PassThroughObserver::OnAddEntry(
+    net::NetLog::EventType type,
+    const base::TimeTicks& time,
+    const net::NetLog::Source& source,
+    net::NetLog::EventPhase phase,
+    net::NetLog::EventParameters* params) {
+  owner_->OnAddEntry(type, time, source, phase, params);
+}
+
+void ChromeNetLog::ThreadSafeObserverImpl::PassThroughObserver::SetLogLevel(
+    net::NetLog::LogLevel log_level) {
+  log_level_ = log_level;
+}
+
 ChromeNetLog::Entry::Entry(uint32 order,
                            net::NetLog::EventType type,
                            const base::TimeTicks& time,
@@ -79,21 +119,21 @@ ChromeNetLog::ChromeNetLog()
     }
   }
 
-  AddObserver(passive_collector_.get());
-  AddObserver(load_timing_observer_.get());
+  passive_collector_->AddAsObserver(this);
+  load_timing_observer_->AddAsObserver(this);
 
   if (command_line->HasSwitch(switches::kLogNetLog)) {
     net_log_logger_.reset(new NetLogLogger(
-            command_line->GetSwitchValuePath(switches::kLogNetLog)));
-    AddObserver(net_log_logger_.get());
+        command_line->GetSwitchValuePath(switches::kLogNetLog)));
+    net_log_logger_->AddAsObserver(this);
   }
 }
 
 ChromeNetLog::~ChromeNetLog() {
-  RemoveObserver(passive_collector_.get());
-  RemoveObserver(load_timing_observer_.get());
+  passive_collector_->RemoveAsObserver();
+  load_timing_observer_->RemoveAsObserver();
   if (net_log_logger_.get()) {
-    RemoveObserver(net_log_logger_.get());
+    net_log_logger_->RemoveAsObserver();
   }
 }
 
@@ -119,21 +159,21 @@ net::NetLog::LogLevel ChromeNetLog::GetLogLevel() const {
   return static_cast<net::NetLog::LogLevel>(log_level);
 }
 
-void ChromeNetLog::AddObserver(ThreadSafeObserver* observer) {
+void ChromeNetLog::AddThreadSafeObserver(
+    net::NetLog::ThreadSafeObserver* observer) {
   base::AutoLock lock(lock_);
   AddObserverWhileLockHeld(observer);
 }
 
-void ChromeNetLog::RemoveObserver(ThreadSafeObserver* observer) {
+void ChromeNetLog::RemoveThreadSafeObserver(
+    net::NetLog::ThreadSafeObserver* observer) {
   base::AutoLock lock(lock_);
-  DCHECK_EQ(observer->net_log_, this);
-  observer->net_log_ = NULL;
   observers_.RemoveObserver(observer);
   UpdateLogLevel();
 }
 
 void ChromeNetLog::AddObserverAndGetAllPassivelyCapturedEvents(
-    ThreadSafeObserver* observer, EntryList* passive_entries) {
+    net::NetLog::ThreadSafeObserver* observer, EntryList* passive_entries) {
   base::AutoLock lock(lock_);
   AddObserverWhileLockHeld(observer);
   passive_collector_->GetAllCapturedEvents(passive_entries);
@@ -165,9 +205,8 @@ void ChromeNetLog::UpdateLogLevel() {
                                 new_effective_log_level);
 }
 
-void ChromeNetLog::AddObserverWhileLockHeld(ThreadSafeObserver* observer) {
-  DCHECK(!observer->net_log_);
-  observer->net_log_ = this;
+void ChromeNetLog::AddObserverWhileLockHeld(
+    net::NetLog::ThreadSafeObserver* observer) {
   observers_.AddObserver(observer);
   UpdateLogLevel();
 }

chrome/browser/net/chrome_net_log.h

@@ -23,7 +23,7 @@ class PassiveLogCollector;
 // messages to a list of observers.
 //
 // All methods are thread safe, with the exception that no ChromeNetLog or
-// ChromeNetLog::ThreadSafeObserver functions may be called by an observer's
+// ChromeNetLog::ThreadSafeObserverImpl functions may be called by an observer's
 // OnAddEntry() method.  Doing so will result in a deadlock.
 //
 // By default, ChromeNetLog will attach the observer PassiveLogCollector which
@@ -54,53 +54,73 @@ class ChromeNetLog : public net::NetLog {
 
   typedef std::vector<Entry> EntryList;
 
-  // Interface for observing the events logged by the network stack.
-  class ThreadSafeObserver {
+  // Base class for observing the events logged by the network
+  // stack. This has some nice-to-have functionality for use by code
+  // within chrome/, but any net::NetLog::ThreadSafeObserver may be
+  // registered to observe the NetLog.
+  //
+  // This class uses composition rather than inheritance so that
+  // certain invariants can be maintained when subclasses of it are
+  // added as observers (through the AddAsObserver and
+  // RemoveAsObserver methods on this class).
+  class ThreadSafeObserverImpl {
    public:
-    // Constructs an observer that wants to see network events, with
-    // the specified minimum event granularity.  A ThreadSafeObserver can only
-    // observe a single ChromeNetLog at a time.
-    //
-    // Typical observers should specify LOG_BASIC.
-    //
     // Observers that need to see the full granularity of events can
     // specify LOG_ALL. However doing so will have performance consequences,
     // and may cause PassiveLogCollector to use more memory than anticipated.
-    //
-    // Observers will be called on the same thread an entry is added on,
-    // and are responsible for ensuring their own thread safety.
-    explicit ThreadSafeObserver(LogLevel log_level);
-
-    virtual ~ThreadSafeObserver();
-
-    // This method will be called on the thread that the event occurs on.  It
-    // is the responsibility of the observer to handle it in a thread safe
-    // manner.
-    //
-    // It is illegal for an Observer to call any ChromeNetLog or
-    // ChromeNetLog::ThreadSafeObserver functions in response to a call to
-    // OnAddEntry.
+    explicit ThreadSafeObserverImpl(LogLevel log_level);
+    virtual ~ThreadSafeObserverImpl();
+
     virtual void OnAddEntry(EventType type,
                             const base::TimeTicks& time,
                             const Source& source,
                             EventPhase phase,
                             EventParameters* params) = 0;
-    LogLevel log_level() const;
 
-   protected:
-    void AssertNetLogLockAcquired() const;
+    // These must be used instead of
+    // ChromeNetLog::Add/RemoveThreadSafeObserver to manage the
+    // association in this class with a given ChromeNetLog instance.
+    void AddAsObserver(ChromeNetLog* net_log);
+    void RemoveAsObserver();
 
-    // Can only be called when actively observing a ChromeNetLog.
     void SetLogLevel(LogLevel log_level);
 
-    // ChromeNetLog currently being observed, if any.  Set by ChromeNetLog's
-    // AddObserver and RemoveObserver methods.
-    ChromeNetLog* net_log_;
+    void AddAsObserverAndGetAllPassivelyCapturedEvents(
+        ChromeNetLog *net_log,
+        EntryList* passive_entries);
+
+   protected:
+    void AssertNetLogLockAcquired() const;
 
    private:
-    friend class ChromeNetLog;
-    LogLevel log_level_;
-    DISALLOW_COPY_AND_ASSIGN(ThreadSafeObserver);
+    class PassThroughObserver : public ThreadSafeObserver {
+     public:
+      PassThroughObserver(ThreadSafeObserverImpl* owner, LogLevel log_level);
+      virtual ~PassThroughObserver() {}
+      virtual void OnAddEntry(EventType type,
+                              const base::TimeTicks& time,
+                              const Source& source,
+                              EventPhase phase,
+                              EventParameters* params) OVERRIDE;
+
+      // Can only be called when actively observing a ChromeNetLog.
+      void SetLogLevel(LogLevel log_level);
+
+     private:
+      ThreadSafeObserverImpl* owner_;
+    };
+
+    friend class PassThroughObserver;
+
+    // ChromeNetLog currently being observed. Set by
+    // AddAsObserver/RemoveAsObserver.
+    ChromeNetLog* net_log_;
+
+    // The observer we register in AddAsObserver, that passes stuff
+    // through to us.
+    PassThroughObserver internal_observer_;
+
+    DISALLOW_COPY_AND_ASSIGN(ThreadSafeObserverImpl);
   };
 
   ChromeNetLog();
@@ -111,19 +131,9 @@ class ChromeNetLog : public net::NetLog {
                         const base::TimeTicks& time,
                         const Source& source,
                         EventPhase phase,
-                        EventParameters* params);
-  virtual uint32 NextID();
-  virtual LogLevel GetLogLevel() const;
-
-  void AddObserver(ThreadSafeObserver* observer);
-  void RemoveObserver(ThreadSafeObserver* observer);
-
-  // Adds |observer| and writes all passively captured events to
-  // |passive_entries|. Guarantees that no events in |passive_entries| will be
-  // sent to |observer| and all future events that have yet been sent to the
-  // PassiveLogCollector will be sent to |observer|.
-  void AddObserverAndGetAllPassivelyCapturedEvents(ThreadSafeObserver* observer,
-                                                   EntryList* passive_entries);
+                        EventParameters* params) OVERRIDE;
+  virtual uint32 NextID() OVERRIDE;
+  virtual LogLevel GetLogLevel() const OVERRIDE;
 
   void GetAllPassivelyCapturedEvents(EntryList* passive_entries);
 
@@ -136,6 +146,18 @@ class ChromeNetLog : public net::NetLog {
  private:
   void AddObserverWhileLockHeld(ThreadSafeObserver* observer);
 
+  // NetLog implementation
+  virtual void AddThreadSafeObserver(ThreadSafeObserver* observer) OVERRIDE;
+  virtual void RemoveThreadSafeObserver(ThreadSafeObserver* observer) OVERRIDE;
+
+  // Adds |observer| and writes all passively captured events to
+  // |passive_entries|. Guarantees that no events in |passive_entries| will be
+  // sent to |observer| and all future events that have yet been sent to the
+  // PassiveLogCollector will be sent to |observer|.
+  void AddObserverAndGetAllPassivelyCapturedEvents(ThreadSafeObserver* observer,
+                                                   EntryList* passive_entries);
+
+
   // Called whenever an observer is added or removed, or changes its log level.
   // Must have acquired |lock_| prior to calling.
   void UpdateLogLevel();

chrome/browser/net/load_timing_observer.cc

@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -62,7 +62,7 @@ LoadTimingObserver::HTTPStreamJobRecord::HTTPStreamJobRecord()
 }
 
 LoadTimingObserver::LoadTimingObserver()
-    : ThreadSafeObserver(net::NetLog::LOG_BASIC),
+    : ThreadSafeObserverImpl(net::NetLog::LOG_BASIC),
       last_connect_job_id_(net::NetLog::Source::kInvalidId) {
 }
 

chrome/browser/net/load_timing_observer.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -24,7 +24,7 @@ struct ResourceResponse;
 //
 // LoadTimingObserver lives completely on the IOThread and ignores events from
 // other threads.  It is not safe to use from other threads.
-class LoadTimingObserver : public ChromeNetLog::ThreadSafeObserver {
+class LoadTimingObserver : public ChromeNetLog::ThreadSafeObserverImpl {
  public:
   struct URLRequestRecord {
     URLRequestRecord();

chrome/browser/net/net_log_logger.cc

@@ -14,7 +14,7 @@
 #include "chrome/browser/ui/webui/net_internals_ui.h"
 
 NetLogLogger::NetLogLogger(const FilePath &log_path)
-    : ThreadSafeObserver(net::NetLog::LOG_ALL_BUT_BYTES) {
+    : ThreadSafeObserverImpl(net::NetLog::LOG_ALL_BUT_BYTES) {
   if (!log_path.empty()) {
     base::ThreadRestrictions::ScopedAllowIO allow_io;
     file_.Set(file_util::OpenFile(log_path, "w"));

chrome/browser/net/net_log_logger.h

@@ -21,7 +21,7 @@ class FilePath;
 //
 // Relies on ChromeNetLog only calling an Observer once at a time for
 // thread-safety.
-class NetLogLogger : public ChromeNetLog::ThreadSafeObserver {
+class NetLogLogger : public ChromeNetLog::ThreadSafeObserverImpl {
  public:
   // If |log_path| is empty or file creation fails, writes to VLOG(1).
   // Otherwise, writes to |log_path|.  Uses one line per entry, for

chrome/browser/net/passive_log_collector.cc

@@ -50,7 +50,7 @@ PassiveLogCollector::SourceInfo::~SourceInfo() {}
 //----------------------------------------------------------------------------
 
 PassiveLogCollector::PassiveLogCollector()
-    : ThreadSafeObserver(net::NetLog::LOG_BASIC),
+    : ThreadSafeObserverImpl(net::NetLog::LOG_BASIC),
       ALLOW_THIS_IN_INITIALIZER_LIST(connect_job_tracker_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(url_request_tracker_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(socket_stream_tracker_(this)),