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

chrome/browser/extensions/app_host/app_host_upgrade.cc

+// Copyright (c) 2012 The Chromium Authors. All rights reserved.

[grt (UTC plus 2), 2012/09/13 13:14:43]

since this is in the app_host directory, does it need to be called
app_host_upgrade?

[erikwright (departed), 2012/09/13 13:48:26]

I requested the file-name because of a possibly mistaken belief that two .cc
files with the same name, even in different directories, would cause build
failures and 'upgrade' seemed too collision-prone.

But after discussion with grt I think that must have been only two files in the
same target. So this file could be renamed to update.cc (since we are also
switching 'upgrade'->'update').

[huangs, 2012/09/13 18:46:00]

Done.

+// 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/app_host_upgrade.h"
+
+#include <windows.h>
+#include "base/command_line.h"
+#include "base/logging.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 {
+
+const wchar_t kGoogleRegClientsKey[] =
+    L"Software\\Google\\Update\\Clients\\";

[grt (UTC plus 2), 2012/09/13 13:14:43]

this fits on one line

[huangs, 2012/09/13 18:46:00]

Done.

+
+// Copied from util_constants.cc

[grt (UTC plus 2), 2012/09/13 13:14:43]

period at the end

[huangs, 2012/09/13 18:46:00]

Done.

+const char kMultiInstall[] = "multi-install";
+const char kChromeAppHost[] = "app-host";
+const char kVerboseLogging[] = "verbose-logging";
+
+// Copied from google_update_constants.cc

[grt (UTC plus 2), 2012/09/13 13:14:43]

period at the end

[huangs, 2012/09/13 18:46:00]

Done.

+const wchar_t kRegVersionField[] = L"pv";
+
+}
+
+// Copied from chrome_appid.cc

[grt (UTC plus 2), 2012/09/13 13:14:43]

period at the end
also, please file a bug to remove the duplication if you haven't already

[erikwright (departed), 2012/09/13 13:48:26]

I believe we have - perhaps just reference it here.

[huangs, 2012/09/13 18:46:00]

Done.  It was filed as http://code.google.com/p/chromium/issues/detail?id=148538

+const wchar_t kMultiInstallAppGuid[] =
+    L"{4DC8B4CA-1BDA-483e-B5FA-D3C12E15B62D}";
+const wchar_t kChromeAppHostAppGuid[] =
+    L"{FDA71E6F-AC4C-4a00-8B70-9958A68906BF}";
+
+bool GetAppVersionForInstallationLevel(const wchar_t* app_guid,
+                                       bool system_level,
+                                       Version* version) {
+  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(), KEY_QUERY_VALUE) !=

[grt (UTC plus 2), 2012/09/13 13:14:43]

this is more readable if wrapped just before KEY_QUERY_VALUE

[huangs, 2012/09/13 18:46:00]

Done.

+      ERROR_SUCCESS) ||
+      (reg_key.ReadValue(kRegVersionField, &version_str) != ERROR_SUCCESS)) {
+    return false;
+  }
+  *version = Version(WideToASCII(version_str));
+  return version->IsValid();
+}
+
+bool UpgradeAppHost(bool system_level) {

[huangs, 2012/09/13 18:46:00]

Removing redundant parameter, per discussion.

+  using namespace chrome_launcher_support;

[grt (UTC plus 2), 2012/09/13 13:14:43]

is this really needed to make the code below more readable?

[huangs, 2012/09/13 18:46:00]

Used to be used 3 times, now 2 times.  So removing this.

+  // Get the path to the setup.exe.
+  FilePath setup_exe(GetSetupExeForInstallationLevel(
+      system_level ? SYSTEM_LEVEL_INSTALLATION : USER_LEVEL_INSTALLATION));
+  if (setup_exe.empty()) {
+    LOG(INFO) << "Failed to find setup.exe";

[grt (UTC plus 2), 2012/09/13 13:14:43]

INFO -> ERROR?

[huangs, 2012/09/13 18:46:00]

Done.

+    return false;
+  }
+  CommandLine cmd_line(setup_exe);
+  cmd_line.AppendSwitch(kMultiInstall);
+  cmd_line.AppendSwitch(kChromeAppHost);
+  // TODO: Make --verbose-logging depend on setup arguments for the

[grt (UTC plus 2), 2012/09/13 13:14:43]

i pose that it's better to unconditionally add --verbose-logging for all of
eternity.

[huangs, 2012/09/13 18:46:00]

Removing comment.

+  // system-level Chrome Binaries.
+  cmd_line.AppendSwitch(kVerboseLogging);
+  LOG(INFO) << "Upgrading App Host using command line: " <<

[grt (UTC plus 2), 2012/09/13 13:14:43]

suggest: "Launching: "

[huangs, 2012/09/13 18:46:00]

Done.

+      cmd_line.GetCommandLineString();
+  return base::LaunchProcess(cmd_line, base::LaunchOptions(), NULL);
+}
+
+}  // namespace app_host