Invoke trace callback even when trace buffer is full

We used to invoke trace event callbacks only when we successfully
stored an event in the buffer. This does not work for clients that
maintain their own buffers, so let's invoke callbacks unconditionally.

BUG=173184
R=dsinclair@google.com, jar@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@197009 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 22 insertions, 6 deletions

diff --git a/base/debug/trace_event_impl.cc b/base/debug/trace_event_impl.cc
index 860d989..38a6c0f 100644
--- a/base/debug/trace_event_impl.cc
+++ b/base/debug/trace_event_impl.cc
@@ -1000,12 +1000,14 @@ void TraceLog::AddTraceEventWithThreadIdAndTimestamp(
 
   NotificationHelper notifier(this);
 
-  {
+  do {
     AutoLock lock(lock_);
     if (*category_group_enabled != CATEGORY_ENABLED)
       return;
+
+    event_callback_copy = event_callback_;
     if (logged_events_->IsFull())
-      return;
+      break;
 
     const char* new_name = ThreadIdNameManager::GetInstance()->
         GetName(thread_id);
@@ -1047,9 +1049,7 @@ void TraceLog::AddTraceEventWithThreadIdAndTimestamp(
 
     if (watch_category_ == category_group_enabled && watch_event_name_ == name)
       notifier.AddNotificationWhileLocked(EVENT_WATCH_NOTIFICATION);
-
-    event_callback_copy = event_callback_;
-  }  // release lock
+  } while (0); // release lock
 
   notifier.SendNotificationIfAny();
   if (event_callback_copy != NULL) {
diff --git a/base/debug/trace_event_unittest.cc b/base/debug/trace_event_unittest.cc
index 64f992d..a78b002 100644
--- a/base/debug/trace_event_unittest.cc
+++ b/base/debug/trace_event_unittest.cc
@@ -55,6 +55,7 @@ class TraceEventTestFixture : public testing::Test {
   void OnTraceNotification(int notification) {
     if (notification & TraceLog::EVENT_WATCH_NOTIFICATION)
       ++event_watch_notification_;
+      notifications_received_ |= notification;
   }
   DictionaryValue* FindMatchingTraceEntry(const JsonKeyValue* key_values);
   DictionaryValue* FindNamePhase(const char* name, const char* phase);
@@ -73,6 +74,7 @@ class TraceEventTestFixture : public testing::Test {
 
   void BeginTrace() {
     event_watch_notification_ = 0;
+    notifications_received_ = 0;
     TraceLog::GetInstance()->SetEnabled(CategoryFilter("*"),
                                         TraceLog::RECORD_UNTIL_FULL);
   }
@@ -102,6 +104,7 @@ class TraceEventTestFixture : public testing::Test {
   base::debug::TraceResultBuffer trace_buffer_;
   base::debug::TraceResultBuffer::SimpleOutput json_output_;
   int event_watch_notification_;
+  int notifications_received_;
 
  private:
   // We want our singleton torn down after each test.
@@ -1666,11 +1669,24 @@ TEST_F(TraceEventCallbackTest, TraceEventCallback) {
   TraceLog::GetInstance()->SetEventCallback(NULL);
   TRACE_EVENT_INSTANT0("all", "after callback removed",
                        TRACE_EVENT_SCOPE_GLOBAL);
-  EXPECT_EQ(2u, collected_events_.size());
+  ASSERT_EQ(2u, collected_events_.size());
   EXPECT_EQ("event1", collected_events_[0]);
   EXPECT_EQ("event2", collected_events_[1]);
 }
 
+TEST_F(TraceEventCallbackTest, TraceEventCallbackWhileFull) {
+  TraceLog::GetInstance()->SetEnabled(CategoryFilter("*"),
+      TraceLog::RECORD_UNTIL_FULL);
+  do {
+    TRACE_EVENT_INSTANT0("all", "badger badger", TRACE_EVENT_SCOPE_GLOBAL);
+  } while ((notifications_received_ & TraceLog::TRACE_BUFFER_FULL) == 0);
+  TraceLog::GetInstance()->SetEventCallback(Callback);
+  TRACE_EVENT_INSTANT0("all", "a snake", TRACE_EVENT_SCOPE_GLOBAL);
+  TraceLog::GetInstance()->SetEventCallback(NULL);
+  ASSERT_EQ(1u, collected_events_.size());
+  EXPECT_EQ("a snake", collected_events_[0]);
+}
+
 // TODO(dsinclair): Continuous Tracing unit test.
 
 // Test the category filter.