Remove the run verb on Windows 8.

chrome/installer/util/shell_util.cc

Index: chrome/installer/util/shell_util.cc
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index 5ababade20aec9b482ef9497fb9a5c179706bb92..425819b5d2e13e37acf197124945a76e10e2fedd 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -239,11 +239,10 @@ class RegistryEntry {
 
       // <root hkey>\Software\Classes\<app_id>\.exe\shell @=open
       entries->push_back(new RegistryEntry(model_id_shell,
-                                          ShellUtil::kRegVerbOpen));
+                                           ShellUtil::kRegVerbOpen));
 
       const wchar_t* const verbs[] = { ShellUtil::kRegVerbOpen,
-                                       ShellUtil::kRegVerbOpenNewWindow,
-                                       ShellUtil::kRegVerbRun };
+                                       ShellUtil::kRegVerbOpenNewWindow };
       for (size_t i = 0; i < arraysize(verbs); ++i) {
         string16 sub_path(model_id_shell);
         sub_path.push_back(FilePath::kSeparators[0]);
@@ -740,26 +739,31 @@ bool LaunchApplicationAssociationDialog(const string16& app_id) {
 // present.  This affected Chrome 20.0.1115.1 on the dev channel (and anyone who
 // pulled a Canary >= 20.0.1112.0 and installed it manually as Google Chrome).
 // This egregious hack is here to remove the bad values for those installs, and
-// should be removed after a reasonable time, say 2012-08-01.  Anyone on Win8
-// dev channel who hasn't been autoupdated or manually updated by then will have
-// to uninstall and reinstall Chrome to repair.  See http://crbug.com/124666 and
-// http://crbug.com/123994 for gory details.
+// should be removed after a reasonable time, say 2012-08-01 (pushed back see

[grt (UTC plus 2), 2012/10/13 00:34:26]

what do you think about retiring the old "remove bad" code (which is on
M22/stable and M23/beta, and is past its useful lifetime) now and update this to
only do the new "remove bad" code?

[gab, 2012/10/13 03:33:12]

Yea, I was tempted by that, but I want to merge this in M22 and M23 (see
http://crbug.com/155640). We could remove it in a follow-up CL.

[grt (UTC plus 2), 2012/10/13 13:21:37]

Okay
I think it's a good idea.

+// below).  Anyone on Win8 dev channel who hasn't been autoupdated or manually
+// updated by then will have to uninstall and reinstall Chrome to repair.
+// See http://crbug.com/124666 and http://crbug.com/123994 for gory details.
 // This function is also used to remove DelegateExecute verb handler
 // registrations on builds for which Metro is no longer supported. This will
 // also become irrelevant sometime after Windows 8 RC (thus the aforementioned
 // removal date remains correct).
+// For installs where we do register the Windows 8 only registrations, this
+// method will clean up an old verb (run) we used to register in
+// <root>\Software\Classes\Chrome<suffix>\.exe\shell\run.
+// TODO (gab): This was fixed before the official release of Windows 8 and thus

[grt (UTC plus 2), 2012/10/13 00:34:26]

i think it's too late for this to be on stable before Windows 8 GA, so this
comment is misleading.

[gab, 2012/10/13 03:33:12]

As mentioned above, my intent is to merge it in both M22 and M23 so that we
wouldn't have to deal with this verb ever being registered on a real Win8 Chrome
install. If it doesn't get merged, I'll modify this comment.

[grt (UTC plus 2), 2012/10/13 13:21:37]

Merging it will not guarantee that.

[grt (UTC plus 2), 2012/10/13 17:50:29]

To elaborate, there will be a population of machines with M23 (which contains
the OS upgrade handler) that will be updated to Win8 before your change reaches
them.  Updates are never rolled out to 100% of users.

To simplify the merge, I suggest moving this new code into a new function with
its own detailed comment that is invoked immediately before or after
RemoveBadWindows8RegistrationIfNeeded (and revert all changes to this function).
 If I'm not mistaken, the logic to remove the run verb key can be simply "if
Windows 8".

[gab, 2012/10/13 19:02:09]

Right, but see my reply/clarification on the bug.
Will do.

[gab, 2012/10/15 21:15:57]

Done.

+// can safely be removed in February 2013.
 void RemoveBadWindows8RegistrationIfNeeded(
     BrowserDistribution* dist,
     const string16& chrome_exe) {
+  // There's no need to rollback, so forgo the usual work item lists and just
+  // remove the values from the registry.
+  bool is_per_user_install =
+      InstallUtil::IsPerUserInstall(chrome_exe.c_str());
+  const HKEY root_key = is_per_user_install ? HKEY_CURRENT_USER :
+                                              HKEY_LOCAL_MACHINE;
   if (dist->GetCommandExecuteImplClsid(NULL) &&
       (!InstallUtil::HasDelegateExecuteHandler(dist, chrome_exe) ||
        !IsChromeMetroSupported())) {
-    // There's no need to rollback, so forgo the usual work item lists and just
-    // remove the values from the registry.
-    bool is_per_user_install =
-        InstallUtil::IsPerUserInstall(chrome_exe.c_str());
-    const HKEY root_key = is_per_user_install ? HKEY_CURRENT_USER :
-                                                HKEY_LOCAL_MACHINE;
     // Use the current installation's suffix, not the about-to-be-installed
     // suffix.
     const string16 installation_suffix(
@@ -780,6 +784,16 @@ void RemoveBadWindows8RegistrationIfNeeded(
     key.append(ShellUtil::kRegShellOpen);
     InstallUtil::DeleteRegistryValue(root_key, key,
                                      ShellUtil::kRegDelegateExecute);
+  } else {
+    string16 run_verb_key(ShellUtil::kRegClasses);

[grt (UTC plus 2), 2012/10/13 00:34:26]

this code is nearly impenetrable. please add a comment saying that it removes
the key ROOT\Software\Classes\Chrome<.suffix>\.exe\shell\run.

[gab, 2012/10/13 03:33:12]

Done.

+    run_verb_key.push_back(FilePath::kSeparators[0]);
+    run_verb_key.append(ShellUtil::GetBrowserModelId(
+        dist, InstallUtil::IsPerUserInstall(chrome_exe.c_str())));
+    run_verb_key.append(ShellUtil::kRegExePath);
+    run_verb_key.append(ShellUtil::kRegShellPath);
+    run_verb_key.push_back(FilePath::kSeparators[0]);
+    run_verb_key.append(ShellUtil::kRegVerbRun);
+    InstallUtil::DeleteRegistryKey(root_key, run_verb_key);

[grt (UTC plus 2), 2012/10/13 00:34:26]

this could be in HKCU even for a system-level install, no?  (see
RegisterChromeBrowser)

[gab, 2012/10/13 03:33:12]

No, on Win8 all user level stuff is in HKCU and system-level in HKLM.

Technically it's possible for user-level stuff to be in HKLM if the user
upgraded a Win7 install to a Win8 install, but we can't clean that up on upgrade
anyways so no need to bother...

[grt (UTC plus 2), 2012/10/13 13:21:37]

I expect there to be a great number of machines upgraded from Win7, so I think
your statement should read "it's probable" rather than "it's possible".

[gab, 2012/10/13 15:17:50]

Well technically if this is merged in M22 and M23 now, people upgrading their
Win7 to Win8 come Win8 Release will never even have the run verb registered.

In fact, there is no way for us to register it in HKLM as we don't have admin
rights on OSupgrade detection (so change "it's possible" to "it's not
possible"). I think the current flow is to re-register everything in HKCU on
WIn8.

[grt (UTC plus 2), 2012/10/13 17:50:29]

Ah, I just realized that the Win7 -> Win8 path is only relevant if the version
of Chrome present at the time of OS upgrade has the OS migration feature (i.e.,
M23) since the Run verb registration isn't done prior to Win8.

[gab, 2012/10/13 19:02:09]

Well not quite, the migration can also happen on natural update, but given this
goes in, a natural update will remove the run verb anyways...

   }
 }