Avoid using BrowserThread and ContentBrowserClient in NetLogObserver.

To avoid using the BrowserThread we instantiate a ThreadChecker instance on the NetLogObserver::Attach method and use this to validate that calls to the NetLogObserver instance occur on the same thread.

To avoid using ContentBrowserClient, we pass the NetLog pointer in the NetLogObserver::Attach method.

BUG=598073

Review-Url: https://codereview.chromium.org/2115823004
Cr-Commit-Position: refs/heads/master@{#405284}

content/browser/devtools/devtools_manager.cc

@@ -34,7 +34,8 @@ void DevToolsManager::AgentHostStateChanged(
       BrowserThread::PostTask(
           BrowserThread::IO,
           FROM_HERE,
-          base::Bind(&NetLogObserver::Attach));
+          base::Bind(&NetLogObserver::Attach,
+                     GetContentClient()->browser()->GetNetLog()));
     }
     ++attached_hosts_count_;
   } else {

content/browser/loader/netlog_observer.cc

@@ -9,8 +9,6 @@
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "content/browser/loader/resource_request_info_impl.h"
-#include "content/public/browser/browser_thread.h"
-#include "content/public/browser/content_browser_client.h"
 #include "content/public/common/resource_response.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
@@ -22,8 +20,13 @@
 namespace content {
 const size_t kMaxNumEntries = 1000;
 
+// static
 NetLogObserver* NetLogObserver::instance_ = NULL;
 
+// static
+base::LazyInstance<std::unique_ptr<base::ThreadChecker>>::Leaky
+    NetLogObserver::io_thread_checker_;
+
 NetLogObserver::NetLogObserver() {}
 
 NetLogObserver::~NetLogObserver() {}
@@ -36,8 +39,10 @@ NetLogObserver::ResourceInfo* NetLogObserver::GetResourceInfo(uint32_t id) {
 }
 
 void NetLogObserver::OnAddEntry(const net::NetLog::Entry& entry) {
+  DCHECK(io_thread_checker_.Get().get());
+
   // The events that the Observer is interested in only occur on the IO thread.
-  if (!BrowserThread::CurrentlyOn(BrowserThread::IO))
+  if (!io_thread_checker_.Get()->CalledOnValidThread())
     return;
 
   if (entry.source().type == net::NetLog::SOURCE_URL_REQUEST)
@@ -45,8 +50,6 @@ void NetLogObserver::OnAddEntry(const net::NetLog::Entry& entry) {
 }
 
 void NetLogObserver::OnAddURLRequestEntry(const net::NetLog::Entry& entry) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
   bool is_begin = entry.phase() == net::NetLog::PHASE_BEGIN;
   bool is_end = entry.phase() == net::NetLog::PHASE_END;
 
@@ -153,9 +156,9 @@ void NetLogObserver::OnAddURLRequestEntry(const net::NetLog::Entry& entry) {
   }
 }
 
-void NetLogObserver::Attach() {
+void NetLogObserver::Attach(net::NetLog* net_log) {
   DCHECK(!instance_);
-  net::NetLog* net_log = GetContentClient()->browser()->GetNetLog();
+  io_thread_checker_.Get().reset(new base::ThreadChecker());
   if (net_log) {
     instance_ = new NetLogObserver();
     net_log->DeprecatedAddObserver(
@@ -164,8 +167,9 @@ void NetLogObserver::Attach() {
 }
 
 void NetLogObserver::Detach() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
+  DCHECK(io_thread_checker_.Get().get() &&
+         io_thread_checker_.Get()->CalledOnValidThread());
+  io_thread_checker_.Get().reset();
   if (instance_) {
     // Safest not to do this in the destructor to maintain thread safety across
     // refactorings.
@@ -176,7 +180,10 @@ void NetLogObserver::Detach() {
 }
 
 NetLogObserver* NetLogObserver::GetInstance() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  if (!io_thread_checker_.Get().get())
+    return nullptr;
+
+  DCHECK(io_thread_checker_.Get()->CalledOnValidThread());
 
   return instance_;
 }

content/browser/loader/netlog_observer.h

@@ -8,8 +8,10 @@
 #include <stdint.h>
 
 #include "base/containers/hash_tables.h"
+#include "base/lazy_instance.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
+#include "base/threading/thread_checker.h"
 #include "content/public/common/resource_devtools_info.h"
 #include "net/log/net_log.h"
 
@@ -34,9 +36,9 @@ class NetLogObserver : public net::NetLog::ThreadSafeObserver {
   // net::NetLog::ThreadSafeObserver implementation:
   void OnAddEntry(const net::NetLog::Entry& entry) override;
 
-  void OnAddURLRequestEntry(const net::NetLog::Entry& entry);
+  // The NetLog instance is passed in via the |net_log| parameter.
+  static void Attach(net::NetLog* net_log);
 
-  static void Attach();
   static void Detach();
 
   // Must be called on the IO thread. May return NULL if no observers
@@ -52,10 +54,17 @@ class NetLogObserver : public net::NetLog::ThreadSafeObserver {
 
   ResourceInfo* GetResourceInfo(uint32_t id);
 
+  void OnAddURLRequestEntry(const net::NetLog::Entry& entry);
+
   typedef base::hash_map<uint32_t, scoped_refptr<ResourceInfo>>
       RequestToInfoMap;
   RequestToInfoMap request_to_info_;
 
+  // Used to validate that calls to the NetLogObserver methods occur on the
+  // thread it was instantiated on. Typically the IO thread.
+  static base::LazyInstance<std::unique_ptr<base::ThreadChecker>>::Leaky
+      io_thread_checker_;
+
   DISALLOW_COPY_AND_ASSIGN(NetLogObserver);
 };