Add fixing of MSI DisplayVersion to Chrome installs/updates.

chrome/installer/setup/install_worker.cc

Index: chrome/installer/setup/install_worker.cc
diff --git a/chrome/installer/setup/install_worker.cc b/chrome/installer/setup/install_worker.cc
index cca3132c6606beb850f0a1e8213422f4a586343c..71181ceb7da4b2ee7874d0be37209b27726f439b 100644
--- a/chrome/installer/setup/install_worker.cc
+++ b/chrome/installer/setup/install_worker.cc
@@ -11,6 +11,7 @@
 #include <shlobj.h>
 #include <time.h>
 
+#include <algorithm>
 #include <vector>
 
 #include "base/bind.h"
@@ -20,7 +21,9 @@
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
+#include "base/strings/string_tokenizer.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/version.h"
 #include "base/win/registry.h"
@@ -78,6 +81,26 @@ const wchar_t kElevationPolicyKeyPath[] =
 const wchar_t kLegacyCmdInstallApp[] = L"install-application";
 const wchar_t kLegacyCmdInstallExtension[] = L"install-extension";
 
+// The prefix of the ClientState value that contains the MSI product id in the
+// format "<kMSIProductIdPrefix><PRODUCTID>".
+const wchar_t kMSIProductIdPrefix[] = L"EnterpriseProduct";
+
+// The common path for uninstall registry entries that show up in the
+// "Add/Remove Programs" dialog and its later incarnations.
+const wchar_t kUninstallRegPathRoot[] =
+    L"Software\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\";
+
+// This is the path to the MSI user data cache, which should probably never be
+// tampered with but which we'll try tweaking anyway. This is used as a printf
+// format string with the token to be replaced by an MSI user data id generated
+// by ConvertGUIDToMSIUserDataFormat(). Other items of note here are that this
+// hard-codes the Local System SID meaning that this is strictly only usable for
+// system-level installs. This would need to change for http://crbug.com/111058
+// to be fixed.
+const wchar_t kMSIUserDataCacheRoot[] =
+    L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Installer\\UserData\\"
+    L"S-1-5-18\\Products\\%ls\\InstallProperties";
+
 void GetOldIELowRightsElevationPolicyKeyPath(base::string16* key_path) {
   key_path->assign(kElevationPolicyKeyPath,
                    arraysize(kElevationPolicyKeyPath) - 1);
@@ -672,6 +695,30 @@ void CleanupBadCanaryDelegateExecuteRegistration(
   }
 }
 
+// Returns the MSI product ID from the ClientState key that is populated for MSI
+// installs. This property is encoded in a value name whose format is
+// "EnterpriseId<GUID>" where <GUID> is the MSI product id. <GUID> is in the
+// format XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX. The id will be returned if found
+// otherwise this method will return an empty string.
+//
+// This format is strange and its provenance is shrouded in mystery but it has
+// the data we need, so use it.
+base::string16 ExtractMSIProductId(const InstallerState& installer_state,
+                                   const Product& product) {
+  HKEY reg_root = installer_state.root_key();
+  BrowserDistribution* dist = product.distribution();
+  DCHECK(dist);
+
+  base::win::RegistryValueIterator value_iter(reg_root,
+                                              dist->GetStateKey().c_str());
+  for (; value_iter.Valid(); ++value_iter) {
+    base::string16 value_name(value_iter.Name());
+    if (StartsWith(value_name, kMSIProductIdPrefix, false))

[grt (UTC plus 2), 2014/12/01 16:19:30]

uber-nit: "false /* !case_sensitive */))"

+      return value_name.substr(arraysize(kMSIProductIdPrefix) - 1);
+  }
+  return base::string16();
+}
+
 }  // namespace
 
 // This method adds work items to create (or update) Chrome uninstall entry in
@@ -721,102 +768,130 @@ void AddUninstallShortcutWorkItems(const InstallerState& installer_state,
       true);
 
   // MSI installations will manage their own uninstall shortcuts.
