Reverting this as it caused installer_unit_tests to fail. Please fix and reland.

Revert 49346 - Fix problem whereby the "-full" magic value is removed from the "ap" value when a differential update for CF fails (it should remain unless the update succeeds).

Also, fix problem with installer return codes being squashed. This was a regression introduced in http://src.chromium.org/viewvc/chrome?view=rev&revision=41322.

BUG=46051,40607
TEST=Cause a differential update to fail, observe that the "ap" value contains a "-full".


Review URL: http://codereview.chromium.org/2773002

TBR=robertshield@chromium.org
Review URL: http://codereview.chromium.org/2725007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@49359 0039d316-1c4b-4281-b951-d872f2087c98

chrome/installer/setup/setup_main.cc

@@ -742,7 +742,7 @@ int WINAPI wWinMain(HINSTANCE instance, HINSTANCE prev_instance,
     // Note that we allow the status installer_util::UNINSTALL_REQUIRES_REBOOT
     // to pass through, since this is only returned on uninstall which is never
     // invoked directly by Google Update.
-    return_code = dist->GetInstallReturnCode(install_status);
+    dist->GetInstallReturnCode(install_status);
   }
 
   LOG(INFO) << "Installation complete, returning: " << return_code;

chrome/installer/util/chrome_frame_distribution.cc

@@ -17,7 +17,6 @@
 #include "base/string_util.h"
 #include "chrome/installer/util/l10n_string_util.h"
 #include "chrome/installer/util/google_update_constants.h"
-#include "chrome/installer/util/google_update_settings.h"
 
 #include "installer_util_strings.h"
 
@@ -108,7 +107,25 @@ bool ChromeFrameDistribution::CanSetAsDefault() {
 
 void ChromeFrameDistribution::UpdateDiffInstallStatus(bool system_install,
     bool incremental_install, installer_util::InstallStatus install_status) {
-  GoogleUpdateSettings::UpdateDiffInstallStatus(system_install,
-      incremental_install, GetInstallReturnCode(install_status),
-      kChromeFrameGuid);
+  HKEY reg_root = (system_install) ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
+  RegKey key;
+  std::wstring ap_key_value;
+  std::wstring reg_key(google_update::kRegPathClientState);
+  reg_key.append(L"\\");
+  reg_key.append(kChromeFrameGuid);
+  if (!key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS) ||
+      !key.ReadValue(google_update::kRegApField, &ap_key_value)) {
+    LOG(INFO) << "Application key not found.";
+  } else {
+    const char kMagicSuffix[] = "-full";
+    if (LowerCaseEqualsASCII(ap_key_value, kMagicSuffix)) {
+      key.DeleteValue(google_update::kRegApField);
+    } else {
+      size_t pos = ap_key_value.find(ASCIIToWide(kMagicSuffix));
+      if (pos != std::wstring::npos) {
+        ap_key_value.erase(pos, strlen(kMagicSuffix));
+        key.WriteValue(google_update::kRegApField, ap_key_value.c_str());
+      }
+    }
+  }
 }

chrome/installer/util/google_chrome_distribution.cc

@@ -364,6 +364,32 @@ std::wstring GoogleChromeDistribution::GetInstallSubDir() {
   return sub_dir;
 }
 
