[Cleanup] Migrate ChromeBrowserMainExtraPartsMetrics to use the Task Scheduler.

chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc

 // Copyright (c) 2013 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 "chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/cpu.h"
 #include "base/macros.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/sys_info.h"
+#include "base/task_scheduler/post_task.h"
+#include "base/task_scheduler/task_traits.h"
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/about_flags.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_browser_main.h"
 #include "chrome/browser/mac/bluetooth_utility.h"
 #include "chrome/browser/shell_integration.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "components/flags_ui/pref_service_flags_storage.h"
-#include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
 #include "ui/base/touch/touch_device.h"
 #include "ui/base/ui_base_switches.h"
 #include "ui/display/screen.h"
 
 #if !defined(OS_ANDROID)
 #include "chrome/browser/metrics/first_web_contents_profiler.h"
 #include "chrome/browser/metrics/tab_usage_recorder.h"
 #endif  // !defined(OS_ANDROID)
 
 #if defined(OS_ANDROID) && defined(__arm__)
 #include <cpu-features.h>
 #endif  // defined(OS_ANDROID) && defined(__arm__)
 
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 #include <gnu/libc-version.h>
 
 #include "base/linux_util.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
-#include "base/task_scheduler/post_task.h"
 #include "base/version.h"
 #if defined(USE_X11)
 #include "ui/base/x/x11_util.h"
 #endif
 #endif  // defined(OS_LINUX) && !defined(OS_CHROMEOS)
 
 #if defined(USE_OZONE) || defined(USE_X11)
 #include "ui/events/devices/input_device_event_observer.h"
 #include "ui/events/devices/input_device_manager.h"
 #endif  // defined(USE_OZONE) || defined(USE_X11)
@@ skipping 110 matching lines @@
   } else {
     UMA_HISTOGRAM_ENUMERATION("Android.ArmFpu",
                               UMA_ANDROID_ARM_FPU_VFPV3_D16,
                               UMA_ANDROID_ARM_FPU_COUNT);
   }
 #endif  // defined(OS_ANDROID) && defined(__arm__)
   UMA_HISTOGRAM_SPARSE_SLOWLY("Platform.LogicalCpuCount",
                               base::SysInfo::NumberOfProcessors());
 }
 
-// Called on the blocking pool some time after startup to avoid slowing down
+// Called on a background thread, with low priority to avoid slowing down
 // startup with metrics that aren't trivial to compute.
