[tracing] Implement Add/RemoveTraceStateObserver for default platform.

src/libplatform/tracing/tracing-controller.cc

 // Copyright 2016 the V8 project 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 <stdio.h>
 #include <string.h>
 
 #include "include/libplatform/v8-tracing.h"
 
 #include "src/base/platform/mutex.h"
@@ skipping 20 matching lines @@
 // Indexes here have to match the g_category_groups array indexes above.
 const int g_category_already_shutdown = 1;
 const int g_category_categories_exhausted = 2;
 // Metadata category not used in V8.
 // const int g_category_metadata = 3;
 const int g_num_builtin_categories = 4;
 
 // Skip default categories.
 v8::base::AtomicWord g_category_index = g_num_builtin_categories;
 
+TracingController::TracingController() {}
+
+TracingController::~TracingController() {}
+
 void TracingController::Initialize(TraceBuffer* trace_buffer) {
   trace_buffer_.reset(trace_buffer);
+  mutex_.reset(new base::Mutex());
 }
 
 uint64_t TracingController::AddTraceEvent(
     char phase, const uint8_t* category_enabled_flag, const char* name,
     const char* scope, uint64_t id, uint64_t bind_id, int num_args,
     const char** arg_names, const uint8_t* arg_types,
     const uint64_t* arg_values, unsigned int flags) {
   uint64_t handle;
   TraceObject* trace_object = trace_buffer_->AddTraceEvent(&handle);
   if (trace_object) {
@@ skipping 33 matching lines @@
                                                     MAX_CATEGORY_GROUPS));
   uintptr_t category_index =
       (category_ptr - category_begin) / sizeof(g_category_group_enabled[0]);
   return g_category_groups[category_index];
 }
 
 void TracingController::StartTracing(TraceConfig* trace_config) {
   trace_config_.reset(trace_config);
   mode_ = RECORDING_MODE;
   UpdateCategoryGroupEnabledFlags();
+  std::unordered_set<Platform::TraceStateObserver*> observers_copy;
+  {
+    base::LockGuard<base::Mutex> lock(mutex_.get());
+    observers_copy = observers_;

[fmeawad, 2016/09/27 18:05:49]

nit: I would argue that the copy constructor has the same overhead as the
iterator below. So I would put the iterator inside the lock

[caseq, 2016/09/27 18:09:04]

No. This is about not invoking callbacks from under a lock.

+  }
+  for (auto o : observers_copy) {
+    o->OnTraceEnabled();
+  }
 }
 
 void TracingController::StopTracing() {
   mode_ = DISABLED;
   UpdateCategoryGroupEnabledFlags();
+  std::unordered_set<Platform::TraceStateObserver*> observers_copy;
+  {
+    base::LockGuard<base::Mutex> lock(mutex_.get());
+    observers_copy = observers_;
+  }
+  for (auto o : observers_copy) {
+    o->OnTraceDisabled();

[caseq, 2016/09/27 17:50:22]

I'm a bit concerned about this being potentially called from
DefaultPlatform::~DefaultPlatform(), from under a lock and with parts of the
default platform already dead. Should we just ignore/clear the observers within
the destructor?

[fmeawad, 2016/09/27 18:05:49]

We need to disable the observer on stop tracing, if we are collecting cpu
profile, we should stop and not wait until the ~ is called.

[alph, 2016/09/27 19:10:53]

I moved StopTracing to the very beginning of platform destructor outside of
lock.

+  }
   trace_buffer_->Flush();
 }
 
 void TracingController::UpdateCategoryGroupEnabledFlag(size_t category_index) {
   unsigned char enabled_flag = 0;
   const char* category_group = g_category_groups[category_index];
   if (mode_ == RECORDING_MODE &&
       trace_config_->IsCategoryGroupEnabled(category_group)) {
     enabled_flag |= ENABLED_FOR_RECORDING;
   }
@@ skipping 54 matching lines @@
     category_group_enabled = &g_category_group_enabled[category_index];
     // Update the max index now.
     base::Release_Store(&g_category_index, category_index + 1);
   } else {
     category_group_enabled =
         &g_category_group_enabled[g_category_categories_exhausted];
   }
   return category_group_enabled;
 }
 
+void TracingController::AddTraceStateObserver(
+    Platform::TraceStateObserver* observer) {
+  base::LockGuard<base::Mutex> lock(mutex_.get());
+  observers_.insert(observer);
+}
+
+void TracingController::RemoveTraceStateObserver(
+    Platform::TraceStateObserver* observer) {
+  base::LockGuard<base::Mutex> lock(mutex_.get());
+  DCHECK(observers_.find(observer) != observers_.end());
+  observers_.erase(observer);
+}
+
 }  // namespace tracing
 }  // namespace platform
 }  // namespace v8