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

chrome/browser/extensions/app_host/app_host_main.cc

Index: chrome/browser/extensions/app_host/app_host_main.cc
diff --git a/chrome/browser/extensions/app_host/app_host_main.cc b/chrome/browser/extensions/app_host/app_host_main.cc
index 10d044a57cbf93c2e524ceead7b669a8763e726d..1681a8f5f20e2bb27aae4f72c838b3c9dfc5ddb9 100644
--- a/chrome/browser/extensions/app_host/app_host_main.cc
+++ b/chrome/browser/extensions/app_host/app_host_main.cc
@@ -7,16 +7,20 @@
 #include "base/file_path.h"
 #include "base/logging.h"
 #include "base/process_util.h"
+#include "base/version.h"
+#include "chrome/browser/extensions/app_host/app_host_upgrade.h"
 #include "chrome/browser/extensions/app_host/binaries_installer.h"
 #include "chrome/installer/launcher_support/chrome_launcher_support.h"
 
 int main(int /* argc */, char* /* argv[] */) {
+  using namespace app_host;
+  using namespace chrome_launcher_support;
   base::AtExitManager exit_manager;
 
   // Initialize the commandline singleton from the environment.
   CommandLine::Init(0, NULL);
 
-  FilePath chrome_exe(chrome_launcher_support::GetAnyChromePath());
+  FilePath chrome_exe(GetAnyChromePath());
 
   if (chrome_exe.empty()) {
     LOG(INFO) << "No Chrome executable could be found. Let's install it.";
@@ -25,7 +29,7 @@ int main(int /* argc */, char* /* argv[] */) {
       LOG(ERROR) << "Failed to install the Chrome Binaries. Error: " << hr;
       return 1;
     } else {
-      chrome_exe = chrome_launcher_support::GetAnyChromePath();
+      chrome_exe = GetAnyChromePath();
       if (chrome_exe.empty()) {
         LOG(ERROR) << "Failed to find the Chrome Binaries despite a "
                    << "'successful' installation.";
@@ -38,13 +42,39 @@ int main(int /* argc */, char* /* argv[] */) {
   chrome_exe_command_line.AppendArguments(
       *CommandLine::ForCurrentProcess(), false);
 
-  if (base::LaunchProcess(chrome_exe_command_line,
-                          base::LaunchOptions(),
-                          NULL)) {
+  // Launch Chrome before checking for updates, for faster user experience.
+  bool launch_result = base::LaunchProcess(chrome_exe_command_line,
+                                           base::LaunchOptions(),
+                                           NULL);
+  if (launch_result) {

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

no braces for single-line conditionals like this

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

Done.

     LOG(INFO) << "Delegated to Chrome executable at " << chrome_exe.value();
-    return 0;
   } else {
     LOG(INFO) << "Failed to launch Chrome executable at " << chrome_exe.value();
-    return 1;
   }
+
+  // If system-level Chrome Binary is installed, and if its version is

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

i think this block of code is big enough and isolated enough that it would fit
nicely in its own function.

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

Okay, moving into app_host_upgrade.cc.  Also hiding constants and function that
don't need to be exposed any more (per discussion).

+  // newer than App Host's, then upgrade App Host by calling setup.exe.

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

please change "upgrade" in all cases to "update" .the latter implies that the
new version is better in some way, which is subjective) also, "update" is
consistent with Google Update and as much of the codebase as i've been able to
influence.

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

By "the latter" grt@ meant "the former".

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

Makes sense.  Done!

+  Version chrome_binaries_version;
+  if (GetAppVersionForInstallationLevel(kMultiInstallAppGuid,
+                                        true,  // System-level.
+                                        &chrome_binaries_version)) {
+    Version app_host_version;
+    if (GetAppVersionForInstallationLevel(kChromeAppHostAppGuid,

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

if this is used to get the version of the currently running app host exe, a much
more reliable way is to introspect into the version resource of the running
process.  our use of the registry for this has historically been problematic. 
search for CreateFileVersionInfoForCurrentModule in client_util.cc for an
example of this.

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

Done.

+                                          false,  // User-level.
+                                          &app_host_version)) {
+      if (app_host_version.IsOlderThan(chrome_binaries_version.GetString())) {
+        LOG(INFO) << "App Host version (" << app_host_version.GetString() <<

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

wrap use of the streaming operator by putting the "<<" down on the next line and
align it with the one on this line.

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

Correct. Note this is an exception to my previous comment about preferring to
wrap after operators.

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

Done.

+          ") is older than Chrome Binaries version (" <<
+          chrome_binaries_version.GetString() <<
+          "), so running setup.exe to upgrade";

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

general comment: make the log message as concise as possible since it
contributes to the size of the binary and download.  i suggest something simple
and clear like: "Updating App Host from foo to bar".

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

Done.

+        if (!UpgradeAppHost(true)) {  // Use system-level setup.exe

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

no braces

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

Done.

+          LOG(ERROR) << "Failed to upgrade App Host";
+        }
+      }
+    } else {
+      LOG(INFO) << "Failed to get App Host version";

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

why isn't this an ERROR?

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

Done.

+    }
+  }  // Else there is no system-level Chrome, so do nothing.
+
+  return launch_result ? 0 : 1;

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

optional:
return !launch_result; is equivalent and tighter (integral promotion dictates
that a bool is promoted to 0 or 1).

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

Done.

 }