-void RecordStartupMetricsOnBlockingPool() {
+void RecordStartupMetrics() {
 #if defined(OS_WIN)
   GoogleUpdateSettings::RecordChromeUpdatePolicyHistograms();
 
   const base::win::OSInfo& os_info = *base::win::OSInfo::GetInstance();
   UMA_HISTOGRAM_ENUMERATION("Windows.GetVersionExVersion", os_info.version(),
                             base::win::VERSION_WIN_LAST);
   UMA_HISTOGRAM_ENUMERATION("Windows.Kernel32Version",
                             os_info.Kernel32Version(),
                             base::win::VERSION_WIN_LAST);
   UMA_HISTOGRAM_BOOLEAN("Windows.InCompatibilityMode",
@@ skipping 262 matching lines @@
 
   // Used for histograms; do not reorder.
   enum Result { NOT_PINNED = 0, PINNED = 1, FAILURE = 2, NUM_RESULTS };
 
   Result result = FAILURE;
   if (succeeded)
     result = is_pinned_to_taskbar ? PINNED : NOT_PINNED;
   UMA_HISTOGRAM_ENUMERATION("Windows.IsPinnedToTaskbar", result, NUM_RESULTS);
 }
 
-// Records the pinned state of the current executable into a histogram.
+// Records the pinned state of the current executable into a histogram. Should
+// be called on a background thread, with low priority, to avoid slowing down
+// startup.
 void RecordIsPinnedToTaskbarHistogram() {
   shell_integration::win::GetIsPinnedToTaskbarState(
       base::Bind(&OnShellHandlerConnectionError),
       base::Bind(&OnIsPinnedToTaskbarResult));
 }
 #endif  // defined(OS_WIN)
 
 }  // namespace
 
 ChromeBrowserMainExtraPartsMetrics::ChromeBrowserMainExtraPartsMetrics()
@@ skipping 24 matching lines @@
                                                             );
 #endif
 }
 
 void ChromeBrowserMainExtraPartsMetrics::PostBrowserStart() {
   RecordLinuxGlibcVersion();
 #if defined(OS_LINUX) && defined(USE_X11) && !defined(OS_CHROMEOS)
   UMA_HISTOGRAM_ENUMERATION("Linux.WindowManager", GetLinuxWindowManager(),
                             UMA_LINUX_WINDOW_MANAGER_COUNT);
 #endif
+
+  base::TaskTraits background_task_traits = {

[Alexei Svitkine (slow), 2017/07/12 15:12:56]

Nit: const

[Ilya Sherman, 2017/07/12 19:39:46]

Done.

+      base::MayBlock(), base::TaskPriority::BACKGROUND,
+      base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN};
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
-  base::PostTaskWithTraits(FROM_HERE,
-                           {base::MayBlock(), base::TaskPriority::BACKGROUND},
+  base::PostTaskWithTraits(FROM_HERE, background_task_traits,
                            base::BindOnce(&RecordLinuxDistro));
 #endif
 
 #if defined(USE_OZONE) || defined(USE_X11)
   // The touch event state for X11 and Ozone based event sub-systems are based
   // on device scans that happen asynchronously. So we may need to attach an
   // observer to wait until these scans complete.
   if (ui::InputDeviceManager::GetInstance()->AreDeviceListsComplete()) {
     RecordTouchEventState();
   } else {
     input_device_event_observer_.reset(
         new AsynchronousTouchEventStateRecorder());
   }
 #else
   RecordTouchEventState();
 #endif  // defined(USE_OZONE) || defined(USE_X11)
 
 #if defined(OS_MACOSX)
   RecordMacMetrics();
 #endif  // defined(OS_MACOSX)
 
-  constexpr base::TimeDelta kStartupMetricsGatheringDelay =
-      base::TimeDelta::FromSeconds(45);
-  content::BrowserThread::GetBlockingPool()->PostDelayedTask(
-      FROM_HERE, base::BindOnce(&RecordStartupMetricsOnBlockingPool),
-      kStartupMetricsGatheringDelay);
 #if defined(OS_WIN)
-  content::BrowserThread::PostDelayedTask(
-      content::BrowserThread::IO, FROM_HERE,
-      base::Bind(&RecordIsPinnedToTaskbarHistogram),
-      kStartupMetricsGatheringDelay);
+  // RecordStartupMetrics calls into shell_integration::GetDefaultBrowser(),
+  // which requires a COM thread on Windows.
+  base::CreateCOMSTATaskRunnerWithTraits(background_task_traits)
+      ->PostTask(FROM_HERE, base::BindOnce(&RecordStartupMetrics));
+#else
+  base::PostTaskWithTraits(FROM_HERE, background_task_traits,
+                           base::BindOnce(&RecordStartupMetrics));

[Alexei Svitkine (slow), 2017/07/12 15:12:56]

The behavior change is we're now no longer doing this 45 seconds after startup.
Can you add that info to CL description? Mention that you don't expect it to
have an effect on the metrics (since I assume that's the case.)

Probably would be good to verify that as well, once this lands - i.e. manually
check the metrics to ensure they didn't change because of this.

[Ilya Sherman, 2017/07/12 19:39:46]

Good point!  Updated the CL description and filed a follow-up bug.

+#endif  // defined(OS_WIN)
+
+#if defined(OS_WIN)
+  // TODO(isherman): The delay below is currently needed to avoid (flakily)
+  // breaking some tests, including all of the ProcessMemoryMetricsEmitterTest
+  // tests. Figure out why there is a dependency and fix the tests.
+  base::CreateSequencedTaskRunnerWithTraits(background_task_traits)
+      ->PostDelayedTask(FROM_HERE,
+                        base::BindOnce(&RecordIsPinnedToTaskbarHistogram),
+                        base::TimeDelta::FromSeconds(45));
 #endif  // defined(OS_WIN)
 
   display_count_ = display::Screen::GetScreen()->GetNumDisplays();
   UMA_HISTOGRAM_COUNTS_100("Hardware.Display.Count.OnStartup", display_count_);
   display::Screen::GetScreen()->AddObserver(this);
   is_screen_observer_ = true;
 
 #if !defined(OS_ANDROID)
   const BrowserList* browser_list = BrowserList::GetInstance();
 
@@ skipping 32 matching lines @@
   }
 }
 
 namespace chrome {
 
 void AddMetricsExtraParts(ChromeBrowserMainParts* main_parts) {
   main_parts->AddParts(new ChromeBrowserMainExtraPartsMetrics());
 }
 
 }  // namespace chrome