Skip to content

Commit

Permalink
Enable overlapping, usage counted SetEnabled()/SetDisabled() for Trac…
Browse files Browse the repository at this point in the history 
…eLog

This allows multiple clients to enable/disable trace events collection independently on each other.

BUG=none
TEST=TraceEventTestFixture.TraceEnableDisable

Review URL: https://codereview.chromium.org/11802005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@177135 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
caseq@google.com committed Jan 16, 2013
1 parent 0cc3e80 commit fbbe2a5
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 11 deletions.
28 changes: 21 additions & 7 deletions base/debug/trace_event_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ TraceLog* TraceLog::GetInstance() {
}

TraceLog::TraceLog()
: enabled_(false),
: enable_count_(0),
dispatching_to_observer_list_(false),
watch_category_(NULL) {
// Trace is enabled or disabled on one thread while other threads are
Expand Down Expand Up @@ -444,7 +444,7 @@ const unsigned char* TraceLog::GetCategoryEnabledInternal(const char* name) {
ANNOTATE_LEAKING_OBJECT_PTR(new_name);
g_categories[new_index] = new_name;
DCHECK(!g_category_enabled[new_index]);
if (enabled_) {
if (enable_count_) {
// Note that if both included and excluded_categories are empty, the
// else clause below excludes nothing, thereby enabling this category.
if (!included_categories_.empty()) {
Expand Down Expand Up @@ -477,8 +477,23 @@ void TraceLog::GetKnownCategories(std::vector<std::string>* categories) {
void TraceLog::SetEnabled(const std::vector<std::string>& included_categories,
const std::vector<std::string>& excluded_categories) {
AutoLock lock(lock_);
if (enabled_)

if (enable_count_++ > 0) {
// Tracing is already enabled, so just merge in enabled categories.
// We only expand the set of enabled categories upon nested SetEnable().
if (!included_categories_.empty() && !included_categories.empty()) {
included_categories_.insert(included_categories_.end(),
included_categories.begin(),
included_categories.end());
EnableMatchingCategories(included_categories_, CATEGORY_ENABLED, 0);
} else {
// If either old or new included categories are empty, allow all events.
included_categories_.clear();
excluded_categories_.clear();
EnableMatchingCategories(excluded_categories_, 0, CATEGORY_ENABLED);
}
return;
}

if (dispatching_to_observer_list_) {
DLOG(ERROR) <<
Expand All @@ -492,7 +507,6 @@ void TraceLog::SetEnabled(const std::vector<std::string>& included_categories,
dispatching_to_observer_list_ = false;

logged_events_.reserve(1024);
enabled_ = true;
included_categories_ = included_categories;
excluded_categories_ = excluded_categories;
// Note that if both included and excluded_categories are empty, the else
Expand Down Expand Up @@ -528,15 +542,16 @@ void TraceLog::GetEnabledTraceCategories(
std::vector<std::string>* included_out,
std::vector<std::string>* excluded_out) {
AutoLock lock(lock_);
if (enabled_) {
if (enable_count_) {
*included_out = included_categories_;
*excluded_out = excluded_categories_;
}
}

void TraceLog::SetDisabled() {
AutoLock lock(lock_);
if (!enabled_)
DCHECK(enable_count_ > 0);
if (--enable_count_ != 0)
return;

if (dispatching_to_observer_list_) {
Expand All @@ -550,7 +565,6 @@ void TraceLog::SetDisabled() {
OnTraceLogWillDisable());
dispatching_to_observer_list_ = false;

enabled_ = false;
included_categories_.clear();
excluded_categories_.clear();
watch_category_ = NULL;
Expand Down
4 changes: 2 additions & 2 deletions base/debug/trace_event_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ class BASE_EXPORT TraceLog {
void SetDisabled();
// Helper method to enable/disable tracing for all categories.
void SetEnabled(bool enabled);
bool IsEnabled() { return enabled_; }
bool IsEnabled() { return !!enable_count_; }

#if defined(OS_ANDROID)
static void InitATrace();
Expand Down Expand Up @@ -367,7 +367,7 @@ class BASE_EXPORT TraceLog {
// synchronization.
// This lock protects TraceLog member accesses from arbitrary threads.
Lock lock_;
bool enabled_;
int enable_count_;
NotificationCallback notification_callback_;
std::vector<TraceEvent> logged_events_;
std::vector<std::string> included_categories_;
Expand Down
60 changes: 58 additions & 2 deletions base/debug/trace_event_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ class TraceEventTestFixture : public testing::Test {
}

void EndTraceAndFlush() {
TraceLog::GetInstance()->SetDisabled();
while (TraceLog::GetInstance()->IsEnabled())
TraceLog::GetInstance()->SetDisabled();
TraceLog::GetInstance()->Flush(
base::Bind(&TraceEventTestFixture::OnTraceDataCollected,
base::Unretained(this)));
Expand Down Expand Up @@ -696,11 +697,14 @@ TEST_F(TraceEventTestFixture, EnabledObserverDoesntFireOnSecondEnable) {
// Cleanup.
TraceLog::GetInstance()->RemoveEnabledStateObserver(&observer);
TraceLog::GetInstance()->SetEnabled(false);
TraceLog::GetInstance()->SetEnabled(false);
}

TEST_F(TraceEventTestFixture, EnabledObserverDoesntFireOnUselessDisable) {
TEST_F(TraceEventTestFixture, EnabledObserverDoesntFireOnNestedDisable) {
ManualTestSetUp();

TraceLog::GetInstance()->SetEnabled(true);
TraceLog::GetInstance()->SetEnabled(true);

testing::StrictMock<MockEnabledStateChangedObserver> observer;
TraceLog::GetInstance()->AddEnabledStateObserver(&observer);
Expand All @@ -714,6 +718,7 @@ TEST_F(TraceEventTestFixture, EnabledObserverDoesntFireOnUselessDisable) {

// Cleanup.
TraceLog::GetInstance()->RemoveEnabledStateObserver(&observer);
TraceLog::GetInstance()->SetEnabled(false);
}

TEST_F(TraceEventTestFixture, EnabledObserverFiresOnDisable) {
Expand Down Expand Up @@ -1355,5 +1360,56 @@ TEST_F(TraceEventTestFixture, TracingIsLazy) {
EndTraceAndFlush();
}

TEST_F(TraceEventTestFixture, TraceEnableDisable) {
ManualTestSetUp();

TraceLog* trace_log = TraceLog::GetInstance();
trace_log->SetEnabled(std::string());
EXPECT_TRUE(trace_log->IsEnabled());
trace_log->SetDisabled();
EXPECT_FALSE(trace_log->IsEnabled());

trace_log->SetEnabled(true);
EXPECT_TRUE(trace_log->IsEnabled());
const std::vector<std::string> empty;
trace_log->SetEnabled(empty, empty);
EXPECT_TRUE(trace_log->IsEnabled());
trace_log->SetEnabled(false);
EXPECT_TRUE(trace_log->IsEnabled());
trace_log->SetDisabled();
EXPECT_FALSE(trace_log->IsEnabled());
}

TEST_F(TraceEventTestFixture, TraceCategoriesAfterNestedEnable) {
ManualTestSetUp();

TraceLog* trace_log = TraceLog::GetInstance();
trace_log->SetEnabled(std::string("foo,bar"));
EXPECT_TRUE(*trace_log->GetCategoryEnabled("foo"));
EXPECT_TRUE(*trace_log->GetCategoryEnabled("bar"));
EXPECT_FALSE(*trace_log->GetCategoryEnabled("baz"));
trace_log->SetEnabled(std::string("foo2"));
EXPECT_TRUE(*trace_log->GetCategoryEnabled("foo2"));
EXPECT_FALSE(*trace_log->GetCategoryEnabled("baz"));
trace_log->SetEnabled(std::string(""));
EXPECT_TRUE(*trace_log->GetCategoryEnabled("foo"));
EXPECT_TRUE(*trace_log->GetCategoryEnabled("baz"));
trace_log->SetDisabled();
trace_log->SetDisabled();
trace_log->SetDisabled();
EXPECT_FALSE(*trace_log->GetCategoryEnabled("foo"));
EXPECT_FALSE(*trace_log->GetCategoryEnabled("baz"));

trace_log->SetEnabled(std::string("-foo,-bar"));
EXPECT_FALSE(*trace_log->GetCategoryEnabled("foo"));
EXPECT_TRUE(*trace_log->GetCategoryEnabled("baz"));
trace_log->SetEnabled(std::string("moo"));
EXPECT_TRUE(*trace_log->GetCategoryEnabled("baz"));
EXPECT_TRUE(*trace_log->GetCategoryEnabled("moo"));
EXPECT_TRUE(*trace_log->GetCategoryEnabled("foo"));
trace_log->SetDisabled();
trace_log->SetDisabled();
}

} // namespace debug
} // namespace base

0 comments on commit fbbe2a5

Please sign in to comment.