Fix in-use updates for Chrome Frame.

On in-use updates, make a copy of the old chrome launcher's IE low rights elevation policy prior to registering the new npchrome_frame.dll so that running instances of IE can still launch Chrome.

In so doing, I also removed elevation policy addition/removal code from the installer so that npchrome_frame.dll's {un,}registration code is the one and only place where this is done.

BUG=95810
TEST=Install a previous version of GCF, run IE and visit some page that activates GCF, update to a version of GCF containing this fix, then try to visit another page that will activate GCF.  If all goes well, you won't see an IE security prompt.

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

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

13 files changed, 309 insertions, 245 deletions

diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc
index 5a619f0..d817c48 100644
--- a/chrome/installer/setup/install.cc
+++ b/chrome/installer/setup/install.cc
@@ -277,9 +277,6 @@ installer::InstallStatus InstallNewVersion(
                       current_version,
                       install_list.get());
 
-  AddElevationPolicyWorkItems(original_state, installer_state, new_version,
-                              install_list.get());
-
   FilePath new_chrome_exe(
       installer_state.target_path().Append(installer::kChromeNewExe));
 
diff --git a/chrome/installer/setup/install_worker.cc b/chrome/installer/setup/install_worker.cc
index 7164d50..4bcb437 100644
--- a/chrome/installer/setup/install_worker.cc
+++ b/chrome/installer/setup/install_worker.cc
@@ -606,6 +606,11 @@ bool AppendPostInstallTasks(const InstallerState& installer_state,
       // update upgrade_utils::SwapNewChromeExeIfPresent.
     }
 
+    if (installer_state.FindProduct(BrowserDistribution::CHROME_FRAME)) {
+      AddCopyIELowRightsPolicyWorkItems(installer_state,
+                                        in_use_update_work_items.get());
+    }
+
     post_install_task_list->AddWorkItem(in_use_update_work_items.release());
   }
 
@@ -626,6 +631,11 @@ bool AppendPostInstallTasks(const InstallerState& installer_state,
                                             google_update::kRegRenameCmdField);
     }
 
+    if (installer_state.FindProduct(BrowserDistribution::CHROME_FRAME)) {
+      AddDeleteOldIELowRightsPolicyWorkItems(installer_state,
+                                             regular_update_work_items.get());
+    }
+
     post_install_task_list->AddWorkItem(regular_update_work_items.release());
   }
 
@@ -969,63 +979,71 @@ void AddChromeFrameWorkItems(const InstallationState& original_state,
   }
 }
 
