Save uninstall registry values for gcpw MSI during installation

Bug: 1036043
Change-Id: Ia80a822b886b030bc1d7bd6846b810c9e2c20193
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1977392
Commit-Queue: Yusuf Sengul <yusufsn@google.com>
Reviewed-by: Tien Mai <tienmai@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728271}

chrome/credential_provider/common/gcp_strings.cc

@@ -51,6 +51,8 @@ const wchar_t kRegUpdaterClientStateAppPath[] =
     L"SOFTWARE\\Google\\Update\\ClientState\\" GCPW_UPDATE_CLIENT_GUID;
 const wchar_t kRegUpdaterClientsAppPath[] =
     L"SOFTWARE\\Google\\Update\\Clients\\" GCPW_UPDATE_CLIENT_GUID;
+const wchar_t kRegUninstallStringField[] = L"UninstallString";
+const wchar_t kRegUninstallArgumentsField[] = L"UninstallArguments";
 const wchar_t kRegUsageStatsName[] = L"usagestats";
 
 // Chrome is being opened to show the credential provider logon page.  This

chrome/credential_provider/common/gcp_strings.h

@@ -75,6 +75,8 @@ extern const wchar_t kDesktopFullName[];
 // Google Update related registry paths.
 extern const wchar_t kRegUpdaterClientStateAppPath[];
 extern const wchar_t kRegUpdaterClientsAppPath[];
+extern const wchar_t kRegUninstallStringField[];
+extern const wchar_t kRegUninstallArgumentsField[];
 extern const wchar_t kRegUsageStatsName[];
 
 // These are command line switches passed to chrome to start it as a process

chrome/credential_provider/setup/setup_lib.cc

