diff options
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 = |