Delete both regular and Metro user data dirs on uninstall.

chrome/installer/setup/uninstall.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.
 //
 // This file defines the methods useful for uninstalling Chrome.
 
 #include "chrome/installer/setup/uninstall.h"
 
 #include <windows.h>
 
@@ skipping 321 matching lines @@
     if (!parent_dir.empty() && file_util::IsDirectoryEmpty(parent_dir)) {
       if (!file_util::Delete(parent_dir, true)) {
         ret = false;
         LOG(ERROR) << "Failed to delete folder: " << parent_dir.value();
       }
     }
   }
   return ret;
 }
 
-FilePath GetLocalStateFolder(const Product& product) {
+void GetLocalStateFolders(const Product& product,
+                          std::vector<FilePath>* paths) {
   // Obtain the location of the user profile data.
-  FilePath local_state_folder = product.GetUserDataPath();
-  LOG_IF(ERROR, local_state_folder.empty())
+  product.GetUserDataPaths(paths);
+  LOG_IF(ERROR, paths->empty())
       << "Could not retrieve user's profile directory.";
-
-  return local_state_folder;
 }
 
 // Creates a copy of the local state file and returns a path to the copy.
-FilePath BackupLocalStateFile(const FilePath& local_state_folder) {
+FilePath BackupLocalStateFile(
+    const std::vector<FilePath>& local_state_folders) {
   FilePath backup;
-  FilePath state_file(local_state_folder.Append(chrome::kLocalStateFilename));
-  if (!file_util::CreateTemporaryFile(&backup)) {
-    LOG(ERROR) << "Failed to create temporary file for Local State.";
-  } else {
-    file_util::CopyFile(state_file, backup);
+
+  // Copy the first local state file that is found.
+  for (size_t i = 0; i < local_state_folders.size(); ++i) {

[gab, 2012/08/03 17:54:20]

Is it ok if the Metro directory comes out of this? I'm not convinced from the
usage related to Chrome metrics in DoPostUninstallOperations().

[grt (UTC plus 2), 2012/08/03 20:18:37]

Yes.  Ideally, we'd aggregate the metrics from both sides, or at least pick the
more recent one, but something (desktop, falling back to metro) is better than
nothing in this case.

+    const FilePath& local_state_folder = local_state_folders[i];
+    FilePath state_file(local_state_folder.Append(chrome::kLocalStateFilename));
+    if (!file_util::PathExists(state_file))
+      continue;
+    if (!file_util::CreateTemporaryFile(&backup))
+      LOG(ERROR) << "Failed to create temporary file for Local State.";
+    else
+      file_util::CopyFile(state_file, backup);
+    break;
   }
   return backup;
 }
 
 enum DeleteResult {
   DELETE_SUCCEEDED,
   DELETE_FAILED,
   DELETE_REQUIRES_REBOOT,
 };
 
-// Copies the local state to the temp folder and then deletes it.
-// The path to the copy is returned via the local_state_copy parameter.
-DeleteResult DeleteLocalState(const Product& product) {
-  FilePath user_local_state(GetLocalStateFolder(product));
-  if (user_local_state.empty())
+// Deletes all user data directories for a product.
+DeleteResult DeleteLocalState(const std::vector<FilePath>& local_state_folders,
+                              bool schedule_on_failure) {
+  if (local_state_folders.empty())
     return DELETE_SUCCEEDED;

[gab, 2012/08/03 17:54:20]

I think this should be: {
  NOTREACHED();
  return DELETE_FAILED;
}

[grt (UTC plus 2), 2012/08/03 20:18:37]

I added a DCHECK to GetChromeUserDataPaths() rather than sprinkling them around
the code.  Do you think that's sufficient?

[gab, 2012/08/03 20:42:41]

Yes, but I still think the return statement here should return DELETE_FAILED.

[grt (UTC plus 2), 2012/08/03 20:56:34]

hmm.  i disagree (although it's purely academic since the return value is
ignored).  if the vector was empty, the function was told to delete nothing at
all, which is a noop.  i don't think this should imply that the uninstall should
be aborted.

[gab, 2012/08/03 21:58:57]

Ah ok I see your point, given the returned value is not used though what about
making this function return void?

[grt (UTC plus 2), 2012/08/04 19:25:20]

After more thought, I like this as-is.  The fact that the (current) caller
doesn't care about the result doesn't mean that this function shouldn't have a
clean interface.

[gab, 2012/08/05 06:27:39]

sgtm

 
   DeleteResult result = DELETE_SUCCEEDED;
-  VLOG(1) << "Deleting user profile " << user_local_state.value();
-  if (!file_util::Delete(user_local_state, true)) {
-    LOG(ERROR) << "Failed to delete user profile dir: "
-               << user_local_state.value();
-    if (product.is_chrome_frame()) {
-      ScheduleDirectoryForDeletion(user_local_state.value().c_str());
-      result = DELETE_REQUIRES_REBOOT;
-    } else {
-      result = DELETE_FAILED;
+  const FilePath *user_local_state = NULL;

[gab, 2012/08/03 17:54:20]

Move this in the for loop (see comment below as to why you don't need it out of
that scope).

[grt (UTC plus 2), 2012/08/03 20:18:37]

Done.

+  for (size_t i = 0; i < local_state_folders.size(); ++i) {
+    user_local_state = &local_state_folders[i];
+    VLOG(1) << "Deleting user profile " << user_local_state->value();
+    if (!file_util::Delete(*user_local_state, true)) {
+      LOG(ERROR) << "Failed to delete user profile dir: "
+                 << user_local_state->value();
+      if (schedule_on_failure) {
+        ScheduleDirectoryForDeletion(user_local_state->value().c_str());
+        result = DELETE_REQUIRES_REBOOT;
+      } else {
+        result = DELETE_FAILED;
+      }
     }
   }
 
-  if (result == DELETE_REQUIRES_REBOOT) {
-    ScheduleParentAndGrandparentForDeletion(user_local_state);
-  } else {
-    DeleteEmptyParentDir(user_local_state);
+  if (user_local_state) {

[gab, 2012/08/03 17:54:20]

This check is unnecessary given you only proceed if !local_state_folders.empty()
above.

[grt (UTC plus 2), 2012/08/03 20:18:37]

Done.

+    if (result == DELETE_REQUIRES_REBOOT) {
+      ScheduleParentAndGrandparentForDeletion(*user_local_state);

[gab, 2012/08/03 17:54:20]

You can use local_state_folders[0] here instead (given your comment on
GetChromeUserDataPaths() that the first path is the primary one and that the
other ones are guaranteed to be siblings).

[grt (UTC plus 2), 2012/08/03 20:18:37]

Done.

+    } else {
+      DeleteEmptyParentDir(*user_local_state);
+    }
   }
 
   return result;
 }
 
 bool MoveSetupOutOfInstallFolder(const InstallerState& installer_state,
                                  const FilePath& setup_path,
                                  const Version& installed_version) {
   bool ret = false;
   FilePath setup_exe(installer_state.GetInstallerDirectory(installed_version)
@@ skipping 691 matching lines @@
 
   // Finally delete all the files from Chrome folder after moving setup.exe
   // and the user's Local State to a temp location.
   bool delete_profile = ShouldDeleteProfile(installer_state, cmd_line, status,
                                             product);
   ret = installer::UNINSTALL_SUCCESSFUL;
 
   // When deleting files, we must make sure that we're either a "single"
   // (aka non-multi) installation or we are the Chrome Binaries.
 
-  FilePath backup_state_file(
-      BackupLocalStateFile(GetLocalStateFolder(product)));
+  std::vector<FilePath> local_state_folders;
+  GetLocalStateFolders(product, &local_state_folders);
+  FilePath backup_state_file(BackupLocalStateFile(local_state_folders));
 
   DeleteResult delete_result = DELETE_SUCCEEDED;
 
   if (product.is_chrome_app_host()) {
     DeleteAppHostFilesAndFolders(installer_state, product_state->version());
   } else if (!installer_state.is_multi_install() ||
              product.is_chrome_binaries()) {
 
     // In order to be able to remove the folder in which we're running, we
     // need to move setup.exe out of the install folder.
     // TODO(tommi): What if the temp folder is on a different volume?
     MoveSetupOutOfInstallFolder(installer_state, setup_path,
                                 product_state->version());
     delete_result = DeleteChromeFilesAndFolders(installer_state,
                                                 product_state->version());
   }
 
   if (delete_profile)
-    DeleteLocalState(product);
+    DeleteLocalState(local_state_folders, product.is_chrome_frame());
 
   if (delete_result == DELETE_FAILED) {
     ret = installer::UNINSTALL_FAILED;
   } else if (delete_result == DELETE_REQUIRES_REBOOT) {
     ret = installer::UNINSTALL_REQUIRES_REBOOT;
   }
 
   if (!force_uninstall) {
     VLOG(1) << "Uninstallation complete. Launching post-uninstall operations.";
     browser_dist->DoPostUninstallOperations(product_state->version(),
         backup_state_file, distribution_data);
   }
 
   // Try and delete the preserved local state once the post-install
   // operations are complete.
   if (!backup_state_file.empty())
     file_util::Delete(backup_state_file, false);
 
   return ret;
 }
 
 }  // namespace installer