@@ -37,11 +37,13 @@ namespace {
 constexpr base::FilePath::CharType kCredentialProviderDll[] =
     FILE_PATH_LITERAL("Gaia1_0.dll");
 
+constexpr base::FilePath::CharType kCredentialProviderSetupExe[] =
+    FILE_PATH_LITERAL("gcp_setup.exe");
+
 // List of files to install.  If the file list is changed here, make sure to
 // update the files added in make_setup.py.
 constexpr const base::FilePath::CharType* kFilenames[] = {
-    FILE_PATH_LITERAL("gcp_setup.exe"),
-    FILE_PATH_LITERAL("gcp_eventlog_provider.dll"),
+    kCredentialProviderSetupExe, FILE_PATH_LITERAL("gcp_eventlog_provider.dll"),
     kCredentialProviderDll,  // Base name to the CP dll.
 };
 
@@ -225,6 +227,18 @@ HRESULT DoInstall(const base::FilePath& installer_path,
   if (SUCCEEDED(hr))
     DeleteVersionsExcept(gcp_path, product_version);
 
+  base::FilePath setup_exe_path = dest_path.Append(kCredentialProviderSetupExe);
+  hr = WriteUninstallRegistryValues(setup_exe_path);
+  if (FAILED(hr)) {
+    LOGFN(ERROR) << "WriteUninstallRegistryValues failed hr=" << putHR(hr);
+    // Uninstall registry values are written for MSI wrapper. Failing to write
+    // them will only impact uninstalling through uninstall shortcuts on
+    // Windows. There is still a workaround to uninstall by calling
+    // "gcp_setup.exe --uninstall" from a terminal. So, ignoring the failure in
+    // this case until we support rollback of installation that fails mid-way
+    // through.
+  }
+
   return S_OK;
 }
 
@@ -309,26 +323,61 @@ void GetInstalledFileBasenames(const base::FilePath::CharType* const** names,
 }
 
 int EnableStatsCollection(const base::CommandLine& cmdline) {
-  DCHECK(cmdline.HasSwitch(credential_provider::switches::kEnableStats) ||
-         cmdline.HasSwitch(credential_provider::switches::kDisableStats));
+  DCHECK(cmdline.HasSwitch(switches::kEnableStats) ||
+         cmdline.HasSwitch(switches::kDisableStats));
 
-  bool enable =
-      !cmdline.HasSwitch(credential_provider::switches::kDisableStats);
+  bool enable = !cmdline.HasSwitch(switches::kDisableStats);
 
   base::win::RegKey key;
-  LONG sts = key.Create(HKEY_LOCAL_MACHINE,
-                        credential_provider::kRegUpdaterClientStateAppPath,
-                        KEY_SET_VALUE | KEY_WOW64_32KEY);
-  if (sts != ERROR_SUCCESS) {
-    LOGFN(ERROR) << "Unable to open omaha key sts=" << sts;
+  LONG status = key.Create(HKEY_LOCAL_MACHINE, kRegUpdaterClientStateAppPath,
+                           KEY_SET_VALUE | KEY_WOW64_32KEY);
+  if (status != ERROR_SUCCESS) {
+    LOGFN(ERROR) << "Unable to open omaha key=" << kRegUpdaterClientStateAppPath
+                 << " status=" << status;
   } else {
-    sts =
-        key.WriteValue(credential_provider::kRegUsageStatsName, enable ? 1 : 0);
-    if (sts != ERROR_SUCCESS)
-      LOGFN(ERROR) << "Unable to write userstats value sts=" << sts;
+    status = key.WriteValue(kRegUsageStatsName, enable ? 1 : 0);
+    if (status != ERROR_SUCCESS) {
+      LOGFN(ERROR) << "Unable to write " << kRegUsageStatsName
+                   << " value status=" << status;
+    }
+  }
+
+  return status == ERROR_SUCCESS ? 0 : -1;
+}
+
+HRESULT WriteUninstallRegistryValues(const base::FilePath& setup_exe) {
+  base::win::RegKey key;
+  LONG status = key.Create(HKEY_LOCAL_MACHINE, kRegUpdaterClientStateAppPath,
+                           KEY_SET_VALUE | KEY_WOW64_32KEY);
+  if (status != ERROR_SUCCESS) {
+    HRESULT hr = HRESULT_FROM_WIN32(status);
+    LOGFN(ERROR) << "Unable to open " << kRegUpdaterClientStateAppPath
+                 << " hr=" << putHR(hr);
+    return hr;
+  } else {
+    status =
+        key.WriteValue(kRegUninstallStringField, setup_exe.value().c_str());
+    if (status != ERROR_SUCCESS) {
+      HRESULT hr = HRESULT_FROM_WIN32(status);
+      LOGFN(ERROR) << "Unable to write " << kRegUninstallStringField
+                   << " hr=" << putHR(hr);
+      return hr;
+    }
+
+    base::CommandLine uninstall_arguments(base::CommandLine::NO_PROGRAM);
+    uninstall_arguments.AppendSwitch(switches::kUninstall);
+
+    status = key.WriteValue(kRegUninstallArgumentsField,
+                            uninstall_arguments.GetCommandLineString().c_str());
+    if (status != ERROR_SUCCESS) {
+      HRESULT hr = HRESULT_FROM_WIN32(status);
+      LOGFN(ERROR) << "Unable to write " << kRegUninstallArgumentsField
+                   << " hr=" << putHR(hr);
+      return hr;
+    }
   }
 
-  return sts == ERROR_SUCCESS ? 0 : -1;
+  return HRESULT_FROM_WIN32(status);
 }
 
 }  // namespace credential_provider

chrome/credential_provider/setup/setup_lib.h

@@ -54,10 +54,14 @@ HRESULT RelaunchUninstaller(const base::FilePath& installer_path);
 void GetInstalledFileBasenames(const base::FilePath::CharType* const** names,
                                size_t* count);
 
-// Enable or disable stats and crash report collection.  Returns 0 on success
+// Enable or disable stats and crash report collection. Returns 0 on success
 // and -1 on failure.
 int EnableStatsCollection(const base::CommandLine& cmdline);
 
+// Writes the UninstallString and UninstallArguments values to the product's
+// ClientState key in support of uninstallation by the MSI wrapper.
+HRESULT WriteUninstallRegistryValues(const base::FilePath& setup_exe);
+
 }  // namespace credential_provider
 
 #endif  // CHROME_CREDENTIAL_PROVIDER_SETUP_SETUP_LIB_H_