-  if (!installer_state.is_msi() && product.ShouldCreateUninstallEntry()) {
-    // We need to quote the command line for the Add/Remove Programs dialog.
-    CommandLine quoted_uninstall_cmd(installer_path);
-    DCHECK_EQ(quoted_uninstall_cmd.GetCommandLineString()[0], '"');
-    quoted_uninstall_cmd.AppendArguments(uninstall_arguments, false);
-
-    base::string16 uninstall_reg = browser_dist->GetUninstallRegPath();
-    install_list->AddCreateRegKeyWorkItem(
-        reg_root, uninstall_reg, KEY_WOW64_32KEY);
-    install_list->AddSetRegValueWorkItem(reg_root,
-                                         uninstall_reg,
-                                         KEY_WOW64_32KEY,
-                                         installer::kUninstallDisplayNameField,
-                                         browser_dist->GetDisplayName(),
-                                         true);
-    install_list->AddSetRegValueWorkItem(
-        reg_root,
-        uninstall_reg,
-        KEY_WOW64_32KEY,
-        installer::kUninstallStringField,
-        quoted_uninstall_cmd.GetCommandLineString(),
-        true);
-    install_list->AddSetRegValueWorkItem(reg_root,
-                                         uninstall_reg,
-                                         KEY_WOW64_32KEY,
-                                         L"InstallLocation",
-                                         install_path.value(),
-                                         true);
-
-    BrowserDistribution* dist = product.distribution();
-    base::string16 chrome_icon = ShellUtil::FormatIconLocation(
-        install_path.Append(dist->GetIconFilename()).value(),
-        dist->GetIconIndex(BrowserDistribution::SHORTCUT_CHROME));
-    install_list->AddSetRegValueWorkItem(reg_root,
-                                         uninstall_reg,
-                                         KEY_WOW64_32KEY,
-                                         L"DisplayIcon",
-                                         chrome_icon,
-                                         true);
-    install_list->AddSetRegValueWorkItem(reg_root,
-                                         uninstall_reg,
-                                         KEY_WOW64_32KEY,
-                                         L"NoModify",
-                                         static_cast<DWORD>(1),
-                                         true);
-    install_list->AddSetRegValueWorkItem(reg_root,
-                                         uninstall_reg,
-                                         KEY_WOW64_32KEY,
-                                         L"NoRepair",
-                                         static_cast<DWORD>(1),
-                                         true);
-
-    install_list->AddSetRegValueWorkItem(reg_root,
-                                         uninstall_reg,
-                                         KEY_WOW64_32KEY,
-                                         L"Publisher",
-                                         browser_dist->GetPublisherName(),
-                                         true);
-    install_list->AddSetRegValueWorkItem(reg_root,
-                                         uninstall_reg,
-                                         KEY_WOW64_32KEY,
-                                         L"Version",
-                                         ASCIIToUTF16(new_version.GetString()),
-                                         true);
-    install_list->AddSetRegValueWorkItem(reg_root,
-                                         uninstall_reg,
-                                         KEY_WOW64_32KEY,
-                                         L"DisplayVersion",
-                                         ASCIIToUTF16(new_version.GetString()),
-                                         true);
-    // TODO(wfh): Ensure that this value is preserved in the 64-bit hive when
-    // 64-bit installs place the uninstall information into the 64-bit registry.
-    install_list->AddSetRegValueWorkItem(reg_root,
-                                         uninstall_reg,
-                                         KEY_WOW64_32KEY,
-                                         L"InstallDate",
-                                         InstallUtil::GetCurrentDate(),
-                                         false);
-
-    const std::vector<uint16>& version_components = new_version.components();
-    if (version_components.size() == 4) {
-      // Our version should be in major.minor.build.rev.
+  if (product.ShouldCreateUninstallEntry()) {
+    if (!installer_state.is_msi()) {
+      // We need to quote the command line for the Add/Remove Programs dialog.
+      CommandLine quoted_uninstall_cmd(installer_path);
+      DCHECK_EQ(quoted_uninstall_cmd.GetCommandLineString()[0], '"');
+      quoted_uninstall_cmd.AppendArguments(uninstall_arguments, false);
+
+      base::string16 uninstall_reg = browser_dist->GetUninstallRegPath();
+      install_list->AddCreateRegKeyWorkItem(reg_root, uninstall_reg,
+                                            KEY_WOW64_32KEY);
       install_list->AddSetRegValueWorkItem(
-          reg_root,
-          uninstall_reg,
-          KEY_WOW64_32KEY,
-          L"VersionMajor",
-          static_cast<DWORD>(version_components[2]),
+          reg_root, uninstall_reg, KEY_WOW64_32KEY,
+          installer::kUninstallDisplayNameField, browser_dist->GetDisplayName(),
           true);
       install_list->AddSetRegValueWorkItem(
-          reg_root,
-          uninstall_reg,
-          KEY_WOW64_32KEY,
-          L"VersionMinor",
-          static_cast<DWORD>(version_components[3]),
-          true);
+          reg_root, uninstall_reg, KEY_WOW64_32KEY,
+          installer::kUninstallStringField,
+          quoted_uninstall_cmd.GetCommandLineString(), true);
+      install_list->AddSetRegValueWorkItem(reg_root, uninstall_reg,
+                                           KEY_WOW64_32KEY, L"InstallLocation",
+                                           install_path.value(), true);
+
+      BrowserDistribution* dist = product.distribution();
+      base::string16 chrome_icon = ShellUtil::FormatIconLocation(
+          install_path.Append(dist->GetIconFilename()).value(),
+          dist->GetIconIndex(BrowserDistribution::SHORTCUT_CHROME));
+      install_list->AddSetRegValueWorkItem(reg_root, uninstall_reg,
+                                           KEY_WOW64_32KEY, L"DisplayIcon",
+                                           chrome_icon, true);
+      install_list->AddSetRegValueWorkItem(reg_root, uninstall_reg,
+                                           KEY_WOW64_32KEY, L"NoModify",
+                                           static_cast<DWORD>(1), true);
+      install_list->AddSetRegValueWorkItem(reg_root, uninstall_reg,
+                                           KEY_WOW64_32KEY, L"NoRepair",
+                                           static_cast<DWORD>(1), true);
+
+      install_list->AddSetRegValueWorkItem(
+          reg_root, uninstall_reg, KEY_WOW64_32KEY, L"Publisher",
+          browser_dist->GetPublisherName(), true);
+      install_list->AddSetRegValueWorkItem(
+          reg_root, uninstall_reg, KEY_WOW64_32KEY, L"Version",
+          ASCIIToUTF16(new_version.GetString()), true);
+      install_list->AddSetRegValueWorkItem(
+          reg_root, uninstall_reg, KEY_WOW64_32KEY, L"DisplayVersion",
+          ASCIIToUTF16(new_version.GetString()), true);
+      // TODO(wfh): Ensure that this value is preserved in the 64-bit hive when
+      // 64-bit installs place the uninstall information into the 64-bit
+      // registry.
+      install_list->AddSetRegValueWorkItem(
+          reg_root, uninstall_reg, KEY_WOW64_32KEY, L"InstallDate",
+          InstallUtil::GetCurrentDate(), false);
+
+      const std::vector<uint16>& version_components = new_version.components();
+      if (version_components.size() == 4) {
+        // Our version should be in major.minor.build.rev.
+        install_list->AddSetRegValueWorkItem(
+            reg_root, uninstall_reg, KEY_WOW64_32KEY, L"VersionMajor",
+            static_cast<DWORD>(version_components[2]), true);
+        install_list->AddSetRegValueWorkItem(
+            reg_root, uninstall_reg, KEY_WOW64_32KEY, L"VersionMinor",
+            static_cast<DWORD>(version_components[3]), true);
+      }
+    } else {
+      // This is an MSI install. Largely, leave all uninstall shortcuts and the
+      // like up to the MSI machinery EXCEPT for the DisplayVersion property.
+      //
+      // Due to reasons, the Chrome version number is wider than can fit inside
+      // the <8bits>.<16bits>.<8bits> allotted to MSI version numbers. To make
+      // things work the A.B.C.D Chrome version is lossily encoded into an X.Y.Z
+      // version, dropping the unneeded A and B terms. For full details, see
+      // https://code.google.com/p/chromium/issues/detail?id=67348#c62
+      //
+      // The upshot of this is that Chrome MSIs have a different visible version
+      // than actual Chrome releases which causes trouble with some workflows.
+      // To help mitigate this, attempt to keep the DisplayVersion property in
+      // sync for MSI installs in both the Add/Remove Programs dialog and the
+      // corresponding MSI user data cache.
+
+      // Figure out MSI ProductID by looking in ClientState.
+      // The ProductID is encoded as a key named "EnterpriseProduct<ProductID>".
+      base::string16 msi_product_id(
+          ExtractMSIProductId(installer_state, product));
+      if (!msi_product_id.empty()) {
+        base::string16 uninstall_reg_path(kUninstallRegPathRoot);
+        uninstall_reg_path.append(L"{");
+        uninstall_reg_path.append(msi_product_id);
+        uninstall_reg_path.append(L"}");
+
+        install_list->AddCreateRegKeyWorkItem(reg_root, uninstall_reg_path,
+                                              WorkItem::kWow64Default);
+        install_list->AddSetRegValueWorkItem(
+            reg_root, uninstall_reg_path, WorkItem::kWow64Default,
+            installer::kUninstallDisplayVersionField,
+            base::ASCIIToUTF16(new_version.GetString()), true)->
+                set_ignore_failure(true);
+
+        // Also fix up the MSI user data cache. This is a little hacky, there
+        // may be a better way and I am not certain this won't cause undesired
+        // side-effects. Note that the user data cache registry path includes
+        // the product id without dashes. Also note that the
+        // kMSIUserDataCacheRoot currently hardcodes the local system SID, which
+        // will need to be changed for user-level MSI installs to happen as
+        // tracked by http://crbug.com/111058. Leave a DCHECK here to warn those
+        // who next venture here to use a correct SID in kMSIUserDataCacheRoot.
+        DCHECK(installer_state.system_install());
+
+        base::string16 msi_user_data_product_id(
+            ConvertGUIDToMSIUserDataFormat(msi_product_id));
+        if (!msi_user_data_product_id.empty()) {
+          base::string16 msi_cache_reg_path(base::StringPrintf(
+              kMSIUserDataCacheRoot, msi_user_data_product_id.c_str()));
+
+          install_list->AddSetRegValueWorkItem(
+              reg_root, msi_cache_reg_path, KEY_WOW64_64KEY,
+              installer::kUninstallDisplayVersionField,
+              base::ASCIIToUTF16(new_version.GetString()), true)->
+                  set_ignore_failure(true);
+        } else {
+          VLOG(1) << "Failed to convert MSI user data id from product id: "
+                  << msi_product_id;
+        }
+      } else {
+        VLOG(1) << "Failed to extract MSI product id.";
+      }
     }
   }
 }