+std::wstring GoogleChromeDistribution::GetNewGoogleUpdateApKey(
+    bool diff_install, installer_util::InstallStatus status,
+    const std::wstring& value) {
+  // Magic suffix that we need to add or remove to "ap" key value.
+  const std::wstring kMagicSuffix = L"-full";
+
+  bool has_magic_string = false;
+  if ((value.length() >= kMagicSuffix.length()) &&
+      (value.rfind(kMagicSuffix) == (value.length() - kMagicSuffix.length()))) {
+    LOG(INFO) << "Incremental installer failure key already set.";
+    has_magic_string = true;
+  }
+
+  std::wstring new_value(value);
+  if ((!diff_install || !GetInstallReturnCode(status)) && has_magic_string) {
+    LOG(INFO) << "Removing failure key from value " << value;
+    new_value = value.substr(0, value.length() - kMagicSuffix.length());
+  } else if ((diff_install && GetInstallReturnCode(status)) &&
+             !has_magic_string) {
+    LOG(INFO) << "Incremental installer failed, setting failure key.";
+    new_value.append(kMagicSuffix);
+  }
+
+  return new_value;
+}
+
 std::wstring GoogleChromeDistribution::GetPublisherName() {
   const std::wstring& publisher_name =
       installer_util::GetLocalizedString(IDS_ABOUT_VERSION_COMPANY_NAME_BASE);
@@ -467,9 +493,39 @@ std::wstring GoogleChromeDistribution::GetEnvVersionKey() {
 // There is no fall-back for full installer :)
 void GoogleChromeDistribution::UpdateDiffInstallStatus(bool system_install,
     bool incremental_install, installer_util::InstallStatus install_status) {
-  GoogleUpdateSettings::UpdateDiffInstallStatus(system_install,
-      incremental_install, GetInstallReturnCode(install_status),
-      product_guid().c_str());
+  HKEY reg_root = (system_install) ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
+
+  RegKey key;
+  std::wstring ap_key_value;
+  std::wstring reg_key(google_update::kRegPathClientState);
+  reg_key.append(L"\\");
+  reg_key.append(product_guid());
+  if (!key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS) ||
+      !key.ReadValue(google_update::kRegApField, &ap_key_value)) {
+    LOG(INFO) << "Application key not found.";
+    if (!incremental_install || !GetInstallReturnCode(install_status)) {
+      LOG(INFO) << "Returning without changing application key.";
+      key.Close();
+      return;
+    } else if (!key.Valid()) {
+      reg_key.assign(google_update::kRegPathClientState);
+      if (!key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS) ||
+          !key.CreateKey(product_guid().c_str(), KEY_ALL_ACCESS)) {
+        LOG(ERROR) << "Failed to create application key.";
+        key.Close();
+        return;
+      }
+    }
+  }
+
+  std::wstring new_value = GoogleChromeDistribution::GetNewGoogleUpdateApKey(
+      incremental_install, install_status, ap_key_value);
+  if ((new_value.compare(ap_key_value) != 0) &&
+      !key.WriteValue(google_update::kRegApField, new_value.c_str())) {
+    LOG(ERROR) << "Failed to write value " << new_value
+               << " to the registry field " << google_update::kRegApField;
+  }
+  key.Close();
 }
 
 // The functions below are not used by the 64-bit Windows binary -

chrome/installer/util/google_chrome_distribution.h

