Make application shortcuts point to app_host.exe, install App Host during app installation.

chrome/browser/extensions/crx_installer.cc

Index: chrome/browser/extensions/crx_installer.cc
diff --git a/chrome/browser/extensions/crx_installer.cc b/chrome/browser/extensions/crx_installer.cc
index e51b225e73cc7387a1d99a46ae2642909fd82dcb..ee9ced1e2a92e7ea94ea9d66c4d8f1b9379e8b7b 100644
--- a/chrome/browser/extensions/crx_installer.cc
+++ b/chrome/browser/extensions/crx_installer.cc
@@ -21,6 +21,7 @@
 #include "base/utf_string_conversions.h"
 #include "base/version.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/extensions/app_host_installer.h"
 #include "chrome/browser/extensions/convert_user_script.h"
 #include "chrome/browser/extensions/convert_web_app.h"
 #include "chrome/browser/extensions/default_apps_trial.h"
@@ -457,7 +458,7 @@ void CrxInstaller::ConfirmInstall() {
   } else {
     if (!BrowserThread::PostTask(
             BrowserThread::FILE, FROM_HERE,
-            base::Bind(&CrxInstaller::CompleteInstall, this)))
+            base::Bind(&CrxInstaller::BeginInstall, this)))
       NOTREACHED();
   }
   return;
@@ -466,7 +467,7 @@ void CrxInstaller::ConfirmInstall() {
 void CrxInstaller::InstallUIProceed() {
   if (!BrowserThread::PostTask(
           BrowserThread::FILE, FROM_HERE,
-          base::Bind(&CrxInstaller::CompleteInstall, this)))
+          base::Bind(&CrxInstaller::BeginInstall, this)))
     NOTREACHED();
 
   Release();  // balanced in ConfirmInstall().
@@ -494,7 +495,7 @@ void CrxInstaller::InstallUIAbort(bool user_initiated) {
   // should go to zero and we die. The destructor will clean up the temp dir.
 }
 
-void CrxInstaller::CompleteInstall() {
+void CrxInstaller::BeginInstall() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   if (!current_version_.empty()) {
@@ -508,6 +509,23 @@ void CrxInstaller::CompleteInstall() {
       return;
     }
   }
+  app_host_installer_.InstallAppHostIfNecessary(*extension_,

[benwells, 2012/10/08 06:14:33]

Always going through this function and its callbacks obfuscates the flow of
installation for all extensions - most of which don't need the app host. This is
the main reason I suggested having the IsAppHostRequired function (the other
reason is that something similar will be needed for the app launcher).

Putting something back like: 
  if (something) {
    do this stuff with callbacks
    return;
  }

  CompleteInstall();

will make the flow nice and simple again for extensions which don't need the app
host.

"something" could be calling into the app_host_installation code, or maybe even
better could just call a new function extension->RequiresAppHost(), keeping the
app host installation code completely decoupled from extensions.

[erikwright (departed), 2012/10/09 15:54:53]

Part of the checking has to occur on the FILE thread.

It's true that for non-Windows platforms we don't need to go to the FILE thread,
but I think it is more appropriate to encapsulate this detail in the AppHost
specific code, rather than requiring each of the clients to be aware of it.

The AppHost code Does The Right Thing, which is to say it doesn't bounce across
threads unnecessarily on unsupported platforms.

[huangs, 2012/10/09 18:25:37]

Some minor rearrangement for style; otherwise not changing the interface.

[benwells, 2012/10/10 03:22:04]

I should have been clearer. The "something" would only be part of the check, the
is_platform_app part. But having that scattered everywhere would be yuck so it
would be better instead encapsulated on extension.

Again, the objective is to keep the install flow simpler for the majority of
extensions that are not platform apps.

+      base::Bind(&CrxInstaller::OnAppHostInstallationComplete, this));
+}
+
+void CrxInstaller::OnAppHostInstallationComplete(bool success) {
+  if (!success) {
+    ReportFailureFromFileThread(
+        CrxInstallerError(
+            l10n_util::GetStringUTF16(
+                IDS_EXTENSION_APP_HOST_INSTALL_ERROR)));
+    return;
+  }

[benwells, 2012/10/08 06:14:33]

Blank line here after early return.

[huangs, 2012/10/09 18:25:37]

Done.

+  CompleteInstall();
+}
+
+void CrxInstaller::CompleteInstall() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
   // See how long extension install paths are.  This is important on
   // windows, because file operations may fail if the path to a file