Update NetLog to be thread safe.

chrome/browser/dom_ui/net_internals_ui.cc

 // Copyright (c) 2010 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/dom_ui/net_internals_ui.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 133 matching lines @@
 
  private:
   class IOThreadImpl;
 
   // This is the "real" message handler, which lives on the IO thread.
   scoped_refptr<IOThreadImpl> proxy_;
 
   DISALLOW_COPY_AND_ASSIGN(NetInternalsMessageHandler);
 };
 
-// This class is the "real" message handler. With the exception of being
-// allocated and destroyed on the UI thread, its methods are expected to be
-// called from the IO thread.
+// This class is the "real" message handler. It is allocated and destroyed on
+// the UI thread.  With the exception of OnAddEntry, OnDOMUIDeleted, and
+// CallJavascriptFunction, its methods are all expected to be called from the IO
+// thread.  OnAddEntry and CallJavascriptFunction can be called from any thread,
+// and OnDOMUIDeleted can only be called from the UI thread.
 class NetInternalsMessageHandler::IOThreadImpl
     : public base::RefCountedThreadSafe<
           NetInternalsMessageHandler::IOThreadImpl,
           BrowserThread::DeleteOnUIThread>,
-      public ChromeNetLog::Observer,
+      public ChromeNetLog::ThreadSafeObserver,
       public ConnectionTester::Delegate {
  public:
   // Type for methods that can be used as MessageHandler callbacks.
   typedef void (IOThreadImpl::*MessageHandler)(const ListValue*);
 
   // Creates a proxy for |handler| that will live on the IO thread.
   // |handler| is a weak pointer, since it is possible for the DOMMessageHandler
   // to be deleted on the UI thread while we were executing on the IO thread.
   // |io_thread| is the global IOThread (it is passed in as an argument since
   // we need to grab it from the UI thread).
   IOThreadImpl(
       const base::WeakPtr<NetInternalsMessageHandler>& handler,
       IOThread* io_thread,
       URLRequestContextGetter* context_getter);
 
   ~IOThreadImpl();
 
   // Creates a callback that will run |method| on the IO thread.
   //
   // This can be used with DOMUI::RegisterMessageCallback() to bind to a method
   // on the IO thread.
   DOMUI::MessageCallback* CreateCallback(MessageHandler method);
 
   // Called once the DOMUI has been deleted (i.e. renderer went away), on the
   // IO thread.
   void Detach();
 
+  // Sends all passive log entries in |entries| to the Javascript handler,

[eroman, 2010/11/18 18:04:03]

|passive_entries|

+  // called on the IO thread.
+  void SendPassiveLogEntries(const ChromeNetLog::EntryList& passive_entries);
+
+  // Called when the DOMUI is deleted.  Prevents calling Javascript functions
+  // afterwards.  Called on UI thread.
+  void OnDOMUIDeleted();
+
   //--------------------------------
   // Javascript message handlers:
   //--------------------------------
 
-  // This message is called after the webpage's onloaded handler has fired.
-  // it indicates that the renderer is ready to start receiving captured data.
   void OnRendererReady(const ListValue* list);
 
   void OnGetProxySettings(const ListValue* list);
   void OnReloadProxySettings(const ListValue* list);
   void OnGetBadProxies(const ListValue* list);
   void OnClearBadProxies(const ListValue* list);
   void OnGetHostResolverInfo(const ListValue* list);
   void OnClearHostResolverCache(const ListValue* list);
   void OnEnableIPv6(const ListValue* list);
-  void OnGetPassiveLogEntries(const ListValue* list);
   void OnStartConnectionTests(const ListValue* list);
   void OnGetHttpCacheInfo(const ListValue* list);
   void OnGetSocketPoolInfo(const ListValue* list);
   void OnGetSpdySessionInfo(const ListValue* list);
 #ifdef OS_WIN
   void OnGetServiceProviders(const ListValue* list);
 #endif
 
   void OnSetLogLevel(const ListValue* list);
 
-  // ChromeNetLog::Observer implementation:
+  // ChromeNetLog::ThreadSafeObserver implementation:
   virtual void OnAddEntry(net::NetLog::EventType type,
                           const base::TimeTicks& time,
                           const net::NetLog::Source& source,
                           net::NetLog::EventPhase phase,
                           net::NetLog::EventParameters* params);
 
   // ConnectionTester::Delegate implementation:
   virtual void OnStartConnectionTestSuite();
   virtual void OnStartConnectionTestExperiment(
       const ConnectionTester::Experiment& experiment);
   virtual void OnCompletedConnectionTestExperiment(
       const ConnectionTester::Experiment& experiment,
       int result);
   virtual void OnCompletedConnectionTestSuite();
 
  private:
   class CallbackHelper;
 
   // Helper that runs |method| with |arg|, and deletes |arg| on completion.
   void DispatchToMessageHandler(ListValue* arg, MessageHandler method);
 
   // Helper that executes |function_name| in the attached renderer.
-  // The function takes ownership of |arg|.
+  // The function takes ownership of |arg|.  Note that this can be called from
+  // any thread.
   void CallJavascriptFunction(const std::wstring& function_name,
                               Value* arg);
 
   // Pointer to the UI-thread message handler. Only access this from
   // the UI thread.
   base::WeakPtr<NetInternalsMessageHandler> handler_;
 
+  // True if the DOM UI has been deleted.  This is used to prevent calling
+  // Javascript functions after the DOM UI is destroyed.  On refresh, the
+  // messages can end up being sent to the refreshed page, causing duplicate
+  // or partial entries.
+  //
+  // This is only read and written to on the UI thread.
+  bool was_domui_deleted_;
+
   // The global IOThread, which contains the global NetLog to observer.
   IOThread* io_thread_;
 
   scoped_refptr<URLRequestContextGetter> context_getter_;
 
   // Helper that runs the suite of connection tests.
   scoped_ptr<ConnectionTester> connection_tester_;
 
   // True if we have attached an observer to the NetLog already.
   bool is_observing_log_;
+
   friend class base::RefCountedThreadSafe<IOThreadImpl>;
 };
 
 // Helper class for a DOMUI::MessageCallback which when excuted calls
 // instance->*method(value) on the IO thread.
 class NetInternalsMessageHandler::IOThreadImpl::CallbackHelper
     : public DOMUI::MessageCallback {
  public:
   CallbackHelper(IOThreadImpl* instance, IOThreadImpl::MessageHandler method)
       : instance_(instance),
@@ skipping 71 matching lines @@
 ////////////////////////////////////////////////////////////////////////////////
 //
 // NetInternalsMessageHandler
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 NetInternalsMessageHandler::NetInternalsMessageHandler() {}
 
 NetInternalsMessageHandler::~NetInternalsMessageHandler() {
   if (proxy_) {
+    proxy_.get()->OnDOMUIDeleted();
     // Notify the handler on the IO thread that the renderer is gone.
     BrowserThread::PostTask(BrowserThread::IO, FROM_HERE,
         NewRunnableMethod(proxy_.get(), &IOThreadImpl::Detach));
   }
 }
 
 DOMMessageHandler* NetInternalsMessageHandler::Attach(DOMUI* dom_ui) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   proxy_ = new IOThreadImpl(this->AsWeakPtr(), g_browser_process->io_thread(),
                             dom_ui->GetProfile()->GetRequestContext());
@@ skipping 22 matching lines @@
   dom_ui_->RegisterMessageCallback(
       "getHostResolverInfo",
       proxy_->CreateCallback(&IOThreadImpl::OnGetHostResolverInfo));
   dom_ui_->RegisterMessageCallback(
       "clearHostResolverCache",
       proxy_->CreateCallback(&IOThreadImpl::OnClearHostResolverCache));
   dom_ui_->RegisterMessageCallback(
       "enableIPv6",
       proxy_->CreateCallback(&IOThreadImpl::OnEnableIPv6));
   dom_ui_->RegisterMessageCallback(
-      "getPassiveLogEntries",
-      proxy_->CreateCallback(&IOThreadImpl::OnGetPassiveLogEntries));
-  dom_ui_->RegisterMessageCallback(
       "startConnectionTests",
       proxy_->CreateCallback(&IOThreadImpl::OnStartConnectionTests));
   dom_ui_->RegisterMessageCallback(
       "getHttpCacheInfo",
       proxy_->CreateCallback(&IOThreadImpl::OnGetHttpCacheInfo));
   dom_ui_->RegisterMessageCallback(
       "getSocketPoolInfo",
       proxy_->CreateCallback(&IOThreadImpl::OnGetSocketPoolInfo));
   dom_ui_->RegisterMessageCallback(
       "getSpdySessionInfo",
@@ skipping 23 matching lines @@
 ////////////////////////////////////////////////////////////////////////////////
 //
 // NetInternalsMessageHandler::IOThreadImpl
 //
 ////////////////////////////////////////////////////////////////////////////////
 
 NetInternalsMessageHandler::IOThreadImpl::IOThreadImpl(
     const base::WeakPtr<NetInternalsMessageHandler>& handler,
     IOThread* io_thread,
     URLRequestContextGetter* context_getter)
-    : Observer(net::NetLog::LOG_ALL_BUT_BYTES),
+    : ThreadSafeObserver(net::NetLog::LOG_ALL_BUT_BYTES),
       handler_(handler),
       io_thread_(io_thread),
       context_getter_(context_getter),
+      was_domui_deleted_(false),
       is_observing_log_(false) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
 NetInternalsMessageHandler::IOThreadImpl::~IOThreadImpl() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 }
 
 DOMUI::MessageCallback*
 NetInternalsMessageHandler::IOThreadImpl::CreateCallback(
     MessageHandler method) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   return new CallbackHelper(this, method);
 }
 
 void NetInternalsMessageHandler::IOThreadImpl::Detach() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   // Unregister with network stack to observe events.
   if (is_observing_log_)