@@ -38,6 +38,21 @@ class GoogleChromeDistribution : public BrowserDistribution {
 
   virtual std::wstring GetInstallSubDir();
 
+  // This method generates the new value for Google Update "ap" key for Chrome
+  // based on whether we are doing incremental install (or not) and whether
+  // the install succeeded.
+  // - If install worked, remove the magic string (if present).
+  // - If incremental installer failed, append a magic string (if
+  //   not present already).
+  // - If full installer failed, still remove this magic
+  //   string (if it is present already).
+  //
+  // diff_install: tells whether this is incremental install or not.
+  // install_status: if 0, means installation was successful.
+  // value: current value of Google Update "ap" key.
+  std::wstring GetNewGoogleUpdateApKey(bool diff_install,
+      installer_util::InstallStatus status, const std::wstring& value);
+
   virtual std::wstring GetPublisherName();
 
   virtual std::wstring GetAppDescription();

chrome/installer/util/google_chrome_distribution_unittest.cc

@@ -6,14 +6,185 @@
 
 #include <windows.h>
 
+#include "base/registry.h"
 #include "base/scoped_ptr.h"
+#include "base/file_util.h"
 #include "chrome/common/json_value_serializer.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/google_chrome_distribution.h"
+#include "chrome/installer/util/work_item_list.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
-TEST(GoogleChromeDistributionTest, TestExtractUninstallMetrics) {
+namespace {
+class GoogleChromeDistributionTest : public testing::Test {
+ protected:
+  virtual void SetUp() {
+    // Currently no setup required.
+  }
+
+  virtual void TearDown() {
+    // Currently no tear down required.
+  }
+
+  // Creates "ap" key with the value given as parameter. Also adds work
+  // items to work_item_list given so that they can be rolled back later.
+  bool CreateApKey(WorkItemList* work_item_list, const std::wstring& value) {
+    HKEY reg_root = HKEY_CURRENT_USER;
+    std::wstring reg_key = GetApKeyPath();
+    work_item_list->AddCreateRegKeyWorkItem(reg_root, reg_key);
+    work_item_list->AddSetRegValueWorkItem(reg_root, reg_key,
+        google_update::kRegApField, value.c_str(), true);
+    if (!work_item_list->Do()) {
+      work_item_list->Rollback();
+      return false;
+    }
+    return true;
+  }
+
+  // Returns the key path of "ap" key Google\Update\ClientState\<chrome-guid>
+  std::wstring GetApKeyPath() {
+    std::wstring reg_key(google_update::kRegPathClientState);
+    reg_key.append(L"\\");
+
+    BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+    reg_key.append(dist->GetAppGuid());
+    return reg_key;
+  }
+
+  // Utility method to read "ap" key value
+  std::wstring ReadApKeyValue() {
+    RegKey key;
+    std::wstring ap_key_value;
+    std::wstring reg_key = GetApKeyPath();
+    if (key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS) &&
+        key.ReadValue(google_update::kRegApField, &ap_key_value)) {
+      return ap_key_value;
+    }
+    return std::wstring();
+  }
+};
+}  // namespace
+
+#if defined(GOOGLE_CHROME_BUILD)
+TEST_F(GoogleChromeDistributionTest, GetNewGoogleUpdateApKeyTest) {
+  GoogleChromeDistribution* dist = static_cast<GoogleChromeDistribution*>(
+      BrowserDistribution::GetDistribution());
+  installer_util::InstallStatus s = installer_util::FIRST_INSTALL_SUCCESS;
+  installer_util::InstallStatus f = installer_util::INSTALL_FAILED;
+
+  // Incremental Installer that worked.
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, s, L""), L"");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, s, L"1.1"), L"1.1");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, s, L"1.1-dev"), L"1.1-dev");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, s, L"-full"), L"");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, s, L"1.1-full"), L"1.1");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, s, L"1.1-dev-full"),
+            L"1.1-dev");
+
+  // Incremental Installer that failed.
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, f, L""), L"-full");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, f, L"1.1"), L"1.1-full");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, f, L"1.1-dev"),
+            L"1.1-dev-full");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, f, L"-full"), L"-full");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, f, L"1.1-full"), L"1.1-full");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(true, f, L"1.1-dev-full"),
+            L"1.1-dev-full");
+
+  // Full Installer that worked.
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, s, L""), L"");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, s, L"1.1"), L"1.1");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, s, L"1.1-dev"), L"1.1-dev");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, s, L"-full"), L"");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, s, L"1.1-full"), L"1.1");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, s, L"1.1-dev-full"),
+            L"1.1-dev");
+
+  // Full Installer that failed.
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, f, L""), L"");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, f, L"1.1"), L"1.1");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, f, L"1.1-dev"), L"1.1-dev");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, f, L"-full"), L"");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, f, L"1.1-full"), L"1.1");
+  EXPECT_EQ(dist->GetNewGoogleUpdateApKey(false, f, L"1.1-dev-full"),
+            L"1.1-dev");
+}
+
+TEST_F(GoogleChromeDistributionTest, UpdateDiffInstallStatusTest) {
+  // Get Google Chrome distribution
+  GoogleChromeDistribution* dist = static_cast<GoogleChromeDistribution*>(
+      BrowserDistribution::GetDistribution());
+
+  scoped_ptr<WorkItemList> work_item_list(WorkItem::CreateWorkItemList());
+  // Test incremental install failure
+  if (!CreateApKey(work_item_list.get(), L""))
+    FAIL() << "Failed to create ap key.";
+  dist->UpdateDiffInstallStatus(false, true, installer_util::INSTALL_FAILED);
+  EXPECT_STREQ(ReadApKeyValue().c_str(), L"-full");
+  work_item_list->Rollback();
+
+  work_item_list.reset(WorkItem::CreateWorkItemList());
+  // Test incremental install success
+  if (!CreateApKey(work_item_list.get(), L""))
+    FAIL() << "Failed to create ap key.";
+  dist->UpdateDiffInstallStatus(false, true,
+                                installer_util::FIRST_INSTALL_SUCCESS);
+  EXPECT_STREQ(ReadApKeyValue().c_str(), L"");
+  work_item_list->Rollback();
+
+  work_item_list.reset(WorkItem::CreateWorkItemList());
+  // Test full install failure
+  if (!CreateApKey(work_item_list.get(), L"-full"))
+    FAIL() << "Failed to create ap key.";
+  dist->UpdateDiffInstallStatus(false, false, installer_util::INSTALL_FAILED);
+  EXPECT_STREQ(ReadApKeyValue().c_str(), L"");
+  work_item_list->Rollback();
+
+  work_item_list.reset(WorkItem::CreateWorkItemList());
+  // Test full install success
+  if (!CreateApKey(work_item_list.get(), L"-full"))
+    FAIL() << "Failed to create ap key.";
+  dist->UpdateDiffInstallStatus(false, false,
+                                installer_util::FIRST_INSTALL_SUCCESS);
+  EXPECT_STREQ(ReadApKeyValue().c_str(), L"");
+  work_item_list->Rollback();
+
+  work_item_list.reset(WorkItem::CreateWorkItemList());
+  // Test the case of when "ap" key doesnt exist at all
+  std::wstring ap_key_value = ReadApKeyValue();
+  std::wstring reg_key = GetApKeyPath();
+  HKEY reg_root = HKEY_CURRENT_USER;
+  bool ap_key_deleted = false;
+  RegKey key;
+  if (!key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS)) {
+    work_item_list->AddCreateRegKeyWorkItem(reg_root, reg_key);
+    if (!work_item_list->Do())
+      FAIL() << "Failed to create ClientState key.";
+  } else if (key.DeleteValue(google_update::kRegApField)) {
+    ap_key_deleted = true;
+  }
+  // try differential installer
+  dist->UpdateDiffInstallStatus(false, true, installer_util::INSTALL_FAILED);
+  EXPECT_STREQ(ReadApKeyValue().c_str(), L"-full");
+  // try full installer now
+  dist->UpdateDiffInstallStatus(false, false, installer_util::INSTALL_FAILED);
+  EXPECT_STREQ(ReadApKeyValue().c_str(), L"");
+  // Now cleanup to leave the system in unchanged state.
+  // - Diff installer creates an ap key if it didnt exist, so delete this ap key
+  // - If we created any reg key path for ap, roll it back
+  // - Finally restore the original value of ap key.
+  key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS);
+  key.DeleteValue(google_update::kRegApField);
+  work_item_list->Rollback();
+  if (ap_key_deleted) {
+    work_item_list.reset(WorkItem::CreateWorkItemList());
+    if (!CreateApKey(work_item_list.get(), ap_key_value))
+      FAIL() << "Failed to restore ap key.";
+  }
+}
+
+TEST_F(GoogleChromeDistributionTest, TestExtractUninstallMetrics) {
   // A make-believe JSON preferences file.
   std::string pref_string(
       "{ \n"
@@ -57,3 +228,4 @@ TEST(GoogleChromeDistributionTest, TestExtractUninstallMetrics) {
                                     &uninstall_metrics_string));
   EXPECT_EQ(expected_url_string, uninstall_metrics_string);
 }
