Remove installation of Chrome Frame.

chrome/installer/setup/install.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/installer/setup/install.h"
 
 #include <windows.h>
 #include <shlobj.h>
 #include <time.h>
 
@@ skipping 489 matching lines @@
 InstallStatus InstallOrUpdateProduct(
     const InstallationState& original_state,
     const InstallerState& installer_state,
     const base::FilePath& setup_path,
     const base::FilePath& archive_path,
     const base::FilePath& install_temp_path,
     const base::FilePath& src_path,
     const base::FilePath& prefs_path,
     const MasterPreferences& prefs,
     const Version& new_version) {
-  // TODO(robertshield): Removing the pending on-reboot moves should be done
-  // elsewhere.
-  // TODO(erikwright): Understand why this is Chrome Frame only and whether
-  // it also applies to App Host. Shouldn't it apply to any multi-install too?
-  const Products& products = installer_state.products();
-  DCHECK(products.size());
-  if (installer_state.FindProduct(BrowserDistribution::CHROME_FRAME)) {
-    // Make sure that we don't end up deleting installed files on next reboot.
-    if (!RemoveFromMovesPendingReboot(installer_state.target_path()))

[gab, 2013/12/19 01:18:31]

This appears to have been the only remaining call to this method; remove it too?

[grt (UTC plus 2), 2013/12/19 20:57:06]

I've re-added Erik's TODO since I think it still applies. I wonder if there's
actually a bug here for the non-Chrome Frame case.

[gab, 2013/12/20 19:05:54]

Hmmm... so we're going to leave a call that's used by nobody else than a
comment?!

I don't actually want this CL to take this on, but we should make an action item
(i.e. a bug + link to bug in comment) to promptly look into this, rotting
unreachable code isn't desired.

[grt (UTC plus 2), 2013/12/23 17:58:34]

Well, when you put it that way...
Agreed. To play devil's advocate, though, it will rot no more or less now than
it would have before since it has test coverage.

I've noodled in my noodle about Erik's concern. From memory, this was needed for
Chrome Frame but not Chrome since the installer requires the user to quit Chrome
before proceeding. As such, uninstalling Chrome should always be able to delete
the files without having to resort to the DeleteAfterRebootHelper. In practice,
there is a race so perhaps the safest thing to do is to leave the call here for
all products. I can't think of a downside to this, so I've made it so.

Thanks for pushing back on this.

-      LOG(ERROR) << "Error accessing pending moves value.";
-  }
+  DCHECK(installer_state.products().size());

[gab, 2013/12/19 01:18:31]

Replace check for foo.size(), but !foo.empty()

[grt (UTC plus 2), 2013/12/19 20:57:06]

Done.

 
   // Create VisualElementManifest.xml in |src_path| (if required) so that it
   // looks as if it had been extracted from the archive when calling
   // InstallNewVersion() below.
   installer_state.UpdateStage(installer::CREATING_VISUAL_MANIFEST);
   CreateVisualElementsManifest(src_path, new_version);
 
   scoped_ptr<Version> existing_version;
   InstallStatus result = InstallNewVersion(original_state, installer_state,
       setup_path, archive_path, src_path, install_temp_path, new_version,
@@ skipping 172 matching lines @@
   if (app_host_path.empty())
     return false;
 
   CommandLine cmd(app_host_path);
   cmd.AppendSwitchASCII(::switches::kInstallFromWebstore, app_code);
   VLOG(1) << "App install command: " << cmd.GetCommandLineString();
   return base::LaunchProcess(cmd, base::LaunchOptions(), NULL);
 }
 
 }  // namespace installer