Update chrome_cleaner/os directory from internal repo

Get changes from the internal repo since last branch point:
- Replace hardcoded company name with COMPANY_SHORTNAME_STRING
- Add more paths to PreFetchedPaths
- Replace some VLOG statements with LOG(INFO)
- Enforce that RegKeyPath takes only predefined keys in rootkey
- Add chrome-cleanup-tool-test to keys in registry unit tests
- Add Chrome policy key constants to registry_util.h
- Replace StringPrintf with StrCat

R=csharp

Bug: 830892
Change-Id: I71c0c3f03ce852c4af4738056187512cfea9f82e
Reviewed-on: https://chromium-review.googlesource.com/1195802Reviewed-by: Chris Sharp <csharp@chromium.org>
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587692}

chrome/chrome_cleaner/os/BUILD.gn

@@ -142,11 +142,13 @@ source_set("unittest_sources") {
     ":cleaner_os",
     ":common_os",
     ":file_remover_api",
-    "//base",
+    "//base:base",
     "//base/test:test_config",
     "//base/test:test_support",
     "//chrome/chrome_cleaner/constants:common_strings",
+    "//chrome/chrome_cleaner/constants:version_header",
     "//chrome/chrome_cleaner/logging/proto:removal_status_proto",
+    "//chrome/chrome_cleaner/proto:shared_pup_enums_proto",
     "//chrome/chrome_cleaner/strings",
     "//chrome/chrome_cleaner/test:test_branding_header",
     "//chrome/chrome_cleaner/test:test_executables",
@@ -155,6 +157,7 @@ source_set("unittest_sources") {
     "//chrome/chrome_cleaner/test:test_util",
     "//chrome/chrome_cleaner/test/resources:test_resources",
     "//components/chrome_cleaner/public/constants:constants",
+    "//testing/gmock",
     "//testing/gtest",
   ]
 }

chrome/chrome_cleaner/os/disk_util.cc

@@ -32,6 +32,7 @@
 #include "base/win/registry.h"
 #include "base/win/shortcut.h"
 #include "base/win/windows_version.h"
+#include "chrome/chrome_cleaner/constants/version.h"
 #include "chrome/chrome_cleaner/os/file_path_sanitization.h"
 #include "chrome/chrome_cleaner/os/layered_service_provider_api.h"
 #include "chrome/chrome_cleaner/os/pre_fetched_paths.h"
@@ -232,7 +233,8 @@ base::FilePath AppendProductPath(const base::FilePath& base_path) {
     return base_path.Append(file_version_info->company_short_name())
         .Append(file_version_info->product_short_name());
   } else {
-    return base_path.Append(L"Google").Append(L"Chrome Cleanup Tool Test");
+    return base_path.Append(COMPANY_SHORTNAME_STRING)
+        .Append(L"Chrome Cleanup Tool Test");
   }
 }
 