@@ -1670,4 +1745,44 @@ void AddQuickEnableChromeFrameWorkItems(const InstallerState& installer_state,
                         " command");
 }
 
+// Converts a correctly formatted guid thusly:
+// 12345678-90ab-cdef-ghij-klmnopqrstuv -> 87654321ba09fedchgjilknmporqtsvu
+// This reverses the string of the first three terms and inverts the bytes of
+// the last two. It also removes the dashes. It expects the input to NOT be
+// wrapped in braces.
+// Returns an empty string on failure.
+base::string16 ConvertGUIDToMSIUserDataFormat(const base::string16& guid) {
+  // Perform some very mild validation of the input.
+  if (guid.size() != 36)
+    return base::string16();
+
+  base::string16 msi_formatted_guid;
+  int token_count = 0;
+  base::WStringTokenizer tokenizer(guid, L"-");
+  while (tokenizer.GetNext()) {
+    // There should be exactly five tokens, bail if not.
+    if (token_count >= 5)
+      return base::string16();
+
+    base::string16 token(tokenizer.token());
+    if (token_count < 3) {
+      // For the first three tokens, just reverse the characters.
+      std::reverse(token.begin(), token.end());
+      msi_formatted_guid += token;
+    } else {
+      // The last two tokens need to be of even length.
+      if (token.size() % 2)
+        return base::string16();
+      // Swap the halves of each byte (i.e. pair of chars). I don't even.
+      for (size_t i = 0; i < token.size(); i += 2) {
+        msi_formatted_guid += token[i+1];
+        msi_formatted_guid += token[i];
+      }
+    }
+    token_count++;
+  }
+
+  return (token_count == 5) ? msi_formatted_guid : base::string16();
+}
+
 }  // namespace installer