Remove sentinel file during uninstall so new versions can run successfully

Bug: 1051082: Remove startup sentinel during gcpw uninstall.
Change-Id: I4d2db7eda7778abf791a1bada4a533562a284195
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2050621Reviewed-by: Tien Mai <tienmai@chromium.org>
Reviewed-by: Yusuf Sengul <yusufsn@google.com>
Commit-Queue: Emre Kanlikilicer <emreknlk@google.com>
Cr-Commit-Position: refs/heads/master@{#741449}

chrome/credential_provider/gaiacp/gcp_utils.cc

@@ -84,7 +84,7 @@ constexpr base::win::i18n::LanguageSelector::LangToOffset
 #undef HANDLE_LANGUAGE
 };
 
-base::FilePath GetStartupSentinelLocation() {
+base::FilePath GetStartupSentinelLocation(const base::string16& version) {
   base::FilePath sentienal_path;
   if (!base::PathService::Get(base::DIR_COMMON_APP_DATA, &sentienal_path)) {
     HRESULT hr = HRESULT_FROM_WIN32(::GetLastError());
@@ -95,8 +95,7 @@ base::FilePath GetStartupSentinelLocation() {
   sentienal_path = sentienal_path.Append(GetInstallParentDirectoryName())
                        .Append(kCredentialProviderFolder);
 
-  return sentienal_path.Append(TEXT(CHROME_VERSION_STRING))
-      .AppendASCII(kSentinelFilename);
+  return sentienal_path.Append(version).AppendASCII(kSentinelFilename);
 }
 
 const base::win::i18n::LanguageSelector& GetLanguageSelector() {
@@ -218,6 +217,7 @@ void DeleteVersionsExcept(const base::FilePath& gcp_path,
     // best effort only.  If any errors occurred they are logged by
     // DeleteVersionDirectory().
     DeleteVersionDirectory(gcp_path.Append(basename));
+    DeleteStartupSentinelForVersion(basename.value());
   }
 }
 
@@ -693,7 +693,8 @@ bool VerifyStartupSentinel() {
   // fails for any reason (file locked, no access etc) consider this a failure.
   // If no sentinel file path can be found this probably means that we are
   // running in a unit test so just let the verification pass in this case.
-  base::FilePath startup_sentinel_path = GetStartupSentinelLocation();
+  base::FilePath startup_sentinel_path =
+      GetStartupSentinelLocation(TEXT(CHROME_VERSION_STRING));
   if (!startup_sentinel_path.empty()) {
     base::FilePath startup_sentinel_directory = startup_sentinel_path.DirName();
     if (!base::DirectoryExists(startup_sentinel_directory)) {
@@ -724,7 +725,11 @@ bool VerifyStartupSentinel() {
 }
 
 void DeleteStartupSentinel() {
-  base::FilePath startup_sentinel_path = GetStartupSentinelLocation();
+  DeleteStartupSentinelForVersion(TEXT(CHROME_VERSION_STRING));
+}
+
+void DeleteStartupSentinelForVersion(const base::string16& version) {
+  base::FilePath startup_sentinel_path = GetStartupSentinelLocation(version);
   if (base::PathExists(startup_sentinel_path) &&
       !base::DeleteFile(startup_sentinel_path, false)) {
     LOGFN(ERROR) << "Failed to delete sentinel file: " << startup_sentinel_path;

chrome/credential_provider/gaiacp/gcp_utils.h

@@ -225,6 +225,7 @@ HRESULT LookupLocalizedNameForWellKnownSid(WELL_KNOWN_SID_TYPE sid_type,
 // winlogon process unusable.
 bool VerifyStartupSentinel();
 void DeleteStartupSentinel();
+void DeleteStartupSentinelForVersion(const base::string16& version);
 
 // Gets a string resource from the DLL with the given id.
 base::string16 GetStringResource(int base_message_id);

chrome/credential_provider/setup/setup_lib.cc

@@ -258,6 +258,12 @@ HRESULT DoUninstall(const base::FilePath& installer_path,
   has_failures |= FAILED(UnregisterDlls(dest_path, kRegsiterDlls,
                                         base::size(kRegsiterDlls), fakes));
 
+  // If the DLLs are unregistered, Credential Provider will not be loaded by
+  // Winlogon. Therefore, it is safe to delete the startup sentinel file at this
+  // time.
+  if (!has_failures)
+    DeleteStartupSentinel();
+
   // Delete all files in the destination directory.  This directory does not
   // contain any configuration files or anything else user generated.
   if (!base::DeleteFileRecursively(dest_path)) {

chrome/credential_provider/test/gcp_setup_unittests.cc

@@ -51,7 +51,10 @@ class GcpSetupTest : public ::testing::Test {
   const base::FilePath& module_path() const { return module_path_; }
   const base::string16& product_version() const { return product_version_; }
 
+  void CreateSentinelFileToSimulateCrash(const base::string16& product_version);
+
   void ExpectAllFilesToExist(bool exist, const base::string16& product_version);
+  void ExpectSentinelFileToNotExist(const base::string16& product_version);
   void ExpectCredentialProviderToBeRegistered(
       bool registered,
       const base::string16& product_version);
@@ -65,6 +68,15 @@ class GcpSetupTest : public ::testing::Test {
         .Append(product_version);
   }
 
+  base::FilePath sentinel_path_for_version(
+      const base::string16& product_version) {
+    return scoped_temp_progdata_dir_.GetPath()
+        .Append(GetInstallParentDirectoryName())
+        .Append(FILE_PATH_LITERAL("Credential Provider"))
+        .Append(product_version)
+        .Append(FILE_PATH_LITERAL("gcpw_startup.sentinel"));
+  }
+
   base::FilePath installed_path() {
     return installed_path_for_version(product_version_);
   }
@@ -86,8 +98,10 @@ class GcpSetupTest : public ::testing::Test {
   registry_util::RegistryOverrideManager registry_override_;
   base::ScopedTempDir scoped_temp_prog_dir_;
   base::ScopedTempDir scoped_temp_start_menu_dir_;
+  base::ScopedTempDir scoped_temp_progdata_dir_;
   std::unique_ptr<base::ScopedPathOverride> program_files_override_;
   std::unique_ptr<base::ScopedPathOverride> start_menu_override_;
+  std::unique_ptr<base::ScopedPathOverride> programdata_override_;
   std::unique_ptr<base::ScopedPathOverride> dll_path_override_;
   base::FilePath module_path_;
   base::string16 product_version_;
@@ -118,6 +132,24 @@ void GcpSetupTest::GetModulePathAndProductVersion(
   ASSERT_FALSE(product_version->empty());
 }
 
+void GcpSetupTest::CreateSentinelFileToSimulateCrash(
+    const base::string16& product_version) {
+  base::FilePath sentinel_file = sentinel_path_for_version(product_version);
+
+  // Create the destination folder
+  ASSERT_TRUE(base::CreateDirectory(sentinel_file.DirName()));
+  base::win::ScopedHandle file(
+      CreateFile(sentinel_file.value().c_str(), GENERIC_WRITE, 0, nullptr,
+                 CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, nullptr));
+  ASSERT_TRUE(file.IsValid());
+}
+
+void GcpSetupTest::ExpectSentinelFileToNotExist(
+    const base::string16& product_version) {
+  base::FilePath sentinel_file = sentinel_path_for_version(product_version);
+  EXPECT_EQ(false, base::PathExists(sentinel_file));
+}
+
 void GcpSetupTest::ExpectAllFilesToExist(
     bool exist,
     const base::string16& product_version) {
@@ -211,6 +243,10 @@ void GcpSetupTest::SetUp() {
   start_menu_override_.reset(new base::ScopedPathOverride(
       base::DIR_COMMON_START_MENU, scoped_temp_start_menu_dir_.GetPath()));
 
+  ASSERT_TRUE(scoped_temp_progdata_dir_.CreateUniqueTempDir());
+  programdata_override_.reset(new base::ScopedPathOverride(
+      base::DIR_COMMON_APP_DATA, scoped_temp_progdata_dir_.GetPath()));
+
   // In non-component builds, base::FILE_MODULE will always return the path
   // to base.dll because of the way CURRENT_MODULE works.  Therefore overriding
   // to point to gaia1_0.dll's destination path (i.e. after it is installed).
@@ -256,6 +292,7 @@ TEST_F(GcpSetupTest, DoInstallOverOldInstall) {
   const base::string16 old_version(L"1.0.0.0");
   ASSERT_EQ(S_OK, DoInstall(module_path(), old_version, fakes_for_testing()));
   ExpectAllFilesToExist(true, old_version);
+  CreateSentinelFileToSimulateCrash(old_version);
 
   FakeOSUserManager::UserInfo old_user_info =
       fake_os_user_manager()->GetUserInfo(kDefaultGaiaAccountName);
@@ -276,6 +313,7 @@ TEST_F(GcpSetupTest, DoInstallOverOldInstall) {
   // Make sure newer version exists and old version is gone.
   ExpectAllFilesToExist(true, product_version());
   ExpectAllFilesToExist(false, old_version);
+  ExpectSentinelFileToNotExist(old_version);
 
   // Make sure kGaiaAccountName info and private data are unchanged.
   EXPECT_EQ(old_user_info,
@@ -375,12 +413,13 @@ TEST_F(GcpSetupTest, DoUninstall) {
 
   ASSERT_EQ(S_OK,
             DoInstall(module_path(), product_version(), fakes_for_testing()));
-
+  CreateSentinelFileToSimulateCrash(product_version());
   logging::ResetEventSourceForTesting();
 
   ASSERT_EQ(S_OK,
             DoUninstall(module_path(), installed_path(), fakes_for_testing()));
   ExpectAllFilesToExist(false, product_version());
+  ExpectSentinelFileToNotExist(product_version());
   ExpectCredentialProviderToBeRegistered(false, product_version());
   EXPECT_TRUE(
       fake_os_user_manager()->GetUserInfo(kDefaultGaiaAccountName).sid.empty());