@@ -444,15 +446,11 @@ bool ExpandEnvPath(const base::FilePath& path, base::FilePath* expanded_path) {
 void ExpandWow64Path(const base::FilePath& path,
                      base::FilePath* expanded_path) {
   DCHECK(expanded_path);
-  base::FilePath system_path;
+  base::FilePath system_path =
+      PreFetchedPaths::GetInstance()->GetCsidlSystemFolder();
 
   *expanded_path = path;
 
-  if (!base::PathService::Get(CsidlToPathServiceKey(CSIDL_SYSTEM),
-                              &system_path)) {
-    LOG(ERROR) << "Can't retrieve system directory.";
-    return;
-  }
   base::FilePath native_path = system_path.DirName().Append(L"sysnative");
   if (system_path.IsParent(path) &&
       system_path.AppendRelativePath(path, &native_path) &&

chrome/chrome_cleaner/os/file_remover.cc

@@ -45,7 +45,7 @@ void DeleteEmptyDirectories(base::FilePath directory) {
     // Empty directories deleted in this cleanup are not logged in the matched
     // folders list for the corresponding UwS, because they are not necessarily
     // matched by any rule by the scanner.
-    VLOG(1) << "Deleting empty directory " << SanitizePath(directory);
+    LOG(INFO) << "Deleting empty directory " << SanitizePath(directory);
     if (!base::DeleteFile(directory, /*recursive=*/false))
       break;
     directory = directory.DirName();

chrome/chrome_cleaner/os/initializer.cc

@@ -15,7 +15,6 @@
 #include "base/win/win_util.h"
 #include "chrome/chrome_cleaner/constants/chrome_cleaner_switches.h"
 #include "chrome/chrome_cleaner/os/disk_util.h"
-#include "chrome/chrome_cleaner/os/file_path_sanitization.h"
 #include "chrome/chrome_cleaner/os/pre_fetched_paths.h"
 
 namespace chrome_cleaner {
@@ -48,7 +47,6 @@ std::unique_ptr<base::WaitableEvent> SignalInitializationDone() {
 }  // namespace
 
 bool InitializeOSUtils() {
-  chrome_cleaner::InitializeFilePathSanitization();
   chrome_cleaner::InitializeDiskUtil();
 
   if (!chrome_cleaner::PreFetchedPaths::GetInstance()->Initialize()) {

chrome/chrome_cleaner/os/post_reboot_registration.cc

@@ -71,8 +71,8 @@ bool PostRebootRegistration::RegisterRunOnceOnRestart(
     return false;
   }
 
-  VLOG(1) << "Successfully registered RunOnce value with: "
-          << SanitizeCommandLine(command_line);
+  LOG(INFO) << "Successfully registered RunOnce value with: "
+            << SanitizeCommandLine(command_line);
 
   return true;
 }

chrome/chrome_cleaner/os/pre_fetched_paths.cc

@@ -24,10 +24,19 @@ PreFetchedPaths::~PreFetchedPaths() = default;
 // successful.
 bool PreFetchedPaths::Initialize() {
   DCHECK(!initialized_);
-
-  initialized_ = FetchPath(base::FILE_EXE) &&
-                 FetchPath(base::DIR_PROGRAM_FILES) &&
-                 FetchPath(base::DIR_WINDOWS);
+  chrome_cleaner::InitializeFilePathSanitization();
+
+  initialized_ =
+      FetchPath(base::FILE_EXE) && FetchPath(base::DIR_PROGRAM_FILES) &&
+      FetchPath(base::DIR_WINDOWS) && FetchPath(base::DIR_COMMON_APP_DATA) &&
+      FetchPath(base::DIR_LOCAL_APP_DATA) &&
+      FetchPath(CsidlToPathServiceKey(CSIDL_PROGRAM_FILES)) &&
+      FetchPath(CsidlToPathServiceKey(CSIDL_PROGRAM_FILESX86)) &&
+      FetchPath(CsidlToPathServiceKey(CSIDL_WINDOWS)) &&
+      FetchPath(CsidlToPathServiceKey(CSIDL_STARTUP)) &&
+      FetchPath(CsidlToPathServiceKey(CSIDL_SYSTEM)) &&
+      FetchPath(CsidlToPathServiceKey(CSIDL_COMMON_APPDATA)) &&
+      FetchPath(CsidlToPathServiceKey(CSIDL_LOCAL_APPDATA));
   return initialized_;
 }
 
@@ -47,6 +56,42 @@ base::FilePath PreFetchedPaths::GetWindowsFolder() const {
   return Get(base::DIR_WINDOWS);
 }
 
+base::FilePath PreFetchedPaths::GetCommonAppDataFolder() const {
+  return Get(base::DIR_COMMON_APP_DATA);
+}
+
+base::FilePath PreFetchedPaths::GetLocalAppDataFolder() const {
+  return Get(base::DIR_LOCAL_APP_DATA);
+}
+
+base::FilePath PreFetchedPaths::GetCsidlProgramFilesFolder() const {
+  return Get(CsidlToPathServiceKey(CSIDL_PROGRAM_FILES));
+}
+
+base::FilePath PreFetchedPaths::GetCsidlProgramFilesX86Folder() const {
+  return Get(CsidlToPathServiceKey(CSIDL_PROGRAM_FILESX86));
+}
+
+base::FilePath PreFetchedPaths::GetCsidlWindowsFolder() const {
+  return Get(CsidlToPathServiceKey(CSIDL_WINDOWS));
+}
+
+base::FilePath PreFetchedPaths::GetCsidlStartupFolder() const {
+  return Get(CsidlToPathServiceKey(CSIDL_STARTUP));
+}
+
+base::FilePath PreFetchedPaths::GetCsidlSystemFolder() const {
+  return Get(CsidlToPathServiceKey(CSIDL_SYSTEM));
+}
+
+base::FilePath PreFetchedPaths::GetCsidlCommonAppDataFolder() const {
+  return Get(CsidlToPathServiceKey(CSIDL_COMMON_APPDATA));
+}
+
+base::FilePath PreFetchedPaths::GetCsidlLocalAppDataFolder() const {
+  return Get(CsidlToPathServiceKey(CSIDL_LOCAL_APPDATA));
+}
+
 PreFetchedPaths::PreFetchedPaths() = default;
 
 bool PreFetchedPaths::FetchPath(int key) {

chrome/chrome_cleaner/os/pre_fetched_paths.h

@@ -35,6 +35,15 @@ class PreFetchedPaths {
   base::FilePath GetExecutablePath() const;
   base::FilePath GetProgramFilesFolder() const;
   base::FilePath GetWindowsFolder() const;
+  base::FilePath GetCommonAppDataFolder() const;
+  base::FilePath GetLocalAppDataFolder() const;
+  base::FilePath GetCsidlProgramFilesFolder() const;
+  base::FilePath GetCsidlProgramFilesX86Folder() const;
+  base::FilePath GetCsidlWindowsFolder() const;
+  base::FilePath GetCsidlStartupFolder() const;
+  base::FilePath GetCsidlSystemFolder() const;
+  base::FilePath GetCsidlCommonAppDataFolder() const;
+  base::FilePath GetCsidlLocalAppDataFolder() const;
 
  protected:
   PreFetchedPaths();

chrome/chrome_cleaner/os/pre_fetched_paths_unittest.cc

@@ -4,6 +4,12 @@
 
 #include "chrome/chrome_cleaner/os/pre_fetched_paths.h"
 
+#include <shlobj.h>
+
+#include "base/base_paths_win.h"
+#include "base/files/file_path.h"
+#include "base/path_service.h"
+#include "chrome/chrome_cleaner/os/file_path_sanitization.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chrome_cleaner {
@@ -16,6 +22,52 @@ TEST(PreFetchedPathsTest, PathsArePreFetched) {
 
   EXPECT_FALSE(pre_fetched_paths->GetExecutablePath().empty());
   EXPECT_FALSE(pre_fetched_paths->GetProgramFilesFolder().empty());
+  EXPECT_FALSE(pre_fetched_paths->GetWindowsFolder().empty());
+  EXPECT_FALSE(pre_fetched_paths->GetCommonAppDataFolder().empty());
+  EXPECT_FALSE(pre_fetched_paths->GetLocalAppDataFolder().empty());
+  EXPECT_FALSE(pre_fetched_paths->GetCsidlProgramFilesFolder().empty());
+  EXPECT_FALSE(pre_fetched_paths->GetCsidlProgramFilesX86Folder().empty());
+  EXPECT_FALSE(pre_fetched_paths->GetCsidlWindowsFolder().empty());
+  EXPECT_FALSE(pre_fetched_paths->GetCsidlStartupFolder().empty());
+  EXPECT_FALSE(pre_fetched_paths->GetCsidlSystemFolder().empty());
+  EXPECT_FALSE(pre_fetched_paths->GetCsidlCommonAppDataFolder().empty());
+  EXPECT_FALSE(pre_fetched_paths->GetCsidlLocalAppDataFolder().empty());
+}
+
+TEST(PreFetchedPathsTest, CheckExpectedPaths) {
+  PreFetchedPaths* pre_fetched_paths = PreFetchedPaths::GetInstance();
+  // Note: pre_fetched_paths->Initialized() is already called in test_main.cc.
+
+  auto GetExpectedPath = [](int key) -> base::FilePath {
+    base::FilePath expected_path;
+    CHECK(base::PathService::Get(key, &expected_path));
+    return expected_path;
+  };
+
+  EXPECT_EQ(GetExpectedPath(base::FILE_EXE),
+            pre_fetched_paths->GetExecutablePath());
+  EXPECT_EQ(GetExpectedPath(base::DIR_PROGRAM_FILES),
+            pre_fetched_paths->GetProgramFilesFolder());
+  EXPECT_EQ(GetExpectedPath(base::DIR_WINDOWS),
+            pre_fetched_paths->GetWindowsFolder());
+  EXPECT_EQ(GetExpectedPath(base::DIR_COMMON_APP_DATA),
+            pre_fetched_paths->GetCommonAppDataFolder());
+  EXPECT_EQ(GetExpectedPath(base::DIR_LOCAL_APP_DATA),
+            pre_fetched_paths->GetLocalAppDataFolder());
+  EXPECT_EQ(GetExpectedPath(CsidlToPathServiceKey(CSIDL_PROGRAM_FILES)),
+            pre_fetched_paths->GetCsidlProgramFilesFolder());
+  EXPECT_EQ(GetExpectedPath(CsidlToPathServiceKey(CSIDL_PROGRAM_FILESX86)),
+            pre_fetched_paths->GetCsidlProgramFilesX86Folder());
+  EXPECT_EQ(GetExpectedPath(CsidlToPathServiceKey(CSIDL_WINDOWS)),
+            pre_fetched_paths->GetCsidlWindowsFolder());
+  EXPECT_EQ(GetExpectedPath(CsidlToPathServiceKey(CSIDL_STARTUP)),
+            pre_fetched_paths->GetCsidlStartupFolder());
+  EXPECT_EQ(GetExpectedPath(CsidlToPathServiceKey(CSIDL_SYSTEM)),
+            pre_fetched_paths->GetCsidlSystemFolder());
+  EXPECT_EQ(GetExpectedPath(CsidlToPathServiceKey(CSIDL_COMMON_APPDATA)),
+            pre_fetched_paths->GetCsidlCommonAppDataFolder());
+  EXPECT_EQ(GetExpectedPath(CsidlToPathServiceKey(CSIDL_LOCAL_APPDATA)),
+            pre_fetched_paths->GetCsidlLocalAppDataFolder());
 }
 
 }  // namespace

chrome/chrome_cleaner/os/registry.cc

@@ -29,6 +29,13 @@ const wchar_t* HKeyToString(HKEY hkey) {
     return L"<unknown>";
 }
 
+bool IsPredefinedRegistryRootKey(HKEY key) {
+  return key == nullptr || key == INVALID_HANDLE_VALUE ||
+         key == HKEY_CLASSES_ROOT || key == HKEY_CURRENT_CONFIG ||
+         key == HKEY_CURRENT_USER || key == HKEY_LOCAL_MACHINE ||
+         key == HKEY_USERS;
+}
+
 }  // namespace
 
 bool GetNativeKeyPath(const base::win::RegKey& key,
@@ -67,13 +74,16 @@ bool GetNativeKeyPath(const base::win::RegKey& key,
 RegKeyPath::RegKeyPath() : rootkey_(nullptr), wow64access_(0) {}
 
 RegKeyPath::RegKeyPath(HKEY rootkey, const base::string16& subkey)
-    : RegKeyPath(rootkey, subkey, 0) {}
+    : RegKeyPath(rootkey, subkey, 0) {
+  DCHECK(IsPredefinedRegistryRootKey(rootkey));
+}
 
 RegKeyPath::RegKeyPath(HKEY rootkey,
                        const base::string16& subkey,
                        REGSAM wow64access)
     : rootkey_(rootkey), subkey_(subkey), wow64access_(wow64access) {
   DCHECK_NE(static_cast<HKEY>(nullptr), rootkey_);
+  DCHECK(IsPredefinedRegistryRootKey(rootkey));
   DCHECK(wow64access_ == KEY_WOW64_32KEY || wow64access_ == KEY_WOW64_64KEY ||
          wow64access_ == 0);
 }

chrome/chrome_cleaner/os/registry.h

@@ -26,6 +26,10 @@ bool GetNativeKeyPath(const base::win::RegKey& key,
 
 // Utility class to store a registry key path. Unlike a RegKey, this class is
 // copy/moveable.
+// It stores |rootkey|, which must be a predefined registry root key.
+// TODO(veranika): to make the requirement of a predefined registry root key
+// explicit, change the constructors to take the PredefinedHandle enum instead
+// of the broader HKEY.
 class RegKeyPath {
  public:
   RegKeyPath();

chrome/chrome_cleaner/os/registry_unittest.cc

@@ -26,9 +26,11 @@ namespace {
 // Construct paths from a randomly-generated GUID, which is unlikely to
 // conflict with anything else.
 const wchar_t kRegistryKeyPathEnvTest[] =
-    L"environment\\4ee21cdb-ecc5-47d6-aadc-342ee9dcb463";
+    L"environment\\chrome-cleanup-tool-test-4ee21cdb-ecc5-47d6-aadc-"
+    L"342ee9dcb463";
 const wchar_t kRegistryKeyPathEnvTestUpCase[] =
-    L"ENVIRONMENT\\4EE21CDB-ECC5-47D6-AADC-342EE9DCB463";
+    L"ENVIRONMENT\\CHROME-CLEANUP-TOOL-TEST-4EE21CDB-ECC5-47D6-AADC-"
+    L"342EE9DCB463";
 
 const wchar_t kRegistryKeyPathClsidTest[] =
     L"software\\classes\\clsid\\{4ee21cdb-ecc5-47d6-aadc-342ee9dcb463}";

chrome/chrome_cleaner/os/registry_util.cc

@@ -125,6 +125,18 @@ void CollectMatchingRegistryPathsRecursive(
 const wchar_t kUninstallerKeyPath[] =
     L"software\\microsoft\\windows\\currentversion\\uninstall";
 
+const wchar_t kChromePoliciesKeyPath[] = L"software\\policies\\google\\chrome";
+
+const wchar_t kChromePoliciesForcelistKeyPath[] =
+    L"software\\policies\\google\\chrome\\ExtensionInstallForcelist";
+const wchar_t kChromePoliciesWhitelistKeyPath[] =
+    L"software\\policies\\google\\chrome\\ExtensionInstallWhitelist";
+
+const wchar_t kChromiumPoliciesForcelistKeyPath[] =
+    L"software\\policies\\chromium\\ExtensionInstallForcelist";
+const wchar_t kChromiumPoliciesWhitelistKeyPath[] =
+    L"software\\policies\\chromium\\ExtensionInstallWhitelist";
+
 base::string16 RegistryValueTypeToString(DWORD value_type) {
   switch (value_type) {
     case REG_BINARY:
@@ -147,7 +159,7 @@ base::string16 RegistryValueTypeToString(DWORD value_type) {
       return L"REG_SZ";
     default:
       LOG(WARNING) << "Unknown registry value type (" << value_type << ").";
-      return base::StringPrintf(L"%lu", value_type);
+      return base::NumberToString16(value_type);
   }
 }
 

chrome/chrome_cleaner/os/registry_util.h

@@ -53,6 +53,17 @@ struct RegistryValue {
 // The key for program uninstallers.
 extern const wchar_t kUninstallerKeyPath[];
 
+// The key for Chrome policies.
+extern const wchar_t kChromePoliciesKeyPath[];
+
+// The keys for the Chrome policy forcelist and whitelist.
+extern const wchar_t kChromePoliciesForcelistKeyPath[];
+extern const wchar_t kChromePoliciesWhitelistKeyPath[];
+
+// The keys for the Chromium policy forcelist and whitelist.
+extern const wchar_t kChromiumPoliciesForcelistKeyPath[];
+extern const wchar_t kChromiumPoliciesWhitelistKeyPath[];
+
 // Returns a string representation of the registry value type.
 base::string16 RegistryValueTypeToString(DWORD value_type);
 

chrome/chrome_cleaner/os/system_util_cleaner.cc

@@ -209,7 +209,7 @@ bool IsPrivilegeEnabled(const wchar_t* privilege_name) {
 
 bool SendStopToService(const wchar_t* service_name) {
   DCHECK(service_name);
-  VLOG(1) << "Stopping service '" << service_name << "'.";
+  LOG(INFO) << "Stopping service '" << service_name << "'.";
 
   ScopedServiceHandle service;
   if (!service.OpenService(service_name, SC_MANAGER_ALL_ACCESS, SERVICE_STOP))
@@ -222,7 +222,7 @@ bool SendStopToService(const wchar_t* service_name) {
   SERVICE_STATUS service_state;
   if (!::ControlService(service.get(), SERVICE_CONTROL_STOP, &service_state)) {
     if (::GetLastError() == ERROR_SERVICE_NOT_ACTIVE) {
-      VLOG(1) << "Service '" << service_name << "' is not active.";
+      LOG(INFO) << "Service '" << service_name << "' is not active.";
       return true;
     }
     PLOG(WARNING) << "Control service failed: could not stop the service.";
@@ -258,7 +258,7 @@ bool IsProcessRunning(const wchar_t* executable) {
 
 bool WaitForProcessesStopped(const wchar_t* executable) {
   DCHECK(executable);
-  VLOG(1) << "Wait for processes '" << executable << "'.";
+  LOG(INFO) << "Wait for processes '" << executable << "'.";
 
   // Wait until the process is completely stopped.
   for (unsigned int iteration = 0; iteration < kMaxProcessQueryIterations;
@@ -294,7 +294,7 @@ bool DoesServiceExist(const wchar_t* service_name) {
 
 bool WaitForServiceStopped(const wchar_t* service_name) {
   DCHECK(service_name);
-  VLOG(1) << "Wait for service '" << service_name << "'.";
+  LOG(INFO) << "Wait for service '" << service_name << "'.";
 
   ScopedServiceHandle service;
   if (!service.OpenService(service_name, SC_MANAGER_ALL_ACCESS,
@@ -348,7 +348,7 @@ bool StopService(const wchar_t* service_name) {
 
 bool DeleteService(const wchar_t* service_name) {
   DCHECK(service_name);
-  VLOG(1) << "Delete service '" << service_name << "'.";
+  LOG(INFO) << "Delete service '" << service_name << "'.";
 
   // Attempt to stop the service before deleting it, but don't worry if it
   // doesn't stop.

chrome/chrome_cleaner/os/task_scheduler.cc

@@ -15,6 +15,7 @@
 #include "base/logging.h"
 #include "base/native_library.h"
 #include "base/path_service.h"
+#include "base/strings/strcat.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
 #include "base/time/time.h"
@@ -324,7 +325,7 @@ class TaskSchedulerV2 : public TaskScheduler {
     if (!root_task_folder_)
       return false;
 
-    VLOG(1) << "Delete Task '" << task_name << "'.";
+    LOG(INFO) << "Delete Task '" << task_name << "'.";
 
     HRESULT hr =
         root_task_folder_->DeleteTask(base::win::ScopedBstr(task_name), 0);
@@ -490,11 +491,12 @@ class TaskSchedulerV2 : public TaskScheduler {
         task_trigger_type = TASK_TRIGGER_DAILY;
         if (base::CommandLine::ForCurrentProcess()->HasSwitch(
                 kLogUploadRetryIntervalSwitch)) {
-          base::string16 interval_switch(base::StringPrintf(
-              L"PT%lsM",
-              base::CommandLine::ForCurrentProcess()
-                  ->GetSwitchValueNative(kLogUploadRetryIntervalSwitch)
-                  .c_str()));
+          // String format: PT%lsM
+          const base::string16 interval_switch = base::StrCat(
+              {L"PT",
+               base::CommandLine::ForCurrentProcess()->GetSwitchValueNative(
+                   kLogUploadRetryIntervalSwitch),
+               L"M"});
           LOG(WARNING) << "Command line switch overriding retry interval to: "
                        << interval_switch;
           repetition_interval.Reset(::SysAllocString(interval_switch.c_str()));
@@ -648,7 +650,8 @@ class TaskSchedulerV2 : public TaskScheduler {
 
     DCHECK(IsTaskRegistered(task_name));
 
-    VLOG(1) << "Successfully registered: " << SanitizeCommandLine(run_command);
+    LOG(INFO) << "Successfully registered: "
+              << SanitizeCommandLine(run_command);
     return true;
   }