[ETW Export] Add polling for ETW keyword.

base/trace_event/trace_event_etw_export_win.cc

 // Copyright 2015 The Chromium 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 "base/trace_event/trace_event_etw_export_win.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #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"
 
 // The GetProcAddress technique is borrowed from
 // https://github.com/google/UIforETW/tree/master/ETWProviders
 //
 // EVNTAPI is used in evntprov.h which is included by chrome_events_win.h.
 // We define EVNTAPI without the DECLSPEC_IMPORT specifier so that we can
 // implement these functions locally instead of using the import library, and
 // can therefore still run on Windows XP.
@@ skipping 61 matching lines @@
     "toplevel",                                        // 0x200
     "v8",                                              // 0x400
     "disabled-by-default-cc.debug",                    // 0x800
     "disabled-by-default-cc.debug.picture",            // 0x1000
     "disabled-by-default-toplevel.flow"};              // 0x2000
 const char* other_events_group_name = "__OTHER_EVENTS";  // 0x2000000000000000
 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
 // chrome_events_win.h
 ULONG EVNTAPI EventRegister(LPCGUID ProviderId,
                             PENABLECALLBACK EnableCallback,
                             PVOID CallbackContext,
                             PREGHANDLE RegHandle) {
   if (EventRegisterProc)
     return EventRegisterProc(ProviderId, EnableCallback, CallbackContext,
@@ skipping 50 matching lines @@
 }
 
 // static
 TraceEventETWExport* TraceEventETWExport::GetInstance() {
   return Singleton<TraceEventETWExport,
                    StaticMemorySingletonTraits<TraceEventETWExport>>::get();
 }
 
 // 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;
 }
 
 // static
 bool TraceEventETWExport::IsETWExportEnabled() {
   return (GetInstance() && GetInstance()->etw_export_enabled_);
 }
 
 // static
 void TraceEventETWExport::AddEvent(
@@ skipping 189 matching lines @@
     DCHECK(categories_status_.find(disabled_other_events_group_name) !=
            categories_status_.end());
     return categories_status_.find(disabled_other_events_group_name)->second;
   } else {
     DCHECK(categories_status_.find(other_events_group_name) !=
            categories_status_.end());
     return categories_status_.find(other_events_group_name)->second;
   }
 }
 
+// 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