If Chrome Binaries version > App Host version, then update App Host.

chrome/browser/extensions/app_host/update.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.
+
+#include "chrome/browser/extensions/app_host/update.h"
+
+#include <windows.h>
+#include "base/command_line.h"
+#include "base/file_version_info.h"
+#include "base/logging.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/process_util.h"
+#include "base/string16.h"
+#include "base/string_util.h"
+#include "base/version.h"
+#include "base/win/registry.h"
+#include "chrome/installer/launcher_support/chrome_launcher_support.h"
+
+namespace app_host {
+
+namespace {
+
+// TODO(huangs) Refactor the constants:
+// http://code.google.com/p/chromium/issues/detail?id=148538

[grt (UTC plus 2), 2012/09/13 20:04:12]

please use http://crbug.com/148538 and move to previous line

[huangs, 2012/09/13 22:25:37]

Done.

[grt (UTC plus 2), 2012/09/14 14:26:52]

I meant to put the bug number on the same line as the comment to save vertical
space.  Apologies for the ambiguous request.

[huangs, 2012/09/14 16:34:23]

Done.  Just realized that comments can appear in older patches...

+const wchar_t kGoogleRegClientsKey[] = L"Software\\Google\\Update\\Clients\\";
+
+// Copied from util_constants.cc.
+const char kMultiInstall[] = "multi-install";
+const char kChromeAppHost[] = "app-host";
+const char kVerboseLogging[] = "verbose-logging";
+
+// Copied from google_update_constants.cc.
+const wchar_t kRegVersionField[] = L"pv";
+
+// Copied from chrome_appid.cc.
+const wchar_t kMultiInstallAppGuid[] =
+    L"{4DC8B4CA-1BDA-483e-B5FA-D3C12E15B62D}";
+
+// Fetches the version of the App Host, directly from the process.
+// This is the preferred method to

[erikwright (departed), 2012/09/13 19:27:49]

no need for the "This is the preferred.." bit.

[huangs, 2012/09/13 22:25:37]

Done.

+// GetAppVersion(L"{FDA71E6F-AC4C-4a00-8B70-9958A68906BF}", false, &version).
+Version GetAppHostVersion() {
+  scoped_ptr<FileVersionInfo> version_info(
+      FileVersionInfo::CreateFileVersionInfoForCurrentModule());
+  DCHECK(version_info.get() != NULL);

[grt (UTC plus 2), 2012/09/13 20:04:12]

remove DCHECK since scoped_ptr<> operator-> asserts

[huangs, 2012/09/13 22:25:37]

Done.

+  Version version = Version(WideToASCII(version_info->product_version()));

[erikwright (departed), 2012/09/13 19:27:49]

just return directly. Don't assign to a local variable.

[huangs, 2012/09/13 22:25:37]

Done.  Vestige of old "delete version_info;" code.

+  return version;
+}
+
+// Fetches the app version ("pv" entry) of a Google product from the registry,
+// given the app GUID ("{###...###}") and the installation level.
+Version GetAppVersion(const wchar_t* app_guid, bool system_level) {

[erikwright (departed), 2012/09/13 19:27:49]

remove the system_level parameter. It will always be true now.

[huangs, 2012/09/13 22:25:37]

Done.

+  HKEY root_key = system_level ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
+  string16 client_key(kGoogleRegClientsKey);
+  client_key.append(app_guid);
+  base::win::RegKey reg_key;
+  string16 version_str;
+  if ((reg_key.Open(root_key, client_key.c_str(),

[grt (UTC plus 2), 2012/09/13 20:04:12]

optional nit: it seems more natural to me to reverse the logic here:
  if (this && that)
    return Version(WideToASCII(version_str));
  return Version();

[huangs, 2012/09/13 22:25:37]

Done.  de'Morgan's Law.

+      KEY_QUERY_VALUE) != ERROR_SUCCESS) ||

[erikwright (departed), 2012/09/13 19:27:49]

KEY_QUERY_VALUE should line up with root_key

[huangs, 2012/09/13 22:25:37]

Done.

+      (reg_key.ReadValue(kRegVersionField, &version_str) != ERROR_SUCCESS)) {
+    return Version();
+  }
+  return Version(WideToASCII(version_str));
+}
+
+// Calls setup.exe to update App Host, using the system-level setup.exe.
+bool UpdateAppHost() {
+  // Get the path to the setup.exe.
+  FilePath setup_exe(chrome_launcher_support::GetSetupExeForInstallationLevel(
+                     chrome_launcher_support::SYSTEM_LEVEL_INSTALLATION));

[erikwright (departed), 2012/09/13 19:27:49]

indent 4 from FilePath.

[grt (UTC plus 2), 2012/09/13 20:04:12]

hmm, my gut tells me:

  FilePath setup_exe(
      chrome_launcher_support::GetSetupExeForInstallationLevel(
          chrome_launcher_support::SYSTEM_LEVEL_INSTALLATION));

[huangs, 2012/09/13 22:25:37]

Done.  I prefer staircase.

+  if (setup_exe.empty()) {
+    LOG(ERROR) << "Failed to find setup.exe";
+    return false;
+  }
+  CommandLine cmd_line(setup_exe);
+  cmd_line.AppendSwitch(kMultiInstall);
+  cmd_line.AppendSwitch(kChromeAppHost);
+  cmd_line.AppendSwitch(kVerboseLogging);
+  LOG(INFO) << "Launching: " << cmd_line.GetCommandLineString();
+  return base::LaunchProcess(cmd_line, base::LaunchOptions(), NULL);
+}
+
+}

[erikwright (departed), 2012/09/13 19:27:49]

}  // namespace

[huangs, 2012/09/13 22:25:37]

Done.

+
+void EnsureAppHostUpToDate() {
+  // Only checking system-level Chrome Binaries.
+  Version chrome_binaries_version(GetAppVersion(kMultiInstallAppGuid, true));
+  if (chrome_binaries_version.IsValid()) {

[erikwright (departed), 2012/09/13 19:27:49]

if chrome_binaries_version is not valid, next try to get chrome's version (to
support single-install at system-level mode).

[huangs, 2012/09/13 22:25:37]

Done, but shuffled things around a lot.

+    Version app_host_version(GetAppHostVersion());
+    if (app_host_version.IsValid()) {
+      if (app_host_version.IsOlderThan(chrome_binaries_version.GetString())) {
+        LOG(INFO) << "Updating App Host from"

[grt (UTC plus 2), 2012/09/13 20:04:12]

space after "from", and condense as much as possible:
        LOG(INFO) << "Updating App Host from " << app_host_version.GetString()
                  << " to " << chrome_binaries_version.GetString();

[huangs, 2012/09/13 22:25:37]

Done.

+                  << app_host_version.GetString()
+                  << " to "
+                  << chrome_binaries_version.GetString();
+        if (!UpdateAppHost())
+          LOG(ERROR) << "Failed to update App Host";
+      }
+    } else {
+      LOG(ERROR) << "Failed to get App Host version";
+    }
+  }  // Else there is no system-level Chrome, so do nothing.
+}
+
+}  // namespace app_host