Skip to content

Commit

Permalink
Update NetLog to be threadsafe.
Browse files Browse the repository at this point in the history
The ChromeNetLog is now owned by the browser process,
and passed to the IOThread on creation.

NetLog entries can be added from any thread.

Observers must be able to handle having log entries added
from any thread.  Observers can add/remove themselves on any
thread.

BUG=63334
TEST=None, yet

Review URL: http://codereview.chromium.org/4118004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@67851 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
mmenke@chromium.org committed Dec 1, 2010
1 parent 087a03f commit b2fcd0e
Show file tree
Hide file tree
Showing 47 changed files with 977 additions and 785 deletions.
5 changes: 4 additions & 1 deletion chrome/browser/browser_process_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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))
Expand Down
4 changes: 4 additions & 0 deletions chrome/browser/browser_process_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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_;

Expand Down
8 changes: 6 additions & 2 deletions chrome/browser/debugger/devtools_netlog_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down Expand Up @@ -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() {
Expand Down
8 changes: 6 additions & 2 deletions chrome/browser/debugger/devtools_netlog_observer.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
110 changes: 62 additions & 48 deletions chrome/browser/dom_ui/net_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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);

Expand All @@ -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>;
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -385,9 +402,6 @@ void NetInternalsMessageHandler::RegisterMessages() {
dom_ui_->RegisterMessageCallback(
"enableIPv6",
proxy_->CreateCallback(&IOThreadImpl::OnEnableIPv6));
dom_ui_->RegisterMessageCallback(
"getPassiveLogEntries",
proxy_->CreateCallback(&IOThreadImpl::OnGetPassiveLogEntries));
dom_ui_->RegisterMessageCallback(
"startConnectionTests",
proxy_->CreateCallback(&IOThreadImpl::OnStartConnectionTests));
Expand Down Expand Up @@ -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));
}
Expand All @@ -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.
{
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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>].
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -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(
Expand Down
Loading

0 comments on commit b2fcd0e

Please sign in to comment.