diff options
author | mmenke@chromium.org <mmenke@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-01 15:19:40 +0000 |
---|---|---|
committer | mmenke@chromium.org <mmenke@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-12-01 15:19:40 +0000 |
commit | b2fcd0e0c8850eda2135b6fe270af5f453682f23 (patch) | |
tree | 2858497a3551d59093a5dfd233bf3550d40a4e8d | |
parent | 087a03f6aeded6397081304e191b1ac027b8e134 (diff) | |
download | chromium_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
47 files changed, 977 insertions, 785 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', diff --git a/net/base/capturing_net_log.cc b/net/base/capturing_net_log.cc index c488e06..e785990 100644 --- a/net/base/capturing_net_log.cc +++ b/net/base/capturing_net_log.cc @@ -18,7 +18,7 @@ CapturingNetLog::Entry::Entry(EventType type, CapturingNetLog::Entry::~Entry() {} CapturingNetLog::CapturingNetLog(size_t max_num_entries) - : next_id_(0), max_num_entries_(max_num_entries) { + : last_id_(-1), max_num_entries_(max_num_entries) { } CapturingNetLog::~CapturingNetLog() {} @@ -28,16 +28,23 @@ void CapturingNetLog::AddEntry(EventType type, const Source& source, EventPhase phase, EventParameters* extra_parameters) { + AutoLock lock(lock_); Entry entry(type, time, source, phase, extra_parameters); if (entries_.size() + 1 < max_num_entries_) entries_.push_back(entry); } uint32 CapturingNetLog::NextID() { - return next_id_++; + return base::subtle::NoBarrier_AtomicIncrement(&last_id_, 1); +} + +void CapturingNetLog::GetEntries(EntryList* entry_list) const { + AutoLock lock(lock_); + *entry_list = entries_; } void CapturingNetLog::Clear() { + AutoLock lock(lock_); entries_.clear(); } @@ -51,16 +58,13 @@ CapturingBoundNetLog::CapturingBoundNetLog(size_t max_num_entries) CapturingBoundNetLog::~CapturingBoundNetLog() {} -void CapturingBoundNetLog::Clear() { - capturing_net_log_->Clear(); +void CapturingBoundNetLog::GetEntries( + CapturingNetLog::EntryList* entry_list) const { + capturing_net_log_->GetEntries(entry_list); } -void CapturingBoundNetLog::AppendTo(const BoundNetLog& net_log) const { - for (size_t i = 0; i < entries().size(); ++i) { - const CapturingNetLog::Entry& entry = entries()[i]; - net_log.AddEntryWithTime(entry.type, entry.time, entry.phase, - entry.extra_parameters); - } +void CapturingBoundNetLog::Clear() { + capturing_net_log_->Clear(); } } // namespace net diff --git a/net/base/capturing_net_log.h b/net/base/capturing_net_log.h index ff7cf16..3b6a39e 100644 --- a/net/base/capturing_net_log.h +++ b/net/base/capturing_net_log.h @@ -8,7 +8,9 @@ #include <vector> +#include "base/atomicops.h" #include "base/basictypes.h" +#include "base/lock.h" #include "base/ref_counted.h" #include "base/scoped_ptr.h" #include "base/time.h" @@ -55,12 +57,17 @@ class CapturingNetLog : public NetLog { virtual LogLevel GetLogLevel() const { return LOG_ALL_BUT_BYTES; } // Returns the list of all entries in the log. - const EntryList& entries() const { return entries_; } + void GetEntries(EntryList* entry_list) const; void Clear(); private: - uint32 next_id_; + // Needs to be "mutable" so can use it in GetEntries(). + mutable Lock lock_; + + // Last assigned source ID. Incremented to get the next one. + base::subtle::Atomic32 last_id_; + size_t max_num_entries_; EntryList entries_; @@ -85,17 +92,11 @@ class CapturingBoundNetLog { return BoundNetLog(source_, capturing_net_log_.get()); } - // Returns the list of all entries in the log. - const CapturingNetLog::EntryList& entries() const { - return capturing_net_log_->entries(); - } + // Fills |entry_list| with all entries in the log. + void GetEntries(CapturingNetLog::EntryList* entry_list) const; void Clear(); - // Sends all of captured messages to |net_log|, using the same source ID - // as |net_log|. - void AppendTo(const BoundNetLog& net_log) const; - private: NetLog::Source source_; scoped_ptr<CapturingNetLog> capturing_net_log_; diff --git a/net/base/forwarding_net_log.cc b/net/base/forwarding_net_log.cc deleted file mode 100644 index 7cfd6a9..0000000 --- a/net/base/forwarding_net_log.cc +++ /dev/null @@ -1,96 +0,0 @@ -// 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 "net/base/forwarding_net_log.h" - -#include "base/lock.h" -#include "base/logging.h" -#include "base/message_loop.h" - -namespace net { - -// Reference-counted wrapper, so we can use PostThread and it can safely -// outlive the parent ForwardingNetLog. -class ForwardingNetLog::Core - : public base::RefCountedThreadSafe<ForwardingNetLog::Core> { - public: - Core(NetLog* impl, MessageLoop* loop) : impl_(impl), loop_(loop) { - DCHECK(impl); - DCHECK(loop); - } - - // Called once the parent ForwardingNetLog is being destroyed. It - // is invalid to access |loop_| and |impl_| afterwards. - void Orphan() { - AutoLock l(lock_); - loop_ = NULL; - impl_ = NULL; - } - - void AddEntry(EventType type, - const base::TimeTicks& time, - const Source& source, - EventPhase phase, - EventParameters* params) { - AutoLock l(lock_); - if (!loop_) - return; // Was orphaned. - - loop_->PostTask( - FROM_HERE, - NewRunnableMethod( - this, &Core::AddEntryOnLoop, type, time, source, phase, - scoped_refptr<EventParameters>(params))); - } - - private: - void AddEntryOnLoop(EventType type, - const base::TimeTicks& time, - const Source& source, - EventPhase phase, - scoped_refptr<EventParameters> params) { - AutoLock l(lock_); - if (!loop_) - return; // Was orphaned. - - DCHECK_EQ(MessageLoop::current(), loop_); - - impl_->AddEntry(type, time, source, phase, params); - } - - Lock lock_; - NetLog* impl_; - MessageLoop* loop_; -}; - -ForwardingNetLog::ForwardingNetLog(NetLog* impl, MessageLoop* loop) - : core_(new Core(impl, loop)) { -} - -ForwardingNetLog::~ForwardingNetLog() { - core_->Orphan(); -} - -void ForwardingNetLog::AddEntry(EventType type, - const base::TimeTicks& time, - const Source& source, - EventPhase phase, - EventParameters* params) { - core_->AddEntry(type, time, source, phase, params); -} - -uint32 ForwardingNetLog::NextID() { - // Can't forward a synchronous API. - CHECK(false) << "Not supported"; - return 0; -} - -NetLog::LogLevel ForwardingNetLog::GetLogLevel() const { - // Can't forward a synchronous API. - CHECK(false) << "Not supported"; - return LOG_ALL_BUT_BYTES; -} - -} // namespace net - diff --git a/net/base/forwarding_net_log.h b/net/base/forwarding_net_log.h deleted file mode 100644 index 257b4c7..0000000 --- a/net/base/forwarding_net_log.h +++ /dev/null @@ -1,54 +0,0 @@ -// 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. - -#ifndef NET_BASE_FORWARDING_NET_LOG_H_ -#define NET_BASE_FORWARDING_NET_LOG_H_ -#pragma once - -#include "base/basictypes.h" -#include "net/base/net_log.h" - -class MessageLoop; - -namespace net { - -// ForwardingNetLog is a wrapper that can be called on any thread, and will -// forward any calls to NetLog::AddEntry() over to |impl| on the specified -// message loop. -// -// This allows using a non-threadsafe NetLog implementation from another -// thread. -// -// TODO(eroman): Explore making NetLog threadsafe and obviating the need -// for this class. -class ForwardingNetLog : public NetLog { - public: - // Both |impl| and |loop| must outlive the lifetime of this instance. - // |impl| will be operated only from |loop|. - ForwardingNetLog(NetLog* impl, MessageLoop* loop); - - // On destruction any outstanding call to AddEntry() which didn't make - // it to |loop| yet will be cancelled. - ~ForwardingNetLog(); - - // NetLog methods: - virtual void AddEntry(EventType type, - const base::TimeTicks& time, - const Source& source, - EventPhase phase, - EventParameters* params); - virtual uint32 NextID(); - virtual LogLevel GetLogLevel() const; - - private: - class Core; - scoped_refptr<Core> core_; - - DISALLOW_COPY_AND_ASSIGN(ForwardingNetLog); -}; - -} // namespace net - -#endif // NET_BASE_FORWARDING_NET_LOG_H_ - diff --git a/net/base/forwarding_net_log_unittest.cc b/net/base/forwarding_net_log_unittest.cc deleted file mode 100644 index 3f25129..0000000 --- a/net/base/forwarding_net_log_unittest.cc +++ /dev/null @@ -1,84 +0,0 @@ -// 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 "net/base/forwarding_net_log.h" - -#include "base/message_loop.h" -#include "net/base/capturing_net_log.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace net { - -namespace { - -// Test forwarding a call to AddEntry() to another implementation, operating -// on this same message loop. -TEST(ForwardingNetLogTest, Basic) { - // Create a forwarding NetLog that sends messages to this same thread. - CapturingNetLog log(CapturingNetLog::kUnbounded); - ForwardingNetLog forwarding(&log, MessageLoop::current()); - - EXPECT_EQ(0u, log.entries().size()); - - NetLogStringParameter* params = new NetLogStringParameter("xxx", "yyy"); - - forwarding.AddEntry( - NetLog::TYPE_PAC_JAVASCRIPT_ALERT, - base::TimeTicks(), - NetLog::Source(), - NetLog::PHASE_NONE, - params); - - // Should still be empty, since we posted an async task. - EXPECT_EQ(0u, log.entries().size()); - - MessageLoop::current()->RunAllPending(); - - // After draining the message loop, we should now have executed the task - // and hence emitted the log entry. - ASSERT_EQ(1u, log.entries().size()); - - // Check that the forwarded call contained received all the right inputs. - EXPECT_EQ(NetLog::TYPE_PAC_JAVASCRIPT_ALERT, log.entries()[0].type); - EXPECT_EQ(NetLog::SOURCE_NONE, log.entries()[0].source.type); - EXPECT_EQ(NetLog::PHASE_NONE, log.entries()[0].phase); - EXPECT_EQ(params, log.entries()[0].extra_parameters.get()); - - // Check that the parameters is still referenced. (if the reference was - // lost then this will be a memory error and probaby crash). - EXPECT_EQ("yyy", params->value()); -} - -// Test forwarding a call to AddEntry() to another implementation that runs -// on the same message loop. However destroy the forwarder before the posted -// task has a chance to run. -TEST(ForwardingNetLogTest, Orphan) { - // Create a forwarding NetLog that sends messages to this same thread. - CapturingNetLog log(CapturingNetLog::kUnbounded); - { - ForwardingNetLog forwarding(&log, MessageLoop::current()); - EXPECT_EQ(0u, log.entries().size()); - - forwarding.AddEntry( - NetLog::TYPE_PAC_JAVASCRIPT_ALERT, - base::TimeTicks(), - NetLog::Source(), - NetLog::PHASE_NONE, - NULL); - - // Should still be empty, since we posted an async task. - EXPECT_EQ(0u, log.entries().size()); - } - - // At this point the ForwardingNetLog is deleted. However it had already - // posted a task to the message loop. Once we drain the message loop, we - // verify that the task didn't actually try to emit to the NetLog. - MessageLoop::current()->RunAllPending(); - EXPECT_EQ(0u, log.entries().size()); -} - -} // namespace - -} // namespace net - diff --git a/net/base/host_resolver_impl_unittest.cc b/net/base/host_resolver_impl_unittest.cc index f3bdb74..4d26ab9 100644 --- a/net/base/host_resolver_impl_unittest.cc +++ b/net/base/host_resolver_impl_unittest.cc @@ -271,11 +271,14 @@ TEST_F(HostResolverImplTest, SynchronousLookup) { int err = host_resolver->Resolve(info, &addrlist, NULL, NULL, log.bound()); EXPECT_EQ(OK, err); - EXPECT_EQ(2u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(2u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_HOST_RESOLVER_IMPL)); + entries, 0, NetLog::TYPE_HOST_RESOLVER_IMPL)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 1, NetLog::TYPE_HOST_RESOLVER_IMPL)); + entries, 1, NetLog::TYPE_HOST_RESOLVER_IMPL)); const struct addrinfo* ainfo = addrlist.head(); EXPECT_EQ(static_cast<addrinfo*>(NULL), ainfo->ai_next); @@ -304,18 +307,23 @@ TEST_F(HostResolverImplTest, AsynchronousLookup) { log.bound()); EXPECT_EQ(ERR_IO_PENDING, err); - EXPECT_EQ(1u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(1u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_HOST_RESOLVER_IMPL)); + entries, 0, NetLog::TYPE_HOST_RESOLVER_IMPL)); MessageLoop::current()->Run(); ASSERT_TRUE(callback_called_); ASSERT_EQ(OK, callback_result_); - EXPECT_EQ(2u, log.entries().size()); + log.GetEntries(&entries); + + EXPECT_EQ(2u, entries.size()); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 1, NetLog::TYPE_HOST_RESOLVER_IMPL)); + entries, 1, NetLog::TYPE_HOST_RESOLVER_IMPL)); const struct addrinfo* ainfo = addrlist.head(); EXPECT_EQ(static_cast<addrinfo*>(NULL), ainfo->ai_next); @@ -356,31 +364,37 @@ TEST_F(HostResolverImplTest, CanceledAsynchronousLookup) { resolver_proc->Signal(); - EXPECT_EQ(2u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(2u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_HOST_RESOLVER_IMPL)); + entries, 0, NetLog::TYPE_HOST_RESOLVER_IMPL)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 1, NetLog::TYPE_HOST_RESOLVER_IMPL)); + entries, 1, NetLog::TYPE_HOST_RESOLVER_IMPL)); + + net::CapturingNetLog::EntryList net_log_entries; + net_log.GetEntries(&net_log_entries); - int pos = net::ExpectLogContainsSomewhereAfter(net_log.entries(), 0, + int pos = net::ExpectLogContainsSomewhereAfter(net_log_entries, 0, net::NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST, net::NetLog::PHASE_BEGIN); - pos = net::ExpectLogContainsSomewhereAfter(net_log.entries(), pos + 1, + pos = net::ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, net::NetLog::TYPE_HOST_RESOLVER_IMPL_JOB, net::NetLog::PHASE_BEGIN); // Both Job and Request need to be cancelled. - pos = net::ExpectLogContainsSomewhereAfter(net_log.entries(), pos + 1, + pos = net::ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, net::NetLog::TYPE_CANCELLED, net::NetLog::PHASE_NONE); // Don't care about order in which they end, or when the other one is // cancelled. - net::ExpectLogContainsSomewhereAfter(net_log.entries(), pos + 1, + net::ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, net::NetLog::TYPE_CANCELLED, net::NetLog::PHASE_NONE); - net::ExpectLogContainsSomewhereAfter(net_log.entries(), pos + 1, + net::ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, net::NetLog::TYPE_HOST_RESOLVER_IMPL_REQUEST, net::NetLog::PHASE_END); - net::ExpectLogContainsSomewhereAfter(net_log.entries(), pos + 1, + net::ExpectLogContainsSomewhereAfter(net_log_entries, pos + 1, net::NetLog::TYPE_HOST_RESOLVER_IMPL_JOB, net::NetLog::PHASE_END); @@ -943,11 +957,14 @@ TEST_F(HostResolverImplTest, Observers) { int rv = host_resolver->Resolve(info1, &addrlist, NULL, NULL, log.bound()); EXPECT_EQ(OK, rv); - EXPECT_EQ(2u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(2u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_HOST_RESOLVER_IMPL)); + entries, 0, NetLog::TYPE_HOST_RESOLVER_IMPL)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 1, NetLog::TYPE_HOST_RESOLVER_IMPL)); + entries, 1, NetLog::TYPE_HOST_RESOLVER_IMPL)); EXPECT_EQ(1U, observer.start_log.size()); EXPECT_EQ(1U, observer.finish_log.size()); diff --git a/net/base/net_log.h b/net/base/net_log.h index 9c670ec..5a8f197 100644 --- a/net/base/net_log.h +++ b/net/base/net_log.h @@ -29,12 +29,9 @@ namespace net { // is usually accessed through a BoundNetLog, which will always pass in a // specific source ID. // -// Note that NetLog is NOT THREADSAFE. -// // ******** The NetLog (and associated logging) is a work in progress ******** // // TODO(eroman): Remove the 'const' qualitifer from the BoundNetLog methods. -// TODO(eroman): Make the DNS jobs emit into the NetLog. // TODO(eroman): Start a new Source each time net::URLRequest redirects // (simpler to reason about each as a separate entity). diff --git a/net/http/http_auth_handler_unittest.cc b/net/http/http_auth_handler_unittest.cc index d63b9fb..c3d0114 100644 --- a/net/http/http_auth_handler_unittest.cc +++ b/net/http/http_auth_handler_unittest.cc @@ -49,11 +49,12 @@ TEST(HttpAuthHandlerTest, NetLog) { if (async) test_callback.WaitForResult(); - EXPECT_EQ(2u, capturing_net_log.entries().size()); - EXPECT_TRUE(LogContainsBeginEvent(capturing_net_log.entries(), - 0, event_type)); - EXPECT_TRUE(LogContainsEndEvent(capturing_net_log.entries(), - 1, event_type)); + net::CapturingNetLog::EntryList entries; + capturing_net_log.GetEntries(&entries); + + EXPECT_EQ(2u, entries.size()); + EXPECT_TRUE(LogContainsBeginEvent(entries, 0, event_type)); + EXPECT_TRUE(LogContainsEndEvent(entries, 1, event_type)); } } } diff --git a/net/http/http_cache_unittest.cc b/net/http/http_cache_unittest.cc index 6ca1da1..239f34e 100644 --- a/net/http/http_cache_unittest.cc +++ b/net/http/http_cache_unittest.cc @@ -1044,19 +1044,22 @@ TEST(HttpCache, SimpleGETNoDiskCache) { // Check that the NetLog was filled as expected. // (We attempted to both Open and Create entries, but both failed). - EXPECT_EQ(6u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(6u, entries.size()); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 0, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 0, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 1, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 1, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 2, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); + entries, 2, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 3, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); + entries, 3, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 4, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); + entries, 4, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 5, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); + entries, 5, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); EXPECT_EQ(1, cache.network_layer()->transaction_count()); EXPECT_EQ(0, cache.disk_cache()->open_count()); @@ -1145,23 +1148,26 @@ TEST(HttpCache, SimpleGET_LoadOnlyFromCache_Hit) { log.bound()); // Check that the NetLog was filled as expected. - EXPECT_EQ(8u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(8u, entries.size()); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 0, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 0, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 1, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 1, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 2, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); + entries, 2, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 3, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); + entries, 3, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 4, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); + entries, 4, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 5, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); + entries, 5, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 6, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 6, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 7, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 7, net::NetLog::TYPE_HTTP_CACHE_WAITING)); // force this transaction to read from the cache MockTransaction transaction(kSimpleGET_Transaction); @@ -1172,23 +1178,25 @@ TEST(HttpCache, SimpleGET_LoadOnlyFromCache_Hit) { RunTransactionTestWithLog(cache.http_cache(), transaction, log.bound()); // Check that the NetLog was filled as expected. - EXPECT_EQ(8u, log.entries().size()); + log.GetEntries(&entries); + + EXPECT_EQ(8u, entries.size()); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 0, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 0, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 1, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 1, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 2, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); + entries, 2, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 3, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); + entries, 3, net::NetLog::TYPE_HTTP_CACHE_OPEN_ENTRY)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 4, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 4, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 5, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 5, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 6, net::NetLog::TYPE_HTTP_CACHE_READ_INFO)); + entries, 6, net::NetLog::TYPE_HTTP_CACHE_READ_INFO)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 7, net::NetLog::TYPE_HTTP_CACHE_READ_INFO)); + entries, 7, net::NetLog::TYPE_HTTP_CACHE_READ_INFO)); EXPECT_EQ(1, cache.network_layer()->transaction_count()); EXPECT_EQ(1, cache.disk_cache()->open_count()); @@ -1268,23 +1276,26 @@ TEST(HttpCache, SimpleGET_LoadBypassCache) { RunTransactionTestWithLog(cache.http_cache(), transaction, log.bound()); // Check that the NetLog was filled as expected. - EXPECT_EQ(8u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(8u, entries.size()); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 0, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 0, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 1, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 1, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 2, net::NetLog::TYPE_HTTP_CACHE_DOOM_ENTRY)); + entries, 2, net::NetLog::TYPE_HTTP_CACHE_DOOM_ENTRY)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 3, net::NetLog::TYPE_HTTP_CACHE_DOOM_ENTRY)); + entries, 3, net::NetLog::TYPE_HTTP_CACHE_DOOM_ENTRY)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 4, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); + entries, 4, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 5, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); + entries, 5, net::NetLog::TYPE_HTTP_CACHE_CREATE_ENTRY)); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 6, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 6, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_TRUE(net::LogContainsEndEvent( - log.entries(), 7, net::NetLog::TYPE_HTTP_CACHE_WAITING)); + entries, 7, net::NetLog::TYPE_HTTP_CACHE_WAITING)); EXPECT_EQ(2, cache.network_layer()->transaction_count()); EXPECT_EQ(0, cache.disk_cache()->open_count()); diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc index f765696..95a8599 100644 --- a/net/http/http_network_transaction_unittest.cc +++ b/net/http/http_network_transaction_unittest.cc @@ -174,15 +174,18 @@ class HttpNetworkTransactionTest : public PlatformTest { rv = ReadTransaction(trans.get(), &out.response_data); EXPECT_EQ(OK, rv); + + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); size_t pos = ExpectLogContainsSomewhere( - log.entries(), 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_REQUEST_HEADERS, + entries, 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_REQUEST_HEADERS, NetLog::PHASE_NONE); ExpectLogContainsSomewhere( - log.entries(), pos, + entries, pos, NetLog::TYPE_HTTP_TRANSACTION_READ_RESPONSE_HEADERS, NetLog::PHASE_NONE); - CapturingNetLog::Entry entry = log.entries()[pos]; + CapturingNetLog::Entry entry = entries[pos]; NetLogHttpRequestParameter* request_params = static_cast<NetLogHttpRequestParameter*>(entry.extra_parameters.get()); EXPECT_EQ("GET / HTTP/1.1\r\n", request_params->GetLine()); @@ -1525,11 +1528,13 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyNoKeepAlive) { rv = callback1.WaitForResult(); EXPECT_EQ(OK, rv); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); size_t pos = ExpectLogContainsSomewhere( - log.entries(), 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, + entries, 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, NetLog::PHASE_NONE); ExpectLogContainsSomewhere( - log.entries(), pos, + entries, pos, NetLog::TYPE_HTTP_TRANSACTION_READ_TUNNEL_RESPONSE_HEADERS, NetLog::PHASE_NONE); @@ -1629,11 +1634,13 @@ TEST_F(HttpNetworkTransactionTest, BasicAuthProxyKeepAlive) { rv = callback1.WaitForResult(); EXPECT_EQ(OK, rv); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); size_t pos = ExpectLogContainsSomewhere( - log.entries(), 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, + entries, 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, NetLog::PHASE_NONE); ExpectLogContainsSomewhere( - log.entries(), pos, + entries, pos, NetLog::TYPE_HTTP_TRANSACTION_READ_TUNNEL_RESPONSE_HEADERS, NetLog::PHASE_NONE); @@ -1831,11 +1838,13 @@ TEST_F(HttpNetworkTransactionTest, HttpsServerRequestsProxyAuthThroughProxy) { rv = callback1.WaitForResult(); EXPECT_EQ(ERR_UNEXPECTED_PROXY_AUTH, rv); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); size_t pos = ExpectLogContainsSomewhere( - log.entries(), 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, + entries, 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, NetLog::PHASE_NONE); ExpectLogContainsSomewhere( - log.entries(), pos, + entries, pos, NetLog::TYPE_HTTP_TRANSACTION_READ_TUNNEL_RESPONSE_HEADERS, NetLog::PHASE_NONE); } @@ -7724,11 +7733,13 @@ TEST_F(HttpNetworkTransactionTest, ProxyTunnelGet) { rv = callback1.WaitForResult(); EXPECT_EQ(OK, rv); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); size_t pos = ExpectLogContainsSomewhere( - log.entries(), 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, + entries, 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, NetLog::PHASE_NONE); ExpectLogContainsSomewhere( - log.entries(), pos, + entries, pos, NetLog::TYPE_HTTP_TRANSACTION_READ_TUNNEL_RESPONSE_HEADERS, NetLog::PHASE_NONE); @@ -7786,11 +7797,13 @@ TEST_F(HttpNetworkTransactionTest, ProxyTunnelGetHangup) { rv = callback1.WaitForResult(); EXPECT_EQ(ERR_EMPTY_RESPONSE, rv); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); size_t pos = ExpectLogContainsSomewhere( - log.entries(), 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, + entries, 0, NetLog::TYPE_HTTP_TRANSACTION_SEND_TUNNEL_HEADERS, NetLog::PHASE_NONE); ExpectLogContainsSomewhere( - log.entries(), pos, + entries, pos, NetLog::TYPE_HTTP_TRANSACTION_READ_TUNNEL_RESPONSE_HEADERS, NetLog::PHASE_NONE); } diff --git a/net/net.gyp b/net/net.gyp index a5134da..bf4e2d6 100644 --- a/net/net.gyp +++ b/net/net.gyp @@ -76,8 +76,6 @@ 'base/file_stream_win.cc', 'base/filter.cc', 'base/filter.h', - 'base/forwarding_net_log.cc', - 'base/forwarding_net_log.h', 'base/gzip_filter.cc', 'base/gzip_filter.h', 'base/gzip_header.cc', @@ -839,7 +837,6 @@ 'base/file_stream_unittest.cc', 'base/filter_unittest.cc', 'base/filter_unittest.h', - 'base/forwarding_net_log_unittest.cc', 'base/gzip_filter_unittest.cc', 'base/host_cache_unittest.cc', 'base/host_mapping_rules_unittest.cc', diff --git a/net/proxy/init_proxy_resolver_unittest.cc b/net/proxy/init_proxy_resolver_unittest.cc index 0c7e8a1..f9858614 100644 --- a/net/proxy/init_proxy_resolver_unittest.cc +++ b/net/proxy/init_proxy_resolver_unittest.cc @@ -181,19 +181,22 @@ TEST(InitProxyResolverTest, CustomPacSucceeds) { EXPECT_EQ(rule.text(), resolver.script_data()->utf16()); // Check the NetLog was filled correctly. - EXPECT_EQ(6u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(6u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_INIT_PROXY_RESOLVER)); + entries, 0, NetLog::TYPE_INIT_PROXY_RESOLVER)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 3, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); + entries, 3, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 4, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); + entries, 4, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 5, NetLog::TYPE_INIT_PROXY_RESOLVER)); + entries, 5, NetLog::TYPE_INIT_PROXY_RESOLVER)); } // Fail downloading the custom PAC script. @@ -215,15 +218,18 @@ TEST(InitProxyResolverTest, CustomPacFails1) { EXPECT_EQ(NULL, resolver.script_data()); // Check the NetLog was filled correctly. - EXPECT_EQ(4u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(4u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_INIT_PROXY_RESOLVER)); + entries, 0, NetLog::TYPE_INIT_PROXY_RESOLVER)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 3, NetLog::TYPE_INIT_PROXY_RESOLVER)); + entries, 3, NetLog::TYPE_INIT_PROXY_RESOLVER)); } // Fail parsing the custom PAC script. @@ -326,31 +332,34 @@ TEST(InitProxyResolverTest, AutodetectFailCustomSuccess2) { // Check the NetLog was filled correctly. // (Note that the Fetch and Set states are repeated since both WPAD and custom // PAC scripts are tried). - EXPECT_EQ(11u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(11u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_INIT_PROXY_RESOLVER)); + entries, 0, NetLog::TYPE_INIT_PROXY_RESOLVER)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 3, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); + entries, 3, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 4, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); + entries, 4, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEvent( - log.entries(), 5, + entries, 5, NetLog::TYPE_INIT_PROXY_RESOLVER_FALLING_BACK_TO_NEXT_PAC_URL, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 6, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 6, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 7, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 7, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 8, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); + entries, 8, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 9, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); + entries, 9, NetLog::TYPE_INIT_PROXY_RESOLVER_SET_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 10, NetLog::TYPE_INIT_PROXY_RESOLVER)); + entries, 10, NetLog::TYPE_INIT_PROXY_RESOLVER)); } // Fails at WPAD (downloading), and fails at custom PAC (downloading). @@ -438,19 +447,22 @@ TEST(InitProxyResolverTest, CustomPacFails1_WithPositiveDelay) { EXPECT_EQ(NULL, resolver.script_data()); // Check the NetLog was filled correctly. - EXPECT_EQ(6u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(6u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_INIT_PROXY_RESOLVER)); + entries, 0, NetLog::TYPE_INIT_PROXY_RESOLVER)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 1, NetLog::TYPE_INIT_PROXY_RESOLVER_WAIT)); + entries, 1, NetLog::TYPE_INIT_PROXY_RESOLVER_WAIT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 2, NetLog::TYPE_INIT_PROXY_RESOLVER_WAIT)); + entries, 2, NetLog::TYPE_INIT_PROXY_RESOLVER_WAIT)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 3, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 3, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 4, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 4, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 5, NetLog::TYPE_INIT_PROXY_RESOLVER)); + entries, 5, NetLog::TYPE_INIT_PROXY_RESOLVER)); } // This is a copy-paste of CustomPacFails1, with the exception that we give it @@ -475,15 +487,18 @@ TEST(InitProxyResolverTest, CustomPacFails1_WithNegativeDelay) { EXPECT_EQ(NULL, resolver.script_data()); // Check the NetLog was filled correctly. - EXPECT_EQ(4u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(4u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_INIT_PROXY_RESOLVER)); + entries, 0, NetLog::TYPE_INIT_PROXY_RESOLVER)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 1, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); + entries, 2, NetLog::TYPE_INIT_PROXY_RESOLVER_FETCH_PAC_SCRIPT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 3, NetLog::TYPE_INIT_PROXY_RESOLVER)); + entries, 3, NetLog::TYPE_INIT_PROXY_RESOLVER)); } } // namespace diff --git a/net/proxy/multi_threaded_proxy_resolver.cc b/net/proxy/multi_threaded_proxy_resolver.cc index d696438a1..a373c1a 100644 --- a/net/proxy/multi_threaded_proxy_resolver.cc +++ b/net/proxy/multi_threaded_proxy_resolver.cc @@ -8,8 +8,8 @@ #include "base/string_util.h" #include "base/stringprintf.h" #include "base/thread.h" -#include "net/base/capturing_net_log.h" #include "net/base/net_errors.h" +#include "net/base/net_log.h" #include "net/proxy/proxy_info.h" // TODO(eroman): Have the MultiThreadedProxyResolver clear its PAC script @@ -253,13 +253,9 @@ class MultiThreadedProxyResolver::GetProxyForURLJob // Runs on the worker thread. virtual void Run(MessageLoop* origin_loop) { - const size_t kNetLogBound = 50u; - worker_log_.reset(new CapturingNetLog(kNetLogBound)); - BoundNetLog bound_worker_log(NetLog::Source(), worker_log_.get()); - ProxyResolver* resolver = executor()->resolver(); int rv = resolver->GetProxyForURL( - url_, &results_buf_, NULL, NULL, bound_worker_log); + url_, &results_buf_, NULL, NULL, net_log_); DCHECK_NE(rv, ERR_IO_PENDING); origin_loop->PostTask( @@ -272,12 +268,6 @@ class MultiThreadedProxyResolver::GetProxyForURLJob void QueryComplete(int result_code) { // The Job may have been cancelled after it was started. if (!was_cancelled()) { - // Merge the load log that was generated on the worker thread, into the - // main log. - CapturingBoundNetLog bound_worker_log(NetLog::Source(), - worker_log_.release()); - bound_worker_log.AppendTo(net_log_); - if (result_code >= OK) { // Note: unit-tests use values > 0. results_->Use(results_buf_); } @@ -288,16 +278,14 @@ class MultiThreadedProxyResolver::GetProxyForURLJob // Must only be used on the "origin" thread. ProxyInfo* results_; + + // Can be used on either "origin" or worker thread. BoundNetLog net_log_; const GURL url_; // Usable from within DoQuery on the worker thread. ProxyInfo results_buf_; - // Used to pass the captured events between DoQuery [worker thread] and - // QueryComplete [origin thread]. - scoped_ptr<CapturingNetLog> worker_log_; - bool was_waiting_for_thread_; }; diff --git a/net/proxy/multi_threaded_proxy_resolver_unittest.cc b/net/proxy/multi_threaded_proxy_resolver_unittest.cc index 7027161..6f168a7 100644 --- a/net/proxy/multi_threaded_proxy_resolver_unittest.cc +++ b/net/proxy/multi_threaded_proxy_resolver_unittest.cc @@ -260,9 +260,11 @@ TEST(MultiThreadedProxyResolverTest, SingleThread_Basic) { // on completion, this should have been copied into |log0|. // We also have 1 log entry that was emitted by the // MultiThreadedProxyResolver. - ASSERT_EQ(2u, log0.entries().size()); - EXPECT_EQ(NetLog::TYPE_SUBMITTED_TO_RESOLVER_THREAD, - log0.entries()[0].type); + net::CapturingNetLog::EntryList entries0; + log0.GetEntries(&entries0); + + ASSERT_EQ(2u, entries0.size()); + EXPECT_EQ(NetLog::TYPE_SUBMITTED_TO_RESOLVER_THREAD, entries0[0].type); // Start 3 more requests (request1 to request3). @@ -368,30 +370,42 @@ TEST(MultiThreadedProxyResolverTest, // 1 entry from the mock proxy resolver. EXPECT_EQ(0, callback0.WaitForResult()); EXPECT_EQ("PROXY request0:80", results0.ToPacString()); - ASSERT_EQ(2u, log0.entries().size()); + + net::CapturingNetLog::EntryList entries0; + log0.GetEntries(&entries0); + + ASSERT_EQ(2u, entries0.size()); EXPECT_EQ(NetLog::TYPE_SUBMITTED_TO_RESOLVER_THREAD, - log0.entries()[0].type); + entries0[0].type); // Check that request 1 completed as expected. EXPECT_EQ(1, callback1.WaitForResult()); EXPECT_EQ("PROXY request1:80", results1.ToPacString()); - ASSERT_EQ(4u, log1.entries().size()); + + net::CapturingNetLog::EntryList entries1; + log1.GetEntries(&entries1); + + ASSERT_EQ(4u, entries1.size()); EXPECT_TRUE(LogContainsBeginEvent( - log1.entries(), 0, + entries1, 0, NetLog::TYPE_WAITING_FOR_PROXY_RESOLVER_THREAD)); EXPECT_TRUE(LogContainsEndEvent( - log1.entries(), 1, + entries1, 1, NetLog::TYPE_WAITING_FOR_PROXY_RESOLVER_THREAD)); // Check that request 2 completed as expected. EXPECT_EQ(2, callback2.WaitForResult()); EXPECT_EQ("PROXY request2:80", results2.ToPacString()); - ASSERT_EQ(4u, log2.entries().size()); + + net::CapturingNetLog::EntryList entries2; + log2.GetEntries(&entries2); + + ASSERT_EQ(4u, entries2.size()); EXPECT_TRUE(LogContainsBeginEvent( - log2.entries(), 0, + entries2, 0, NetLog::TYPE_WAITING_FOR_PROXY_RESOLVER_THREAD)); EXPECT_TRUE(LogContainsEndEvent( - log2.entries(), 1, + entries2, 1, NetLog::TYPE_WAITING_FOR_PROXY_RESOLVER_THREAD)); } diff --git a/net/proxy/proxy_resolver_js_bindings_unittest.cc b/net/proxy/proxy_resolver_js_bindings_unittest.cc index 6b33a34..ecb36ca 100644 --- a/net/proxy/proxy_resolver_js_bindings_unittest.cc +++ b/net/proxy/proxy_resolver_js_bindings_unittest.cc @@ -304,65 +304,82 @@ TEST(ProxyResolverJSBindingsTest, NetLog) { bindings->set_current_request_context(&context); std::string ip_address; - - ASSERT_EQ(0u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + ASSERT_EQ(0u, entries.size()); // Call all the bindings. Each call should be logging something to // our NetLog. bindings->MyIpAddress(&ip_address); - EXPECT_EQ(2u, log.entries().size()); + + log.GetEntries(&entries); + EXPECT_EQ(2u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_PAC_JAVASCRIPT_MY_IP_ADDRESS)); + entries, 0, NetLog::TYPE_PAC_JAVASCRIPT_MY_IP_ADDRESS)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 1, NetLog::TYPE_PAC_JAVASCRIPT_MY_IP_ADDRESS)); + entries, 1, NetLog::TYPE_PAC_JAVASCRIPT_MY_IP_ADDRESS)); bindings->MyIpAddressEx(&ip_address); - EXPECT_EQ(4u, log.entries().size()); + + log.GetEntries(&entries); + EXPECT_EQ(4u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 2, NetLog::TYPE_PAC_JAVASCRIPT_MY_IP_ADDRESS_EX)); + entries, 2, NetLog::TYPE_PAC_JAVASCRIPT_MY_IP_ADDRESS_EX)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 3, NetLog::TYPE_PAC_JAVASCRIPT_MY_IP_ADDRESS_EX)); + entries, 3, NetLog::TYPE_PAC_JAVASCRIPT_MY_IP_ADDRESS_EX)); bindings->DnsResolve("foo", &ip_address); - EXPECT_EQ(6u, log.entries().size()); + + log.GetEntries(&entries); + EXPECT_EQ(6u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 4, NetLog::TYPE_PAC_JAVASCRIPT_DNS_RESOLVE)); + entries, 4, NetLog::TYPE_PAC_JAVASCRIPT_DNS_RESOLVE)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 5, NetLog::TYPE_PAC_JAVASCRIPT_DNS_RESOLVE)); + entries, 5, NetLog::TYPE_PAC_JAVASCRIPT_DNS_RESOLVE)); bindings->DnsResolveEx("foo", &ip_address); - EXPECT_EQ(8u, log.entries().size()); + + log.GetEntries(&entries); + EXPECT_EQ(8u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 6, NetLog::TYPE_PAC_JAVASCRIPT_DNS_RESOLVE_EX)); + entries, 6, NetLog::TYPE_PAC_JAVASCRIPT_DNS_RESOLVE_EX)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 7, NetLog::TYPE_PAC_JAVASCRIPT_DNS_RESOLVE_EX)); + entries, 7, NetLog::TYPE_PAC_JAVASCRIPT_DNS_RESOLVE_EX)); // Nothing has been emitted globally yet. - EXPECT_EQ(0u, global_log.entries().size()); + net::CapturingNetLog::EntryList global_log_entries; + global_log.GetEntries(&global_log_entries); + EXPECT_EQ(0u, global_log_entries.size()); bindings->OnError(30, string16()); - EXPECT_EQ(9u, log.entries().size()); + + log.GetEntries(&entries); + EXPECT_EQ(9u, entries.size()); EXPECT_TRUE(LogContainsEvent( - log.entries(), 8, NetLog::TYPE_PAC_JAVASCRIPT_ERROR, + entries, 8, NetLog::TYPE_PAC_JAVASCRIPT_ERROR, NetLog::PHASE_NONE)); // We also emit errors to the top-level log stream. - EXPECT_EQ(1u, global_log.entries().size()); + global_log.GetEntries(&global_log_entries); + EXPECT_EQ(1u, global_log_entries.size()); EXPECT_TRUE(LogContainsEvent( - global_log.entries(), 0, NetLog::TYPE_PAC_JAVASCRIPT_ERROR, + global_log_entries, 0, NetLog::TYPE_PAC_JAVASCRIPT_ERROR, NetLog::PHASE_NONE)); bindings->Alert(string16()); - EXPECT_EQ(10u, log.entries().size()); + + log.GetEntries(&entries); + EXPECT_EQ(10u, entries.size()); EXPECT_TRUE(LogContainsEvent( - log.entries(), 9, NetLog::TYPE_PAC_JAVASCRIPT_ALERT, + entries, 9, NetLog::TYPE_PAC_JAVASCRIPT_ALERT, NetLog::PHASE_NONE)); // We also emit javascript alerts to the top-level log stream. - EXPECT_EQ(2u, global_log.entries().size()); + global_log.GetEntries(&global_log_entries); + EXPECT_EQ(2u, global_log_entries.size()); EXPECT_TRUE(LogContainsEvent( - global_log.entries(), 1, NetLog::TYPE_PAC_JAVASCRIPT_ALERT, + global_log_entries, 1, NetLog::TYPE_PAC_JAVASCRIPT_ALERT, NetLog::PHASE_NONE)); } diff --git a/net/proxy/proxy_resolver_v8_unittest.cc b/net/proxy/proxy_resolver_v8_unittest.cc index 3e61cd7..eff0413 100644 --- a/net/proxy/proxy_resolver_v8_unittest.cc +++ b/net/proxy/proxy_resolver_v8_unittest.cc @@ -138,8 +138,10 @@ TEST(ProxyResolverV8Test, Direct) { EXPECT_EQ(0U, resolver.mock_js_bindings()->alerts.size()); EXPECT_EQ(0U, resolver.mock_js_bindings()->errors.size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); // No bindings were called, so no log entries. - EXPECT_EQ(0u, log.entries().size()); + EXPECT_EQ(0u, entries.size()); } TEST(ProxyResolverV8Test, ReturnEmptyString) { diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc index beeab51..669ae083 100644 --- a/net/proxy/proxy_service.cc +++ b/net/proxy/proxy_service.cc @@ -12,7 +12,6 @@ #include "base/message_loop.h" #include "base/string_util.h" #include "googleurl/src/gurl.h" -#include "net/base/forwarding_net_log.h" #include "net/base/net_log.h" #include "net/base/net_errors.h" #include "net/base/net_util.h" @@ -157,16 +156,14 @@ class ProxyResolverFactoryForV8 : public ProxyResolverFactory { public: // |async_host_resolver|, |io_loop| and |net_log| must remain // valid for the duration of our lifetime. - // Both |async_host_resolver| and |net_log| will only be operated on - // |io_loop|. + // |async_host_resolver| will only be operated on |io_loop|. ProxyResolverFactoryForV8(HostResolver* async_host_resolver, MessageLoop* io_loop, NetLog* net_log) : ProxyResolverFactory(true /*expects_pac_bytes*/), async_host_resolver_(async_host_resolver), io_loop_(io_loop), - forwarding_net_log_( - net_log ? new ForwardingNetLog(net_log, io_loop) : NULL) { + net_log_(net_log) { } virtual ProxyResolver* CreateProxyResolver() { @@ -176,8 +173,7 @@ class ProxyResolverFactoryForV8 : public ProxyResolverFactory { new SyncHostResolverBridge(async_host_resolver_, io_loop_); ProxyResolverJSBindings* js_bindings = - ProxyResolverJSBindings::CreateDefault(sync_host_resolver, - forwarding_net_log_.get()); + ProxyResolverJSBindings::CreateDefault(sync_host_resolver, net_log_); // ProxyResolverV8 takes ownership of |js_bindings|. return new ProxyResolverV8(js_bindings); @@ -186,10 +182,7 @@ class ProxyResolverFactoryForV8 : public ProxyResolverFactory { private: HostResolver* const async_host_resolver_; MessageLoop* io_loop_; - - // Thread-safe wrapper around a non-threadsafe NetLog implementation. This - // enables the proxy resolver to emit log messages from the PAC thread. - scoped_ptr<ForwardingNetLog> forwarding_net_log_; + NetLog* net_log_; }; // Creates ProxyResolvers using a platform-specific implementation. diff --git a/net/proxy/proxy_service_unittest.cc b/net/proxy/proxy_service_unittest.cc index 1c77876..3c8ef2a 100644 --- a/net/proxy/proxy_service_unittest.cc +++ b/net/proxy/proxy_service_unittest.cc @@ -130,14 +130,17 @@ TEST(ProxyServiceTest, Direct) { EXPECT_TRUE(info.is_direct()); // Check the NetLog was filled correctly. - EXPECT_EQ(3u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(3u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_PROXY_SERVICE)); + entries, 0, NetLog::TYPE_PROXY_SERVICE)); EXPECT_TRUE(LogContainsEvent( - log.entries(), 1, NetLog::TYPE_PROXY_SERVICE_RESOLVED_PROXY_LIST, + entries, 1, NetLog::TYPE_PROXY_SERVICE_RESOLVED_PROXY_LIST, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 2, NetLog::TYPE_PROXY_SERVICE)); + entries, 2, NetLog::TYPE_PROXY_SERVICE)); } TEST(ProxyServiceTest, PAC) { @@ -174,15 +177,18 @@ TEST(ProxyServiceTest, PAC) { EXPECT_EQ("foopy:80", info.proxy_server().ToURI()); // Check the NetLog was filled correctly. - EXPECT_EQ(5u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(5u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_PROXY_SERVICE)); + entries, 0, NetLog::TYPE_PROXY_SERVICE)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 1, NetLog::TYPE_PROXY_SERVICE_WAITING_FOR_INIT_PAC)); + entries, 1, NetLog::TYPE_PROXY_SERVICE_WAITING_FOR_INIT_PAC)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 2, NetLog::TYPE_PROXY_SERVICE_WAITING_FOR_INIT_PAC)); + entries, 2, NetLog::TYPE_PROXY_SERVICE_WAITING_FOR_INIT_PAC)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 4, NetLog::TYPE_PROXY_SERVICE)); + entries, 4, NetLog::TYPE_PROXY_SERVICE)); } // Test that the proxy resolver does not see the URL's username/password @@ -1149,18 +1155,21 @@ TEST(ProxyServiceTest, CancelWhilePACFetching) { EXPECT_FALSE(callback1.have_result()); // Cancelled. EXPECT_FALSE(callback2.have_result()); // Cancelled. + net::CapturingNetLog::EntryList entries1; + log1.GetEntries(&entries1); + // Check the NetLog for request 1 (which was cancelled) got filled properly. - EXPECT_EQ(4u, log1.entries().size()); + EXPECT_EQ(4u, entries1.size()); EXPECT_TRUE(LogContainsBeginEvent( - log1.entries(), 0, NetLog::TYPE_PROXY_SERVICE)); + entries1, 0, NetLog::TYPE_PROXY_SERVICE)); EXPECT_TRUE(LogContainsBeginEvent( - log1.entries(), 1, NetLog::TYPE_PROXY_SERVICE_WAITING_FOR_INIT_PAC)); + entries1, 1, NetLog::TYPE_PROXY_SERVICE_WAITING_FOR_INIT_PAC)); // Note that TYPE_PROXY_SERVICE_WAITING_FOR_INIT_PAC is never completed before // the cancellation occured. EXPECT_TRUE(LogContainsEvent( - log1.entries(), 2, NetLog::TYPE_CANCELLED, NetLog::PHASE_NONE)); + entries1, 2, NetLog::TYPE_CANCELLED, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsEndEvent( - log1.entries(), 3, NetLog::TYPE_PROXY_SERVICE)); + entries1, 3, NetLog::TYPE_PROXY_SERVICE)); } // Test that if auto-detect fails, we fall-back to the custom pac. @@ -1679,11 +1688,14 @@ TEST(ProxyServiceTest, NetworkChangeTriggersPacRefetch) { // In particular, PROXY_CONFIG_CHANGED should have only been emitted once // (for the initial setup), and NOT a second time when the IP address // changed. - EXPECT_TRUE(LogContainsEntryWithType(log.entries(), 0, + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_TRUE(LogContainsEntryWithType(entries, 0, NetLog::TYPE_PROXY_CONFIG_CHANGED)); - ASSERT_EQ(13u, log.entries().size()); - for (size_t i = 1; i < log.entries().size(); ++i) - EXPECT_NE(NetLog::TYPE_PROXY_CONFIG_CHANGED, log.entries()[i].type); + ASSERT_EQ(13u, entries.size()); + for (size_t i = 1; i < entries.size(); ++i) + EXPECT_NE(NetLog::TYPE_PROXY_CONFIG_CHANGED, entries[i].type); } } // namespace net diff --git a/net/socket/client_socket_pool_base_unittest.cc b/net/socket/client_socket_pool_base_unittest.cc index d145bdf..fd9e337 100644 --- a/net/socket/client_socket_pool_base_unittest.cc +++ b/net/socket/client_socket_pool_base_unittest.cc @@ -620,21 +620,24 @@ TEST_F(ClientSocketPoolBaseTest, ConnectJob_TimedOut) { PlatformThread::Sleep(1); EXPECT_EQ(ERR_TIMED_OUT, delegate.WaitForResult()); - EXPECT_EQ(6u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(6u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKET_POOL_CONNECT_JOB)); + entries, 0, NetLog::TYPE_SOCKET_POOL_CONNECT_JOB)); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 1, NetLog::TYPE_SOCKET_POOL_CONNECT_JOB_CONNECT)); + entries, 1, NetLog::TYPE_SOCKET_POOL_CONNECT_JOB_CONNECT)); EXPECT_TRUE(LogContainsEvent( - log.entries(), 2, NetLog::TYPE_CONNECT_JOB_SET_SOCKET, + entries, 2, NetLog::TYPE_CONNECT_JOB_SET_SOCKET, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsEvent( - log.entries(), 3, NetLog::TYPE_SOCKET_POOL_CONNECT_JOB_TIMED_OUT, + entries, 3, NetLog::TYPE_SOCKET_POOL_CONNECT_JOB_TIMED_OUT, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 4, NetLog::TYPE_SOCKET_POOL_CONNECT_JOB_CONNECT)); + entries, 4, NetLog::TYPE_SOCKET_POOL_CONNECT_JOB_CONNECT)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 5, NetLog::TYPE_SOCKET_POOL_CONNECT_JOB)); + entries, 5, NetLog::TYPE_SOCKET_POOL_CONNECT_JOB)); } TEST_F(ClientSocketPoolBaseTest, BasicSynchronous) { @@ -655,17 +658,20 @@ TEST_F(ClientSocketPoolBaseTest, BasicSynchronous) { EXPECT_TRUE(handle.socket()); handle.Reset(); - EXPECT_EQ(4u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(4u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKET_POOL)); + entries, 0, NetLog::TYPE_SOCKET_POOL)); EXPECT_TRUE(LogContainsEvent( - log.entries(), 1, NetLog::TYPE_SOCKET_POOL_BOUND_TO_CONNECT_JOB, + entries, 1, NetLog::TYPE_SOCKET_POOL_BOUND_TO_CONNECT_JOB, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsEvent( - log.entries(), 2, NetLog::TYPE_SOCKET_POOL_BOUND_TO_SOCKET, + entries, 2, NetLog::TYPE_SOCKET_POOL_BOUND_TO_SOCKET, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 3, NetLog::TYPE_SOCKET_POOL)); + entries, 3, NetLog::TYPE_SOCKET_POOL)); } TEST_F(ClientSocketPoolBaseTest, InitConnectionFailure) { @@ -692,14 +698,17 @@ TEST_F(ClientSocketPoolBaseTest, InitConnectionFailure) { EXPECT_FALSE(handle.is_ssl_error()); EXPECT_TRUE(handle.ssl_error_response_info().headers.get() == NULL); - EXPECT_EQ(3u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(3u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKET_POOL)); + entries, 0, NetLog::TYPE_SOCKET_POOL)); EXPECT_TRUE(LogContainsEvent( - log.entries(), 1, NetLog::TYPE_SOCKET_POOL_BOUND_TO_CONNECT_JOB, + entries, 1, NetLog::TYPE_SOCKET_POOL_BOUND_TO_CONNECT_JOB, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 2, NetLog::TYPE_SOCKET_POOL)); + entries, 2, NetLog::TYPE_SOCKET_POOL)); } TEST_F(ClientSocketPoolBaseTest, TotalLimit) { @@ -1503,17 +1512,20 @@ TEST_F(ClientSocketPoolBaseTest, BasicAsynchronous) { EXPECT_TRUE(handle.socket()); handle.Reset(); - EXPECT_EQ(4u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(4u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKET_POOL)); + entries, 0, NetLog::TYPE_SOCKET_POOL)); EXPECT_TRUE(LogContainsEvent( - log.entries(), 1, NetLog::TYPE_SOCKET_POOL_BOUND_TO_CONNECT_JOB, + entries, 1, NetLog::TYPE_SOCKET_POOL_BOUND_TO_CONNECT_JOB, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsEvent( - log.entries(), 2, NetLog::TYPE_SOCKET_POOL_BOUND_TO_SOCKET, + entries, 2, NetLog::TYPE_SOCKET_POOL_BOUND_TO_SOCKET, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 3, NetLog::TYPE_SOCKET_POOL)); + entries, 3, NetLog::TYPE_SOCKET_POOL)); } TEST_F(ClientSocketPoolBaseTest, @@ -1540,14 +1552,17 @@ TEST_F(ClientSocketPoolBaseTest, EXPECT_FALSE(handle.is_ssl_error()); EXPECT_TRUE(handle.ssl_error_response_info().headers.get() == NULL); - EXPECT_EQ(3u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_EQ(3u, entries.size()); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKET_POOL)); + entries, 0, NetLog::TYPE_SOCKET_POOL)); EXPECT_TRUE(LogContainsEvent( - log.entries(), 1, NetLog::TYPE_SOCKET_POOL_BOUND_TO_CONNECT_JOB, + entries, 1, NetLog::TYPE_SOCKET_POOL_BOUND_TO_CONNECT_JOB, NetLog::PHASE_NONE)); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), 2, NetLog::TYPE_SOCKET_POOL)); + entries, 2, NetLog::TYPE_SOCKET_POOL)); } TEST_F(ClientSocketPoolBaseTest, TwoRequestsCancelOne) { @@ -1892,8 +1907,11 @@ TEST_F(ClientSocketPoolBaseTest, CleanupTimedOutIdleSockets) { log.bound()); EXPECT_EQ(OK, rv); EXPECT_TRUE(handle.is_reused()); + + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(LogContainsEntryWithType( - log.entries(), 1, NetLog::TYPE_SOCKET_POOL_REUSED_AN_EXISTING_SOCKET)); + entries, 1, NetLog::TYPE_SOCKET_POOL_REUSED_AN_EXISTING_SOCKET)); } // Make sure that we process all pending requests even when we're stalling diff --git a/net/socket/socks5_client_socket_unittest.cc b/net/socket/socks5_client_socket_unittest.cc index 2152c86..5458411 100644 --- a/net/socket/socks5_client_socket_unittest.cc +++ b/net/socket/socks5_client_socket_unittest.cc @@ -131,14 +131,19 @@ TEST_F(SOCKS5ClientSocketTest, CompleteHandshake) { int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); EXPECT_FALSE(user_sock_->IsConnected()); - EXPECT_TRUE(LogContainsBeginEvent(net_log_.entries(), 0, + + net::CapturingNetLog::EntryList net_log_entries; + net_log_.GetEntries(&net_log_entries); + EXPECT_TRUE(LogContainsBeginEvent(net_log_entries, 0, NetLog::TYPE_SOCKS5_CONNECT)); rv = callback_.WaitForResult(); EXPECT_EQ(OK, rv); EXPECT_TRUE(user_sock_->IsConnected()); - EXPECT_TRUE(LogContainsEndEvent(net_log_.entries(), -1, + + net_log_.GetEntries(&net_log_entries); + EXPECT_TRUE(LogContainsEndEvent(net_log_entries, -1, NetLog::TYPE_SOCKS5_CONNECT)); scoped_refptr<IOBuffer> buffer(new IOBuffer(payload_write.size())); @@ -248,12 +253,18 @@ TEST_F(SOCKS5ClientSocketTest, PartialReadWrites) { hostname, 80, &net_log_)); int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_TRUE(LogContainsBeginEvent(net_log_.entries(), 0, + + net::CapturingNetLog::EntryList net_log_entries; + net_log_.GetEntries(&net_log_entries); + EXPECT_TRUE(LogContainsBeginEvent(net_log_entries, 0, NetLog::TYPE_SOCKS5_CONNECT)); + rv = callback_.WaitForResult(); EXPECT_EQ(OK, rv); EXPECT_TRUE(user_sock_->IsConnected()); - EXPECT_TRUE(LogContainsEndEvent(net_log_.entries(), -1, + + net_log_.GetEntries(&net_log_entries); + EXPECT_TRUE(LogContainsEndEvent(net_log_entries, -1, NetLog::TYPE_SOCKS5_CONNECT)); } @@ -273,12 +284,16 @@ TEST_F(SOCKS5ClientSocketTest, PartialReadWrites) { hostname, 80, &net_log_)); int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_TRUE(LogContainsBeginEvent(net_log_.entries(), 0, + + net::CapturingNetLog::EntryList net_log_entries; + net_log_.GetEntries(&net_log_entries); + EXPECT_TRUE(LogContainsBeginEvent(net_log_entries, 0, NetLog::TYPE_SOCKS5_CONNECT)); rv = callback_.WaitForResult(); EXPECT_EQ(OK, rv); EXPECT_TRUE(user_sock_->IsConnected()); - EXPECT_TRUE(LogContainsEndEvent(net_log_.entries(), -1, + net_log_.GetEntries(&net_log_entries); + EXPECT_TRUE(LogContainsEndEvent(net_log_entries, -1, NetLog::TYPE_SOCKS5_CONNECT)); } @@ -299,12 +314,15 @@ TEST_F(SOCKS5ClientSocketTest, PartialReadWrites) { hostname, 80, &net_log_)); int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_TRUE(LogContainsBeginEvent(net_log_.entries(), 0, + net::CapturingNetLog::EntryList net_log_entries; + net_log_.GetEntries(&net_log_entries); + EXPECT_TRUE(LogContainsBeginEvent(net_log_entries, 0, NetLog::TYPE_SOCKS5_CONNECT)); rv = callback_.WaitForResult(); EXPECT_EQ(OK, rv); EXPECT_TRUE(user_sock_->IsConnected()); - EXPECT_TRUE(LogContainsEndEvent(net_log_.entries(), -1, + net_log_.GetEntries(&net_log_entries); + EXPECT_TRUE(LogContainsEndEvent(net_log_entries, -1, NetLog::TYPE_SOCKS5_CONNECT)); } @@ -327,12 +345,15 @@ TEST_F(SOCKS5ClientSocketTest, PartialReadWrites) { hostname, 80, &net_log_)); int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); - EXPECT_TRUE(LogContainsBeginEvent(net_log_.entries(), 0, + net::CapturingNetLog::EntryList net_log_entries; + net_log_.GetEntries(&net_log_entries); + EXPECT_TRUE(LogContainsBeginEvent(net_log_entries, 0, NetLog::TYPE_SOCKS5_CONNECT)); rv = callback_.WaitForResult(); EXPECT_EQ(OK, rv); EXPECT_TRUE(user_sock_->IsConnected()); - EXPECT_TRUE(LogContainsEndEvent(net_log_.entries(), -1, + net_log_.GetEntries(&net_log_entries); + EXPECT_TRUE(LogContainsEndEvent(net_log_entries, -1, NetLog::TYPE_SOCKS5_CONNECT)); } } diff --git a/net/socket/socks_client_socket_unittest.cc b/net/socket/socks_client_socket_unittest.cc index 11a88ae..aa5338a 100644 --- a/net/socket/socks_client_socket_unittest.cc +++ b/net/socket/socks_client_socket_unittest.cc @@ -142,16 +142,20 @@ TEST_F(SOCKSClientSocketTest, CompleteHandshake) { int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); + + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE( - LogContainsBeginEvent(log.entries(), 0, NetLog::TYPE_SOCKS_CONNECT)); + LogContainsBeginEvent(entries, 0, NetLog::TYPE_SOCKS_CONNECT)); EXPECT_FALSE(user_sock_->IsConnected()); - rv = callback_.WaitForResult(); + rv = callback_.WaitForResult(); EXPECT_EQ(OK, rv); EXPECT_TRUE(user_sock_->IsConnected()); EXPECT_EQ(SOCKSClientSocket::kSOCKS4, user_sock_->socks_version_); + log.GetEntries(&entries); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), -1, NetLog::TYPE_SOCKS_CONNECT)); + entries, -1, NetLog::TYPE_SOCKS_CONNECT)); scoped_refptr<IOBuffer> buffer(new IOBuffer(payload_write.size())); memcpy(buffer->data(), payload_write.data(), payload_write.size()); @@ -208,14 +212,19 @@ TEST_F(SOCKSClientSocketTest, HandshakeFailures) { int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); + + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKS_CONNECT)); + entries, 0, NetLog::TYPE_SOCKS_CONNECT)); + rv = callback_.WaitForResult(); EXPECT_EQ(tests[i].fail_code, rv); EXPECT_FALSE(user_sock_->IsConnected()); EXPECT_TRUE(tcp_sock_->IsConnected()); + log.GetEntries(&entries); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), -1, NetLog::TYPE_SOCKS_CONNECT)); + entries, -1, NetLog::TYPE_SOCKS_CONNECT)); } } @@ -240,13 +249,17 @@ TEST_F(SOCKSClientSocketTest, PartialServerReads) { int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKS_CONNECT)); + entries, 0, NetLog::TYPE_SOCKS_CONNECT)); + rv = callback_.WaitForResult(); EXPECT_EQ(OK, rv); EXPECT_TRUE(user_sock_->IsConnected()); + log.GetEntries(&entries); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), -1, NetLog::TYPE_SOCKS_CONNECT)); + entries, -1, NetLog::TYPE_SOCKS_CONNECT)); } // Tests scenario when the client sends the handshake request in @@ -274,13 +287,17 @@ TEST_F(SOCKSClientSocketTest, PartialClientWrites) { int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKS_CONNECT)); + entries, 0, NetLog::TYPE_SOCKS_CONNECT)); + rv = callback_.WaitForResult(); EXPECT_EQ(OK, rv); EXPECT_TRUE(user_sock_->IsConnected()); + log.GetEntries(&entries); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), -1, NetLog::TYPE_SOCKS_CONNECT)); + entries, -1, NetLog::TYPE_SOCKS_CONNECT)); } // Tests the case when the server sends a smaller sized handshake data @@ -302,13 +319,17 @@ TEST_F(SOCKSClientSocketTest, FailedSocketRead) { int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKS_CONNECT)); + entries, 0, NetLog::TYPE_SOCKS_CONNECT)); + rv = callback_.WaitForResult(); EXPECT_EQ(ERR_CONNECTION_CLOSED, rv); EXPECT_FALSE(user_sock_->IsConnected()); + log.GetEntries(&entries); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), -1, NetLog::TYPE_SOCKS_CONNECT)); + entries, -1, NetLog::TYPE_SOCKS_CONNECT)); } // Tries to connect to an unknown DNS and on failure should revert to SOCKS4A. @@ -335,14 +356,18 @@ TEST_F(SOCKSClientSocketTest, SOCKS4AFailedDNS) { int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKS_CONNECT)); + entries, 0, NetLog::TYPE_SOCKS_CONNECT)); + rv = callback_.WaitForResult(); EXPECT_EQ(OK, rv); EXPECT_TRUE(user_sock_->IsConnected()); EXPECT_EQ(SOCKSClientSocket::kSOCKS4a, user_sock_->socks_version_); + log.GetEntries(&entries); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), -1, NetLog::TYPE_SOCKS_CONNECT)); + entries, -1, NetLog::TYPE_SOCKS_CONNECT)); } // Tries to connect to a domain that resolves to IPv6. @@ -371,14 +396,18 @@ TEST_F(SOCKSClientSocketTest, SOCKS4AIfDomainInIPv6) { int rv = user_sock_->Connect(&callback_); EXPECT_EQ(ERR_IO_PENDING, rv); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(LogContainsBeginEvent( - log.entries(), 0, NetLog::TYPE_SOCKS_CONNECT)); + entries, 0, NetLog::TYPE_SOCKS_CONNECT)); + rv = callback_.WaitForResult(); EXPECT_EQ(OK, rv); EXPECT_TRUE(user_sock_->IsConnected()); EXPECT_EQ(SOCKSClientSocket::kSOCKS4a, user_sock_->socks_version_); + log.GetEntries(&entries); EXPECT_TRUE(LogContainsEndEvent( - log.entries(), -1, NetLog::TYPE_SOCKS_CONNECT)); + entries, -1, NetLog::TYPE_SOCKS_CONNECT)); } // Calls Disconnect() while a host resolve is in progress. The outstanding host diff --git a/net/socket/ssl_client_socket_snapstart_unittest.cc b/net/socket/ssl_client_socket_snapstart_unittest.cc index 13b2636..bb12fb3 100644 --- a/net/socket/ssl_client_socket_snapstart_unittest.cc +++ b/net/socket/ssl_client_socket_snapstart_unittest.cc @@ -234,8 +234,9 @@ class SSLClientSocketSnapStartTest : public PlatformTest { // SnapStartEventType extracts the type of Snap Start from the NetLog. See // the SSL_SNAP_START_* defines in sslt.h int SnapStartEventType() { - const std::vector<CapturingNetLog::Entry>& entries = log_.entries(); - for (std::vector<CapturingNetLog::Entry>::const_iterator + CapturingNetLog::EntryList entries; + log_.GetEntries(&entries); + for (CapturingNetLog::EntryList::const_iterator i = entries.begin(); i != entries.end(); i++) { if (i->type == NetLog::TYPE_SSL_SNAP_START) { scoped_ptr<Value> value(i->extra_parameters->ToValue()); @@ -253,8 +254,9 @@ class SSLClientSocketSnapStartTest : public PlatformTest { // it's certificate validation with the optimistic validation from the // SSLHostInfo. bool DidMerge() { - const std::vector<CapturingNetLog::Entry>& entries = log_.entries(); - for (std::vector<CapturingNetLog::Entry>::const_iterator + CapturingNetLog::EntryList entries; + log_.GetEntries(&entries); + for (CapturingNetLog::EntryList::const_iterator i = entries.begin(); i != entries.end(); i++) { if (i->type == NetLog::TYPE_SSL_VERIFICATION_MERGED) return true; diff --git a/net/socket/ssl_client_socket_unittest.cc b/net/socket/ssl_client_socket_unittest.cc index e736d5b..a95585c 100644 --- a/net/socket/ssl_client_socket_unittest.cc +++ b/net/socket/ssl_client_socket_unittest.cc @@ -72,13 +72,17 @@ TEST_F(SSLClientSocketTest, Connect) { EXPECT_FALSE(sock->IsConnected()); rv = sock->Connect(&callback); + + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 5, net::NetLog::TYPE_SSL_CONNECT)); + entries, 5, net::NetLog::TYPE_SSL_CONNECT)); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); EXPECT_EQ(net::OK, rv); EXPECT_TRUE(sock->IsConnected()); - EXPECT_TRUE(LogContainsSSLConnectEndEvent(log.entries(), -1)); + log.GetEntries(&entries); + EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1)); sock->Disconnect(); EXPECT_FALSE(sock->IsConnected()); @@ -109,8 +113,11 @@ TEST_F(SSLClientSocketTest, ConnectExpired) { EXPECT_FALSE(sock->IsConnected()); rv = sock->Connect(&callback); + + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 5, net::NetLog::TYPE_SSL_CONNECT)); + entries, 5, net::NetLog::TYPE_SSL_CONNECT)); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); @@ -120,7 +127,8 @@ TEST_F(SSLClientSocketTest, ConnectExpired) { // test that the handshake has finished. This is because it may be // desirable to disconnect the socket before showing a user prompt, since // the user may take indefinitely long to respond. - EXPECT_TRUE(LogContainsSSLConnectEndEvent(log.entries(), -1)); + log.GetEntries(&entries); + EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1)); } TEST_F(SSLClientSocketTest, ConnectMismatched) { @@ -149,8 +157,10 @@ TEST_F(SSLClientSocketTest, ConnectMismatched) { rv = sock->Connect(&callback); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 5, net::NetLog::TYPE_SSL_CONNECT)); + entries, 5, net::NetLog::TYPE_SSL_CONNECT)); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); @@ -160,7 +170,8 @@ TEST_F(SSLClientSocketTest, ConnectMismatched) { // test that the handshake has finished. This is because it may be // desirable to disconnect the socket before showing a user prompt, since // the user may take indefinitely long to respond. - EXPECT_TRUE(LogContainsSSLConnectEndEvent(log.entries(), -1)); + log.GetEntries(&entries); + EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1)); } // Attempt to connect to a page which requests a client certificate. It should @@ -191,11 +202,16 @@ TEST_F(SSLClientSocketTest, FLAKY_ConnectClientAuthCertRequested) { EXPECT_FALSE(sock->IsConnected()); rv = sock->Connect(&callback); + + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 5, net::NetLog::TYPE_SSL_CONNECT)); + entries, 5, net::NetLog::TYPE_SSL_CONNECT)); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); + log.GetEntries(&entries); + EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1)); EXPECT_EQ(net::ERR_SSL_CLIENT_AUTH_CERT_NEEDED, rv); EXPECT_FALSE(sock->IsConnected()); } @@ -235,14 +251,18 @@ TEST_F(SSLClientSocketTest, ConnectClientAuthSendNullCert) { // Our test server accepts certificate-less connections. // TODO(davidben): Add a test which requires them and verify the error. rv = sock->Connect(&callback); + + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 5, net::NetLog::TYPE_SSL_CONNECT)); + entries, 5, net::NetLog::TYPE_SSL_CONNECT)); if (rv == net::ERR_IO_PENDING) rv = callback.WaitForResult(); EXPECT_EQ(net::OK, rv); EXPECT_TRUE(sock->IsConnected()); - EXPECT_TRUE(LogContainsSSLConnectEndEvent(log.entries(), -1)); + log.GetEntries(&entries); + EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1)); sock->Disconnect(); EXPECT_FALSE(sock->IsConnected()); @@ -553,8 +573,10 @@ TEST_F(SSLClientSocketTest, MAYBE_CipherSuiteDisables) { EXPECT_FALSE(sock->IsConnected()); rv = sock->Connect(&callback); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); EXPECT_TRUE(net::LogContainsBeginEvent( - log.entries(), 5, net::NetLog::TYPE_SSL_CONNECT)); + entries, 5, net::NetLog::TYPE_SSL_CONNECT)); // NSS has special handling that maps a handshake_failure alert received // immediately after a client_hello to be a mismatched cipher suite error, @@ -569,12 +591,13 @@ TEST_F(SSLClientSocketTest, MAYBE_CipherSuiteDisables) { // The exact ordering differs between SSLClientSocketNSS (which issues an // extra read) and SSLClientSocketMac (which does not). Just make sure the // error appears somewhere in the log. - net::ExpectLogContainsSomewhere(log.entries(), 0, + log.GetEntries(&entries); + net::ExpectLogContainsSomewhere(entries, 0, net::NetLog::TYPE_SSL_HANDSHAKE_ERROR, net::NetLog::PHASE_NONE); // We cannot test sock->IsConnected(), as the NSS implementation disconnects // the socket when it encounters an error, whereas other implementations // leave it connected. - EXPECT_TRUE(LogContainsSSLConnectEndEvent(log.entries(), -1)); + EXPECT_TRUE(LogContainsSSLConnectEndEvent(entries, -1)); } diff --git a/net/socket/tcp_client_socket_unittest.cc b/net/socket/tcp_client_socket_unittest.cc index 64d78f3..2ed9441 100644 --- a/net/socket/tcp_client_socket_unittest.cc +++ b/net/socket/tcp_client_socket_unittest.cc @@ -105,10 +105,13 @@ TEST_F(TCPClientSocketTest, Connect) { EXPECT_FALSE(sock_->IsConnected()); int rv = sock_->Connect(&callback); + + net::CapturingNetLog::EntryList net_log_entries; + net_log_.GetEntries(&net_log_entries); EXPECT_TRUE(net::LogContainsBeginEvent( - net_log_.entries(), 0, net::NetLog::TYPE_SOCKET_ALIVE)); + net_log_entries, 0, net::NetLog::TYPE_SOCKET_ALIVE)); EXPECT_TRUE(net::LogContainsBeginEvent( - net_log_.entries(), 1, net::NetLog::TYPE_TCP_CONNECT)); + net_log_entries, 1, net::NetLog::TYPE_TCP_CONNECT)); if (rv != OK) { ASSERT_EQ(rv, ERR_IO_PENDING); rv = callback.WaitForResult(); @@ -116,8 +119,9 @@ TEST_F(TCPClientSocketTest, Connect) { } EXPECT_TRUE(sock_->IsConnected()); + net_log_.GetEntries(&net_log_entries); EXPECT_TRUE(net::LogContainsEndEvent( - net_log_.entries(), -1, net::NetLog::TYPE_TCP_CONNECT)); + net_log_entries, -1, net::NetLog::TYPE_TCP_CONNECT)); sock_->Disconnect(); EXPECT_FALSE(sock_->IsConnected()); diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc index e3ce486..7400756 100644 --- a/net/spdy/spdy_network_transaction_unittest.cc +++ b/net/spdy/spdy_network_transaction_unittest.cc @@ -3343,33 +3343,36 @@ TEST_P(SpdyNetworkTransactionTest, NetLog) { // This test is intentionally non-specific about the exact ordering of the // log; instead we just check to make sure that certain events exist, and that // they are in the right order. - EXPECT_LT(0u, log.entries().size()); + net::CapturingNetLog::EntryList entries; + log.GetEntries(&entries); + + EXPECT_LT(0u, entries.size()); int pos = 0; - pos = net::ExpectLogContainsSomewhere(log.entries(), 0, + pos = net::ExpectLogContainsSomewhere(entries, 0, net::NetLog::TYPE_HTTP_TRANSACTION_SEND_REQUEST, net::NetLog::PHASE_BEGIN); - pos = net::ExpectLogContainsSomewhere(log.entries(), pos + 1, + pos = net::ExpectLogContainsSomewhere(entries, pos + 1, net::NetLog::TYPE_HTTP_TRANSACTION_SEND_REQUEST, net::NetLog::PHASE_END); - pos = net::ExpectLogContainsSomewhere(log.entries(), pos + 1, + pos = net::ExpectLogContainsSomewhere(entries, pos + 1, net::NetLog::TYPE_HTTP_TRANSACTION_READ_HEADERS, net::NetLog::PHASE_BEGIN); - pos = net::ExpectLogContainsSomewhere(log.entries(), pos + 1, + pos = net::ExpectLogContainsSomewhere(entries, pos + 1, net::NetLog::TYPE_HTTP_TRANSACTION_READ_HEADERS, net::NetLog::PHASE_END); - pos = net::ExpectLogContainsSomewhere(log.entries(), pos + 1, + pos = net::ExpectLogContainsSomewhere(entries, pos + 1, net::NetLog::TYPE_HTTP_TRANSACTION_READ_BODY, net::NetLog::PHASE_BEGIN); - pos = net::ExpectLogContainsSomewhere(log.entries(), pos + 1, + pos = net::ExpectLogContainsSomewhere(entries, pos + 1, net::NetLog::TYPE_HTTP_TRANSACTION_READ_BODY, net::NetLog::PHASE_END); // Check that we logged all the headers correctly pos = net::ExpectLogContainsSomewhere( - log.entries(), 0, + entries, 0, net::NetLog::TYPE_SPDY_SESSION_SYN_STREAM, net::NetLog::PHASE_NONE); - CapturingNetLog::Entry entry = log.entries()[pos]; + CapturingNetLog::Entry entry = entries[pos]; NetLogSpdySynParameter* request_params = static_cast<NetLogSpdySynParameter*>(entry.extra_parameters.get()); spdy::SpdyHeaderBlock* headers = |