Use GetDeterministicMachineSpecificId instead of RLZ for device_id

components/user_prefs/tracked/pref_hash_store_impl.cc

Index: components/user_prefs/tracked/pref_hash_store_impl.cc
diff --git a/components/user_prefs/tracked/pref_hash_store_impl.cc b/components/user_prefs/tracked/pref_hash_store_impl.cc
index 80438d6954a45c9c791a2c7d620e108fdeff3141..6ba2c77a9a8450cfce9b6d7d4062f824300e52b7 100644
--- a/components/user_prefs/tracked/pref_hash_store_impl.cc
+++ b/components/user_prefs/tracked/pref_hash_store_impl.cc
@@ -10,8 +10,22 @@
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/metrics/histogram.h"
+#include "components/user_prefs/tracked/device_id.h"
 #include "components/user_prefs/tracked/hash_store_contents.h"
 
+namespace {
+
+// Returns a deterministic ID for this machine.
+std::string GenerateDeviceId() {

[proberge, 2017/01/18 16:10:58]

Possible optimization:

This method is called up to 6 times on Windows on startup. Maybe we could cache
the result static-ly to avoid calls; I don't know how fast the
GetDeterministicMachineSpecificId methods are on Windows or MacOSX. wdyt? 

[gab, 2017/01/19 18:52:05]

Ah yes, that's a great idea let's do it. A static variable in this method would
do just fine, thanks

[proberge, 2017/01/19 20:42:59]

Done.

+  std::string device_id;
+  if (GetDeterministicMachineSpecificId(&device_id) == MachineIdStatus::SUCCESS)

[proberge, 2017/01/18 16:10:58]

One thing I worry about is the stability of this method. I assume that the
implementation is correct and that the returned ID is deterministic, but what if
there are intermittent failures? It seems like this method would return the
empty string and we would reset preferences on Windows.

I think I'll add a DCHECK and an histogram. wdyt?

[gab, 2017/01/19 18:52:05]

It better be 100% deterministic and non-flaky. I'm pretty sure the Windows one
is because it's a subset of the logic for the RLZ id we've been using for a
while and have never had complaints about AFAIK.

Let's confirm that the Mac one also is.

A DCHECK and a histogram sounds good to me. In fact we might even want to CHECK
against failure because failure here is very serious and would result in erasing
user settings... but I'm happy to start with a histogram.

[proberge, 2017/01/19 20:42:59]

Done.

+    return device_id;
+
+  return std::string();
+}
+
+}  // namespace
+
 class PrefHashStoreImpl::PrefHashStoreTransactionImpl
     : public PrefHashStoreTransaction {
  public:
@@ -49,10 +63,10 @@ class PrefHashStoreImpl::PrefHashStoreTransactionImpl
 };
 
 PrefHashStoreImpl::PrefHashStoreImpl(const std::string& seed,
-                                     const std::string& device_id,
+                                     const std::string& legacy_device_id,
                                      bool use_super_mac)
-    : pref_hash_calculator_(seed, device_id), use_super_mac_(use_super_mac) {
-}
+    : pref_hash_calculator_(seed, GenerateDeviceId(), legacy_device_id),
+      use_super_mac_(use_super_mac) {}
 
 PrefHashStoreImpl::~PrefHashStoreImpl() {
 }