Update NetLog to be thread safe.

chrome/browser/dom_ui/net_internals_ui.cc

Index: chrome/browser/dom_ui/net_internals_ui.cc
===================================================================
--- chrome/browser/dom_ui/net_internals_ui.cc	(revision 65207)
+++ chrome/browser/dom_ui/net_internals_ui.cc	(working copy)
@@ -186,12 +186,13 @@
   // IO thread.
   void Detach();
 
+  // Sends all passive log entries in |entries| to Javascript handler.

[eroman, 2010/11/17 05:59:02]

nit: "to the javascript handler" (added "the").

+  void SendPassiveLogEntries(const ChromeNetLog::EntryList& passive_entries);
+
   //--------------------------------
   // 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);
@@ -201,7 +202,6 @@
   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);
@@ -386,9 +386,6 @@
       "enableIPv6",
       proxy_->CreateCallback(&IOThreadImpl::OnEnableIPv6));
   dom_ui_->RegisterMessageCallback(
-      "getPassiveLogEntries",
-      proxy_->CreateCallback(&IOThreadImpl::OnGetPassiveLogEntries));
-  dom_ui_->RegisterMessageCallback(
       "startConnectionTests",
       proxy_->CreateCallback(&IOThreadImpl::OnStartConnectionTests));
   dom_ui_->RegisterMessageCallback(
@@ -455,7 +452,7 @@
   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();
@@ -466,10 +463,6 @@
   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.
   {
@@ -617,7 +610,11 @@
                                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()->AddObserverAndGetAllCapturedEvents(this, &entries);

[eroman, 2010/11/17 05:59:02]

The impact of threading here is fairly subtle.

I don't remember, can you check whether the javascript cares if it receives
active events BEFORE receiving the passively captured events?

I don't think it will matter, but it is worth checking and additionally noting
here that it can happen (although unlikely).

Basically I am thinking that right after we attach the observer and grab the
passively logged entries, the observer might be called on some other thread, and
in turn queue up a task in the UI thread. Then we reach the next line
(SendPassiveLogEntries) which also posts a task to the UI thread, this time to
send the renderer the passively logged entries.

[mmenke, 2010/11/17 21:42:14]

Thanks for catching that.  I hadn't thought about that case.  It looks to me
like there is actually an issue: All events within a source are assumed to be
received in the order they occurred.  I've updated the JS code to hold onto
actively logged events until passively logged ones are received.

Could try to fix it C++-side, but the Javascript solution seems a lot simpler.

+  SendPassiveLogEntries(entries);
 }
 
 void NetInternalsMessageHandler::IOThreadImpl::OnGetProxySettings(
@@ -774,16 +771,11 @@
   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);
-
+void NetInternalsMessageHandler::IOThreadImpl::SendPassiveLogEntries(
+    const ChromeNetLog::EntryList& passive_entries) {
   ListValue* dict_list = new ListValue();
   for (size_t i = 0; i < passive_entries.size(); ++i) {
-    const PassiveLogCollector::Entry& e = passive_entries[i];
+    const ChromeNetLog::Entry& e = passive_entries[i];
     dict_list->Append(net::NetLog::EntryToDictionaryValue(e.type,
                                                           e.time,
                                                           e.source,
@@ -911,7 +903,8 @@
 
   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),
+               io_thread_->net_log());

[eroman, 2010/11/17 05:59:02]

indentation is off by one.

 }
 
 void NetInternalsMessageHandler::IOThreadImpl::OnAddEntry(
@@ -980,10 +973,6 @@
     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(