+#endif

chrome/installer/util/google_update_settings.cc

@@ -95,15 +95,15 @@ bool GoogleUpdateSettings::SetEULAConsent(bool consented) {
 }
 
 int GoogleUpdateSettings::GetLastRunTime() {
-  std::wstring time_s;
-  if (!ReadGoogleUpdateStrKey(google_update::kRegLastRunTimeField, &time_s))
-    return -1;
-  int64 time_i;
-  if (!StringToInt64(time_s, &time_i))
-    return -1;
-  base::TimeDelta td =
+ std::wstring time_s;
+ if (!ReadGoogleUpdateStrKey(google_update::kRegLastRunTimeField, &time_s))
+   return -1;
+ int64 time_i;
+ if (!StringToInt64(time_s, &time_i))
+   return -1;
+ base::TimeDelta td =
     base::Time::NowFromSystemTime() - base::Time::FromInternalValue(time_i);
-  return td.InDays();
+ return td.InDays();
 }
 
 bool GoogleUpdateSettings::SetLastRunTime() {
@@ -175,67 +175,3 @@ bool GoogleUpdateSettings::GetChromeChannel(bool system_install,
 
   return true;
 }
-
-void GoogleUpdateSettings::UpdateDiffInstallStatus(bool system_install,
-    bool incremental_install, int install_return_code,
-    const std::wstring& product_guid) {
-  HKEY reg_root = (system_install) ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
-
-  RegKey key;
-  std::wstring ap_key_value;
-  std::wstring reg_key(google_update::kRegPathClientState);
-  reg_key.append(L"\\");
-  reg_key.append(product_guid);
-  if (!key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS) ||
-      !key.ReadValue(google_update::kRegApField, &ap_key_value)) {
-    LOG(INFO) << "Application key not found.";
-    if (!incremental_install || !install_return_code) {
-      LOG(INFO) << "Returning without changing application key.";
-      key.Close();
-      return;
-    } else if (!key.Valid()) {
-      reg_key.assign(google_update::kRegPathClientState);
-      if (!key.Open(reg_root, reg_key.c_str(), KEY_ALL_ACCESS) ||
-          !key.CreateKey(product_guid.c_str(), KEY_ALL_ACCESS)) {
-        LOG(ERROR) << "Failed to create application key.";
-        key.Close();
-        return;
-      }
-    }
-  }
-
-  std::wstring new_value = GetNewGoogleUpdateApKey(
-      incremental_install, install_return_code, ap_key_value);
-  if ((new_value.compare(ap_key_value) != 0) &&
-      !key.WriteValue(google_update::kRegApField, new_value.c_str())) {
-    LOG(ERROR) << "Failed to write value " << new_value
-               << " to the registry field " << google_update::kRegApField;
-  }
-  key.Close();
-}
-
-std::wstring GoogleUpdateSettings::GetNewGoogleUpdateApKey(
-    bool diff_install, int install_return_code, const std::wstring& value) {
-  // Magic suffix that we need to add or remove to "ap" key value.
-  const std::wstring kMagicSuffix = L"-full";
-
-  bool has_magic_string = false;
-  if ((value.length() >= kMagicSuffix.length()) &&
-      (value.rfind(kMagicSuffix) == (value.length() - kMagicSuffix.length()))) {
-    LOG(INFO) << "Incremental installer failure key already set.";
-    has_magic_string = true;
-  }
-
-  std::wstring new_value(value);
-  if ((!diff_install || !install_return_code) && has_magic_string) {
-    LOG(INFO) << "Removing failure key from value " << value;
-    new_value = value.substr(0, value.length() - kMagicSuffix.length());
-  } else if ((diff_install && install_return_code) &&
-             !has_magic_string) {
-    LOG(INFO) << "Incremental installer failed, setting failure key.";
-    new_value.append(kMagicSuffix);
-  }
-
-  return new_value;
-}
-

