Use GetDeterministicMachineSpecificId instead of RLZ for device_id

components/user_prefs/tracked/pref_hash_store_impl.cc

 // Copyright 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 "components/user_prefs/tracked/pref_hash_store_impl.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/logging.h"
 #include "base/macros.h"
-#include "base/metrics/histogram.h"
+#include "base/metrics/histogram_macros.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() {
+  std::string device_id;
+  MachineIdStatus status = GetDeterministicMachineSpecificId(&device_id);
+  // TODO(proberge): Remove this histogram once we validate that machine id
+  // generation is not flaky.
+  UMA_HISTOGRAM_ENUMERATION("Settings.MachineIdGenerationStatus", status,

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

I don't think we want to report not implemented, i.e.:

if (status != MachineIdStatus::NOT_IMPLEMENTED) {
  UMA_HISTOGRAM_BOOLEAN("Settings.MachineIdGenerationSucess", 
                        status == MachineIdStatus::SUCCESS);
}

with a "BooleanSuccess" histogram type in histograms.xml instead of the enum.

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

Done.

+                            MachineIdStatus::NUM_STATUS);
+
+  if (status == MachineIdStatus::SUCCESS)
+    return device_id;
+
+#if defined(OS_WIN) || defined(OS_MACOSX)

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

Use MachineIdStatus::NOT_IMPLEMENTED vs FAILURE instead of ifdefs here.

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

Done.

+  // On Windows and MacOSX we need a machine id to validate and sign
+  // preferences.
+  DCHECK("Failed to generate machine specific id for tracked preferences.");

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

To generate a DCHECK message you need to use << (right now this is checking that
the char* for this string is non-null).

Overall I think the histogram is more useful than the DCHECK so let's remove the
DCHECK for now (we can re-evaluate whether to make it into a CHECK after we get
histogram results).

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

doh! Good catch. 

Removed the DCHECK for now.

+#endif
+  return std::string();
+}
+
+}  // namespace
+
 class PrefHashStoreImpl::PrefHashStoreTransactionImpl
     : public PrefHashStoreTransaction {
  public:
   // Constructs a PrefHashStoreTransactionImpl which can use the private
   // members of its |outer| PrefHashStoreImpl.
   PrefHashStoreTransactionImpl(PrefHashStoreImpl* outer,
                                HashStoreContents* storage);
   ~PrefHashStoreTransactionImpl() override;
 
   // PrefHashStoreTransaction implementation.
@@ skipping 17 matching lines @@
   PrefHashStoreImpl* outer_;
   HashStoreContents* contents_;
 
   bool super_mac_valid_;
   bool super_mac_dirty_;
 
   DISALLOW_COPY_AND_ASSIGN(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() {
 }
 
 std::unique_ptr<PrefHashStoreTransaction> PrefHashStoreImpl::BeginTransaction(
     HashStoreContents* storage) {
   return std::unique_ptr<PrefHashStoreTransaction>(
       new PrefHashStoreTransactionImpl(this, std::move(storage)));
 }
 
@@ skipping 216 matching lines @@
 bool PrefHashStoreImpl::PrefHashStoreTransactionImpl::IsSuperMACValid() const {
   return super_mac_valid_;
 }
 
 bool PrefHashStoreImpl::PrefHashStoreTransactionImpl::StampSuperMac() {
   if (!outer_->use_super_mac_ || super_mac_valid_)
     return false;
   super_mac_dirty_ = true;
   return true;
 }