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

chrome/installer/setup/uninstall.cc

Index: chrome/installer/setup/uninstall.cc
diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index e09be35b587edb877a8e5577fcd7a175cc36f8ca..6b0d7b11e765ad1fcc253e7edeedb3a5bf382e53 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -533,13 +533,18 @@ bool DeleteChromeRegistrationKeys(BrowserDistribution* dist, HKEY root,
   // 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);
@@ -809,6 +814,17 @@ InstallStatus UninstallProduct(const InstallationState& original_state,
   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