Persist setup metrics and have Chrome report them during UMA upload.

chrome/installer/setup/setup_main.cc

Index: chrome/installer/setup/setup_main.cc
diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index 997b8342c8cf71e3ce0fd163d6ca3c9a1f5c2e01..9e9e7f979797e01c971fce08290c70afbfd82f84 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/chrome/installer/setup/setup_main.cc
@@ -21,6 +21,9 @@
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram_base.h"
+#include "base/metrics/histogram_persistence.h"
+#include "base/metrics/persistent_memory_allocator.h"
 #include "base/path_service.h"
 #include "base/process/launch.h"
 #include "base/process/memory.h"
@@ -69,6 +72,7 @@
 #include "chrome/installer/util/self_cleaning_temp_dir.h"
 #include "chrome/installer/util/shell_util.h"
 #include "chrome/installer/util/user_experiment.h"
+#include "chrome/installer/util/util_constants.h"
 
 #if defined(GOOGLE_CHROME_BUILD)
 #include "chrome/installer/util/updating_app_registration_data.h"
@@ -1621,6 +1625,23 @@ int WINAPI wWinMain(HINSTANCE instance, HINSTANCE prev_instance,
   if (!installer::IsProcessorSupported())
     return installer::CPU_NOT_SUPPORTED;
 
+  // Create a persistent space for any histograms that can be reported later

[grt (UTC plus 2), 2016/02/04 15:36:42]

how about creating an installer_metrics.{cc,h} pair (or somesuch) with two
functions, one each for this block of code and the one below?

[bcwhite, 2016/02/04 21:51:13]

Done.

+  // by the Chrome process. The "id" is a timestamp so that Chrome can tell

[grt (UTC plus 2), 2016/02/04 15:36:42]

nit: The comment on line 1795 refers to the browser rather than Chrome. If it
truly is the browser process that is responsible for picking up the data and
reporting it (I think this is true), please harmonize both comments to "...by
the browser process...".

[bcwhite, 2016/02/04 21:51:13]

Done.

+  // if it has already reported these metrics. No name is given so no internal
+  // histograms are created or managed; such would be pointless because they
+  // would have to be created on the heap and thus never be persisted for
+  // reporting.
+  uint64_t allocator_id = base::Time::Now().ToTimeT();
+  base::SetPersistentHistogramMemoryAllocator(
+      new base::LocalPersistentMemoryAllocator(1 << 20,  // 1 MiB
+                                               allocator_id,
+                                               std::string()));
+
+  // Now that histograms have a persistent place to be stored, the allocator
+  // itself can create histograms within that space.
+  base::GetPersistentHistogramMemoryAllocator()
+      ->CreateTrackingHistograms(installer::kSetupHistogramAllocatorName);
+
   // The exit manager is in charge of calling the dtors of singletons.
   base::AtExitManager exit_manager;
   base::CommandLine::Init(0, NULL);
@@ -1770,6 +1791,34 @@ int WINAPI wWinMain(HINSTANCE instance, HINSTANCE prev_instance,
     return_code = InstallUtil::GetInstallReturnCode(install_status);
   }
 
+  // Save all the persistent histograms to a single file on disk for reading

[grt (UTC plus 2), 2016/02/04 15:36:42]

using a single file means that we'll get only a random subset of installer UMA
data. is it possible to not lose data in some reasonable way? multiple files,
perhaps?

[bcwhite, 2016/02/04 21:51:13]

All things are possible... for a cost.  Metrics are reported at launch and every
30 minutes after that.  We'll only fail to report data if there are multiple
runs between those times.

Once read/write memory-mapped files are supported (a few months down my
timeline), it may be possible to switch to that which will mean that no records
will get lost no matter how many runs occur.

[grt (UTC plus 2), 2016/02/08 18:09:18]

Okay. Getting some data is better than none, so I'm okay with this for now. Is
there a way to measure how much data we're losing in the current scheme? Please
open a new crbug issue for this (r/w mapped files so there's no data loss) so
that it isn't forgotten about.

[bcwhite, 2016/02/09 21:08:45]

I've got some ideas on how to extend this but it needs more thought.  It's
complicated by the privilege separation (i.e. Chrome can't change, delete, or
rename files created by setup).

[grt (UTC plus 2), 2016/02/10 16:01:53]

For a system-level install, setup.exe and chrome.exe can interact via
ClientStateMedium. Perhaps chrome.exe can relay its collection state to
setup.exe by way of a value there. Happy to brainstorm with you when the time is
right.

+  // by the browser so it can be reported. For atomicity, first write to a
+  // temporary file and then rename it. The ImportantFileWriter would be good
+  // for this except it supports only std::string for its data.
+  base::FilePath file_path = installer_state.target_path().AppendASCII(
+      installer::kSetupHistogramAllocatorName).AddExtension(L".pma");
+  base::FilePath tmp_file_path;
+  base::DeleteFile(file_path, false);
+  if (base::CreateTemporaryFileInDir(file_path.DirName(), &tmp_file_path)) {
+    base::File histograms(tmp_file_path,
+                          base::File::FLAG_OPEN | base::File::FLAG_WRITE);
+    if (histograms.IsValid()) {
+      base::PersistentMemoryAllocator* alloc =
+          base::GetPersistentHistogramMemoryAllocator();
+      int used = static_cast<int>(alloc->used());
+      if (histograms.Write(0, static_cast<const char*>(alloc->data()),
+                           used) == used &&
+          histograms.Flush()) {
+        histograms.Close();
+        if (base::ReplaceFile(tmp_file_path, file_path, nullptr)) {

[grt (UTC plus 2), 2016/02/04 15:36:42]

this will fail so long as chrome.exe has the |file_path| mapped in memory since
FileMetricsProvider::CheckAndMapNewMetrics opens the file without SHARE_DELETE.

[bcwhite, 2016/02/04 21:51:13]

Basically part of the multiple-runs problem.  FileMetricsProvider only keep the
file mapped until the values are sent, after which it releases it.

[grt (UTC plus 2), 2016/02/08 18:09:18]

Right now that window is pretty big (~30m), right? Are you planning to change
the Chrome side so that it reads the file during the scanning phase rather than
keeping it mapped until histograms are collected?

[bcwhite, 2016/02/09 21:08:45]

Yes.  Either I'll pre-read it into an alternate memory buffer or do everything
off the UI thread or... something else.

+          VLOG(1) << "Persistent histograms saved in file: "
+                  << file_path.AsUTF8Unsafe();
+        }
+      }
+    }
+  }
+  base::DeleteFile(tmp_file_path, false);
+
   VLOG(1) << "Installation complete, returning: " << return_code;
 
   return return_code;