Use a better registration suffix that will always be unique while respecting the MSDN rules (2nd tr…

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 <vector>
 
@@ skipping 515 matching lines @@
   // well anyways.
   string16 html_prog_id(ShellUtil::kRegClasses);
   html_prog_id.push_back(FilePath::kSeparators[0]);
   html_prog_id.append(ShellUtil::kChromeHTMLProgId);
   html_prog_id.append(browser_entry_suffix);
   InstallUtil::DeleteRegistryKey(root, html_prog_id);
 
   // Delete Software\Classes\Chrome (Same comment as above applies for this too)
   string16 chrome_app_id(ShellUtil::kRegClasses);
   chrome_app_id.push_back(FilePath::kSeparators[0]);
-  if (browser_entry_suffix.empty()) {
-    // An unsuffixed appid used to be registered on some user-level install
-    // (dev-channel 21.0.1171.0). Make sure it gets cleaned up here.
+  string16 old_style_suffix;
+  if (browser_entry_suffix.empty() ||
+      (ShellUtil::GetOldUserSpecificRegistrySuffix(&old_style_suffix) &&
+       browser_entry_suffix.compare(old_style_suffix) == 0)) {

[robertshield, 2012/06/27 02:22:29]

on uninstall, should we not just unconditionally attempt to delete all relevant
registry keys? why bother checking suffixes when we could instead just always
try to delete the suffixed and unsuffixed versions (not caring if the deletion
attempts fail)?

[gab, 2012/06/27 03:56:56]

This would be more work in most cases, but I agree it would make the uninstall
logic simpler.

i.e. for user-level uninstall:
1) new-style suffixed entries
2) old-style suffixed entries
3) unsuffixed entries (in HKCU only if !remove_all)

[grt (UTC plus 2), 2012/06/27 15:47:32]

nit: i think "browser_entry_suffix == old_style_suffix" is more readable than
.compare() == 0

[gab, 2012/07/03 22:36:06]

Done.

[gab, 2012/07/03 22:36:06]

I removed this to always suffix with the passed in appid as that makes more
sense.

I however left the logic on line 823 as it only requires a small check (instead
of deleting all entries for all possible suffixes without checking).

[grt (UTC plus 2), 2012/07/05 20:00:19]

Does this mean old values will be left behind now?

[gab, 2012/07/05 21:19:53]

No, it means I'm using the browser_entry_suffix passed in (instead of doing some
crazy logic to bypass GetBrowserModelId() here (which I was doing up to this
patchset)).

The logic on line 823 below was kept to pass in the old suffixes when
appropriate.

+    // Some appids were incorrectly registered (unsuffixed in dev-channel
+    // 21.0.1171.0 and username suffixed in dev 21.0.1180.0).
+    // Make sure the bad registrations get cleaned up here if they are present.
     // Note: this couldn't be cleaned on update as a currently running old
-    // chrome might still be using the unsuffixed appid when the registration
-    // update steps run.
-    InstallUtil::DeleteRegistryKey(root, chrome_app_id + dist->GetBaseAppId());
+    // chrome might still be using the bad appid when the registration update
+    // steps run.
+    InstallUtil::DeleteRegistryKey(
+        root, chrome_app_id + dist->GetBaseAppId() + browser_entry_suffix);
   }
   chrome_app_id.append(ShellUtil::GetBrowserModelId(dist, chrome_exe.value()));
   InstallUtil::DeleteRegistryKey(root, chrome_app_id);
 
   // Delete all Start Menu Internet registrations that refer to this Chrome.
   {
     using base::win::RegistryKeyIterator;
     InstallUtil::ProgramCompare open_command_pred(chrome_exe);
     string16 client_name;
     string16 client_key;
@@ skipping 249 matching lines @@
   // If the user's Chrome is registered with a suffix: it is possible that old
   // unsuffixed registrations were left in HKCU (e.g. if this install was
   // previously installed with no suffix in HKCU (old suffix rules if the user
   // is not an admin (or declined UAC at first run)) and later had to be
   // suffixed when fully registered in HKLM (e.g. when later making Chrome
   // default through the UI)).
   // Remove remaining HKCU entries with no suffix if any.
   if (!suffix.empty()) {
     DeleteChromeRegistrationKeys(browser_dist, HKEY_CURRENT_USER, string16(),
                                  installer_state.target_path(), &ret);
+
+    // For similar reasons it is possible in very few installs (from 21.0.1180.0
+    // and fixed shortly after) to be installed with the new-style suffix, but
+    // have some old-style suffix registrations left behind.
+    string16 old_style_suffix;
+    if (ShellUtil::GetOldUserSpecificRegistrySuffix(&old_style_suffix) &&
+        suffix.compare(old_style_suffix) != 0) {

[grt (UTC plus 2), 2012/06/27 15:47:32]

same nit as above: "suffix != old_style_suffix" is more readable

[gab, 2012/07/03 22:36:06]

Done.

+      DeleteChromeRegistrationKeys(browser_dist, HKEY_CURRENT_USER,
+                                   old_style_suffix,
+                                   installer_state.target_path(), &ret);
+    }
   }
 
   // Chrome is registered in HKLM for all system-level installs and for
   // user-level installs for which Chrome has been made the default browser.
   // Always remove the HKLM registration for system-level installs.  For
   // user-level installs, only remove it if both: 1) this uninstall isn't a self
   // destruct following the installation of a system-level Chrome (because the
   // system-level Chrome owns the HKLM registration now), and 2) this user has
   // made Chrome their default browser (i.e. has shell integration entries
   // registered with |suffix| (note: |suffix| will be the empty string if
@@ skipping 135 matching lines @@
 
   // 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