Avoid using BrowserThread and ContentBrowserClient in NetLogObserver.

content/browser/loader/netlog_observer.cc

 // 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 "content/browser/loader/netlog_observer.h"
 
 #include <stddef.h>
 
 #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"
 #include "net/http/http_util.h"
 #include "net/spdy/spdy_header_block.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_netlog_params.h"
 
 namespace content {
 const size_t kMaxNumEntries = 1000;
 
+// static
 NetLogObserver* NetLogObserver::instance_ = NULL;
 
+// static
+base::LazyInstance<scoped_refptr<base::SingleThreadTaskRunner>>::Leaky
+    NetLogObserver::io_thread_task_runner_;
+
 NetLogObserver::NetLogObserver() {}
 
 NetLogObserver::~NetLogObserver() {}
 
 NetLogObserver::ResourceInfo* NetLogObserver::GetResourceInfo(uint32_t id) {
   RequestToInfoMap::iterator it = request_to_info_.find(id);
   if (it != request_to_info_.end())
     return it->second.get();
   return NULL;
 }
 
 void NetLogObserver::OnAddEntry(const net::NetLog::Entry& entry) {
+  DCHECK(io_thread_task_runner_.Get().get());
+
   // The events that the Observer is interested in only occur on the IO thread.
-  if (!BrowserThread::CurrentlyOn(BrowserThread::IO))
+  if (!io_thread_task_runner_.Get()->BelongsToCurrentThread())
     return;
 
   if (entry.source().type == net::NetLog::SOURCE_URL_REQUEST)
     OnAddURLRequestEntry(entry);
 }
 
 void NetLogObserver::OnAddURLRequestEntry(const net::NetLog::Entry& entry) {

[pfeldman, 2016/07/13 17:17:49]

nit: seems like this method could be private, then we could drop the check.

[ananta, 2016/07/13 18:12:26]

Done.

-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(io_thread_task_runner_.Get().get() &&
+         io_thread_task_runner_.Get()->BelongsToCurrentThread());
 
   bool is_begin = entry.phase() == net::NetLog::PHASE_BEGIN;
   bool is_end = entry.phase() == net::NetLog::PHASE_END;
 
   if (entry.type() == net::NetLog::TYPE_URL_REQUEST_START_JOB) {
     if (is_begin) {
       if (request_to_info_.size() > kMaxNumEntries) {
         LOG(WARNING) << "The raw headers observer url request count has grown "
                         "larger than expected, resetting";
         request_to_info_.clear();
@@ skipping 87 matching lines @@
         // SPDY request.
         info->response_headers_text = "";
       }
       break;
     }
     default:
       break;
   }
 }
 
-void NetLogObserver::Attach() {
+void NetLogObserver::Attach(
+    net::NetLog* net_log,
+    scoped_refptr<base::SingleThreadTaskRunner> io_thread_runner) {
   DCHECK(!instance_);
-  net::NetLog* net_log = GetContentClient()->browser()->GetNetLog();
+  io_thread_task_runner_.Get() = io_thread_runner;
   if (net_log) {
     instance_ = new NetLogObserver();
     net_log->DeprecatedAddObserver(
         instance_, net::NetLogCaptureMode::IncludeCookiesAndCredentials());
   }
 }
 
 void NetLogObserver::Detach() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-
+  DCHECK(io_thread_task_runner_.Get().get() &&
+         io_thread_task_runner_.Get()->BelongsToCurrentThread());

[pfeldman, 2016/07/13 17:17:49]

reset io_thread_task_runner_ (or checker) for symmetry

[ananta, 2016/07/13 18:12:26]

Done.

   if (instance_) {
     // Safest not to do this in the destructor to maintain thread safety across
     // refactorings.
     instance_->net_log()->DeprecatedRemoveObserver(instance_);
     delete instance_;
     instance_ = NULL;
   }
 }
 
 NetLogObserver* NetLogObserver::GetInstance() {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  DCHECK(io_thread_task_runner_.Get().get() &&

[pfeldman, 2016/07/13 17:17:49]

This logic has changed - we used to return nullptr in case called before attach
or after detach.

[ananta, 2016/07/13 18:12:26]

Please clarify. The only change in this patch is to change the DCHECK?
The instance_ will be null if we call this before Attach or after Detach

+         io_thread_task_runner_.Get()->BelongsToCurrentThread());
 
   return instance_;
 }
 
 // static
 void NetLogObserver::PopulateResponseInfo(net::URLRequest* request,
                                           ResourceResponse* response) {
   const ResourceRequestInfoImpl* request_info =
       ResourceRequestInfoImpl::ForRequest(request);
   if (!request_info || !request_info->ShouldReportRawHeaders())
     return;
 
   uint32_t source_id = request->net_log().source().id;
   NetLogObserver* dev_tools_net_log_observer = NetLogObserver::GetInstance();
   if (dev_tools_net_log_observer == NULL)
     return;
   response->head.devtools_info =
       dev_tools_net_log_observer->GetResourceInfo(source_id);
   response->head.encoded_data_length = request->GetTotalReceivedBytes();
 }
 
 }  // namespace content