summaryrefslogtreecommitdiffstats
path: root/chrome
diff options
context:
space:
mode:
authormmenke@chromium.org <mmenke@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-12-01 15:19:40 +0000
committermmenke@chromium.org <mmenke@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-12-01 15:19:40 +0000
commitb2fcd0e0c8850eda2135b6fe270af5f453682f23 (patch)
tree2858497a3551d59093a5dfd233bf3550d40a4e8d /chrome
parent087a03f6aeded6397081304e191b1ac027b8e134 (diff)
downloadchromium_src-b2fcd0e0c8850eda2135b6fe270af5f453682f23.zip
chromium_src-b2fcd0e0c8850eda2135b6fe270af5f453682f23.tar.gz
chromium_src-b2fcd0e0c8850eda2135b6fe270af5f453682f23.tar.bz2
Update NetLog to be threadsafe.
The ChromeNetLog is now owned by the browser process, and passed to the IOThread on creation. NetLog entries can be added from any thread. Observers must be able to handle having log entries added from any thread. Observers can add/remove themselves on any thread. BUG=63334 TEST=None, yet Review URL: http://codereview.chromium.org/4118004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67851 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r--chrome/browser/browser_process_impl.cc5
-rw-r--r--chrome/browser/browser_process_impl.h4
-rw-r--r--chrome/browser/debugger/devtools_netlog_observer.cc8
-rw-r--r--chrome/browser/debugger/devtools_netlog_observer.h8
-rw-r--r--chrome/browser/dom_ui/net_internals_ui.cc110
-rw-r--r--chrome/browser/io_thread.cc25
-rw-r--r--chrome/browser/io_thread.h18
-rw-r--r--chrome/browser/net/chrome_net_log.cc114
-rw-r--r--chrome/browser/net/chrome_net_log.h109
-rw-r--r--chrome/browser/net/chrome_net_log_unittest.cc73
-rw-r--r--chrome/browser/net/chrome_url_request_context.cc14
-rw-r--r--chrome/browser/net/connection_tester_unittest.cc2
-rw-r--r--chrome/browser/net/load_timing_observer.cc16
-rw-r--r--chrome/browser/net/load_timing_observer.h7
-rw-r--r--chrome/browser/net/load_timing_observer_unittest.cc33
-rw-r--r--chrome/browser/net/net_log_logger.cc4
-rw-r--r--chrome/browser/net/net_log_logger.h4
-rw-r--r--chrome/browser/net/passive_log_collector.cc86
-rw-r--r--chrome/browser/net/passive_log_collector.h79
-rw-r--r--chrome/browser/net/passive_log_collector_unittest.cc16
-rw-r--r--chrome/browser/resources/net_internals/main.js26
-rw-r--r--chrome/chrome_tests.gypi1
22 files changed, 503 insertions, 259 deletions
diff --git a/chrome/browser/browser_process_impl.cc b/chrome/browser/browser_process_impl.cc
index 44f459b..f281667 100644
--- a/chrome/browser/browser_process_impl.cc
+++ b/chrome/browser/browser_process_impl.cc
@@ -32,6 +32,7 @@
#include "chrome/browser/intranet_redirect_detector.h"
#include "chrome/browser/io_thread.h"
#include "chrome/browser/metrics/metrics_service.h"
+#include "chrome/browser/net/chrome_net_log.h"
#include "chrome/browser/net/predictor_api.h"
#include "chrome/browser/net/sdch_dictionary_fetcher.h"
#include "chrome/browser/net/sqlite_persistent_cookie_store.h"
@@ -111,6 +112,8 @@ BrowserProcessImpl::BrowserProcessImpl(const CommandLine& command_line)
print_job_manager_.reset(new printing::PrintJobManager);
shutdown_event_.reset(new base::WaitableEvent(true, false));
+
+ net_log_.reset(new ChromeNetLog);
}
BrowserProcessImpl::~BrowserProcessImpl() {
@@ -562,7 +565,7 @@ void BrowserProcessImpl::CreateIOThread() {
background_x11_thread_.swap(background_x11_thread);
#endif
- scoped_ptr<IOThread> thread(new IOThread(local_state()));
+ scoped_ptr<IOThread> thread(new IOThread(local_state(), net_log_.get()));
base::Thread::Options options;
options.message_loop_type = MessageLoop::TYPE_IO;
if (!thread->StartWithOptions(options))
diff --git a/chrome/browser/browser_process_impl.h b/chrome/browser/browser_process_impl.h
index 0aeb976..4cfab10 100644
--- a/chrome/browser/browser_process_impl.h
+++ b/chrome/browser/browser_process_impl.h
@@ -24,6 +24,7 @@
#include "chrome/browser/tab_contents/thumbnail_generator.h"
#include "ipc/ipc_message.h"
+class ChromeNetLog;
class CommandLine;
class DebuggerWrapper;
class FilePath;
@@ -211,6 +212,9 @@ class BrowserProcessImpl : public BrowserProcess, public NonThreadSafe {
// notifications are properly added and removed.
PrefChangeRegistrar pref_change_registrar_;
+ // Lives here so can safely log events on shutdown.
+ scoped_ptr<ChromeNetLog> net_log_;
+
#if (defined(OS_WIN) || defined(OS_LINUX)) && !defined(OS_CHROMEOS)
base::RepeatingTimer<BrowserProcessImpl> autoupdate_timer_;
diff --git a/chrome/browser/debugger/devtools_netlog_observer.cc b/chrome/browser/debugger/devtools_netlog_observer.cc
index c5d49f1..aa4c705 100644
--- a/chrome/browser/debugger/devtools_netlog_observer.cc
+++ b/chrome/browser/debugger/devtools_netlog_observer.cc
@@ -19,7 +19,7 @@ const size_t kMaxNumEntries = 1000;
DevToolsNetLogObserver* DevToolsNetLogObserver::instance_ = NULL;
DevToolsNetLogObserver::DevToolsNetLogObserver(ChromeNetLog* chrome_net_log)
- : ChromeNetLog::Observer(net::NetLog::LOG_ALL_BUT_BYTES),
+ : ChromeNetLog::ThreadSafeObserver(net::NetLog::LOG_ALL_BUT_BYTES),
chrome_net_log_(chrome_net_log) {
chrome_net_log_->AddObserver(this);
}
@@ -41,6 +41,10 @@ void DevToolsNetLogObserver::OnAddEntry(net::NetLog::EventType type,
const net::NetLog::Source& source,
net::NetLog::EventPhase phase,
net::NetLog::EventParameters* params) {
+ // The events that the Observer is interested in only occur on the IO thread.
+ if (!BrowserThread::CurrentlyOn(BrowserThread::IO))
+ return;
+
if (type == net::NetLog::TYPE_URL_REQUEST_START_JOB) {
if (phase != net::NetLog::PHASE_BEGIN)
return;
@@ -108,7 +112,7 @@ void DevToolsNetLogObserver::Attach(IOThread* io_thread) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
DCHECK(!instance_);
- instance_ = new DevToolsNetLogObserver(io_thread->globals()->net_log.get());
+ instance_ = new DevToolsNetLogObserver(io_thread->net_log());
}
void DevToolsNetLogObserver::Detach() {
diff --git a/chrome/browser/debugger/devtools_netlog_observer.h b/chrome/browser/debugger/devtools_netlog_observer.h
index 2fad712..d6ce2a4 100644
--- a/chrome/browser/debugger/devtools_netlog_observer.h
+++ b/chrome/browser/debugger/devtools_netlog_observer.h
@@ -21,11 +21,15 @@ struct ResourceResponse;
// DevToolsNetLogObserver watches the NetLog event stream and collects the
// stuff that may be of interest to DevTools. Currently, this only includes
// actual HTTP/SPDY headers sent and received over the network.
-class DevToolsNetLogObserver: public ChromeNetLog::Observer {
+//
+// As DevToolsNetLogObserver shares live data with objects that live on the
+// IO Thread, it must also reside on the IO Thread. Only OnAddEntry can be
+// called from other threads.
+class DevToolsNetLogObserver: public ChromeNetLog::ThreadSafeObserver {
typedef webkit_glue::ResourceDevToolsInfo ResourceInfo;
public:
- // Observer implementation:
+ // ThreadSafeObserver implementation:
virtual void OnAddEntry(net::NetLog::EventType type,
const base::TimeTicks& time,
const net::NetLog::Source& source,
diff --git a/chrome/browser/dom_ui/net_internals_ui.cc b/chrome/browser/dom_ui/net_internals_ui.cc
index 90d2a09..785ec1a 100644
--- a/chrome/browser/dom_ui/net_internals_ui.cc
+++ b/chrome/browser/dom_ui/net_internals_ui.cc
@@ -151,14 +151,16 @@ class NetInternalsMessageHandler
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.
@@ -186,12 +188,18 @@ class NetInternalsMessageHandler::IOThreadImpl
// IO thread.
void Detach();
+ // Sends all passive log entries in |passive_entries| to the Javascript
+ // handler, 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);
@@ -201,7 +209,6 @@ class NetInternalsMessageHandler::IOThreadImpl
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);
@@ -212,7 +219,7 @@ class NetInternalsMessageHandler::IOThreadImpl
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,
@@ -235,7 +242,8 @@ class NetInternalsMessageHandler::IOThreadImpl
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);
@@ -251,6 +259,14 @@ class NetInternalsMessageHandler::IOThreadImpl
// Helper that runs the suite of connection tests.
scoped_ptr<ConnectionTester> connection_tester_;
+ // 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_;
+
// True if we have attached an observer to the NetLog already.
bool is_observing_log_;
friend class base::RefCountedThreadSafe<IOThreadImpl>;
@@ -344,6 +360,7 @@ 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));
@@ -386,9 +403,6 @@ void NetInternalsMessageHandler::RegisterMessages() {
"enableIPv6",
proxy_->CreateCallback(&IOThreadImpl::OnEnableIPv6));
dom_ui_->RegisterMessageCallback(
- "getPassiveLogEntries",
- proxy_->CreateCallback(&IOThreadImpl::OnGetPassiveLogEntries));
- dom_ui_->RegisterMessageCallback(
"startConnectionTests",
proxy_->CreateCallback(&IOThreadImpl::OnStartConnectionTests));
dom_ui_->RegisterMessageCallback(
@@ -432,10 +446,11 @@ 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));
}
@@ -455,21 +470,39 @@ 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.
{
@@ -617,7 +650,12 @@ void NetInternalsMessageHandler::IOThreadImpl::OnRendererReady(
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(
@@ -774,27 +812,6 @@ void NetInternalsMessageHandler::IOThreadImpl::OnEnableIPv6(
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>].
@@ -911,17 +928,17 @@ void NetInternalsMessageHandler::IOThreadImpl::OnSetLogLevel(
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,
@@ -967,11 +984,12 @@ void NetInternalsMessageHandler::IOThreadImpl::DispatchToMessageHandler(
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_) {
// 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);
@@ -980,10 +998,6 @@ void NetInternalsMessageHandler::IOThreadImpl::CallJavascriptFunction(
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(
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 94d3126..b9e8ef9 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -214,8 +214,9 @@ IOThread::Globals::~Globals() {}
// |local_state| is passed in explicitly in order to (1) reduce implicit
// dependencies and (2) make IOThread more flexible for testing.
-IOThread::IOThread(PrefService* local_state)
+IOThread::IOThread(PrefService* local_state, ChromeNetLog* net_log)
: BrowserProcessSubThread(BrowserThread::IO),
+ net_log_(net_log),
globals_(NULL),
speculative_interceptor_(NULL),
predictor_(NULL) {
@@ -245,6 +246,10 @@ IOThread::Globals* IOThread::globals() {
return globals_;
}
+ChromeNetLog* IOThread::net_log() {
+ return net_log_;
+}
+
void IOThread::InitNetworkPredictor(
bool prefetching_enabled,
base::TimeDelta max_dns_queue_delay,
@@ -320,16 +325,14 @@ void IOThread::Init() {
DCHECK(!globals_);
globals_ = new Globals;
- globals_->net_log.reset(new ChromeNetLog());
-
// Add an observer that will emit network change events to the ChromeNetLog.
// Assuming NetworkChangeNotifier dispatches in FIFO order, we should be
// logging the network change before other IO thread consumers respond to it.
network_change_observer_.reset(
- new LoggingNetworkChangeObserver(globals_->net_log.get()));
+ new LoggingNetworkChangeObserver(net_log_));
globals_->host_resolver.reset(
- CreateGlobalHostResolver(globals_->net_log.get()));
+ CreateGlobalHostResolver(net_log_));
globals_->dnsrr_resolver.reset(new net::DnsRRResolver);
globals_->http_auth_handler_factory.reset(CreateDefaultAuthHandlerFactory(
globals_->host_resolver.get()));
@@ -416,10 +419,6 @@ void IOThread::CleanUp() {
globals_->host_resolver.get()->GetAsHostResolverImpl()->Shutdown();
}
- // We will delete the NetLog as part of CleanUpAfterMessageLoopDestruction()
- // in case any of the message loop destruction observers try to access it.
- deferred_net_log_to_delete_.reset(globals_->net_log.release());
-
delete globals_;
globals_ = NULL;
@@ -427,12 +426,6 @@ void IOThread::CleanUp() {
}
void IOThread::CleanUpAfterMessageLoopDestruction() {
- // TODO(eroman): get rid of this special case for 39723. If we could instead
- // have a method that runs after the message loop destruction observers have
- // run, but before the message loop itself is destroyed, we could safely
- // combine the two cleanups.
- deferred_net_log_to_delete_.reset();
-
// This will delete the |notification_service_|. Make sure it's done after
// anything else can reference it.
BrowserProcessSubThread::CleanUpAfterMessageLoopDestruction();
@@ -529,5 +522,5 @@ void IOThread::ChangedToOnTheRecordOnIOThread() {
// Clear all of the passively logged data.
// TODO(eroman): this is a bit heavy handed, really all we need to do is
// clear the data pertaining to off the record context.
- globals_->net_log->passive_collector()->Clear();
+ net_log_->ClearAllPassivelyCapturedEvents();
}
diff --git a/chrome/browser/io_thread.h b/chrome/browser/io_thread.h
index a2dad58..3443af1 100644
--- a/chrome/browser/io_thread.h
+++ b/chrome/browser/io_thread.h
@@ -43,7 +43,6 @@ class IOThread : public BrowserProcessSubThread {
Globals();
~Globals();
- scoped_ptr<ChromeNetLog> net_log;
scoped_ptr<net::HostResolver> host_resolver;
scoped_ptr<net::DnsRRResolver> dnsrr_resolver;
scoped_ptr<net::HttpAuthHandlerFactory> http_auth_handler_factory;
@@ -51,13 +50,16 @@ class IOThread : public BrowserProcessSubThread {
ChromeNetworkDelegate network_delegate;
};
- explicit IOThread(PrefService* local_state);
+ // |net_log| must either outlive the IOThread or be NULL.
+ IOThread(PrefService* local_state, ChromeNetLog* net_log);
virtual ~IOThread();
// Can only be called on the IO thread.
Globals* globals();
+ ChromeNetLog* net_log();
+
// Initializes the network predictor, which induces DNS pre-resolution and/or
// TCP/IP preconnections. |prefetching_enabled| indicates whether or not DNS
// prefetching should be enabled, and |preconnect_enabled| controls whether
@@ -120,22 +122,20 @@ class IOThread : public BrowserProcessSubThread {
void ChangedToOnTheRecordOnIOThread();
+ // The NetLog is owned by the browser process, to allow logging from other
+ // threads during shutdown, but is used most frequently on the IOThread.
+ ChromeNetLog* net_log_;
+
// These member variables are basically global, but their lifetimes are tied
// to the IOThread. IOThread owns them all, despite not using scoped_ptr.
// This is because the destructor of IOThread runs on the wrong thread. All
- // member variables should be deleted in CleanUp(), except ChromeNetLog
- // which is deleted later in CleanUpAfterMessageLoopDestruction().
+ // member variables should be deleted in CleanUp().
// These member variables are initialized in Init() and do not change for the
// lifetime of the IO thread.
Globals* globals_;
- // This variable is only meaningful during shutdown. It is used to defer
- // deletion of the NetLog to CleanUpAfterMessageLoopDestruction() even
- // though |globals_| is reset by CleanUp().
- scoped_ptr<ChromeNetLog> deferred_net_log_to_delete_;
-
// Observer that logs network changes to the ChromeNetLog.
scoped_ptr<net::NetworkChangeNotifier::Observer> network_change_observer_;
diff --git a/chrome/browser/net/chrome_net_log.cc b/chrome/browser/net/chrome_net_log.cc
index b6ef86e..6cfc893 100644
--- a/chrome/browser/net/chrome_net_log.cc
+++ b/chrome/browser/net/chrome_net_log.cc
@@ -9,27 +9,59 @@
#include "base/command_line.h"
#include "base/logging.h"
#include "base/string_util.h"
-#include "chrome/browser/browser_thread.h"
+#include "base/values.h"
#include "chrome/browser/net/load_timing_observer.h"
#include "chrome/browser/net/net_log_logger.h"
#include "chrome/browser/net/passive_log_collector.h"
#include "chrome/common/chrome_switches.h"
-ChromeNetLog::Observer::Observer(LogLevel log_level) : log_level_(log_level) {}
+ChromeNetLog::ThreadSafeObserver::ThreadSafeObserver(LogLevel log_level)
+ : net_log_(NULL),
+ log_level_(log_level) {
+}
+
+ChromeNetLog::ThreadSafeObserver::~ThreadSafeObserver() {
+ DCHECK(!net_log_);
+}
-net::NetLog::LogLevel ChromeNetLog::Observer::log_level() const {
+net::NetLog::LogLevel ChromeNetLog::ThreadSafeObserver::log_level() const {
return log_level_;
}
-void ChromeNetLog::Observer::set_log_level(net::NetLog::LogLevel log_level) {
+void ChromeNetLog::ThreadSafeObserver::AssertNetLogLockAcquired() const {
+ if (net_log_)
+ net_log_->lock_.AssertAcquired();
+}
+
+void ChromeNetLog::ThreadSafeObserver::SetLogLevel(
+ net::NetLog::LogLevel log_level) {
+ DCHECK(net_log_);
+ AutoLock lock(net_log_->lock_);
log_level_ = log_level;
+ net_log_->UpdateLogLevel_();
+}
+
+ChromeNetLog::Entry::Entry(uint32 order,
+ net::NetLog::EventType type,
+ const base::TimeTicks& time,
+ net::NetLog::Source source,
+ net::NetLog::EventPhase phase,
+ net::NetLog::EventParameters* params)
+ : order(order),
+ type(type),
+ time(time),
+ source(source),
+ phase(phase),
+ params(params) {
}
+ChromeNetLog::Entry::~Entry() {}
+
ChromeNetLog::ChromeNetLog()
- : next_id_(1),
+ : last_id_(0),
+ log_level_(LOG_BASIC),
passive_collector_(new PassiveLogCollector),
load_timing_observer_(new LoadTimingObserver) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
AddObserver(passive_collector_.get());
AddObserver(load_timing_observer_.get());
@@ -41,7 +73,6 @@ ChromeNetLog::ChromeNetLog()
}
ChromeNetLog::~ChromeNetLog() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
RemoveObserver(passive_collector_.get());
RemoveObserver(load_timing_observer_.get());
if (net_log_logger_.get()) {
@@ -54,42 +85,69 @@ void ChromeNetLog::AddEntry(EventType type,
const Source& source,
EventPhase phase,
EventParameters* params) {
- // This must be invoked when we're on the IO thread, or if the IO thread's
- // message loop isn't valid. The later can happen if this is invoked when the
- // IOThread is shuting down the MessageLoop.
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO) ||
- !BrowserThread::IsMessageLoopValid(BrowserThread::IO));
+ AutoLock lock(lock_);
// Notify all of the log observers.
- FOR_EACH_OBSERVER(Observer, observers_,
+ FOR_EACH_OBSERVER(ThreadSafeObserver, observers_,
OnAddEntry(type, time, source, phase, params));
}
uint32 ChromeNetLog::NextID() {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- return next_id_++;
+ return base::subtle::NoBarrier_AtomicIncrement(&last_id_, 1);
}
net::NetLog::LogLevel ChromeNetLog::GetLogLevel() const {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+ base::subtle::Atomic32 log_level = base::subtle::NoBarrier_Load(&log_level_);
+ return static_cast<net::NetLog::LogLevel>(log_level);
+}
+
+void ChromeNetLog::AddObserver(ThreadSafeObserver* observer) {
+ AutoLock lock(lock_);
+ AddObserverWhileLockHeld(observer);
+}
+
+void ChromeNetLog::RemoveObserver(ThreadSafeObserver* observer) {
+ 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) {
+ AutoLock lock(lock_);
+ AddObserverWhileLockHeld(observer);
+ passive_collector_->GetAllCapturedEvents(passive_entries);
+}
+
+void ChromeNetLog::GetAllPassivelyCapturedEvents(EntryList* passive_entries) {
+ AutoLock lock(lock_);
+ passive_collector_->GetAllCapturedEvents(passive_entries);
+}
+
+void ChromeNetLog::ClearAllPassivelyCapturedEvents() {
+ AutoLock lock(lock_);
+ passive_collector_->Clear();
+}
+
+void ChromeNetLog::UpdateLogLevel_() {
+ lock_.AssertAcquired();
// Look through all the observers and find the finest granularity
// log level (higher values of the enum imply *lower* log levels).
- LogLevel log_level = LOG_BASIC;
- ObserverListBase<Observer>::Iterator it(observers_);
- Observer* observer;
+ LogLevel new_log_level = LOG_BASIC;
+ ObserverListBase<ThreadSafeObserver>::Iterator it(observers_);
+ ThreadSafeObserver* observer;
while ((observer = it.GetNext()) != NULL) {
- log_level = std::min(log_level, observer->log_level());
+ new_log_level = std::min(new_log_level, observer->log_level());
}
- return log_level;
+ base::subtle::NoBarrier_Store(&log_level_, new_log_level);
}
-void ChromeNetLog::AddObserver(Observer* observer) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+void ChromeNetLog::AddObserverWhileLockHeld(ThreadSafeObserver* observer) {
+ DCHECK(!observer->net_log_);
+ observer->net_log_ = this;
observers_.AddObserver(observer);
-}
-
-void ChromeNetLog::RemoveObserver(Observer* observer) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- observers_.RemoveObserver(observer);
+ UpdateLogLevel_();
}
diff --git a/chrome/browser/net/chrome_net_log.h b/chrome/browser/net/chrome_net_log.h
index aac09e6..f6a5c20 100644
--- a/chrome/browser/net/chrome_net_log.h
+++ b/chrome/browser/net/chrome_net_log.h
@@ -6,8 +6,13 @@
#define CHROME_BROWSER_NET_CHROME_NET_LOG_H_
#pragma once
+#include <vector>
+
+#include "base/atomicops.h"
+#include "base/lock.h"
#include "base/observer_list.h"
#include "base/scoped_ptr.h"
+#include "chrome/browser/browser_thread.h"
#include "net/base/net_log.h"
class LoadTimingObserver;
@@ -17,39 +22,85 @@ class PassiveLogCollector;
// ChromeNetLog is an implementation of NetLog that dispatches network log
// 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
+// OnAddEntry() method. Doing so will result in a deadlock.
+//
// By default, ChromeNetLog will attach the observer PassiveLogCollector which
// will keep track of recent request information (which used when displaying
// the about:net-internals page).
//
-// TODO(eroman): Move this default observer out of ChromeNetLog.
-//
class ChromeNetLog : public net::NetLog {
public:
+ // This structure encapsulates all of the parameters of an event,
+ // including an "order" field that identifies when it was captured relative
+ // to other events.
+ struct Entry {
+ Entry(uint32 order,
+ net::NetLog::EventType type,
+ const base::TimeTicks& time,
+ net::NetLog::Source source,
+ net::NetLog::EventPhase phase,
+ net::NetLog::EventParameters* params);
+ ~Entry();
+
+ uint32 order;
+ net::NetLog::EventType type;
+ base::TimeTicks time;
+ net::NetLog::Source source;
+ net::NetLog::EventPhase phase;
+ scoped_refptr<net::NetLog::EventParameters> params;
+ };
+
+ typedef std::vector<Entry> EntryList;
+
// Interface for observing the events logged by the network stack.
- class Observer {
+ class ThreadSafeObserver {
public:
// Constructs an observer that wants to see network events, with
- // the specified minimum event granularity.
+ // 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 anticiapted.
- explicit Observer(LogLevel log_level);
+ // 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();
- virtual ~Observer() {}
+ // 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.
virtual void OnAddEntry(EventType type,
const base::TimeTicks& time,
const Source& source,
EventPhase phase,
EventParameters* params) = 0;
LogLevel log_level() const;
+
protected:
- void set_log_level(LogLevel log_level);
+ void AssertNetLogLockAcquired() const;
+
+ // 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_;
+
private:
+ friend class ChromeNetLog;
LogLevel log_level_;
- DISALLOW_COPY_AND_ASSIGN(Observer);
+ DISALLOW_COPY_AND_ASSIGN(ThreadSafeObserver);
};
ChromeNetLog();
@@ -64,26 +115,48 @@ class ChromeNetLog : public net::NetLog {
virtual uint32 NextID();
virtual LogLevel GetLogLevel() const;
- void AddObserver(Observer* observer);
- void RemoveObserver(Observer* observer);
+ void AddObserver(ThreadSafeObserver* observer);
+ void RemoveObserver(ThreadSafeObserver* observer);
- PassiveLogCollector* passive_collector() {
- return passive_collector_.get();
- }
+ // 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);
+
+ void GetAllPassivelyCapturedEvents(EntryList* passive_entries);
+
+ void ClearAllPassivelyCapturedEvents();
LoadTimingObserver* load_timing_observer() {
return load_timing_observer_.get();
}
private:
- uint32 next_id_;
+ void AddObserverWhileLockHeld(ThreadSafeObserver* observer);
+
+ // Called whenever an observer is added or removed, or changes its log level.
+ // Must have acquired |lock_| prior to calling.
+ void UpdateLogLevel_();
+
+ // |lock_| protects access to |observers_| and, indirectly, to
+ // |passive_collector_|. Should not be acquired by observers.
+ Lock lock_;
+
+ // Last assigned source ID. Incremented to get the next one.
+ base::subtle::Atomic32 last_id_;
+
+ base::subtle::Atomic32 log_level_;
+
+ // Not thread safe. Must only be used when |lock_| is acquired.
scoped_ptr<PassiveLogCollector> passive_collector_;
+
scoped_ptr<LoadTimingObserver> load_timing_observer_;
scoped_ptr<NetLogLogger> net_log_logger_;
- // Note that this needs to be "mutable" so we can iterate over the observer
- // list in GetLogLevel().
- mutable ObserverList<Observer, true> observers_;
+ // |lock_| must be acquired whenever reading or writing to this.
+ ObserverList<ThreadSafeObserver, true> observers_;
DISALLOW_COPY_AND_ASSIGN(ChromeNetLog);
};
diff --git a/chrome/browser/net/chrome_net_log_unittest.cc b/chrome/browser/net/chrome_net_log_unittest.cc
new file mode 100644
index 0000000..7bd9301
--- /dev/null
+++ b/chrome/browser/net/chrome_net_log_unittest.cc
@@ -0,0 +1,73 @@
+// 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/net/chrome_net_log.h"
+
+#include "base/waitable_event.h"
+#include "base/simple_thread.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+const int kThreads = 10;
+const int kEvents = 100;
+
+class ChromeNetLogTestThread : public base::SimpleThread {
+ public:
+ ChromeNetLogTestThread() : base::SimpleThread("ChromeNetLogTest"),
+ can_start_loop_(false, false),
+ log_(NULL) {
+ }
+
+ void Init(ChromeNetLog* log) {
+ log_ = log;
+ }
+
+ virtual void Run() {
+ can_start_loop_.Wait();
+ for (int i = 0; i < kEvents; ++i) {
+ net::NetLog::Source source(net::NetLog::SOURCE_SOCKET, log_->NextID());
+ log_->AddEntry(net::NetLog::TYPE_SOCKET_ALIVE, base::TimeTicks(),
+ source, net::NetLog::PHASE_BEGIN, NULL);
+ }
+ log_->ClearAllPassivelyCapturedEvents();
+ }
+
+ void ReallyStart() {
+ can_start_loop_.Signal();
+ }
+
+ private:
+ // Only triggered once all threads have been created, to make it much less
+ // likely each thread completes before the next one starts.
+ base::WaitableEvent can_start_loop_;
+
+ ChromeNetLog* log_;
+
+ DISALLOW_COPY_AND_ASSIGN(ChromeNetLogTestThread);
+};
+
+} // namespace
+
+// Attempts to check thread safety, exercising checks in ChromeNetLog and
+// PassiveLogCollector.
+TEST(ChromeNetLogTest, NetLogThreads) {
+ ChromeNetLog log;
+ ChromeNetLogTestThread threads[kThreads];
+
+ for (int i = 0; i < kThreads; ++i) {
+ threads[i].Init(&log);
+ threads[i].Start();
+ }
+
+ for (int i = 0; i < kThreads; ++i)
+ threads[i].ReallyStart();
+
+ for (int i = 0; i < kThreads; ++i)
+ threads[i].Join();
+
+ ChromeNetLog::EntryList entries;
+ log.GetAllPassivelyCapturedEvents(&entries);
+ EXPECT_EQ(0u, entries.size());
+}
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 81a0402..f153162 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -273,7 +273,7 @@ ChromeURLRequestContext* FactoryForOriginal::Create() {
const CommandLine& command_line = *CommandLine::ForCurrentProcess();
context->set_proxy_service(
- CreateProxyService(io_thread_globals->net_log.get(),
+ CreateProxyService(io_thread()->net_log(),
context,
proxy_config_service_.release(),
command_line,
@@ -290,7 +290,7 @@ ChromeURLRequestContext* FactoryForOriginal::Create() {
context->ssl_config_service(),
context->http_auth_handler_factory(),
&io_thread_globals->network_delegate,
- io_thread_globals->net_log.get(),
+ io_thread()->net_log(),
backend);
bool record_mode = chrome::kRecordModeEnabled &&
@@ -324,7 +324,7 @@ ChromeURLRequestContext* FactoryForOriginal::Create() {
appcache_service_->set_request_context(context);
- context->set_net_log(io_thread_globals->net_log.get());
+ context->set_net_log(io_thread()->net_log());
return context;
}
@@ -417,7 +417,7 @@ ChromeURLRequestContext* FactoryForOffTheRecord::Create() {
context->ssl_config_service(),
context->http_auth_handler_factory(),
&io_thread_globals->network_delegate,
- io_thread_globals->net_log.get(),
+ io_thread()->net_log(),
backend);
context->set_cookie_store(new net::CookieMonster(NULL,
cookie_monster_delegate_));
@@ -430,7 +430,7 @@ ChromeURLRequestContext* FactoryForOffTheRecord::Create() {
appcache_service_->set_request_context(context);
- context->set_net_log(io_thread_globals->net_log.get());
+ context->set_net_log(io_thread()->net_log());
return context;
}
@@ -510,12 +510,12 @@ ChromeURLRequestContext* FactoryForMedia::Create() {
main_context->ssl_config_service(),
main_context->http_auth_handler_factory(),
&io_thread_globals->network_delegate,
- io_thread_globals->net_log.get(),
+ io_thread()->net_log(),
backend);
}
context->set_http_transaction_factory(cache);
- context->set_net_log(io_thread_globals->net_log.get());
+ context->set_net_log(io_thread()->net_log());
return context;
}
diff --git a/chrome/browser/net/connection_tester_unittest.cc b/chrome/browser/net/connection_tester_unittest.cc
index 5bab81e..409c676 100644
--- a/chrome/browser/net/connection_tester_unittest.cc
+++ b/chrome/browser/net/connection_tester_unittest.cc
@@ -78,7 +78,7 @@ class ConnectionTesterTest : public PlatformTest {
FilePath(FILE_PATH_LITERAL("net/data/url_request_unittest"))),
message_loop_(MessageLoop::TYPE_IO),
pref_service(new TestingPrefService()),
- io_thread_(pref_service.get()) {
+ io_thread_(pref_service.get(), NULL) {
scoped_refptr<net::RuleBasedHostResolverProc> catchall_resolver(
new net::RuleBasedHostResolverProc(NULL));
diff --git a/chrome/browser/net/load_timing_observer.cc b/chrome/browser/net/load_timing_observer.cc
index f5b27d5..7d2571c 100644
--- a/chrome/browser/net/load_timing_observer.cc
+++ b/chrome/browser/net/load_timing_observer.cc
@@ -44,7 +44,7 @@ static int32 TimeTicksToOffset(
(time_ticks - record->base_ticks).InMillisecondsRoundedUp());
}
-}
+} // namespace
LoadTimingObserver::URLRequestRecord::URLRequestRecord()
: connect_job_id(net::NetLog::Source::kInvalidId),
@@ -53,7 +53,7 @@ LoadTimingObserver::URLRequestRecord::URLRequestRecord()
}
LoadTimingObserver::LoadTimingObserver()
- : Observer(net::NetLog::LOG_BASIC),
+ : ThreadSafeObserver(net::NetLog::LOG_BASIC),
last_connect_job_id_(net::NetLog::Source::kInvalidId) {
}
@@ -62,6 +62,8 @@ LoadTimingObserver::~LoadTimingObserver() {
LoadTimingObserver::URLRequestRecord*
LoadTimingObserver::GetURLRequestRecord(uint32 source_id) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
URLRequestToRecordMap::iterator it = url_request_to_record_.find(source_id);
if (it != url_request_to_record_.end())
return &it->second;
@@ -73,6 +75,9 @@ void LoadTimingObserver::OnAddEntry(net::NetLog::EventType type,
const net::NetLog::Source& source,
net::NetLog::EventPhase phase,
net::NetLog::EventParameters* params) {
+ // The events that the Observer is interested in only occur on the IO thread.
+ if (!BrowserThread::CurrentlyOn(BrowserThread::IO))
+ return;
if (source.type == net::NetLog::SOURCE_URL_REQUEST)
OnAddURLRequestEntry(type, time, source, phase, params);
else if (source.type == net::NetLog::SOURCE_CONNECT_JOB)
@@ -84,6 +89,7 @@ void LoadTimingObserver::OnAddEntry(net::NetLog::EventType type,
// static
void LoadTimingObserver::PopulateTimingInfo(net::URLRequest* request,
ResourceResponse* response) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
if (!(request->load_flags() & net::LOAD_ENABLE_LOAD_TIMING))
return;
@@ -109,6 +115,8 @@ void LoadTimingObserver::OnAddURLRequestEntry(
const net::NetLog::Source& source,
net::NetLog::EventPhase phase,
net::NetLog::EventParameters* params) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
bool is_begin = phase == net::NetLog::PHASE_BEGIN;
bool is_end = phase == net::NetLog::PHASE_END;
@@ -210,6 +218,8 @@ void LoadTimingObserver::OnAddConnectJobEntry(
const net::NetLog::Source& source,
net::NetLog::EventPhase phase,
net::NetLog::EventParameters* params) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
bool is_begin = phase == net::NetLog::PHASE_BEGIN;
bool is_end = phase == net::NetLog::PHASE_END;
@@ -253,6 +263,8 @@ void LoadTimingObserver::OnAddSocketEntry(
const net::NetLog::Source& source,
net::NetLog::EventPhase phase,
net::NetLog::EventParameters* params) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
bool is_begin = phase == net::NetLog::PHASE_BEGIN;
bool is_end = phase == net::NetLog::PHASE_END;
diff --git a/chrome/browser/net/load_timing_observer.h b/chrome/browser/net/load_timing_observer.h
index ec3c2e5..a280398 100644
--- a/chrome/browser/net/load_timing_observer.h
+++ b/chrome/browser/net/load_timing_observer.h
@@ -21,7 +21,10 @@ struct ResourceResponse;
// LoadTimingObserver watches the NetLog event stream and collects the network
// timing information.
-class LoadTimingObserver : public ChromeNetLog::Observer {
+//
+// 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 {
public:
struct URLRequestRecord {
URLRequestRecord();
@@ -48,7 +51,7 @@ class LoadTimingObserver : public ChromeNetLog::Observer {
URLRequestRecord* GetURLRequestRecord(uint32 source_id);
- // Observer implementation:
+ // ThreadSafeObserver implementation:
virtual void OnAddEntry(net::NetLog::EventType type,
const base::TimeTicks& time,
const net::NetLog::Source& source,
diff --git a/chrome/browser/net/load_timing_observer_unittest.cc b/chrome/browser/net/load_timing_observer_unittest.cc
index 98c50be..7d5ffa9 100644
--- a/chrome/browser/net/load_timing_observer_unittest.cc
+++ b/chrome/browser/net/load_timing_observer_unittest.cc
@@ -17,6 +17,17 @@ namespace {
using net::NetLog;
using base::TimeDelta;
+// Serves to Identify the current thread as the IO thread.
+class LoadTimingObserverTest : public testing::Test {
+ public:
+ LoadTimingObserverTest() : io_thread_(BrowserThread::IO, &message_loop_) {
+ }
+
+ private:
+ MessageLoop message_loop_;
+ BrowserThread io_thread_;
+};
+
base::TimeTicks current_time;
void AddStartEntry(LoadTimingObserver& observer,
@@ -82,7 +93,7 @@ void AddEndSocketEntries(LoadTimingObserver& observer, uint32 id) {
} // namespace
// Test that net::URLRequest with no load timing flag is not processed.
-TEST(LoadTimingObserverTest, NoLoadTimingEnabled) {
+TEST_F(LoadTimingObserverTest, NoLoadTimingEnabled) {
LoadTimingObserver observer;
AddStartURLRequestEntries(observer, 0, false);
@@ -92,7 +103,7 @@ TEST(LoadTimingObserverTest, NoLoadTimingEnabled) {
}
// Test that URLRequestRecord is created, deleted and is not growing unbound.
-TEST(LoadTimingObserverTest, URLRequestRecord) {
+TEST_F(LoadTimingObserverTest, URLRequestRecord) {
LoadTimingObserver observer;
// Create record.
@@ -114,7 +125,7 @@ TEST(LoadTimingObserverTest, URLRequestRecord) {
}
// Test that ConnectJobRecord is created, deleted and is not growing unbound.
-TEST(LoadTimingObserverTest, ConnectJobRecord) {
+TEST_F(LoadTimingObserverTest, ConnectJobRecord) {
LoadTimingObserver observer;
// Create record.
@@ -135,7 +146,7 @@ TEST(LoadTimingObserverTest, ConnectJobRecord) {
}
// Test that SocketRecord is created, deleted and is not growing unbound.
-TEST(LoadTimingObserverTest, SocketRecord) {
+TEST_F(LoadTimingObserverTest, SocketRecord) {
LoadTimingObserver observer;
// Create record.
@@ -157,7 +168,7 @@ TEST(LoadTimingObserverTest, SocketRecord) {
}
// Test that basic time is set to the request.
-TEST(LoadTimingObserverTest, BaseTicks) {
+TEST_F(LoadTimingObserverTest, BaseTicks) {
LoadTimingObserver observer;
current_time += TimeDelta::FromSeconds(1);
AddStartURLRequestEntries(observer, 0, true);
@@ -168,7 +179,7 @@ TEST(LoadTimingObserverTest, BaseTicks) {
}
// Test proxy time detection.
-TEST(LoadTimingObserverTest, ProxyTime) {
+TEST_F(LoadTimingObserverTest, ProxyTime) {
LoadTimingObserver observer;
current_time += TimeDelta::FromSeconds(1);
@@ -187,7 +198,7 @@ TEST(LoadTimingObserverTest, ProxyTime) {
}
// Test connect time detection.
-TEST(LoadTimingObserverTest, ConnectTime) {
+TEST_F(LoadTimingObserverTest, ConnectTime) {
LoadTimingObserver observer;
current_time += TimeDelta::FromSeconds(1);
@@ -206,7 +217,7 @@ TEST(LoadTimingObserverTest, ConnectTime) {
}
// Test dns time detection.
-TEST(LoadTimingObserverTest, DnsTime) {
+TEST_F(LoadTimingObserverTest, DnsTime) {
LoadTimingObserver observer;
// Start request.
@@ -240,7 +251,7 @@ TEST(LoadTimingObserverTest, DnsTime) {
}
// Test send time detection.
-TEST(LoadTimingObserverTest, SendTime) {
+TEST_F(LoadTimingObserverTest, SendTime) {
LoadTimingObserver observer;
// Start request.
@@ -266,7 +277,7 @@ TEST(LoadTimingObserverTest, SendTime) {
}
// Test receive time detection.
-TEST(LoadTimingObserverTest, ReceiveTime) {
+TEST_F(LoadTimingObserverTest, ReceiveTime) {
LoadTimingObserver observer;
// Start request.
@@ -292,7 +303,7 @@ TEST(LoadTimingObserverTest, ReceiveTime) {
}
// Test ssl time detection.
-TEST(LoadTimingObserverTest, SslTime) {
+TEST_F(LoadTimingObserverTest, SslTime) {
LoadTimingObserver observer;
// Start request.
diff --git a/chrome/browser/net/net_log_logger.cc b/chrome/browser/net/net_log_logger.cc
index b533e95..bba75ca 100644
--- a/chrome/browser/net/net_log_logger.cc
+++ b/chrome/browser/net/net_log_logger.cc
@@ -7,7 +7,9 @@
#include "base/json/json_writer.h"
#include "base/values.h"
-NetLogLogger::NetLogLogger() : Observer(net::NetLog::LOG_ALL_BUT_BYTES) {}
+NetLogLogger::NetLogLogger()
+ : ThreadSafeObserver(net::NetLog::LOG_ALL_BUT_BYTES) {
+}
NetLogLogger::~NetLogLogger() {}
diff --git a/chrome/browser/net/net_log_logger.h b/chrome/browser/net/net_log_logger.h
index 7f90d94..564f232 100644
--- a/chrome/browser/net/net_log_logger.h
+++ b/chrome/browser/net/net_log_logger.h
@@ -11,12 +11,12 @@
// NetLogLogger watches the NetLog event stream, and sends all entries to
// VLOG(1). This is to debug errors that prevent getting to the
// about:net-internals page.
-class NetLogLogger : public ChromeNetLog::Observer {
+class NetLogLogger : public ChromeNetLog::ThreadSafeObserver {
public:
NetLogLogger();
~NetLogLogger();
- // Observer implementation:
+ // ThreadSafeObserver implementation:
virtual void OnAddEntry(net::NetLog::EventType type,
const base::TimeTicks& time,
const net::NetLog::Source& source,
diff --git a/chrome/browser/net/passive_log_collector.cc b/chrome/browser/net/passive_log_collector.cc
index 84a9403..b7ef11e 100644
--- a/chrome/browser/net/passive_log_collector.cc
+++ b/chrome/browser/net/passive_log_collector.cc
@@ -7,6 +7,7 @@
#include <algorithm>
#include "base/compiler_specific.h"
+#include "base/lock.h"
#include "base/string_util.h"
#include "base/format_macros.h"
#include "chrome/browser/browser_thread.h"
@@ -18,7 +19,7 @@ namespace {
const size_t kMaxNumEntriesPerLog = 30;
-void AddEntryToSourceInfo(const PassiveLogCollector::Entry& entry,
+void AddEntryToSourceInfo(const ChromeNetLog::Entry& entry,
PassiveLogCollector::SourceInfo* out_info) {
// Start dropping new entries when the log has gotten too big.
if (out_info->entries.size() + 1 <= kMaxNumEntriesPerLog) {
@@ -30,29 +31,13 @@ void AddEntryToSourceInfo(const PassiveLogCollector::Entry& entry,
}
// Comparator to sort entries by their |order| property, ascending.
-bool SortByOrderComparator(const PassiveLogCollector::Entry& a,
- const PassiveLogCollector::Entry& b) {
+bool SortByOrderComparator(const ChromeNetLog::Entry& a,
+ const ChromeNetLog::Entry& b) {
return a.order < b.order;
}
} // namespace
-PassiveLogCollector::Entry::Entry(uint32 order,
- net::NetLog::EventType type,
- const base::TimeTicks& time,
- net::NetLog::Source source,
- net::NetLog::EventPhase phase,
- net::NetLog::EventParameters* params)
- : order(order),
- type(type),
- time(time),
- source(source),
- phase(phase),
- params(params) {
-}
-
-PassiveLogCollector::Entry::~Entry() {}
-
PassiveLogCollector::SourceInfo::SourceInfo()
: source_id(net::NetLog::Source::kInvalidId),
num_entries_truncated(0),
@@ -67,7 +52,7 @@ PassiveLogCollector::SourceInfo::~SourceInfo() {}
//----------------------------------------------------------------------------
PassiveLogCollector::PassiveLogCollector()
- : Observer(net::NetLog::LOG_BASIC),
+ : ThreadSafeObserver(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)),
@@ -100,28 +85,33 @@ void PassiveLogCollector::OnAddEntry(
const net::NetLog::Source& source,
net::NetLog::EventPhase phase,
net::NetLog::EventParameters* params) {
+ AssertNetLogLockAcquired();
// Package the parameters into a single struct for convenience.
- Entry entry(num_events_seen_++, type, time, source, phase, params);
+ ChromeNetLog::Entry entry(num_events_seen_++, type, time, source, phase,
+ params);
- SourceTrackerInterface* tracker = GetTrackerForSourceType(entry.source.type);
+ SourceTrackerInterface* tracker = GetTrackerForSourceType_(entry.source.type);
if (tracker)
tracker->OnAddEntry(entry);
}
+void PassiveLogCollector::Clear() {
+ AssertNetLogLockAcquired();
+ for (size_t i = 0; i < arraysize(trackers_); ++i)
+ trackers_[i]->Clear();
+}
+
PassiveLogCollector::SourceTrackerInterface*
-PassiveLogCollector::GetTrackerForSourceType(
+PassiveLogCollector::GetTrackerForSourceType_(
net::NetLog::SourceType source_type) {
DCHECK_LE(source_type, static_cast<int>(arraysize(trackers_)));
DCHECK_GE(source_type, 0);
return trackers_[source_type];
}
-void PassiveLogCollector::Clear() {
- for (size_t i = 0; i < arraysize(trackers_); ++i)
- trackers_[i]->Clear();
-}
-
-void PassiveLogCollector::GetAllCapturedEvents(EntryList* out) const {
+void PassiveLogCollector::GetAllCapturedEvents(
+ ChromeNetLog::EntryList* out) const {
+ AssertNetLogLockAcquired();
out->clear();
// Append all of the captured entries held by the various trackers to
@@ -137,7 +127,7 @@ std::string PassiveLogCollector::SourceInfo::GetURL() const {
// Note: we look at the first *two* entries, since the outer REQUEST_ALIVE
// doesn't actually contain any data.
for (size_t i = 0; i < 2 && i < entries.size(); ++i) {
- const PassiveLogCollector::Entry& entry = entries[i];
+ const ChromeNetLog::Entry& entry = entries[i];
if (entry.phase == net::NetLog::PHASE_BEGIN && entry.params) {
switch (entry.type) {
case net::NetLog::TYPE_URL_REQUEST_START_JOB:
@@ -161,7 +151,8 @@ std::string PassiveLogCollector::SourceInfo::GetURL() const {
PassiveLogCollector::GlobalSourceTracker::GlobalSourceTracker() {}
PassiveLogCollector::GlobalSourceTracker::~GlobalSourceTracker() {}
-void PassiveLogCollector::GlobalSourceTracker::OnAddEntry(const Entry& entry) {
+void PassiveLogCollector::GlobalSourceTracker::OnAddEntry(
+ const ChromeNetLog::Entry& entry) {
const size_t kMaxEntries = 30u;
entries_.push_back(entry);
if (entries_.size() > kMaxEntries)
@@ -173,7 +164,7 @@ void PassiveLogCollector::GlobalSourceTracker::Clear() {
}
void PassiveLogCollector::GlobalSourceTracker::AppendAllEntries(
- EntryList* out) const {
+ ChromeNetLog::EntryList* out) const {
out->insert(out->end(), entries_.begin(), entries_.end());
}
@@ -192,7 +183,8 @@ PassiveLogCollector::SourceTracker::SourceTracker(
PassiveLogCollector::SourceTracker::~SourceTracker() {}
-void PassiveLogCollector::SourceTracker::OnAddEntry(const Entry& entry) {
+void PassiveLogCollector::SourceTracker::OnAddEntry(
+ const ChromeNetLog::Entry& entry) {
// Lookup or insert a new entry into the bounded map.
SourceIDToInfoMap::iterator it = sources_.find(entry.source.id);
if (it == sources_.end()) {
@@ -259,7 +251,7 @@ void PassiveLogCollector::SourceTracker::Clear() {
}
void PassiveLogCollector::SourceTracker::AppendAllEntries(
- EntryList* out) const {
+ ChromeNetLog::EntryList* out) const {
// Append all of the entries for each of the sources.
for (SourceIDToInfoMap::const_iterator it = sources_.begin();
it != sources_.end();
@@ -344,7 +336,7 @@ void PassiveLogCollector::SourceTracker::AddReferenceToSourceDependency(
DCHECK(parent_);
DCHECK_NE(source.type, net::NetLog::SOURCE_NONE);
SourceTracker* tracker = static_cast<SourceTracker*>(
- parent_->GetTrackerForSourceType(source.type));
+ parent_->GetTrackerForSourceType_(source.type));
DCHECK(tracker);
// Tell the owning tracker to increment the reference count of |source|.
@@ -366,7 +358,7 @@ PassiveLogCollector::SourceTracker::ReleaseAllReferencesToDependencies(
DCHECK(parent_);
DCHECK_NE(source.type, net::NetLog::SOURCE_NONE);
SourceTracker* tracker = static_cast<SourceTracker*>(
- parent_->GetTrackerForSourceType(source.type));
+ parent_->GetTrackerForSourceType_(source.type));
DCHECK(tracker);
// Tell the owning tracker to decrement the reference count of |source|.
@@ -389,8 +381,8 @@ PassiveLogCollector::ConnectJobTracker::ConnectJobTracker(
}
PassiveLogCollector::SourceTracker::Action
-PassiveLogCollector::ConnectJobTracker::DoAddEntry(const Entry& entry,
- SourceInfo* out_info) {
+PassiveLogCollector::ConnectJobTracker::DoAddEntry(
+ const ChromeNetLog::Entry& entry, SourceInfo* out_info) {
AddEntryToSourceInfo(entry, out_info);
if (entry.type == net::NetLog::TYPE_CONNECT_JOB_SET_SOCKET) {
@@ -420,7 +412,7 @@ PassiveLogCollector::SocketTracker::SocketTracker()
}
PassiveLogCollector::SourceTracker::Action
-PassiveLogCollector::SocketTracker::DoAddEntry(const Entry& entry,
+PassiveLogCollector::SocketTracker::DoAddEntry(const ChromeNetLog::Entry& entry,
SourceInfo* out_info) {
// TODO(eroman): aggregate the byte counts once truncation starts to happen,
// to summarize transaction read/writes for each SOCKET_IN_USE
@@ -452,8 +444,8 @@ PassiveLogCollector::RequestTracker::RequestTracker(PassiveLogCollector* parent)
}
PassiveLogCollector::SourceTracker::Action
-PassiveLogCollector::RequestTracker::DoAddEntry(const Entry& entry,
- SourceInfo* out_info) {
+PassiveLogCollector::RequestTracker::DoAddEntry(
+ const ChromeNetLog::Entry& entry, SourceInfo* out_info) {
if (entry.type == net::NetLog::TYPE_SOCKET_POOL_BOUND_TO_CONNECT_JOB ||
entry.type == net::NetLog::TYPE_SOCKET_POOL_BOUND_TO_SOCKET) {
const net::NetLog::Source& source_dependency =
@@ -491,7 +483,7 @@ PassiveLogCollector::InitProxyResolverTracker::InitProxyResolverTracker()
PassiveLogCollector::SourceTracker::Action
PassiveLogCollector::InitProxyResolverTracker::DoAddEntry(
- const Entry& entry, SourceInfo* out_info) {
+ const ChromeNetLog::Entry& entry, SourceInfo* out_info) {
AddEntryToSourceInfo(entry, out_info);
if (entry.type == net::NetLog::TYPE_INIT_PROXY_RESOLVER &&
entry.phase == net::NetLog::PHASE_END) {
@@ -513,8 +505,8 @@ PassiveLogCollector::SpdySessionTracker::SpdySessionTracker()
}
PassiveLogCollector::SourceTracker::Action
-PassiveLogCollector::SpdySessionTracker::DoAddEntry(const Entry& entry,
- SourceInfo* out_info) {
+PassiveLogCollector::SpdySessionTracker::DoAddEntry(
+ const ChromeNetLog::Entry& entry, SourceInfo* out_info) {
AddEntryToSourceInfo(entry, out_info);
if (entry.type == net::NetLog::TYPE_SPDY_SESSION &&
entry.phase == net::NetLog::PHASE_END) {
@@ -536,8 +528,8 @@ PassiveLogCollector::DNSRequestTracker::DNSRequestTracker()
}
PassiveLogCollector::SourceTracker::Action
-PassiveLogCollector::DNSRequestTracker::DoAddEntry(const Entry& entry,
- SourceInfo* out_info) {
+PassiveLogCollector::DNSRequestTracker::DoAddEntry(
+ const ChromeNetLog::Entry& entry, SourceInfo* out_info) {
AddEntryToSourceInfo(entry, out_info);
if (entry.type == net::NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST &&
entry.phase == net::NetLog::PHASE_END) {
@@ -559,7 +551,7 @@ PassiveLogCollector::DNSJobTracker::DNSJobTracker()
}
PassiveLogCollector::SourceTracker::Action
-PassiveLogCollector::DNSJobTracker::DoAddEntry(const Entry& entry,
+PassiveLogCollector::DNSJobTracker::DoAddEntry(const ChromeNetLog::Entry& entry,
SourceInfo* out_info) {
AddEntryToSourceInfo(entry, out_info);
if (entry.type == net::NetLog::TYPE_HOST_RESOLVER_IMPL_JOB &&
diff --git a/chrome/browser/net/passive_log_collector.h b/chrome/browser/net/passive_log_collector.h
index 164aa66..114e439 100644
--- a/chrome/browser/net/passive_log_collector.h
+++ b/chrome/browser/net/passive_log_collector.h
@@ -32,29 +32,12 @@
// The data captured by PassiveLogCollector is grouped by NetLog::Source, into
// a SourceInfo structure. These in turn are grouped by NetLog::SourceType, and
// owned by a SourceTracker instance for the specific source type.
-class PassiveLogCollector : public ChromeNetLog::Observer {
+//
+// The PassiveLogCollector is owned by the ChromeNetLog itself, and is not
+// thread safe. The ChromeNetLog is responsible for calling it in a thread safe
+// manner.
+class PassiveLogCollector : public ChromeNetLog::ThreadSafeObserver {
public:
- // This structure encapsulates all of the parameters of a captured event,
- // including an "order" field that identifies when it was captured relative
- // to other events.
- struct Entry {
- Entry(uint32 order,
- net::NetLog::EventType type,
- const base::TimeTicks& time,
- net::NetLog::Source source,
- net::NetLog::EventPhase phase,
- net::NetLog::EventParameters* params);
- ~Entry();
-
- uint32 order;
- net::NetLog::EventType type;
- base::TimeTicks time;
- net::NetLog::Source source;
- net::NetLog::EventPhase phase;
- scoped_refptr<net::NetLog::EventParameters> params;
- };
-
- typedef std::vector<Entry> EntryList;
typedef std::vector<net::NetLog::Source> SourceDependencyList;
struct SourceInfo {
@@ -67,7 +50,7 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
std::string GetURL() const;
uint32 source_id;
- EntryList entries;
+ ChromeNetLog::EntryList entries;
size_t num_entries_truncated;
// List of other sources which contain information relevant to this
@@ -93,13 +76,13 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
public:
virtual ~SourceTrackerInterface() {}
- virtual void OnAddEntry(const Entry& entry) = 0;
+ virtual void OnAddEntry(const ChromeNetLog::Entry& entry) = 0;
// Clears all the passively logged data from this tracker.
virtual void Clear() = 0;
// Appends all the captured entries to |out|. The ordering is undefined.
- virtual void AppendAllEntries(EntryList* out) const = 0;
+ virtual void AppendAllEntries(ChromeNetLog::EntryList* out) const = 0;
};
// This source tracker is intended for TYPE_NONE. All entries go into a
@@ -110,12 +93,12 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
~GlobalSourceTracker();
// SourceTrackerInterface implementation:
- virtual void OnAddEntry(const Entry& entry);
+ virtual void OnAddEntry(const ChromeNetLog::Entry& entry);
virtual void Clear();
- virtual void AppendAllEntries(EntryList* out) const;
+ virtual void AppendAllEntries(ChromeNetLog::EntryList* out) const;
private:
- typedef std::deque<Entry> CircularEntryList;
+ typedef std::deque<ChromeNetLog::Entry> CircularEntryList;
CircularEntryList entries_;
DISALLOW_COPY_AND_ASSIGN(GlobalSourceTracker);
};
@@ -136,9 +119,9 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
virtual ~SourceTracker();
// SourceTrackerInterface implementation:
- virtual void OnAddEntry(const Entry& entry);
+ virtual void OnAddEntry(const ChromeNetLog::Entry& entry);
virtual void Clear();
- virtual void AppendAllEntries(EntryList* out) const;
+ virtual void AppendAllEntries(ChromeNetLog::EntryList* out) const;
#ifdef UNIT_TEST
// Helper used to inspect the current state by unit-tests.
@@ -172,7 +155,8 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
// Updates |out_info| with the information from |entry|. Returns an action
// to perform for this map entry on completion.
- virtual Action DoAddEntry(const Entry& entry, SourceInfo* out_info) = 0;
+ virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
+ SourceInfo* out_info) = 0;
// Removes |source_id| from |sources_|. This also releases any references
// to dependencies held by this source.
@@ -217,7 +201,8 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
explicit ConnectJobTracker(PassiveLogCollector* parent);
protected:
- virtual Action DoAddEntry(const Entry& entry, SourceInfo* out_info);
+ virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
+ SourceInfo* out_info);
private:
DISALLOW_COPY_AND_ASSIGN(ConnectJobTracker);
};
@@ -231,7 +216,8 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
SocketTracker();
protected:
- virtual Action DoAddEntry(const Entry& entry, SourceInfo* out_info);
+ virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
+ SourceInfo* out_info);
private:
DISALLOW_COPY_AND_ASSIGN(SocketTracker);
@@ -246,7 +232,8 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
explicit RequestTracker(PassiveLogCollector* parent);
protected:
- virtual Action DoAddEntry(const Entry& entry, SourceInfo* out_info);
+ virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
+ SourceInfo* out_info);
private:
DISALLOW_COPY_AND_ASSIGN(RequestTracker);
@@ -262,7 +249,8 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
InitProxyResolverTracker();
protected:
- virtual Action DoAddEntry(const Entry& entry, SourceInfo* out_info);
+ virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
+ SourceInfo* out_info);
private:
DISALLOW_COPY_AND_ASSIGN(InitProxyResolverTracker);
@@ -277,7 +265,8 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
SpdySessionTracker();
protected:
- virtual Action DoAddEntry(const Entry& entry, SourceInfo* out_info);
+ virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
+ SourceInfo* out_info);
private:
DISALLOW_COPY_AND_ASSIGN(SpdySessionTracker);
@@ -292,7 +281,8 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
DNSRequestTracker();
protected:
- virtual Action DoAddEntry(const Entry& entry, SourceInfo* out_info);
+ virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
+ SourceInfo* out_info);
private:
DISALLOW_COPY_AND_ASSIGN(DNSRequestTracker);
@@ -307,7 +297,8 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
DNSJobTracker();
protected:
- virtual Action DoAddEntry(const Entry& entry, SourceInfo* out_info);
+ virtual Action DoAddEntry(const ChromeNetLog::Entry& entry,
+ SourceInfo* out_info);
private:
DISALLOW_COPY_AND_ASSIGN(DNSJobTracker);
@@ -316,25 +307,25 @@ class PassiveLogCollector : public ChromeNetLog::Observer {
PassiveLogCollector();
~PassiveLogCollector();
- // Observer implementation:
+ // 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);
- // Returns the tracker to use for sources of type |source_type|, or NULL.
- SourceTrackerInterface* GetTrackerForSourceType(
- net::NetLog::SourceType source_type);
-
// Clears all of the passively logged data.
void Clear();
// Fills |out| with the full list of events that have been passively
// captured. The list is ordered by capture time.
- void GetAllCapturedEvents(EntryList* out) const;
+ void GetAllCapturedEvents(ChromeNetLog::EntryList* out) const;
private:
+ // Returns the tracker to use for sources of type |source_type|, or NULL.
+ SourceTrackerInterface* GetTrackerForSourceType_(
+ net::NetLog::SourceType source_type);
+
FRIEND_TEST_ALL_PREFIXES(PassiveLogCollectorTest,
HoldReferenceToDependentSource);
FRIEND_TEST_ALL_PREFIXES(PassiveLogCollectorTest,
diff --git a/chrome/browser/net/passive_log_collector_unittest.cc b/chrome/browser/net/passive_log_collector_unittest.cc
index e7d6074..fb2a1a2 100644
--- a/chrome/browser/net/passive_log_collector_unittest.cc
+++ b/chrome/browser/net/passive_log_collector_unittest.cc
@@ -19,9 +19,9 @@ using net::NetLog;
const NetLog::SourceType kSourceType = NetLog::SOURCE_NONE;
-PassiveLogCollector::Entry MakeStartLogEntryWithURL(int source_id,
- const std::string& url) {
- return PassiveLogCollector::Entry(
+ChromeNetLog::Entry MakeStartLogEntryWithURL(int source_id,
+ const std::string& url) {
+ return ChromeNetLog::Entry(
0,
NetLog::TYPE_URL_REQUEST_START_JOB,
base::TimeTicks(),
@@ -30,13 +30,13 @@ PassiveLogCollector::Entry MakeStartLogEntryWithURL(int source_id,
new URLRequestStartEventParameters(GURL(url), "GET", 0, net::LOW));
}
-PassiveLogCollector::Entry MakeStartLogEntry(int source_id) {
+ChromeNetLog::Entry MakeStartLogEntry(int source_id) {
return MakeStartLogEntryWithURL(source_id,
StringPrintf("http://req%d", source_id));
}
-PassiveLogCollector::Entry MakeEndLogEntry(int source_id) {
- return PassiveLogCollector::Entry(
+ChromeNetLog::Entry MakeEndLogEntry(int source_id) {
+ return ChromeNetLog::Entry(
0,
NetLog::TYPE_REQUEST_ALIVE,
base::TimeTicks(),
@@ -176,7 +176,7 @@ TEST(SpdySessionTracker, MovesToGraveyard) {
EXPECT_EQ(0u, GetLiveSources(tracker).size());
EXPECT_EQ(0u, GetDeadSources(tracker).size());
- PassiveLogCollector::Entry begin(
+ ChromeNetLog::Entry begin(
0u,
NetLog::TYPE_SPDY_SESSION,
base::TimeTicks(),
@@ -188,7 +188,7 @@ TEST(SpdySessionTracker, MovesToGraveyard) {
EXPECT_EQ(1u, GetLiveSources(tracker).size());
EXPECT_EQ(0u, GetDeadSources(tracker).size());
- PassiveLogCollector::Entry end(
+ ChromeNetLog::Entry end(
0u,
NetLog::TYPE_SPDY_SESSION,
base::TimeTicks(),
diff --git a/chrome/browser/resources/net_internals/main.js b/chrome/browser/resources/net_internals/main.js
index 7bfdb62..b85bc55 100644
--- a/chrome/browser/resources/net_internals/main.js
+++ b/chrome/browser/resources/net_internals/main.js
@@ -308,9 +308,6 @@ BrowserBridge.prototype.setLogLevel = function(logLevel) {
//------------------------------------------------------------------------------
BrowserBridge.prototype.receivedLogEntry = function(logEntry) {
- // Silently drop entries received before ready to receive them.
- if (!this.areLogTypesReady_())
- return;
// Assign unique ID, if needed.
if (logEntry.source.id == 0) {
logEntry.source.id = this.nextSourcelessEventId_;
@@ -388,12 +385,27 @@ BrowserBridge.prototype.receivedServiceProviders = function(serviceProviders) {
};
BrowserBridge.prototype.receivedPassiveLogEntries = function(entries) {
- this.numPassivelyCapturedEvents_ += entries.length;
+ // Due to an expected race condition, it is possible to receive actively
+ // captured log entries before the passively logged entries are received.
+ //
+ // When that happens, we create a copy of the actively logged entries, delete
+ // all entries, and, after handling all the passively logged entries, add back
+ // the deleted actively logged entries.
+ var earlyActivelyCapturedEvents = this.capturedEvents_.slice(0);
+ if (earlyActivelyCapturedEvents.length > 0)
+ this.deleteAllEvents();
+
+ this.numPassivelyCapturedEvents_ = entries.length;
for (var i = 0; i < entries.length; ++i) {
var entry = entries[i];
entry.wasPassivelyCaptured = true;
this.receivedLogEntry(entry);
}
+
+ // Add back early actively captured events, if any.
+ for (var i = 0; i < earlyActivelyCapturedEvents.length; ++i) {
+ this.receivedLogEntry(earlyActivelyCapturedEvents[i]);
+ }
};
@@ -427,12 +439,6 @@ BrowserBridge.prototype.receivedHttpCacheInfo = function(info) {
this.pollableDataHelpers_.httpCacheInfo.update(info);
};
-BrowserBridge.prototype.areLogTypesReady_ = function() {
- return (LogEventType != null &&
- LogEventPhase != null &&
- LogSourceType != null);
-};
-
//------------------------------------------------------------------------------
/**
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index c30b7ba..431c87e 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1390,6 +1390,7 @@
'browser/net/connection_tester_unittest.cc',
'browser/net/gaia/token_service_unittest.cc',
'browser/net/gaia/token_service_unittest.h',
+ 'browser/net/chrome_net_log_unittest.cc',
'browser/net/load_timing_observer_unittest.cc',
'browser/net/passive_log_collector_unittest.cc',
'browser/net/predictor_unittest.cc',