chrome/installer/util/google_update_settings.h

@@ -83,27 +83,6 @@ class GoogleUpdateSettings {
   // on success, channel contains one of "", "unknown", "dev" or "beta".
   static bool GetChromeChannel(bool system_install, std::wstring* channel);
 
-  static void UpdateDiffInstallStatus(bool system_install,
-                                      bool incremental_install,
-                                      int install_return_code,
-                                      const std::wstring& product_guid);
-
-  // This method generates the new value for Google Update "ap" key for Chrome
-  // based on whether we are doing incremental install (or not) and whether
-  // the install succeeded.
-  // - If install worked, remove the magic string (if present).
-  // - If incremental installer failed, append a magic string (if
-  //   not present already).
-  // - If full installer failed, still remove this magic
-  //   string (if it is present already).
-  //
-  // diff_install: tells whether this is incremental install or not.
-  // install_return_code: if 0, means installation was successful.
-  // value: current value of Google Update "ap" key.
-  static std::wstring GetNewGoogleUpdateApKey(bool diff_install,
-                                              int install_return_code,
-                                              const std::wstring& value);
-
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(GoogleUpdateSettings);
 };

chrome/installer/util/google_update_settings_unittest.cc

@@ -5,11 +5,8 @@
 #include <windows.h>
 
 #include "base/registry.h"
-#include "base/scoped_ptr.h"
 #include "chrome/installer/util/browser_distribution.h"
-#include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/google_update_settings.h"
-#include "chrome/installer/util/work_item_list.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -18,8 +15,6 @@ const wchar_t* kHKCUReplacement =
 const wchar_t* kHKLMReplacement =
     L"Software\\Google\\InstallUtilUnittest\\HKLM";
 