-    io_thread_->globals()->net_log->RemoveObserver(this);
+    io_thread_->net_log()->RemoveObserver(this);
 
   // Cancel any in-progress connection tests.
   connection_tester_.reset();
 }
 
+void NetInternalsMessageHandler::IOThreadImpl::SendPassiveLogEntries(
+    const ChromeNetLog::EntryList& passive_entries) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  ListValue* dict_list = new ListValue();
+  for (size_t i = 0; i < passive_entries.size(); ++i) {
+    const ChromeNetLog::Entry& e = passive_entries[i];
+    dict_list->Append(net::NetLog::EntryToDictionaryValue(e.type,
+                                                          e.time,
+                                                          e.source,
+                                                          e.phase,
+                                                          e.params,
+                                                          false));
+  }
+
+  CallJavascriptFunction(L"g_browser.receivedPassiveLogEntries", dict_list);
+}
+
+void NetInternalsMessageHandler::IOThreadImpl::OnDOMUIDeleted() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  was_domui_deleted_ = true;
+}
+
 void NetInternalsMessageHandler::IOThreadImpl::OnRendererReady(
     const ListValue* list) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   DCHECK(!is_observing_log_) << "notifyReady called twice";
 
-  // Register with network stack to observe events.
-  is_observing_log_ = true;
-  io_thread_->globals()->net_log->AddObserver(this);
-
   // Tell the javascript about the relationship between event type enums and
   // their symbolic name.
   {
     std::vector<net::NetLog::EventType> event_types =
         net::NetLog::GetAllEventTypes();
 
     DictionaryValue* dict = new DictionaryValue();
 
     for (size_t i = 0; i < event_types.size(); ++i) {
       const char* name = net::NetLog::EventTypeToString(event_types[i]);
@@ skipping 127 matching lines @@
     //               be part of the time library.
     const int64 kUnixEpochMs = 11644473600000LL;
     int64 tick_to_unix_time_ms = tick_to_time_ms - kUnixEpochMs;
 
     // Pass it as a string, since it may be too large to fit in an integer.
     CallJavascriptFunction(L"g_browser.receivedTimeTickOffset",
                            Value::CreateStringValue(
                                base::Int64ToString(tick_to_unix_time_ms)));
   }
 
-  OnGetPassiveLogEntries(NULL);
+  // Register with network stack to observe events.
+  is_observing_log_ = true;
+  ChromeNetLog::EntryList entries;
+  io_thread_->net_log()->AddObserverAndGetAllPassivelyCapturedEvents(this,
+                                                                     &entries);
+  SendPassiveLogEntries(entries);
 }
 
 void NetInternalsMessageHandler::IOThreadImpl::OnGetProxySettings(
     const ListValue* list) {
   URLRequestContext* context = context_getter_->GetURLRequestContext();
   net::ProxyService* proxy_service = context->proxy_service();
 
   DictionaryValue* dict = new DictionaryValue();
   if (proxy_service->fetched_config().is_valid())
     dict->Set("original", proxy_service->fetched_config().ToValue());
@@ skipping 136 matching lines @@
 
   if (host_resolver_impl) {
     host_resolver_impl->SetDefaultAddressFamily(
         net::ADDRESS_FAMILY_UNSPECIFIED);
   }
 
   // Cause the renderer to be notified of the new value.
   OnGetHostResolverInfo(NULL);
 }
 