chrome/credential_provider/test/gcp_setup_unittests.cc

@@ -40,6 +40,9 @@
 
 namespace credential_provider {
 
+constexpr base::FilePath::CharType kCredentialProviderSetupExe[] =
+    FILE_PATH_LITERAL("gcp_setup.exe");
+
 class GcpSetupTest : public ::testing::Test {
  protected:
   ~GcpSetupTest() override;
@@ -418,21 +421,18 @@ TEST_F(GcpSetupTest, EnableStats) {
   // Make sure usagestats does not exist.
   base::win::RegKey key;
   EXPECT_EQ(ERROR_SUCCESS,
-            key.Create(HKEY_LOCAL_MACHINE,
-                       credential_provider::kRegUpdaterClientStateAppPath,
+            key.Create(HKEY_LOCAL_MACHINE, kRegUpdaterClientStateAppPath,
                        KEY_ALL_ACCESS | KEY_WOW64_32KEY));
   DWORD value;
-  EXPECT_NE(ERROR_SUCCESS,
-            key.ReadValueDW(credential_provider::kRegUsageStatsName, &value));
+  EXPECT_NE(ERROR_SUCCESS, key.ReadValueDW(kRegUsageStatsName, &value));
 
   // Enable stats.
   base::CommandLine cmdline(base::CommandLine::NO_PROGRAM);
-  cmdline.AppendSwitch(credential_provider::switches::kEnableStats);
+  cmdline.AppendSwitch(switches::kEnableStats);
   EXPECT_EQ(0, EnableStatsCollection(cmdline));
 
   // Stats should be enabled.
-  EXPECT_EQ(ERROR_SUCCESS,
-            key.ReadValueDW(credential_provider::kRegUsageStatsName, &value));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(kRegUsageStatsName, &value));
   EXPECT_EQ(1u, value);
 }
 
@@ -440,21 +440,18 @@ TEST_F(GcpSetupTest, DisableStats) {
   // Make sure usagestats does not exist.
   base::win::RegKey key;
   EXPECT_EQ(ERROR_SUCCESS,
-            key.Create(HKEY_LOCAL_MACHINE,
-                       credential_provider::kRegUpdaterClientStateAppPath,
+            key.Create(HKEY_LOCAL_MACHINE, kRegUpdaterClientStateAppPath,
                        KEY_ALL_ACCESS | KEY_WOW64_32KEY));
   DWORD value;
-  EXPECT_NE(ERROR_SUCCESS,
-            key.ReadValueDW(credential_provider::kRegUsageStatsName, &value));
+  EXPECT_NE(ERROR_SUCCESS, key.ReadValueDW(kRegUsageStatsName, &value));
 
   // Disable stats.
   base::CommandLine cmdline(base::CommandLine::NO_PROGRAM);
-  cmdline.AppendSwitch(credential_provider::switches::kDisableStats);
+  cmdline.AppendSwitch(switches::kDisableStats);
   EXPECT_EQ(0, EnableStatsCollection(cmdline));
 
   // Stats should be disabled.
-  EXPECT_EQ(ERROR_SUCCESS,
-            key.ReadValueDW(credential_provider::kRegUsageStatsName, &value));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(kRegUsageStatsName, &value));
   EXPECT_EQ(0u, value);
 }
 
@@ -462,25 +459,81 @@ TEST_F(GcpSetupTest, EnableDisableStats) {
   // Make sure usagestats does not exist.
   base::win::RegKey key;
   EXPECT_EQ(ERROR_SUCCESS,
-            key.Create(HKEY_LOCAL_MACHINE,
-                       credential_provider::kRegUpdaterClientStateAppPath,
+            key.Create(HKEY_LOCAL_MACHINE, kRegUpdaterClientStateAppPath,
                        KEY_ALL_ACCESS | KEY_WOW64_32KEY));
   DWORD value;
-  EXPECT_NE(ERROR_SUCCESS,
-            key.ReadValueDW(credential_provider::kRegUsageStatsName, &value));
+  EXPECT_NE(ERROR_SUCCESS, key.ReadValueDW(kRegUsageStatsName, &value));
 
   // Enable and disable stats.
   base::CommandLine cmdline(base::CommandLine::NO_PROGRAM);
-  cmdline.AppendSwitch(credential_provider::switches::kEnableStats);
-  cmdline.AppendSwitch(credential_provider::switches::kDisableStats);
+  cmdline.AppendSwitch(switches::kEnableStats);
+  cmdline.AppendSwitch(switches::kDisableStats);
   EXPECT_EQ(0, EnableStatsCollection(cmdline));
 
   // Stats should be disabled.
-  EXPECT_EQ(ERROR_SUCCESS,
-            key.ReadValueDW(credential_provider::kRegUsageStatsName, &value));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(kRegUsageStatsName, &value));
   EXPECT_EQ(0u, value);
 }
 
+TEST_F(GcpSetupTest, WriteUninstallStrings) {
+  base::win::RegKey key;
+
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.Create(HKEY_LOCAL_MACHINE, kRegUpdaterClientStateAppPath,
+                       KEY_ALL_ACCESS | KEY_WOW64_32KEY));
+
+  // Write uninstall strings.
+  base::FilePath file_path =
+      installed_path().Append(FILE_PATH_LITERAL("foo.exe"));
+  ASSERT_EQ(S_OK, WriteUninstallRegistryValues(file_path));
+
+  // Verify uninstall strings.
+  base::string16 uninstall_string;
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.ReadValue(kRegUninstallStringField, &uninstall_string));
+  EXPECT_EQ(uninstall_string, file_path.value());
+
+  base::string16 uninstall_arguments;
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.ReadValue(kRegUninstallArgumentsField, &uninstall_arguments));
+
+  base::CommandLine expected_uninstall_arguments(base::CommandLine::NO_PROGRAM);
+  expected_uninstall_arguments.AppendSwitch(switches::kUninstall);
+
+  EXPECT_EQ(uninstall_arguments,
+            expected_uninstall_arguments.GetCommandLineString());
+}
+
+TEST_F(GcpSetupTest, DoInstallWritesUninstallStrings) {
+  logging::ResetEventSourceForTesting();
+
+  ASSERT_EQ(S_OK,
+            DoInstall(module_path(), product_version(), fakes_for_testing()));
+  ExpectAllFilesToExist(true, product_version());
+
+  base::win::RegKey key;
+
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.Create(HKEY_LOCAL_MACHINE, kRegUpdaterClientStateAppPath,
+                       KEY_ALL_ACCESS | KEY_WOW64_32KEY));
+
+  // Verify uninstall strings.
+  base::string16 uninstall_string;
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.ReadValue(kRegUninstallStringField, &uninstall_string));
+  EXPECT_EQ(uninstall_string,
+            installed_path().Append(kCredentialProviderSetupExe).value());
+
+  base::string16 uninstall_arguments;
+  ASSERT_EQ(ERROR_SUCCESS,
+            key.ReadValue(kRegUninstallArgumentsField, &uninstall_arguments));
+
+  base::CommandLine expected_uninstall_arguments(base::CommandLine::NO_PROGRAM);
+  expected_uninstall_arguments.AppendSwitch(switches::kUninstall);
+  EXPECT_EQ(uninstall_arguments,
+            expected_uninstall_arguments.GetCommandLineString());
+}
+
 // This test checks the expect success / failure of DLL registration when
 // a gaia user already exists
 // Parameters: