[Cleanup] Migrate ChromeBrowserMainExtraPartsMetrics to use the Task Scheduler.

chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc

Index: chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
diff --git a/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc b/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
index c142faadcd5de1d5673bc70a1330c898ff44142a..20c0d8ae8d68c5921498050ef8403145e8bdc73b 100644
--- a/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
+++ b/chrome/browser/metrics/chrome_browser_main_extra_parts_metrics.cc
@@ -13,6 +13,8 @@
 #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"
@@ -25,7 +27,6 @@
 #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"
@@ -46,7 +47,6 @@
 #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"
@@ -177,9 +177,9 @@ void RecordMicroArchitectureStats() {
                               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();
 
@@ -462,7 +462,9 @@ void OnIsPinnedToTaskbarResult(bool succeeded, bool is_pinned_to_taskbar) {
   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),
@@ -507,9 +509,12 @@ void ChromeBrowserMainExtraPartsMetrics::PostBrowserStart() {
   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
 
@@ -531,16 +536,24 @@ void ChromeBrowserMainExtraPartsMetrics::PostBrowserStart() {
   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();