diff options
author | gab <gab@chromium.org> | 2015-08-13 05:25:38 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-08-13 12:26:17 +0000 |
commit | 3595a39b33535f5c58199be981d3f6f0aca3f35e (patch) | |
tree | 4618b2ae022ee1175737315ad84b8865a3bb18f2 | |
parent | 7f25099293be10b089bc0c8a66ac54af1dc1e9e6 (diff) | |
download | chromium_src-3595a39b33535f5c58199be981d3f6f0aca3f35e.zip chromium_src-3595a39b33535f5c58199be981d3f6f0aca3f35e.tar.gz chromium_src-3595a39b33535f5c58199be981d3f6f0aca3f35e.tar.bz2 |
Remove helper methods no longer needed after https://codereview.chromium.org/1275023005/
Removing them in a separate CL to make that one a smaller merge.
BUG=502363
Review URL: https://codereview.chromium.org/1278883003
Cr-Commit-Position: refs/heads/master@{#343182}
-rw-r--r-- | chrome/installer/setup/setup_util.cc | 105 | ||||
-rw-r--r-- | chrome/installer/setup/setup_util.h | 7 | ||||
-rw-r--r-- | chrome/installer/setup/setup_util_unittest.cc | 86 |
3 files changed, 0 insertions, 198 deletions
diff --git a/chrome/installer/setup/setup_util.cc b/chrome/installer/setup/setup_util.cc index b9575d0..7f0af71 100644 --- a/chrome/installer/setup/setup_util.cc +++ b/chrome/installer/setup/setup_util.cc @@ -7,7 +7,6 @@ #include "chrome/installer/setup/setup_util.h" #include <windows.h> -#include <stdint.h> #include "base/command_line.h" #include "base/cpu.h" @@ -18,21 +17,15 @@ #include "base/process/kill.h" #include "base/process/launch.h" #include "base/process/process_handle.h" -#include "base/strings/string_number_conversions.h" -#include "base/strings/string_split.h" #include "base/strings/string_util.h" #include "base/strings/utf_string_conversions.h" #include "base/version.h" #include "base/win/registry.h" -#include "base/win/win_util.h" #include "base/win/windows_version.h" #include "chrome/installer/setup/setup_constants.h" -#include "chrome/installer/setup/update_active_setup_version_work_item.h" -#include "chrome/installer/util/app_registration_data.h" #include "chrome/installer/util/app_registration_data.h" #include "chrome/installer/util/copy_tree_work_item.h" #include "chrome/installer/util/google_update_constants.h" -#include "chrome/installer/util/install_util.h" #include "chrome/installer/util/installation_state.h" #include "chrome/installer/util/installer_state.h" #include "chrome/installer/util/master_preferences.h" @@ -102,104 +95,6 @@ bool SupportsSingleInstall(BrowserDistribution::Type type) { } // namespace -bool UpdateLastOSUpgradeHandledByActiveSetup(BrowserDistribution* dist) { - // FIRST: Find the value of the latest OS upgrade registered in the Active - // Setup version (bumped on every major OS upgrade), defaults to 0 if no OS - // upgrade was ever encountered by this install. - DWORD latest_os_upgrade = 0; - - { - const base::string16 active_setup_key_path( - InstallUtil::GetActiveSetupPath(dist)); - - base::win::RegKey active_setup_key; - if (active_setup_key.Open(HKEY_LOCAL_MACHINE, active_setup_key_path.c_str(), - KEY_QUERY_VALUE) == ERROR_SUCCESS) { - base::string16 existing_version; - if (active_setup_key.ReadValue(L"Version", - &existing_version) == ERROR_SUCCESS) { - std::vector<base::string16> version_components = - base::SplitString(existing_version, L",", base::TRIM_WHITESPACE, - base::SPLIT_WANT_NONEMPTY); - uint32_t latest_os_upgrade_uint = 0; - if (version_components.size() == 4U && - base::StringToUint( - version_components[UpdateActiveSetupVersionWorkItem:: - VersionComponent::OS_UPGRADES], - &latest_os_upgrade_uint)) { - latest_os_upgrade = static_cast<DWORD>(latest_os_upgrade_uint); - } else { - LOG(ERROR) << "Failed to parse OS_UPGRADES component of " - << existing_version; - } - } - } - } - - // Whether the read failed or the existing value is 0, do not proceed. - if (latest_os_upgrade == 0U) - return false; - - static const wchar_t kLastOSUpgradeHandledRegName[] = L"LastOSUpgradeHandled"; - - // SECOND: Find out the value of the last OS upgrade handled, defaults to 0 if - // none was ever handled. - DWORD last_os_upgrade_handled = 0; - - base::string16 last_upgrade_handled_key_path = - dist->GetAppRegistrationData().GetStateMediumKey(); - last_upgrade_handled_key_path.push_back(L'\\'); - last_upgrade_handled_key_path.append(kLastOSUpgradeHandledRegName); - - base::string16 user_specific_value; - // This should never fail. If it does, the beacon will be written in the key's - // default value, which is okay since the majority case is likely a machine - // with a single user. - if (!base::win::GetUserSidString(&user_specific_value)) - NOTREACHED(); - - base::win::RegKey last_upgrade_key; - if (last_upgrade_key.Create( - HKEY_LOCAL_MACHINE, last_upgrade_handled_key_path.c_str(), - KEY_WOW64_32KEY | KEY_QUERY_VALUE | KEY_SET_VALUE) != ERROR_SUCCESS) { - LOG(ERROR) << "Failed to create LastOSUpgradeHandled value @ " - << last_upgrade_handled_key_path; - // If the key is not read/writeable, do not proceed as this could result in - // handling an OS upgrade twice. - return false; - } - - // It's okay for this read to fail (i.e. there is an OS upgrade available but - // this user never handled one yet). - last_upgrade_key.ReadValueDW(user_specific_value.c_str(), - &last_os_upgrade_handled); - - // THIRD: Figure out whether the latest OS upgrade has been handled already. - - if (last_os_upgrade_handled >= latest_os_upgrade) { - LOG_IF(ERROR, last_os_upgrade_handled > latest_os_upgrade) - << "Last OS upgrade handled is somehow ahead of the latest OS upgrade?"; - VLOG_IF(1, last_os_upgrade_handled == latest_os_upgrade) - << "Latest OS upgrade already handled."; - return false; - } - - // At this point |last_os_upgrade_handled < latest_os_upgrade| so, - // FOURTH: store the fact that the latest OS upgrade has been handled and - // return true for the caller to act accordingly. - - if (last_upgrade_key.WriteValue(user_specific_value.c_str(), - latest_os_upgrade) != ERROR_SUCCESS) { - LOG(ERROR) << "Failed to save latest_os_upgrade value (" - << latest_os_upgrade << ") to " << last_upgrade_handled_key_path; - // Do not proceed if the write fails as this could otherwise result in - // handling this OS upgrade multiple times. - return false; - } - - return true; -} - int CourgettePatchFiles(const base::FilePath& src, const base::FilePath& patch, const base::FilePath& dest) { diff --git a/chrome/installer/setup/setup_util.h b/chrome/installer/setup/setup_util.h index 4d6303a..0b04b7b 100644 --- a/chrome/installer/setup/setup_util.h +++ b/chrome/installer/setup/setup_util.h @@ -31,13 +31,6 @@ class InstallationState; class InstallerState; class ProductState; -// Sets a bit in the registry to note that the latest OS upgrade notification -// has been handled by this user. Returns true if the previous bit was -// different or absent (i.e., the latest OS update wasn't handled yet), in -// which case subsequent calls to this method will return false until the next -// OS upgrade. This call is only valid on system-level installs. -bool UpdateLastOSUpgradeHandledByActiveSetup(BrowserDistribution* dist); - // Applies a patch file to source file using Courgette. Returns 0 in case of // success. In case of errors, it returns kCourgetteErrorOffset + a Courgette // status code, as defined in courgette/courgette.h diff --git a/chrome/installer/setup/setup_util_unittest.cc b/chrome/installer/setup/setup_util_unittest.cc index 1ac23b8..82b849f 100644 --- a/chrome/installer/setup/setup_util_unittest.cc +++ b/chrome/installer/setup/setup_util_unittest.cc @@ -26,10 +26,8 @@ #include "base/win/windows_version.h" #include "chrome/installer/setup/setup_constants.h" #include "chrome/installer/setup/setup_util.h" -#include "chrome/installer/setup/update_active_setup_version_work_item.h" #include "chrome/installer/util/browser_distribution.h" #include "chrome/installer/util/google_update_constants.h" -#include "chrome/installer/util/install_util.h" #include "chrome/installer/util/installation_state.h" #include "chrome/installer/util/installer_state.h" #include "chrome/installer/util/updating_app_registration_data.h" @@ -89,90 +87,6 @@ bool CurrentProcessHasPrivilege(const wchar_t* privilege_name) { } // namespace -TEST(SetupUtilTest, UpdateLastOSUpgradeHandledByActiveSetup) { - registry_util::RegistryOverrideManager registry_override_manager; - registry_override_manager.OverrideRegistry(HKEY_CURRENT_USER); - registry_override_manager.OverrideRegistry(HKEY_LOCAL_MACHINE); - - BrowserDistribution* chrome_dist = - BrowserDistribution::GetSpecificDistribution( - BrowserDistribution::CHROME_BROWSER); - const base::string16 active_setup_path( - InstallUtil::GetActiveSetupPath(chrome_dist)); - - base::win::RegKey test_key; - base::string16 unused_tmp; - - EXPECT_EQ(ERROR_FILE_NOT_FOUND, - test_key.Open(HKEY_LOCAL_MACHINE, active_setup_path.c_str(), - KEY_QUERY_VALUE)); - // The WorkItem assume the ActiveSetup key itself already exists and only - // handles the Version entry, create it now, but don't fill the "Version" - // entry just yet. - EXPECT_EQ(ERROR_SUCCESS, - test_key.Create(HKEY_LOCAL_MACHINE, active_setup_path.c_str(), - KEY_QUERY_VALUE)); - EXPECT_EQ(ERROR_FILE_NOT_FOUND, test_key.ReadValue(L"Version", &unused_tmp)); - - // Test returns false when no Active Setup version present (and doesn't alter - // that state). - EXPECT_FALSE( - installer::UpdateLastOSUpgradeHandledByActiveSetup(chrome_dist)); - EXPECT_EQ(ERROR_FILE_NOT_FOUND, test_key.ReadValue(L"Version", &unused_tmp)); - - { - UpdateActiveSetupVersionWorkItem active_setup_work_item( - active_setup_path, UpdateActiveSetupVersionWorkItem::UPDATE); - active_setup_work_item.Do(); - EXPECT_EQ(ERROR_SUCCESS, test_key.ReadValue(L"Version", &unused_tmp)); - } - - // Test returns false with default Active Setup version. - EXPECT_FALSE( - installer::UpdateLastOSUpgradeHandledByActiveSetup(chrome_dist)); - EXPECT_EQ(ERROR_SUCCESS, test_key.ReadValue(L"Version", &unused_tmp)); - - // Run through |kIterations| sequences of bumping the OS upgrade version |i| - // times and simulating a regular update |kIterations-i| times, confirming - // that handling any number of OS upgrades only results in a single hit and - // that no amount of regular updates after that result in any hit. - const size_t kIterations = 4U; - for (size_t i = 0U; i < kIterations; ++i) { - SCOPED_TRACE(i); - // Bump the OS_UPGRADES component |i| times. - for (size_t j = 0; j < i; ++j) { - UpdateActiveSetupVersionWorkItem active_setup_work_item( - active_setup_path, UpdateActiveSetupVersionWorkItem:: - UPDATE_AND_BUMP_OS_UPGRADES_COMPONENT); - active_setup_work_item.Do(); - } - - // There should be a single OS upgrade to handle if the OS_UPGRADES - // component was bumped at least once. - EXPECT_EQ(i > 0, installer::UpdateLastOSUpgradeHandledByActiveSetup( - chrome_dist)); - - // We should only be told to handle the latest OS upgrade once above. - EXPECT_FALSE( - installer::UpdateLastOSUpgradeHandledByActiveSetup(chrome_dist)); - EXPECT_FALSE( - installer::UpdateLastOSUpgradeHandledByActiveSetup(chrome_dist)); - - // Run |kIterations-i| regular updates. - for (size_t j = i; j < kIterations; ++j) { - UpdateActiveSetupVersionWorkItem active_setup_work_item( - active_setup_path, UpdateActiveSetupVersionWorkItem::UPDATE); - active_setup_work_item.Do(); - } - - // No amount of regular updates should trigger an OS upgrade to be handled. - EXPECT_FALSE( - installer::UpdateLastOSUpgradeHandledByActiveSetup(chrome_dist)); - EXPECT_FALSE( - installer::UpdateLastOSUpgradeHandledByActiveSetup(chrome_dist)); - } -} - // Test that we are parsing Chrome version correctly. TEST(SetupUtilTest, GetMaxVersionFromArchiveDirTest) { // Create a version dir |