Wire in postmortem report collection

chrome/browser/metrics/chrome_metrics_service_client.cc

Index: chrome/browser/metrics/chrome_metrics_service_client.cc
diff --git a/chrome/browser/metrics/chrome_metrics_service_client.cc b/chrome/browser/metrics/chrome_metrics_service_client.cc
index b1af4ef6ecbcb0294a9c5476fb4f538339abfe9b..032ef6c28d0371aa5ccfbb70dc1433abe74191ea 100644
--- a/chrome/browser/metrics/chrome_metrics_service_client.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_client.cc
@@ -249,6 +249,8 @@ ChromeMetricsServiceClient::ChromeMetricsServiceClient(
 #endif
 #if defined(OS_WIN)
       google_update_metrics_provider_(nullptr),
+      watcher_metrics_provider_(nullptr),
+      antivirus_metrics_provider_(nullptr),

[Sigurður Ásgeirsson, 2016/09/19 14:20:05]

This init was missing?

[manzagop (departed), 2016/09/21 22:00:43]

Correct. Ack.

 #endif
       drive_metrics_provider_(nullptr),
       start_time_(base::TimeTicks::Now()),
@@ -498,11 +500,19 @@ void ChromeMetricsServiceClient::Initialize() {
       std::unique_ptr<metrics::MetricsProvider>(
           google_update_metrics_provider_));
 
+  base::FilePath user_data_dir;
+  base::FilePath crash_dir;
+  if (!base::PathService::Get(chrome::DIR_USER_DATA, &user_data_dir) ||
+      !base::PathService::Get(chrome::DIR_CRASH_DUMPS, &crash_dir)) {
+    // If either call fails, then clear both.
+    user_data_dir = base::FilePath();
+    crash_dir = base::FilePath();
+  }
+  watcher_metrics_provider_ = new browser_watcher::WatcherMetricsProviderWin(
+      chrome::GetBrowserExitCodesRegistryPath(), user_data_dir, crash_dir,
+      content::BrowserThread::GetBlockingPool());
   metrics_service_->RegisterMetricsProvider(
-      std::unique_ptr<metrics::MetricsProvider>(
-          new browser_watcher::WatcherMetricsProviderWin(
-              chrome::GetBrowserExitCodesRegistryPath(),
-              content::BrowserThread::GetBlockingPool())));
+      std::unique_ptr<metrics::MetricsProvider>(watcher_metrics_provider_));
 
   antivirus_metrics_provider_ = new AntiVirusMetricsProvider(
       content::BrowserThread::GetBlockingPool()
@@ -620,6 +630,18 @@ void ChromeMetricsServiceClient::OnInitTaskGotAntiVirusData() {
 }
 
 void ChromeMetricsServiceClient::OnInitTaskGotDriveMetrics() {
+  const base::Closure on_collected_callback = base::Bind(

[Sigurður Ásgeirsson, 2016/09/19 14:20:05]

I'm pretty lost as to what runs on which thread here, and at which point in
time. Is this the UI thread?

[manzagop (departed), 2016/09/21 22:00:43]

When the MetricsService calls OpenNewLog in the initialized state, that posts a
delayed MetricsService::StartInitTask on the UI thread's
base::ThreadTaskRunnerHandle::Get() - I think that may actually be the UI thread
itself. Note that on Windows, this is posted 30 seconds in the future.
StartInitTask leads to chain of tasks on that same thread, which eventually ends
up calling MetricsService::FinishedInitTask, which is the upload of the initial
log (that's *not* the initial *stability* log). 

The trick though is that CollectPostmortemReports is an empty shell that does a
PostTaskAndReply (runs the actual work on an io_task_runner_, then calls back to
the original thread).

+      &ChromeMetricsServiceClient::OnInitTaskCollectedPostmortemReports,
+      weak_ptr_factory_.GetWeakPtr());
+
+#if defined(OS_WIN)
+  watcher_metrics_provider_->CollectPostmortemReports(on_collected_callback);
+#else
+  on_collected_callback.Run();
+#endif  // defined(OS_WIN)
+}
+
+void ChromeMetricsServiceClient::OnInitTaskCollectedPostmortemReports() {
   finished_init_task_callback_.Run();
 }