-void NetInternalsMessageHandler::IOThreadImpl::OnGetPassiveLogEntries(
-    const ListValue* list) {
-  ChromeNetLog* net_log = io_thread_->globals()->net_log.get();
-
-  PassiveLogCollector::EntryList passive_entries;
-  net_log->passive_collector()->GetAllCapturedEvents(&passive_entries);
-
-  ListValue* dict_list = new ListValue();
-  for (size_t i = 0; i < passive_entries.size(); ++i) {
-    const PassiveLogCollector::Entry& e = passive_entries[i];
-    dict_list->Append(net::NetLog::EntryToDictionaryValue(e.type,
-                                                          e.time,
-                                                          e.source,
-                                                          e.phase,
-                                                          e.params,
-                                                          false));
-  }
-
-  CallJavascriptFunction(L"g_browser.receivedPassiveLogEntries", dict_list);
-}
-
 void NetInternalsMessageHandler::IOThreadImpl::OnStartConnectionTests(
     const ListValue* list) {
   // |value| should be: [<URL to test>].
   string16 url_str;
   CHECK(list->GetString(0, &url_str));
 
   // Try to fix-up the user provided URL into something valid.
   // For example, turn "www.google.com" into "http://www.google.com".
   GURL url(URLFixerUpper::FixupURL(UTF16ToUTF8(url_str), std::string()));
 
@@ skipping 96 matching lines @@
   int log_level;
   std::string log_level_string;
   if (!list->GetString(0, &log_level_string) ||
       !base::StringToInt(log_level_string, &log_level)) {
     NOTREACHED();
     return;
   }
 
   DCHECK_GE(log_level, net::NetLog::LOG_ALL);
   DCHECK_LE(log_level, net::NetLog::LOG_BASIC);
-  set_log_level(static_cast<net::NetLog::LogLevel>(log_level));
+  SetLogLevel(static_cast<net::NetLog::LogLevel>(log_level));
 }
 
