Fix a race condition in ~RegistryHashStoreContentsWin

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/files/scoped_temp_dir.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 36 matching lines @@
                    const std::string& split_key_name) {
   base::win::RegKey key;
   if (key.Open(HKEY_CURRENT_USER,
                GetSplitPrefKeyName(reg_key_name, split_key_name).c_str(),
                KEY_SET_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS) {
     return key.DeleteKey(L"") == ERROR_SUCCESS;
   }
   return false;
 }
 
+// Deletes |reg_key_name| if it exists.
+void DeleteRegistryKey(const base::string16& reg_key_name) {
+  base::win::RegKey key;
+  if (key.Open(HKEY_CURRENT_USER, reg_key_name.c_str(),
+               KEY_SET_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS) {
+    LONG result = key.DeleteKey(L"");
+    DCHECK(result == ERROR_SUCCESS || result == ERROR_FILE_NOT_FOUND) << result;
+  }
+}
+
 }  // namespace
 
-RegistryHashStoreContentsWin::RegistryHashStoreContentsWin(
-    const base::string16& registry_path,
-    const base::string16& store_key)
-    : preference_key_name_(registry_path + L"\\PreferenceMACs\\" + store_key),
-      reset_on_delete_(base::StartsWith(store_key,
-                                        base::ScopedTempDir::GetTempDirPrefix(),
-                                        base::CompareCase::INSENSITIVE_ASCII)) {
+void TempScopedDirRegistryCleaner::SetRegistryPath(
+    const base::string16& registry_path) {
+  DCHECK(registry_path_ == L"" || registry_path_ == registry_path);
+  registry_path_ = registry_path;

[gab, 2017/06/12 15:52:16]

if (resitry_path_.empty())
  registry_path_ = registry_path;
else
  DCHECK_EQ(registry_path_, registry_path);

(avoids string copy in = case)

[proberge, 2017/06/12 18:46:02]

Done.

 }
 
-RegistryHashStoreContentsWin::~RegistryHashStoreContentsWin() {
-  if (reset_on_delete_)
-    Reset();
+TempScopedDirRegistryCleaner::~TempScopedDirRegistryCleaner() {
+  DCHECK(registry_path_ != L"");

[gab, 2017/06/12 15:52:16]

DCHECK(!registry_path_.empty());

[proberge, 2017/06/12 18:46:02]

Done.

+  DeleteRegistryKey(registry_path_);
 }
 
 RegistryHashStoreContentsWin::RegistryHashStoreContentsWin(
+    const base::string16& registry_path,
+    const base::string16& store_key,
+    scoped_refptr<TempScopedDirCleaner> temp_dir_cleaner)
+    : preference_key_name_(registry_path + L"\\PreferenceMACs\\" + store_key),
+      temp_dir_cleaner_(temp_dir_cleaner) {

[gab, 2017/06/12 15:52:16]

std::move

[proberge, 2017/06/12 18:46:02]

Done.

+  if (temp_dir_cleaner_)
+    static_cast<TempScopedDirRegistryCleaner*>(temp_dir_cleaner_.get())
+        ->SetRegistryPath(preference_key_name_);
+}
+
+RegistryHashStoreContentsWin::RegistryHashStoreContentsWin(
     const RegistryHashStoreContentsWin& other) = default;
 
 bool RegistryHashStoreContentsWin::IsCopyable() const {
   return true;
 }
 
 std::unique_ptr<HashStoreContents> RegistryHashStoreContentsWin::MakeCopy()
     const {
   return base::WrapUnique(new RegistryHashStoreContentsWin(*this));
 }
 
 base::StringPiece RegistryHashStoreContentsWin::GetUMASuffix() const {
   return user_prefs::tracked::kTrackedPrefRegistryValidationSuffix;
 }
 
 void RegistryHashStoreContentsWin::Reset() {
-  base::win::RegKey key;
-  if (key.Open(HKEY_CURRENT_USER, preference_key_name_.c_str(),
-               KEY_SET_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS) {
-    LONG result = key.DeleteKey(L"");
-    DCHECK(result == ERROR_SUCCESS || result == ERROR_FILE_NOT_FOUND) << result;
-  }
+  DeleteRegistryKey(preference_key_name_);
 }
 
 bool RegistryHashStoreContentsWin::GetMac(const std::string& path,
                                           std::string* out_value) {
   base::win::RegKey key;
   if (key.Open(HKEY_CURRENT_USER, preference_key_name_.c_str(),
                KEY_QUERY_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS) {
     return ReadMacFromRegistry(key, path, out_value);
   }
 
@@ skipping 64 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";
 }