-const wchar_t kTestProductGuid[] = L"{89F1B351-B15D-48D4-8F10-1298721CF13D}";
-
 // This test fixture redirects the HKLM and HKCU registry hives for
 // the duration of the test to make it independent of the machine
 // and user settings.
@@ -122,42 +117,6 @@ class GoogleUpdateSettingsTest: public testing::Test {
     }
   }
 
-  // Creates "ap" key with the value given as parameter. Also adds work
-  // items to work_item_list given so that they can be rolled back later.
-  bool CreateApKey(WorkItemList* work_item_list, const std::wstring& value) {
-    HKEY reg_root = HKEY_CURRENT_USER;
-    std::wstring reg_key = GetApKeyPath();
-    work_item_list->AddCreateRegKeyWorkItem(reg_root, reg_key);
-    work_item_list->AddSetRegValueWorkItem(reg_root, reg_key,
-        google_update::kRegApField, value.c_str(), true);
-    if (!work_item_list->Do()) {
-      work_item_list->Rollback();
-      return false;
-    }
-    return true;
-  }
-
-  // Returns the key path of "ap" key, e.g.:
-  // Google\Update\ClientState\<kTestProductGuid>
-  std::wstring GetApKeyPath() {
-    std::wstring reg_key(google_update::kRegPathClientState);
-    reg_key.append(L"\\");
-    reg_key.append(kTestProductGuid);
-    return reg_key;
-  }
-
-  // Utility method to read "ap" key value
-  std::wstring ReadApKeyValue() {
-    RegKey key;
-    std::wstring ap_key_value;
-    std::wstring reg_key = GetApKeyPath();
-    if (key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS) &&
-        key.ReadValue(google_update::kRegApField, &ap_key_value)) {
-      return ap_key_value;
-    }
-    return std::wstring();
-  }
-
   RegKey hkcu_;
   RegKey hklm_;
 };