-void AddElevationPolicyWorkItems(const InstallationState& original_state,
-                                 const InstallerState& installer_state,
-                                 const Version& new_version,
-                                 WorkItemList* install_list) {
-  if (!installer_state.is_multi_install()) {
-    VLOG(1) << "Not adding elevation policy for single installs";
-    return;
+namespace {
+
+enum ElevationPolicyId {
+  CURRENT_ELEVATION_POLICY,
+  OLD_ELEVATION_POLICY,
+};
+
+// Although the UUID of the ChromeFrame class is used for the "current" value,
+// this is done only as a convenience; there is no need for the GUID of the Low
+// Rights policies to match the ChromeFrame class's GUID.  Hence, it is safe to
+// use this completely unrelated GUID for the "old" policies.
+const wchar_t kIELowRightsPolicyOldGuid[] =
+    L"{6C288DD7-76FB-4721-B628-56FAC252E199}";
+
+const wchar_t kElevationPolicyKeyPath[] =
+    L"SOFTWARE\\Microsoft\\Internet Explorer\\Low Rights\\ElevationPolicy\\";
+
+void GetIELowRightsElevationPolicyKeyPath(ElevationPolicyId policy,
+                                          std::wstring* key_path) {
+  DCHECK(policy == CURRENT_ELEVATION_POLICY || policy == OLD_ELEVATION_POLICY);
+
+  key_path->assign(kElevationPolicyKeyPath,
+                   arraysize(kElevationPolicyKeyPath) - 1);
+  if (policy == CURRENT_ELEVATION_POLICY) {
+    wchar_t cf_clsid[64];
+    int len = StringFromGUID2(__uuidof(ChromeFrame), &cf_clsid[0],
+                              arraysize(cf_clsid));
+    key_path->append(&cf_clsid[0], len - 1);
   } else {
-    const ProductState* cf_state =
-        original_state.GetProductState(installer_state.system_install(),
-                                       BrowserDistribution::CHROME_FRAME);
-    if (cf_state && !cf_state->is_multi_install()) {
-      LOG(WARNING) << "Not adding elevation policy since a single install "
-                      "of CF exists";
-      return;
-    }
+    key_path->append(kIELowRightsPolicyOldGuid,
+                     arraysize(kIELowRightsPolicyOldGuid)- 1);
   }
+}
 
-  FilePath binary_dir(
-      GetChromeInstallPath(installer_state.system_install(),
-          BrowserDistribution::GetSpecificDistribution(
-              BrowserDistribution::CHROME_BINARIES)));
-
-  struct {
-    const wchar_t* sub_key;
-    const wchar_t* executable;
-    const FilePath exe_dir;
-  } low_rights_entries[] = {
-    { L"ElevationPolicy\\", kChromeLauncherExe,
-      binary_dir.Append(ASCIIToWide(new_version.GetString())) },
-    { L"DragDrop\\", chrome::kBrowserProcessExecutableName, binary_dir },
-  };
-
-  bool uninstall = (installer_state.operation() == InstallerState::UNINSTALL);
-  HKEY root = installer_state.root_key();
-  const wchar_t kLowRightsKeyPath[] =
-      L"SOFTWARE\\Microsoft\\Internet Explorer\\Low Rights\\";
-  std::wstring key_path(kLowRightsKeyPath);
+}  // namespace
 
-  wchar_t cf_classid[64] = {0};
-  StringFromGUID2(__uuidof(ChromeFrame), cf_classid, arraysize(cf_classid));
+void AddDeleteOldIELowRightsPolicyWorkItems(
+    const InstallerState& installer_state,
+    WorkItemList* install_list) {
+  DCHECK(install_list);
 
-  for (size_t i = 0; i < arraysize(low_rights_entries); ++i) {
-    key_path.append(low_rights_entries[i].sub_key).append(cf_classid);
-    if (uninstall) {
-      install_list->AddDeleteRegKeyWorkItem(root, key_path);
-    } else {
-      install_list->AddCreateRegKeyWorkItem(root, key_path);
-      install_list->AddSetRegValueWorkItem(root, key_path, L"Policy",
-          static_cast<DWORD>(3), true);
-      install_list->AddSetRegValueWorkItem(root, key_path, L"AppName",
-          low_rights_entries[i].executable, true);
-      install_list->AddSetRegValueWorkItem(root, key_path, L"AppPath",
-          low_rights_entries[i].exe_dir.value(), true);
-    }
-    key_path.resize(arraysize(kLowRightsKeyPath) - 1);
-  }
+  std::wstring key_path;
+  GetIELowRightsElevationPolicyKeyPath(OLD_ELEVATION_POLICY, &key_path);
+  install_list->AddDeleteRegKeyWorkItem(installer_state.root_key(), key_path);
+}
+
+// Adds work items to copy the chrome_launcher IE low rights elevation policy
+// from the primary policy GUID to the "old" policy GUID.  Take care not to
+// perform the copy if there is already an old policy present, as the ones under
+// the main kElevationPolicyGuid would then correspond to an intermediate
+// version (current_version < pv < new_version).
+void AddCopyIELowRightsPolicyWorkItems(const InstallerState& installer_state,
+                                       WorkItemList* install_list) {
+  DCHECK(install_list);
+
+  std::wstring current_key_path;
+  std::wstring old_key_path;
+
+  GetIELowRightsElevationPolicyKeyPath(CURRENT_ELEVATION_POLICY,
+                                       &current_key_path);
+  GetIELowRightsElevationPolicyKeyPath(OLD_ELEVATION_POLICY, &old_key_path);
+  // Do not clobber existing old policies.
+  install_list->AddCopyRegKeyWorkItem(installer_state.root_key(),
+                                      current_key_path, old_key_path,
+                                      WorkItem::IF_NOT_PRESENT);
 }
 
 void AppendUninstallCommandLineFlags(const InstallerState& installer_state,
diff --git a/chrome/installer/setup/install_worker.h b/chrome/installer/setup/install_worker.h
index 82a4901..3da3066 100644
--- a/chrome/installer/setup/install_worker.h
+++ b/chrome/installer/setup/install_worker.h
@@ -139,11 +139,15 @@ void AddVersionKeyWorkItems(HKEY root,
                             const Version& new_version,
                             WorkItemList* list);
 
-// [Un]Registers Chrome and ChromeLauncher in IE's low rights elevation policy.
-void AddElevationPolicyWorkItems(const InstallationState& original_state,
-                                 const InstallerState& installer_state,
-                                 const Version& new_version,
-                                 WorkItemList* install_list);
+// Unregisters the "opv" version of ChromeLauncher from IE's low rights
+// elevation policy.
+void AddDeleteOldIELowRightsPolicyWorkItems(
+    const InstallerState& installer_state,
+    WorkItemList* install_list);
+
+// Adds work items to copy IE low rights policies for an in-use update.
+void AddCopyIELowRightsPolicyWorkItems(const InstallerState& installer_state,
+                                       WorkItemList* install_list);
 
 // Utility method currently shared between install.cc and install_worker.cc
 void AppendUninstallCommandLineFlags(const InstallerState& installer_state,
diff --git a/chrome/installer/setup/install_worker_unittest.cc b/chrome/installer/setup/install_worker_unittest.cc
index 67ef806..e5936df 100644
--- a/chrome/installer/setup/install_worker_unittest.cc
+++ b/chrome/installer/setup/install_worker_unittest.cc
@@ -20,6 +20,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/gmock/include/gmock/gmock.h"
 
+using base::win::RegKey;
 using installer::InstallationState;
 using installer::InstallerState;
 using installer::Product;
@@ -27,13 +28,16 @@ using installer::ProductState;
 
 using ::testing::_;
 using ::testing::AtLeast;
-using ::testing::HasSubstr;
 using ::testing::AtMost;
+using ::testing::Bool;
+using ::testing::Combine;
+using ::testing::HasSubstr;
 using ::testing::Eq;
 using ::testing::Return;
 using ::testing::StrCaseEq;
 using ::testing::StrEq;
 using ::testing::StrictMock;
+using ::testing::Values;
 
 // Mock classes to help with testing
 //------------------------------------------------------------------------------
@@ -42,6 +46,10 @@ class MockWorkItemList : public WorkItemList {
  public:
   MockWorkItemList() {}
 
+  MOCK_METHOD4(AddCopyRegKeyWorkItem, WorkItem* (HKEY,
+                                                 const std::wstring&,
+                                                 const std::wstring&,
+                                                 CopyOverWriteOption));
   MOCK_METHOD5(AddCopyTreeWorkItem, WorkItem*(const std::wstring&,
                                               const std::wstring&,
                                               const std::wstring&,
@@ -176,11 +184,26 @@ class InstallWorkerTest : public testing::Test {
   virtual void TearDown() {
   }
 
+  void MaybeAddBinariesToInstallationState(
+      bool system_level,
+      MockInstallationState* installation_state) {
+    if (installation_state->GetProductState(
+            system_level, BrowserDistribution::CHROME_BINARIES) == NULL) {
+      MockProductState product_state;
+      product_state.set_version(current_version_->Clone());
+      installation_state->SetProductState(system_level,
+                                          BrowserDistribution::CHROME_BINARIES,
+                                          product_state);
+    }
+  }
+
   void AddChromeToInstallationState(
       bool system_level,
       bool multi_install,
       bool with_chrome_frame_ready_mode,
       MockInstallationState* installation_state) {
+    if (multi_install)
+      MaybeAddBinariesToInstallationState(system_level, installation_state);
     MockProductState product_state;
     product_state.set_version(current_version_->Clone());
     product_state.set_multi_install(multi_install);
@@ -192,7 +215,9 @@ class InstallWorkerTest : public testing::Test {
     FilePath install_path =
         installer::GetChromeInstallPath(system_level, dist);
     product_state.SetUninstallProgram(
-        install_path.Append(installer::kSetupExe));
+      install_path.AppendASCII(current_version_->GetString())
+          .Append(installer::kInstallerDir)
+          .Append(installer::kSetupExe));
     product_state.AddUninstallSwitch(installer::switches::kUninstall);
     if (system_level)
       product_state.AddUninstallSwitch(installer::switches::kSystemLevel);
@@ -216,6 +241,8 @@ class InstallWorkerTest : public testing::Test {
       bool multi_install,
       bool ready_mode,
       MockInstallationState* installation_state) {
+    if (multi_install)
+      MaybeAddBinariesToInstallationState(system_level, installation_state);
     MockProductState product_state;
     product_state.set_version(current_version_->Clone());
     product_state.set_multi_install(multi_install);
@@ -226,7 +253,9 @@ class InstallWorkerTest : public testing::Test {
     FilePath install_path =
         installer::GetChromeInstallPath(system_level, dist);
     product_state.SetUninstallProgram(
-        install_path.Append(installer::kSetupExe));
+      install_path.AppendASCII(current_version_->GetString())
+          .Append(installer::kInstallerDir)
+          .Append(installer::kSetupExe));
     product_state.AddUninstallSwitch(installer::switches::kUninstall);
     product_state.AddUninstallSwitch(installer::switches::kChromeFrame);
     if (system_level)
@@ -393,141 +422,72 @@ namespace {
 const wchar_t elevation_key[] =
     L"SOFTWARE\\Microsoft\\Internet Explorer\\Low Rights\\ElevationPolicy\\"
     L"{E0A900DF-9611-4446-86BD-4B1D47E7DB2A}";
-const wchar_t dragdrop_key[] =
-    L"SOFTWARE\\Microsoft\\Internet Explorer\\Low Rights\\DragDrop\\"
-    L"{E0A900DF-9611-4446-86BD-4B1D47E7DB2A}";
+const wchar_t old_elevation_key[] =
+    L"SOFTWARE\\Microsoft\\Internet Explorer\\Low Rights\\ElevationPolicy\\"
+    L"{6C288DD7-76FB-4721-B628-56FAC252E199}";
 
 }  // namespace
 
-TEST_F(InstallWorkerTest, ElevationPolicyWorkItems) {
-  const bool system_level = true;
-  const HKEY root = HKEY_LOCAL_MACHINE;
-  const bool multi_install = true;
-  MockWorkItemList work_item_list;
-
-  scoped_ptr<MockInstallationState> installation_state(
-      BuildChromeInstallationState(system_level, multi_install));
-
-  scoped_ptr<MockInstallerState> installer_state(
-      BuildChromeInstallerState(system_level, multi_install,
-                                *installation_state,
-                                InstallerState::MULTI_INSTALL));
-
-  EXPECT_CALL(work_item_list, AddCreateRegKeyWorkItem(root,
-      StrEq(elevation_key))).Times(1);
-
-  EXPECT_CALL(work_item_list, AddCreateRegKeyWorkItem(root,
-      StrEq(dragdrop_key))).Times(1);
-
-  EXPECT_CALL(work_item_list, AddSetRegDwordValueWorkItem(root,
-      StrEq(elevation_key), StrEq(L"Policy"), 3, _)).Times(1);
-
-  EXPECT_CALL(work_item_list, AddSetRegStringValueWorkItem(root,
-      StrEq(elevation_key), StrEq(L"AppName"),
-      StrEq(installer::kChromeLauncherExe), _)).Times(1);
-
-  EXPECT_CALL(work_item_list, AddSetRegStringValueWorkItem(root,
-      StrEq(elevation_key), StrEq(L"AppPath"), _, _)).Times(1);
-
-  EXPECT_CALL(work_item_list, AddSetRegDwordValueWorkItem(root,
-      StrEq(dragdrop_key), StrEq(L"Policy"), 3, _)).Times(1);
-
-  EXPECT_CALL(work_item_list, AddSetRegStringValueWorkItem(root,
-      StrEq(dragdrop_key), StrEq(L"AppName"),
-      StrEq(chrome::kBrowserProcessExecutableName), _)).Times(1);
-
-  EXPECT_CALL(work_item_list, AddSetRegStringValueWorkItem(root,
-      StrEq(dragdrop_key), StrEq(L"AppPath"), _, _)).Times(1);
+// A test class for worker functions that manipulate the old IE low rights
+// policies.
+// Parameters:
+// bool : system_level_
+// bool : multi_install_
+class OldIELowRightsTests : public InstallWorkerTest,
+  public ::testing::WithParamInterface<std::tr1::tuple<bool, bool> > {
+ protected:
+  virtual void SetUp() OVERRIDE {
+    InstallWorkerTest::SetUp();
 
-  AddElevationPolicyWorkItems(*installation_state.get(),
-                              *installer_state.get(),
-                              *new_version_.get(),
-                              &work_item_list);
-}
+    const ParamType& param = GetParam();
+    system_level_ = std::tr1::get<0>(param);
+    multi_install_ = std::tr1::get<1>(param);
+    root_key_ = system_level_ ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
 
-TEST_F(InstallWorkerTest, ElevationPolicyUninstall) {
-  const bool system_level = true;
-  const HKEY root = HKEY_LOCAL_MACHINE;
-  const bool multi_install = true;
-  MockWorkItemList work_item_list;
+    installation_state_.reset(new MockInstallationState());
+    AddChromeFrameToInstallationState(system_level_, multi_install_, false,
+                                      installation_state_.get());
+    installer_state_.reset(BuildBasicInstallerState(
+        system_level_, multi_install_, *installation_state_,
+        multi_install_ ? InstallerState::MULTI_UPDATE :
+            InstallerState::SINGLE_INSTALL_OR_UPDATE));
+    AddChromeFrameToInstallerState(*installation_state_, false,
+                                   installer_state_.get());
+  }
 
-  scoped_ptr<MockInstallationState> installation_state(
-      BuildChromeInstallationState(system_level, multi_install));
+  scoped_ptr<MockInstallationState> installation_state_;
+  scoped_ptr<MockInstallerState> installer_state_;
+  bool system_level_;
+  bool multi_install_;
+  HKEY root_key_;
+};
 
-  scoped_ptr<MockInstallerState> installer_state(
-      BuildChromeInstallerState(system_level, multi_install,
-                                *installation_state,
-                                InstallerState::UNINSTALL));
+TEST_P(OldIELowRightsTests, AddDeleteOldIELowRightsPolicyWorkItems) {
+  StrictMock<MockWorkItemList> work_item_list;
 
-  EXPECT_CALL(work_item_list, AddDeleteRegKeyWorkItem(root,
-      StrEq(elevation_key))).Times(1);
-  EXPECT_CALL(work_item_list, AddDeleteRegKeyWorkItem(root,
-      StrEq(dragdrop_key))).Times(1);
+  EXPECT_CALL(work_item_list,
+              AddDeleteRegKeyWorkItem(root_key_, StrEq(old_elevation_key)))
+      .Times(1);
 
-  AddElevationPolicyWorkItems(*installation_state.get(),
-                              *installer_state.get(),
-                              *new_version_.get(),
-                              &work_item_list);
+  AddDeleteOldIELowRightsPolicyWorkItems(*installer_state_.get(),
+                                         &work_item_list);
 }
 
-TEST_F(InstallWorkerTest, ElevationPolicySingleNoop) {
-  const bool system_level = true;
-  const bool multi_install = false;  // nothing should be done for single.
-  MockWorkItemList work_item_list;
+TEST_P(OldIELowRightsTests, AddCopyIELowRightsPolicyWorkItems) {
+  StrictMock<MockWorkItemList> work_item_list;
 
-  scoped_ptr<MockInstallationState> installation_state(
-      BuildChromeInstallationState(system_level, multi_install));
+  // The old elevation policy key should only be copied when there's no old
+  // value.
+  EXPECT_CALL(work_item_list,
+              AddCopyRegKeyWorkItem(root_key_, StrEq(elevation_key),
+                                    StrEq(old_elevation_key),
+                                    Eq(WorkItem::IF_NOT_PRESENT))).Times(1);
 
-  scoped_ptr<MockInstallerState> installer_state(
-      BuildChromeInstallerState(system_level, multi_install,
-                                *installation_state,
-                                InstallerState::UNINSTALL));
-
-  EXPECT_CALL(work_item_list, AddDeleteRegKeyWorkItem(_, _)).Times(0);
-  EXPECT_CALL(work_item_list, AddCreateRegKeyWorkItem(_, _)).Times(0);
-  EXPECT_CALL(work_item_list, AddSetRegDwordValueWorkItem(_, _, _, _, _))
-      .Times(0);
-  EXPECT_CALL(work_item_list, AddSetRegStringValueWorkItem(_, _, _, _, _))
-      .Times(0);
-
-  AddElevationPolicyWorkItems(*installation_state.get(),
-                              *installer_state.get(),
-                              *new_version_.get(),
-                              &work_item_list);
+  AddCopyIELowRightsPolicyWorkItems(*installer_state_.get(), &work_item_list);
 }
 
-TEST_F(InstallWorkerTest, ElevationPolicyExistingSingleCFNoop) {
-  const bool system_level = true;
-  const bool multi_install = true;
-  MockWorkItemList work_item_list;
-
-  scoped_ptr<MockInstallationState> installation_state(
-      BuildChromeInstallationState(system_level, multi_install));
-
-  MockProductState cf_state;
-  cf_state.set_version(current_version_->Clone());
-  cf_state.set_multi_install(false);
-
-  installation_state->SetProductState(system_level,
-      BrowserDistribution::CHROME_FRAME, cf_state);
-
-  scoped_ptr<MockInstallerState> installer_state(
-      BuildChromeInstallerState(system_level, multi_install,
-                                *installation_state,
-                                InstallerState::MULTI_INSTALL));
-
-  EXPECT_CALL(work_item_list, AddDeleteRegKeyWorkItem(_, _)).Times(0);
-  EXPECT_CALL(work_item_list, AddCreateRegKeyWorkItem(_, _)).Times(0);
-  EXPECT_CALL(work_item_list, AddSetRegDwordValueWorkItem(_, _, _, _, _))
-      .Times(0);
-  EXPECT_CALL(work_item_list, AddSetRegStringValueWorkItem(_, _, _, _, _))
-      .Times(0);
-
-  AddElevationPolicyWorkItems(*installation_state.get(),
-                              *installer_state.get(),
-                              *new_version_.get(),
-                              &work_item_list);
-}
+INSTANTIATE_TEST_CASE_P(Variations, OldIELowRightsTests,
+                        Combine(Bool(), Bool()));
 
 TEST_F(InstallWorkerTest, GoogleUpdateWorkItemsTest) {
   const bool system_level = true;
@@ -535,7 +495,7 @@ TEST_F(InstallWorkerTest, GoogleUpdateWorkItemsTest) {
   MockWorkItemList work_item_list;
 
   scoped_ptr<MockInstallationState> installation_state(
-      BuildChromeInstallationState(system_level, multi_install));
+      BuildChromeInstallationState(system_level, false));
 
   MockProductState cf_state;
   cf_state.set_version(current_version_->Clone());
diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index 4769337..702893f 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/chrome/installer/setup/setup_main.cc
@@ -30,6 +30,7 @@
 #include "chrome/installer/setup/chrome_frame_quick_enable.h"
 #include "chrome/installer/setup/chrome_frame_ready_mode.h"
 #include "chrome/installer/setup/install.h"
+#include "chrome/installer/setup/install_worker.h"
 #include "chrome/installer/setup/setup_constants.h"
 #include "chrome/installer/setup/setup_util.h"
 #include "chrome/installer/setup/uninstall.h"
@@ -169,6 +170,8 @@ void AddExistingMultiInstalls(const InstallationState& original_state,
 // for Chrome so there should be a file called new_chrome.exe on the file
 // system and a key called 'opv' in the registry. This function will move
 // new_chrome.exe to chrome.exe and delete 'opv' key in one atomic operation.
+// This function also deletes elevation policies associated with the old version
+// if they exist.
 installer::InstallStatus RenameChromeExecutables(
     const InstallationState& original_state,
     InstallerState* installer_state) {
@@ -202,6 +205,12 @@ installer::InstallStatus RenameChromeExecutables(
                                     temp_path.path().value(),
                                     WorkItem::ALWAYS_MOVE);
   install_list->AddDeleteTreeWorkItem(chrome_new_exe, temp_path.path());
+  // Delete an elevation policy associated with the old version, should one
+  // exist.
+  if (installer_state->FindProduct(BrowserDistribution::CHROME_FRAME)) {
+    installer::AddDeleteOldIELowRightsPolicyWorkItems(*installer_state,
+                                                      install_list.get());
+  }
   // old_chrome.exe is still in use in most cases, so ignore failures here.
   install_list->AddDeleteTreeWorkItem(chrome_old_exe, temp_path.path())
       ->set_ignore_failure(true);
@@ -590,11 +599,6 @@ installer::InstallStatus InstallProductsHelper(
       installer_state.WriteInstallerResult(install_status,
           IDS_INSTALL_INVALID_ARCHIVE_BASE, NULL);
     } else {
-      // TODO(tommi): Move towards having only a single version that is common
-      // to all products.  Only the package should have a version since it
-      // represents all the binaries.  When a single product is upgraded, all
-      // currently installed product for the shared binary installation, should
-      // (or rather must) be upgraded.
       VLOG(1) << "version to install: " << installer_version->GetString();
       bool proceed_with_installation = true;
       uint32 higher_products = 0;
@@ -806,24 +810,52 @@ installer::InstallStatus UninstallProduct(
     const InstallationState& original_state,
     const InstallerState& installer_state,
     const CommandLine& cmd_line,
+    bool remove_all,
+    bool force_uninstall,
     const Product& product) {
-  bool force = cmd_line.HasSwitch(installer::switches::kForceUninstall);
   const ProductState* product_state =
       original_state.GetProductState(installer_state.system_install(),
                                      product.distribution()->GetType());
   if (product_state != NULL) {
     VLOG(1) << "version on the system: "
             << product_state->version().GetString();
-  } else if (!force) {
-    LOG(ERROR) << "No Chrome installation found for uninstall.";
+  } else if (!force_uninstall) {
+    LOG(ERROR) << product.distribution()->GetAppShortCutName()
+               << " not found for uninstall.";
     return installer::CHROME_NOT_INSTALLED;
   }
 
-  bool remove_all = !cmd_line.HasSwitch(
+  return installer::UninstallProduct(original_state, installer_state,
+      cmd_line.GetProgram(), product, remove_all, force_uninstall, cmd_line);
+}
+
+installer::InstallStatus UninstallProducts(
+    const InstallationState& original_state,
+    const InstallerState& installer_state,
+    const CommandLine& cmd_line) {
+  const Products& products = installer_state.products();
+  // InstallerState::Initialize always puts Chrome first, and we rely on that
+  // here for this reason: if Chrome is in-use, the user will be prompted to
+  // confirm uninstallation.  Upon cancel, we should not continue with the
+  // other products.
+  DCHECK(products.size() < 2 || products[0]->is_chrome());
+  installer::InstallStatus install_status = installer::UNINSTALL_SUCCESSFUL;
+  installer::InstallStatus prod_status = installer::UNKNOWN_STATUS;
+  const bool force = cmd_line.HasSwitch(installer::switches::kForceUninstall);
+  const bool remove_all = !cmd_line.HasSwitch(
       installer::switches::kDoNotRemoveSharedItems);
 
-  return installer::UninstallProduct(original_state, installer_state,
-      cmd_line.GetProgram(), product, remove_all, force, cmd_line);
+  for (size_t i = 0;
+       install_status != installer::UNINSTALL_CANCELLED &&
+           i < products.size();
+       ++i) {
+    prod_status = UninstallProduct(original_state, installer_state,
+        cmd_line, remove_all, force, *products[i]);
+    if (prod_status != installer::UNINSTALL_SUCCESSFUL)
+      install_status = prod_status;
+  }
+
+  return install_status;
 }
 
 installer::InstallStatus ShowEULADialog(const std::wstring& inner_frame) {
@@ -1260,23 +1292,8 @@ int WINAPI wWinMain(HINSTANCE instance, HINSTANCE prev_instance,
   installer::InstallStatus install_status = installer::UNKNOWN_STATUS;
   // If --uninstall option is given, uninstall the identified product(s)
   if (is_uninstall) {
-    const Products& products = installer_state.products();
-    // InstallerState::Initialize always puts Chrome first, and we rely on that
-    // here for this reason: if Chrome is in-use, the user will be prompted to
-    // confirm uninstallation.  Upon cancel, we should not continue with the
-    // other products.
-    DCHECK(products.size() < 2 || products[0]->is_chrome());
-    install_status = installer::UNINSTALL_SUCCESSFUL;  // I'm an optimist.
-    installer::InstallStatus prod_status = installer::UNKNOWN_STATUS;
-    for (size_t i = 0;
-         install_status != installer::UNINSTALL_CANCELLED &&
-             i < products.size();
-         ++i) {
-      prod_status = UninstallProduct(original_state, installer_state,
-          cmd_line, *products[i]);
-      if (prod_status != installer::UNINSTALL_SUCCESSFUL)
-        install_status = prod_status;
-    }
+    install_status = UninstallProducts(original_state, installer_state,
+                                       cmd_line);
   } else {
     // If --uninstall option is not specified, we assume it is install case.
     install_status = InstallProducts(original_state, cmd_line, prefs,
diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index f0d7a58..382b764 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -119,6 +119,14 @@ void ProcessQuickEnableWorkItems(
     LOG(ERROR) << "Failed to update quick-enable-cf command.";
 }
 
+void ProcessIELowRightsPolicyWorkItems(
+    const installer::InstallerState& installer_state) {
+  scoped_ptr<WorkItemList> work_items(WorkItem::CreateNoRollbackWorkItemList());
+  AddDeleteOldIELowRightsPolicyWorkItems(installer_state, work_items.get());
+  work_items->Do();
+  installer::RefreshElevationPolicy();
+}
+
 void ClearRlzProductState() {
   const rlz_lib::AccessPoint points[] = {rlz_lib::CHROME_OMNIBOX,
                                          rlz_lib::CHROME_HOME_PAGE,
@@ -790,6 +798,9 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
                                  unreg_work_item_list.get());
       unreg_work_item_list->Do();
     }
+
+    if (!is_chrome)
+      ProcessIELowRightsPolicyWorkItems(installer_state);
   }
 
   // Close any Chrome Frame helper processes that may be running.
@@ -825,14 +836,6 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
     LOG(INFO) << (can_delete_files ? "Shared binaries will be deleted." :
                                      "Shared binaries still in use.");
     if (can_delete_files) {
-      // Remove the elevation policy when the last product is uninstalled.
-      scoped_ptr<WorkItemList> work_items(
-          WorkItem::CreateNoRollbackWorkItemList());
-      AddElevationPolicyWorkItems(original_state, installer_state,
-          product_state->version(), work_items.get());
-      work_items->Do();
-      RefreshElevationPolicy();
-
       BrowserDistribution* multi_dist =
           installer_state.multi_package_binaries_distribution();
       InstallUtil::DeleteRegistryKey(reg_root, multi_dist->GetVersionKey());
diff --git a/chrome/installer/util/copy_reg_key_work_item.cc b/chrome/installer/util/copy_reg_key_work_item.cc
index a0d2b1f..71f6fd0 100644
--- a/chrome/installer/util/copy_reg_key_work_item.cc
+++ b/chrome/installer/util/copy_reg_key_work_item.cc
@@ -16,21 +16,33 @@ CopyRegKeyWorkItem::~CopyRegKeyWorkItem() {
 
 CopyRegKeyWorkItem::CopyRegKeyWorkItem(HKEY predefined_root,
                                        const std::wstring& source_key_path,
-                                       const std::wstring& dest_key_path)
+                                       const std::wstring& dest_key_path,
+                                       CopyOverWriteOption overwrite_option)
     : predefined_root_(predefined_root),
       source_key_path_(source_key_path),
-      dest_key_path_(dest_key_path) {
+      dest_key_path_(dest_key_path),
+      overwrite_option_(overwrite_option),
+      cleared_destination_(false) {
   DCHECK(predefined_root);
   // It's a safe bet that we don't want to copy or overwrite one of the root
   // trees.
   DCHECK(!source_key_path.empty());
   DCHECK(!dest_key_path.empty());
+  DCHECK(overwrite_option == ALWAYS || overwrite_option == IF_NOT_PRESENT);
 }
 
 bool CopyRegKeyWorkItem::Do() {
   if (source_key_path_.empty() || dest_key_path_.empty())
     return false;
 
+  // Leave immediately if we're not supposed to overwrite an existing key and
+  // one is there.
+  if (overwrite_option_ == IF_NOT_PRESENT &&
+      RegKey(predefined_root_, dest_key_path_.c_str(),
+             KEY_QUERY_VALUE).Valid()) {
+    return true;
+  }
+
   RegistryKeyBackup backup;
   RegKey dest_key;
 
@@ -48,6 +60,7 @@ bool CopyRegKeyWorkItem::Do() {
     LOG(ERROR) << "Failed to delete key at " << dest_key_path_ << ", result: "
                << result;
   } else {
+    cleared_destination_ = true;
     // We've just modified the registry, so remember any backup we may have
     // made so that Rollback can take us back where we started.
     backup_.swap(backup);
@@ -85,16 +98,18 @@ void CopyRegKeyWorkItem::Rollback() {
   if (ignore_failure_)
     return;
 
-  // Delete anything in the key before restoring the backup in case someone else
-  // put new data in the key after Do().
-  LONG result = SHDeleteKey(predefined_root_, dest_key_path_.c_str());
-  if (result != ERROR_SUCCESS && result != ERROR_FILE_NOT_FOUND) {
-    LOG(ERROR) << "Failed to delete key at " << dest_key_path_
-               << " in rollback, result: " << result;
-  }
+  if (cleared_destination_) {
+    // Delete anything in the key before restoring the backup in case new data
+    // was written after Do().
+    LONG result = SHDeleteKey(predefined_root_, dest_key_path_.c_str());
+    if (result != ERROR_SUCCESS && result != ERROR_FILE_NOT_FOUND) {
+      LOG(ERROR) << "Failed to delete key at " << dest_key_path_
+                 << " in rollback, result: " << result;
+    }
 
-  // Restore the old contents.  The restoration takes on its default security
-  // attributes; any custom attributes are lost.
-  if (!backup_.WriteTo(predefined_root_, dest_key_path_.c_str()))
-    LOG(ERROR) << "Failed to restore key in rollback.";
+    // Restore the old contents.  The restoration takes on its default security
+    // attributes; any custom attributes are lost.
+    if (!backup_.WriteTo(predefined_root_, dest_key_path_.c_str()))
+      LOG(ERROR) << "Failed to restore key in rollback.";
+  }
 }
diff --git a/chrome/installer/util/copy_reg_key_work_item.h b/chrome/installer/util/copy_reg_key_work_item.h
index b1d4631..cba85fe 100644
--- a/chrome/installer/util/copy_reg_key_work_item.h
+++ b/chrome/installer/util/copy_reg_key_work_item.h
@@ -28,9 +28,11 @@ class CopyRegKeyWorkItem : public WorkItem {
   friend class WorkItem;
 
   // Neither |source_key_path| nor |dest_key_path| may be empty.
+  // Only ALWAYS and IF_NOT_PRESENT are supported for |overwrite_option|.
   CopyRegKeyWorkItem(HKEY predefined_key,
                      const std::wstring& source_key_path,
-                     const std::wstring& dest_key_path);
+                     const std::wstring& dest_key_path,
+                     CopyOverWriteOption overwrite_option);
 
   // Root key in which we operate. The root key must be one of the predefined
   // keys on Windows.
@@ -42,9 +44,14 @@ class CopyRegKeyWorkItem : public WorkItem {
   // Path of the destination key.
   std::wstring dest_key_path_;
 
+  CopyOverWriteOption overwrite_option_;
+
   // Backup of the destination key.
   RegistryKeyBackup backup_;
 
+  // Set to true after the destination is cleared for the copy.
+  bool cleared_destination_;
+
   DISALLOW_COPY_AND_ASSIGN(CopyRegKeyWorkItem);
 };
 
diff --git a/chrome/installer/util/copy_reg_key_work_item_unittest.cc b/chrome/installer/util/copy_reg_key_work_item_unittest.cc
index 84a6f64..59399e8 100644
--- a/chrome/installer/util/copy_reg_key_work_item_unittest.cc
+++ b/chrome/installer/util/copy_reg_key_work_item_unittest.cc
@@ -42,7 +42,8 @@ TEST_F(CopyRegKeyWorkItemTest, TestNoKey) {
     const std::wstring& key_path = key_paths[i];
     scoped_ptr<CopyRegKeyWorkItem> item(
         WorkItem::CreateCopyRegKeyWorkItem(test_data_.root_key(), key_path,
-                                           destination_path_));
+                                           destination_path_,
+                                           WorkItem::ALWAYS));
     EXPECT_TRUE(item->Do());
     EXPECT_NE(ERROR_SUCCESS,
               key.Open(test_data_.root_key(), destination_path_.c_str(),
@@ -61,7 +62,8 @@ TEST_F(CopyRegKeyWorkItemTest, TestEmptyKey) {
   scoped_ptr<CopyRegKeyWorkItem> item(
       WorkItem::CreateCopyRegKeyWorkItem(test_data_.root_key(),
                                          test_data_.empty_key_path(),
-                                         destination_path_));
+                                         destination_path_,
+                                         WorkItem::ALWAYS));
   EXPECT_TRUE(item->Do());
   RegistryTestData::ExpectEmptyKey(test_data_.root_key(),
                                    destination_path_.c_str());
@@ -79,7 +81,8 @@ TEST_F(CopyRegKeyWorkItemTest, TestNonEmptyKey) {
   scoped_ptr<CopyRegKeyWorkItem> item(
       WorkItem::CreateCopyRegKeyWorkItem(test_data_.root_key(),
                                          test_data_.non_empty_key_path(),
-                                         destination_path_));
+                                         destination_path_,
+                                         WorkItem::ALWAYS));
   EXPECT_TRUE(item->Do());
   test_data_.ExpectMatchesNonEmptyKey(test_data_.root_key(),
                                       destination_path_.c_str());
@@ -90,6 +93,40 @@ TEST_F(CopyRegKeyWorkItemTest, TestNonEmptyKey) {
                      KEY_READ));
 }
 
+// Test that existing data isn't overwritten.
+TEST_F(CopyRegKeyWorkItemTest, TestNoOverwrite) {
+  RegKey key;
+  // First copy the data into the dest.
+  EXPECT_EQ(ERROR_SUCCESS,
+            key.Create(test_data_.root_key(), destination_path_.c_str(),
+                       KEY_WRITE));
+  EXPECT_EQ(ERROR_SUCCESS,
+            SHCopyKey(test_data_.root_key(),
+                      test_data_.non_empty_key_path().c_str(), key.Handle(),
+                      0));
+  key.Close();
+
+  // Now copy the empty key into the dest, which should do nothing.
+  scoped_ptr<CopyRegKeyWorkItem> item(
+      WorkItem::CreateCopyRegKeyWorkItem(test_data_.root_key(),
+                                         test_data_.empty_key_path(),
+                                         destination_path_,
+                                         WorkItem::IF_NOT_PRESENT));
+  EXPECT_TRUE(item->Do());
+
+  // Make sure it's all there.
+  test_data_.ExpectMatchesNonEmptyKey(test_data_.root_key(),
+                                      destination_path_.c_str());
+
+  // Rollback should do nothing.
+  item->Rollback();
+  item.reset();
+
+  // Make sure it's still all there.
+  test_data_.ExpectMatchesNonEmptyKey(test_data_.root_key(),
+                                      destination_path_.c_str());
+}
+
 // Test that copying an empty key over a key with data succeeds, and that
 // rollback restores the original data.
 TEST_F(CopyRegKeyWorkItemTest, TestOverwriteAndRestore) {
@@ -108,7 +145,8 @@ TEST_F(CopyRegKeyWorkItemTest, TestOverwriteAndRestore) {
   scoped_ptr<CopyRegKeyWorkItem> item(
       WorkItem::CreateCopyRegKeyWorkItem(test_data_.root_key(),
                                          test_data_.empty_key_path(),
-                                         destination_path_));
+                                         destination_path_,
+                                         WorkItem::ALWAYS));
   EXPECT_TRUE(item->Do());
 
   // Make sure the dest is now empty.
@@ -131,7 +169,8 @@ TEST_F(CopyRegKeyWorkItemTest, TestIgnoreFailRollback) {
   scoped_ptr<CopyRegKeyWorkItem> item(
       WorkItem::CreateCopyRegKeyWorkItem(test_data_.root_key(),
                                          test_data_.empty_key_path(),
-                                         test_data_.non_empty_key_path()));
+                                         test_data_.non_empty_key_path(),
+                                         WorkItem::ALWAYS));
   item->set_ignore_failure(true);
   EXPECT_TRUE(item->Do());
   item->Rollback();
diff --git a/chrome/installer/util/work_item.cc b/chrome/installer/util/work_item.cc
index e4982f8..8b7bb11 100644
--- a/chrome/installer/util/work_item.cc
+++ b/chrome/installer/util/work_item.cc
@@ -26,9 +26,10 @@ WorkItem::~WorkItem() {
 CopyRegKeyWorkItem* WorkItem::CreateCopyRegKeyWorkItem(
     HKEY predefined_root,
     const std::wstring& source_key_path,
-    const std::wstring& dest_key_path) {
+    const std::wstring& dest_key_path,
+    CopyOverWriteOption overwrite_option) {
   return new CopyRegKeyWorkItem(predefined_root, source_key_path,
-                                dest_key_path);
+                                dest_key_path, overwrite_option);
 }
 
 CopyTreeWorkItem* WorkItem::CreateCopyTreeWorkItem(
diff --git a/chrome/installer/util/work_item.h b/chrome/installer/util/work_item.h
index cbe9665..6c345a9 100644
--- a/chrome/installer/util/work_item.h
+++ b/chrome/installer/util/work_item.h
@@ -63,7 +63,8 @@ class WorkItem {
   static CopyRegKeyWorkItem* CreateCopyRegKeyWorkItem(
       HKEY predefined_root,
       const std::wstring& source_key_path,
-      const std::wstring& dest_key_path);
+      const std::wstring& dest_key_path,
+      CopyOverWriteOption overwrite_option);
 
   // Create a CopyTreeWorkItem that recursively copies a file system hierarchy
   // from source path to destination path.
diff --git a/chrome/installer/util/work_item_list.cc b/chrome/installer/util/work_item_list.cc
index 23ef381..f9808c0 100644
--- a/chrome/installer/util/work_item_list.cc
+++ b/chrome/installer/util/work_item_list.cc
@@ -76,9 +76,10 @@ void WorkItemList::AddWorkItem(WorkItem* work_item) {
 WorkItem* WorkItemList::AddCopyRegKeyWorkItem(
     HKEY predefined_root,
     const std::wstring& source_key_path,
-    const std::wstring& dest_key_path) {
+    const std::wstring& dest_key_path,
+    CopyOverWriteOption overwrite_option) {
   WorkItem* item = WorkItem::CreateCopyRegKeyWorkItem(
-      predefined_root, source_key_path, dest_key_path);
+      predefined_root, source_key_path, dest_key_path, overwrite_option);
   AddWorkItem(item);
   return item;
 }
diff --git a/chrome/installer/util/work_item_list.h b/chrome/installer/util/work_item_list.h
index e73a1ac..f334a4a 100644
--- a/chrome/installer/util/work_item_list.h
+++ b/chrome/installer/util/work_item_list.h
@@ -40,7 +40,8 @@ class WorkItemList : public WorkItem {
   // Add a CopyRegKeyWorkItem that recursively copies a given registry key.
   virtual WorkItem* AddCopyRegKeyWorkItem(HKEY predefined_root,
                                           const std::wstring& source_key_path,
-                                          const std::wstring& dest_key_path);
+                                          const std::wstring& dest_key_path,
+                                          CopyOverWriteOption overwrite_option);
 
   // Add a CopyTreeWorkItem to the list of work items.
   // See the NOTE in the documentation for the CopyTreeWorkItem class for