[ETW Export] Add polling for ETW keyword.

base/trace_event/trace_event_etw_export_win.cc

Index: base/trace_event/trace_event_etw_export_win.cc
diff --git a/base/trace_event/trace_event_etw_export_win.cc b/base/trace_event/trace_event_etw_export_win.cc
index e61a7b29645992d0e160d3d25cd91e2bc5fb58f9..a834378a9a0d1b8fe84b06b41d16f18b21cd03de 100644
--- a/base/trace_event/trace_event_etw_export_win.cc
+++ b/base/trace_event/trace_event_etw_export_win.cc
@@ -9,6 +9,7 @@
 #include "base/memory/singleton.h"
 #include "base/strings/string_tokenizer.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/threading/platform_thread.h"
 #include "base/trace_event/trace_event.h"
 #include "base/trace_event/trace_event_impl.h"
 
@@ -90,6 +91,29 @@ const char* disabled_other_events_group_name =
     "__DISABLED_OTHER_EVENTS";  // 0x4000000000000000
 uint64 other_events_keyword_bit = 1ULL << 61;
 uint64 disabled_other_events_keyword_bit = 1ULL << 62;
+
+// This object will be created by each process. It's a background (low-priority)
+// thread that will monitor the ETW keyword for any changes..
+class ETWKeywordUpdateThread : public base::PlatformThread::Delegate {
+ public:
+  ETWKeywordUpdateThread() {}
+  ~ETWKeywordUpdateThread() override {}
+
+  // Implementation of PlatformThread::Delegate:
+  void ThreadMain() override {
+    base::PlatformThread::SetName("ETW Keyword Update Thread");
+    while (1) {
+      base::PlatformThread::Sleep(
+          base::TimeDelta::FromMilliseconds(kUpdateTimerDelayMs));
+      base::trace_event::TraceEventETWExport::UpdateKeyword();
+    }
+  }
+
+ private:
+  // Time between checks for ETW keyword changes (in milliseconds).
+  unsigned int kUpdateTimerDelayMs = 1000;
+};
+
 }  // namespace
 
 // Redirector function for EventRegister. Called by macros in
@@ -160,13 +184,23 @@ TraceEventETWExport* TraceEventETWExport::GetInstance() {
 
 // static
 void TraceEventETWExport::EnableETWExport() {
-  if (GetInstance())
+  if (GetInstance() && !GetInstance()->etw_export_enabled_) {

[brucedawson, 2015/08/10 23:47:15]

The repeated calls to GetInstance() seem excessive to me. It feels like it would
be better to call it once and stash it in a pointer:

    auto* instance = GetInstance();
    if (instance && instant->etw_export_enabled_)

This might make the code slightly smaller and it feels like the right thing to
do.

[Georges Khalil, 2015/08/11 18:12:31]

Agreed, done. I did it everywhere for consistency.

     GetInstance()->etw_export_enabled_ = true;
+    // We only create the update thread once and it lives for as long as Chrome
+    // is running.
+    if (GetInstance()->keyword_update_thread_handle_.is_null()) {
+      GetInstance()->keyword_update_thread_.reset(new ETWKeywordUpdateThread);
+      PlatformThread::CreateWithPriority(
+          0, GetInstance()->keyword_update_thread_.get(),
+          &GetInstance()->keyword_update_thread_handle_,
+          ThreadPriority::BACKGROUND);
+    }
+  }
 }
 
 // static
 void TraceEventETWExport::DisableETWExport() {
-  if (GetInstance())
+  if (GetInstance() && GetInstance()->etw_export_enabled_)
     GetInstance()->etw_export_enabled_ = false;
 }
 
@@ -376,5 +410,13 @@ bool TraceEventETWExport::IsCategoryEnabled(const char* category_name) const {
   }
 }
 
+// static
+void TraceEventETWExport::UpdateKeyword() {
+  DCHECK(GetInstance());
+  if (IsETWExportEnabled()) {
+    if (GetInstance()->UpdateEnabledCategories())
+      TraceLog::GetInstance()->UpdateCategoryGroupEnabledFlags();

[brucedawson, 2015/08/10 23:47:15]

I think we need a lock around the data structures that are shared between the
background thread and the rest of Chrome? Also some comments saying which
functions are called from which thread(s).

[Georges Khalil, 2015/08/11 18:12:31]

Technically, yes, there's a very small chance of race here. But locking
UpdateCategoryGroupEnabledFlags would do more harm than good.

Worst case is that the flags don't get enabled correctly. 

+  }
+}
 }  // namespace trace_event
 }  // namespace base