+// Note that unlike other methods of IOThreadImpl, this function
+// can be called from ANY THREAD.
 void NetInternalsMessageHandler::IOThreadImpl::OnAddEntry(
     net::NetLog::EventType type,
     const base::TimeTicks& time,
     const net::NetLog::Source& source,
     net::NetLog::EventPhase phase,
     net::NetLog::EventParameters* params) {
-  DCHECK(is_observing_log_);
-
   CallJavascriptFunction(
       L"g_browser.receivedLogEntry",
       net::NetLog::EntryToDictionaryValue(type, time, source, phase, params,
                                           false));
 }
 
 void NetInternalsMessageHandler::IOThreadImpl::OnStartConnectionTestSuite() {
   CallJavascriptFunction(L"g_browser.receivedStartConnectionTestSuite", NULL);
 }
 
@@ skipping 25 matching lines @@
       NULL);
 }
 
 void NetInternalsMessageHandler::IOThreadImpl::DispatchToMessageHandler(
     ListValue* arg, MessageHandler method) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   (this->*method)(arg);
   delete arg;
 }
 
+// Note that this can be called from ANY THREAD.
 void NetInternalsMessageHandler::IOThreadImpl::CallJavascriptFunction(
     const std::wstring& function_name,
     Value* arg) {
   if (BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    if (handler_) {
+    if (handler_ && !was_domui_deleted_) {

[eroman, 2010/11/18 18:04:03]

ah! so this was why we were getting javascript errors on refresh? good find.

[mmenke, 2010/11/23 16:48:45]

Yea.  Since refresh keeps the same RenderView, and the handler_ is only deleted
after the new page is committed, events related to loading the net_internals_ui
page itself were being passed to the old domui, but only being handled after the
new page was committed...  Or something like that.

We actually still occasionally get Javascript errors from it, but only before
g_browser is instantiated, so the errors don't affect anything.

       // We check |handler_| in case it was deleted on the UI thread earlier
       // while we were running on the IO thread.
       handler_->CallJavascriptFunction(function_name, arg);
     }
     delete arg;
     return;
   }
 
-  // Otherwise if we were called from the IO thread, bridge the request over to
-  // the UI thread.
-
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   if (!BrowserThread::PostTask(
            BrowserThread::UI, FROM_HERE,
            NewRunnableMethod(
                this,
                &IOThreadImpl::CallJavascriptFunction,
                function_name, arg))) {
     // Failed posting the task, avoid leaking.
     delete arg;
   }
 }
@@ skipping 13 matching lines @@
   NetInternalsHTMLSource* html_source = new NetInternalsHTMLSource();
 
   // Set up the chrome://net-internals/ source.
   BrowserThread::PostTask(
       BrowserThread::IO, FROM_HERE,
       NewRunnableMethod(
           Singleton<ChromeURLDataManager>::get(),
           &ChromeURLDataManager::AddDataSource,
           make_scoped_refptr(html_source)));
 }