windows: make CrashIdHelper only log on the main thread

When I looked at this code I convinced myself it's not used
on multiple threads. I'm wrong there. As we only care about
crashes on the main thread, this makes CrashIdHelper only
work on the main thread.

BUG=967947
TEST=none

Change-Id: Ic8e4c81012e31cff1f0ef771789a504de95d876b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1633550Reviewed-by: Michael Wasserman <msw@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#664355}

chrome/browser/chrome_browser_main_win.cc

@@ -93,6 +93,7 @@
 #include "ui/display/win/dpi.h"
 #include "ui/gfx/switches.h"
 #include "ui/gfx/system_fonts_win.h"
+#include "ui/gfx/win/crash_id_helper.h"
 #include "ui/strings/grit/app_locale_settings.h"
 
 #if defined(GOOGLE_CHROME_BUILD)
@@ -501,10 +502,11 @@ ChromeBrowserMainPartsWin::ChromeBrowserMainPartsWin(
     StartupData* startup_data)
     : ChromeBrowserMainParts(parameters, startup_data) {}
 
-ChromeBrowserMainPartsWin::~ChromeBrowserMainPartsWin() {
-}
+ChromeBrowserMainPartsWin::~ChromeBrowserMainPartsWin() = default;
 
 void ChromeBrowserMainPartsWin::ToolkitInitialized() {
+  DCHECK_NE(base::PlatformThread::CurrentId(), base::kInvalidThreadId);
+  gfx::CrashIdHelper::RegisterMainThread(base::PlatformThread::CurrentId());
   ChromeBrowserMainParts::ToolkitInitialized();
   gfx::win::SetAdjustFontCallback(&AdjustUIFont);
   gfx::win::SetGetMinimumFontSizeCallback(&GetMinimumFontSize);

ui/gfx/win/crash_id_helper.cc

@@ -15,6 +15,11 @@ CrashIdHelper* CrashIdHelper::Get() {
   return helper.get();
 }
 
+// static
+void CrashIdHelper::RegisterMainThread(base::PlatformThreadId thread_id) {
+  main_thread_id_ = thread_id;
+}
+
 CrashIdHelper::ScopedLogger::~ScopedLogger() {
   CrashIdHelper::Get()->OnDidProcessMessages();
 }
@@ -23,6 +28,11 @@ CrashIdHelper::ScopedLogger::ScopedLogger() = default;
 
 std::unique_ptr<CrashIdHelper::ScopedLogger>
 CrashIdHelper::OnWillProcessMessages(const std::string& id) {
+  if (main_thread_id_ == base::kInvalidThreadId ||
+      base::PlatformThread::CurrentId() != main_thread_id_) {
+    return nullptr;
+  }
+
   if (!ids_.empty())
     was_nested_ = true;
   ids_.push_back(id.empty() ? "unspecified" : id);
@@ -64,4 +74,7 @@ std::string CrashIdHelper::CurrentCrashId() const {
   return base::JoinString(ids_, ">");
 }
 
+// static
+base::PlatformThreadId CrashIdHelper::main_thread_id_ = base::kInvalidThreadId;
+
 }  // namespace gfx

ui/gfx/win/crash_id_helper.h

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "base/no_destructor.h"
+#include "base/threading/platform_thread.h"
 #include "components/crash/core/common/crash_key.h"
 #include "ui/gfx/gfx_export.h"
 
@@ -20,6 +21,8 @@ namespace gfx {
 // code, while the bug lies in client code. Logging an id helps better identify
 // the client code that created the window/widget.
 //
+// This class only logs ids on the thread identified by RegisterMainThread().
+//
 // Example usage:
 // {
 //   auto logger = CrashIdHelper::Get()->OnWillProcessMessages(crash_id);
@@ -29,6 +32,9 @@ class GFX_EXPORT CrashIdHelper {
  public:
   static CrashIdHelper* Get();
 
+  // Registers the thread used for logging.
+  static void RegisterMainThread(base::PlatformThreadId thread_id);
+
   // RAII style class that unregisters in the destructor.
   class GFX_EXPORT ScopedLogger {
    public:
@@ -43,6 +49,7 @@ class GFX_EXPORT CrashIdHelper {
 
   // Adds |id| to the list of active debugging ids. When the returned object
   // is destroyed, |id| is removed from the list of active debugging ids.
+  // Returns null if logging is not enabled on the current thread.
   std::unique_ptr<ScopedLogger> OnWillProcessMessages(const std::string& id);
 
  private:
@@ -71,6 +78,8 @@ class GFX_EXPORT CrashIdHelper {
   // views, which uses the term Widget.
   crash_reporter::CrashKeyString<128> debugging_crash_key_{"widget-id"};
 
+  static base::PlatformThreadId main_thread_id_;
+
   DISALLOW_COPY_AND_ASSIGN(CrashIdHelper);
 };
 

ui/gfx/win/crash_id_helper_unittest.cc

@@ -25,6 +25,8 @@ class CrashIdHelperTest : public testing::Test {
 // crash_reporter::CrashKeyString, but that class isn't particularly test
 // friendly (and the implementation varies depending upon compile time flags).
 TEST_F(CrashIdHelperTest, Basic) {
+  CrashIdHelper::RegisterMainThread(base::PlatformThread::CurrentId());
+
   const std::string id1 = "id";
   {
     auto scoper = CrashIdHelper::Get()->OnWillProcessMessages(id1);
@@ -45,6 +47,7 @@ TEST_F(CrashIdHelperTest, Basic) {
     }
     EXPECT_EQ("(N) id2", CurrentCrashId());
   }
+  CrashIdHelper::RegisterMainThread(base::kInvalidThreadId);
 }
 
 }  // namespace gfx