Cleanup PreferenceMACs on browser_tests startup

services/preferences/tracked/registry_hash_store_contents_win.cc

 // Copyright (c) 2016 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 "services/preferences/tracked/registry_hash_store_contents_win.h"
 
 #include <windows.h>
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/numerics/safe_conversions.h"
 #include "base/strings/string_split.h"
+#include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "base/win/registry.h"
 #include "services/preferences/public/cpp/tracked/tracked_preference_histogram_names.h"
 
 using base::win::RegistryValueIterator;
 
 namespace {
 
@@ skipping 41 matching lines @@
     return key.DeleteKey(L"") == ERROR_SUCCESS;
   }
   return false;
 }
 
 }  // namespace
 
 RegistryHashStoreContentsWin::RegistryHashStoreContentsWin(
     const base::string16& registry_path,
     const base::string16& store_key)
-    : preference_key_name_(registry_path + L"\\PreferenceMACs\\" + store_key) {}
+    : preference_key_name_(registry_path + L"\\PreferenceMACs\\" + store_key),
+      reset_on_delete_(base::StartsWith(store_key,

[jam, 2017/05/17 19:06:16]

I'm curious, it seems this cleanup code is quite far from the place that's
creating scoped_dir profiles. so there's a risk that as that code changes, say
by updating the name, we will forget this part (and also the test cleanup).

is it possible to mark this object at the callsite that knows it's creating a
scoped temp dir?

[gab, 2017/05/17 20:01:05]

I debated this too... basically the problem is a test does
CreateProfile(scoped_tmp_dir.path(), ...) and that dir eventually trickles here.
The "faulty" test has no idea it's even triggering this code so asking it to
mark anything at the callsite will likely result in someone forgetting to
eventually when writing such a test (their test will still pass but will cause
this issue).

Added a static call on ScopedTempDir to retrieve the prefix it's using in order
to at least link the name it's using to this code.

[grt (UTC plus 2), 2017/05/18 08:21:58]

My suggestion on the bug was to create a ScopedProfile object for tests that
wraps a ScopedTempDir and does the appropriate registry cleanup. This will
require changing the existing tests, but it keeps the cleanup closer to where it
belongs. WDYT?

+                                        L"scoped_dir",
+                                        base::CompareCase::INSENSITIVE_ASCII)) {
+}
+
+RegistryHashStoreContentsWin::~RegistryHashStoreContentsWin() {
+  if (reset_on_delete_)
+    Reset();
+}
 
 RegistryHashStoreContentsWin::RegistryHashStoreContentsWin(
     const RegistryHashStoreContentsWin& other) = default;
 
 bool RegistryHashStoreContentsWin::IsCopyable() const {
   return true;
 }
 
 std::unique_ptr<HashStoreContents> RegistryHashStoreContentsWin::MakeCopy()
     const {
@@ skipping 88 matching lines @@
 std::string RegistryHashStoreContentsWin::GetSuperMac() const {
   NOTREACHED()
       << "RegistryHashStoreContents does not support the GetSuperMac operation";
   return NULL;
 }
 
 void RegistryHashStoreContentsWin::SetSuperMac(const std::string& super_mac) {
   NOTREACHED()
       << "RegistryHashStoreContents does not support the SetSuperMac operation";
 }