@@ -206,145 +165,3 @@ TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelVariousApValuesSystem) {
 TEST_F(GoogleUpdateSettingsTest, CurrentChromeChannelVariousApValuesUser) {
   TestCurrentChromeChannelWithVariousApValues(USER_INSTALL);
 }
-
-TEST_F(GoogleUpdateSettingsTest, GetNewGoogleUpdateApKeyTest) {
-  installer_util::InstallStatus s = installer_util::FIRST_INSTALL_SUCCESS;
-  installer_util::InstallStatus f = installer_util::INSTALL_FAILED;
-
-  // Incremental Installer that worked.
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, s, L""), L"");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, s, L"1.1"),
-            L"1.1");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, s, L"1.1-dev"),
-            L"1.1-dev");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, s, L"-full"),
-            L"");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, s, L"1.1-full"),
-            L"1.1");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, s,
-                                                          L"1.1-dev-full"),
-            L"1.1-dev");
-
-  // Incremental Installer that failed.
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, f, L""),
-            L"-full");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, f, L"1.1"),
-            L"1.1-full");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, f, L"1.1-dev"),
-            L"1.1-dev-full");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, f, L"-full"),
-            L"-full");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, f, L"1.1-full"),
-            L"1.1-full");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(true, f,
-                                                          L"1.1-dev-full"),
-            L"1.1-dev-full");
-
-  // Full Installer that worked.
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, s, L""), L"");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, s, L"1.1"),
-            L"1.1");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, s, L"1.1-dev"),
-            L"1.1-dev");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, s, L"-full"),
-            L"");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, s,
-                                                          L"1.1-full"),
-            L"1.1");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, s,
-                                                          L"1.1-dev-full"),
-            L"1.1-dev");
-
-  // Full Installer that failed.
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, f, L""), L"");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, f, L"1.1"),
-            L"1.1");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, f, L"1.1-dev"),
-            L"1.1-dev");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, f, L"-full"),
-            L"");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, f,
-                                                          L"1.1-full"),
-            L"1.1");
-  EXPECT_EQ(GoogleUpdateSettings::GetNewGoogleUpdateApKey(false, f,
-                                                          L"1.1-dev-full"),
-            L"1.1-dev");
-}
-
-TEST_F(GoogleUpdateSettingsTest, UpdateDiffInstallStatusTest) {
-  scoped_ptr<WorkItemList> work_item_list(WorkItem::CreateWorkItemList());
-  // Test incremental install failure
-  ASSERT_TRUE(CreateApKey(work_item_list.get(), L""))
-      << "Failed to create ap key.";
-  GoogleUpdateSettings::UpdateDiffInstallStatus(false, true,
-                                       installer_util::INSTALL_FAILED,
-                                       kTestProductGuid);
-  EXPECT_STREQ(ReadApKeyValue().c_str(), L"-full");
-  work_item_list->Rollback();
-
-  work_item_list.reset(WorkItem::CreateWorkItemList());
-  // Test incremental install success
-  ASSERT_TRUE(CreateApKey(work_item_list.get(), L""))
-      << "Failed to create ap key.";
-  GoogleUpdateSettings::UpdateDiffInstallStatus(false, true,
-                                       installer_util::FIRST_INSTALL_SUCCESS,
-                                       kTestProductGuid);
-  EXPECT_STREQ(ReadApKeyValue().c_str(), L"");
-  work_item_list->Rollback();
-
-  work_item_list.reset(WorkItem::CreateWorkItemList());
-  // Test full install failure
-  ASSERT_TRUE(CreateApKey(work_item_list.get(), L"-full"))
-      << "Failed to create ap key.";
-  GoogleUpdateSettings::UpdateDiffInstallStatus(false, false,
-                                       installer_util::INSTALL_FAILED,
-                                       kTestProductGuid);
-  EXPECT_STREQ(ReadApKeyValue().c_str(), L"");
-  work_item_list->Rollback();
-
-  work_item_list.reset(WorkItem::CreateWorkItemList());
-  // Test full install success
-  ASSERT_TRUE(CreateApKey(work_item_list.get(), L"-full"))
-      << "Failed to create ap key.";
-  GoogleUpdateSettings::UpdateDiffInstallStatus(false, false,
-                                       installer_util::FIRST_INSTALL_SUCCESS,
-                                       kTestProductGuid);
-  EXPECT_STREQ(ReadApKeyValue().c_str(), L"");
-  work_item_list->Rollback();
-
-  work_item_list.reset(WorkItem::CreateWorkItemList());
-  // Test the case of when "ap" key doesnt exist at all
-  std::wstring ap_key_value = ReadApKeyValue();
-  std::wstring reg_key = GetApKeyPath();
-  HKEY reg_root = HKEY_CURRENT_USER;
-  bool ap_key_deleted = false;
-  RegKey key;
-  if (!key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS)) {
-    work_item_list->AddCreateRegKeyWorkItem(reg_root, reg_key);
-    ASSERT_TRUE(work_item_list->Do()) << "Failed to create ClientState key.";
-  } else if (key.DeleteValue(google_update::kRegApField)) {
-    ap_key_deleted = true;
-  }
-  // try differential installer
-  GoogleUpdateSettings::UpdateDiffInstallStatus(false, true,
-                                       installer_util::INSTALL_FAILED,
-                                       kTestProductGuid);
-  EXPECT_STREQ(ReadApKeyValue().c_str(), L"-full");
-  // try full installer now
-  GoogleUpdateSettings::UpdateDiffInstallStatus(false, false,
-                                       installer_util::INSTALL_FAILED,
-                                       kTestProductGuid);
-  EXPECT_STREQ(ReadApKeyValue().c_str(), L"");
-  // Now cleanup to leave the system in unchanged state.
-  // - Diff installer creates an ap key if it didnt exist, so delete this ap key
-  // - If we created any reg key path for ap, roll it back
-  // - Finally restore the original value of ap key.
-  key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS);
-  key.DeleteValue(google_update::kRegApField);
-  work_item_list->Rollback();
-  if (ap_key_deleted) {
-    work_item_list.reset(WorkItem::CreateWorkItemList());
-    ASSERT_TRUE(CreateApKey(work_item_list.get(), ap_key_value))
-        << "Failed to restore ap key.";
-  }
-}

chrome/installer/util/install_util.cc

@@ -235,6 +235,7 @@ bool InstallUtil::SetMSIMarker(bool system_level, bool set) {
   return success;
 }
 
+
 bool InstallUtil::BuildDLLRegistrationList(const std::wstring& install_path,
                                            const wchar_t** const dll_names,
                                            int dll_names_count,