Resubmit of r49346 that caused mysterious test failures on the bot.

Original change: 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".

TBR=ananta
Review URL: http://codereview.chromium.org/2737006

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

10 files changed, 289 insertions, 280 deletions

diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index c86a95a..c32b2af 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/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.
-    dist->GetInstallReturnCode(install_status);
+    return_code = dist->GetInstallReturnCode(install_status);
   }
 
   LOG(INFO) << "Installation complete, returning: " << return_code;
diff --git a/chrome/installer/util/chrome_frame_distribution.cc b/chrome/installer/util/chrome_frame_distribution.cc
index a8f120f..d19fc95 100644
--- a/chrome/installer/util/chrome_frame_distribution.cc
+++ b/chrome/installer/util/chrome_frame_distribution.cc
@@ -17,6 +17,7 @@
 #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"
 
@@ -107,25 +108,7 @@ bool ChromeFrameDistribution::CanSetAsDefault() {
 
 void ChromeFrameDistribution::UpdateDiffInstallStatus(bool system_install,
     bool incremental_install, installer_util::InstallStatus install_status) {
-  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());
-      }
-    }
-  }
+  GoogleUpdateSettings::UpdateDiffInstallStatus(system_install,
+      incremental_install, GetInstallReturnCode(install_status),
+      kChromeFrameGuid);
 }
diff --git a/chrome/installer/util/google_chrome_distribution.cc b/chrome/installer/util/google_chrome_distribution.cc
index bd085e0..133aba7 100644
--- a/chrome/installer/util/google_chrome_distribution.cc
+++ b/chrome/installer/util/google_chrome_distribution.cc
@@ -369,32 +369,6 @@ 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);
@@ -498,39 +472,9 @@ 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) {
-  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();
+  GoogleUpdateSettings::UpdateDiffInstallStatus(system_install,
+      incremental_install, GetInstallReturnCode(install_status),
+      product_guid().c_str());
 }
 
 // The functions below are not used by the 64-bit Windows binary -
diff --git a/chrome/installer/util/google_chrome_distribution.h b/chrome/installer/util/google_chrome_distribution.h
index 75999da..1be2f13 100644
--- a/chrome/installer/util/google_chrome_distribution.h
+++ b/chrome/installer/util/google_chrome_distribution.h
@@ -40,21 +40,6 @@ 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();
@@ -103,7 +88,7 @@ class GoogleChromeDistribution : public BrowserDistribution {
  private:
   friend class BrowserDistribution;
 
-  FRIEND_TEST(GoogleChromeDistributionTest, TestExtractUninstallMetrics);
+  FRIEND_TEST(GoogleChromeDistTest, TestExtractUninstallMetrics);
 
   // Extracts uninstall metrics from the JSON file located at file_path.
   // Returns them in a form suitable for appending to a url that already
diff --git a/chrome/installer/util/google_chrome_distribution_unittest.cc b/chrome/installer/util/google_chrome_distribution_unittest.cc
index cc63e00..1e0ede8 100644
--- a/chrome/installer/util/google_chrome_distribution_unittest.cc
+++ b/chrome/installer/util/google_chrome_distribution_unittest.cc
@@ -6,185 +6,15 @@
 
 #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"
 
-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) {
+TEST(GoogleChromeDistTest, TestExtractUninstallMetrics) {
   // A make-believe JSON preferences file.
   std::string pref_string(
       "{ \n"
@@ -219,13 +49,13 @@ TEST_F(GoogleChromeDistributionTest, TestExtractUninstallMetrics) {
 
   scoped_ptr<Value> root(json_deserializer.Deserialize(NULL, &error_message));
   ASSERT_TRUE(root.get());
-
   std::wstring uninstall_metrics_string;
   GoogleChromeDistribution* dist = static_cast<GoogleChromeDistribution*>(
       BrowserDistribution::GetDistribution());
+
   EXPECT_TRUE(
       dist->ExtractUninstallMetrics(*static_cast<DictionaryValue*>(root.get()),
                                     &uninstall_metrics_string));
   EXPECT_EQ(expected_url_string, uninstall_metrics_string);
 }
-#endif
+#endif  // defined(GOOGLE_CHROME_BUILD)
diff --git a/chrome/installer/util/google_update_settings.cc b/chrome/installer/util/google_update_settings.cc
index 6765ae1..a5460e2 100644
--- a/chrome/installer/util/google_update_settings.cc
+++ b/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,3 +175,67 @@ 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;
+}
+
diff --git a/chrome/installer/util/google_update_settings.h b/chrome/installer/util/google_update_settings.h
index 70fbaf8..dd7145e 100644
--- a/chrome/installer/util/google_update_settings.h
+++ b/chrome/installer/util/google_update_settings.h
@@ -83,6 +83,27 @@ 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);
 };
diff --git a/chrome/installer/util/google_update_settings_unittest.cc b/chrome/installer/util/google_update_settings_unittest.cc
index e58e23b..e670a8e 100644
--- a/chrome/installer/util/google_update_settings_unittest.cc
+++ b/chrome/installer/util/google_update_settings_unittest.cc
@@ -5,8 +5,11 @@
 #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 {
@@ -15,6 +18,8 @@ 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.
@@ -117,6 +122,42 @@ 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_;
 };
@@ -165,3 +206,145 @@ 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.";
+  }
+}
diff --git a/chrome/installer/util/install_util.cc b/chrome/installer/util/install_util.cc
index 9d36e2e..1940d0d 100644
--- a/chrome/installer/util/install_util.cc
+++ b/chrome/installer/util/install_util.cc
@@ -235,7 +235,6 @@ 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,
diff --git a/chrome/installer/util/util_constants.cc b/chrome/installer/util/util_constants.cc
index 183bb69..d7e7d3d 100644
--- a/chrome/installer/util/util_constants.cc
+++ b/chrome/installer/util/util_constants.cc
@@ -19,7 +19,7 @@ const wchar_t kCreateAllShortcuts[] = L"create-all-shortcuts";
 
 // Delete user profile data. This param is useful only when specified with
 // kUninstall, otherwise it is silently ignored.
-const wchar_t kDeleteProfile[] = L"delete-profile";
+const wchar_t kDeleteProfile[] = L"1";
 
 // Disable logging
 const wchar_t kDisableLogging[] = L"disable-logging";