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

chrome/browser/extensions/crx_installer.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/crx_installer.h"
 
 #include <map>
 #include <set>
 
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/lazy_instance.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/path_service.h"
 #include "base/scoped_temp_dir.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
 #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"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_install_ui.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_system.h"
 #include "chrome/browser/extensions/permissions_updater.h"
 #include "chrome/browser/extensions/requirements_checker.h"
 #include "chrome/browser/extensions/webstore_installer.h"
@@ skipping 416 matching lines @@
 
   current_version_ =
       frontend_weak_->extension_prefs()->GetVersionString(extension_->id());
 
   if (client_ && (!allow_silent_install_ || !approved_)) {
     AddRef();  // Balanced in Proceed() and Abort().
     client_->ConfirmInstall(this, extension_.get());
   } else {
     if (!BrowserThread::PostTask(
             BrowserThread::FILE, FROM_HERE,
-            base::Bind(&CrxInstaller::CompleteInstall, this)))
+            base::Bind(&CrxInstaller::BeginInstall, this)))
       NOTREACHED();
   }
   return;
 }
 
 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().
 }
 
 void CrxInstaller::InstallUIAbort(bool user_initiated) {
   std::string histogram_name = user_initiated ?
       "Extensions.Permissions_InstallCancel" :
       "Extensions.Permissions_InstallAbort";
   ExtensionService::RecordPermissionMessagesHistogram(
       extension_, histogram_name.c_str());
 
   // Kill the theme loading bubble.
   content::NotificationService* service =
       content::NotificationService::current();
   service->Notify(chrome::NOTIFICATION_NO_THEME_DETECTED,
                   content::Source<CrxInstaller>(this),
                   content::NotificationService::NoDetails());
 
   NotifyCrxInstallComplete(NULL);
 
   Release();  // balanced in ConfirmInstall().
 
   // We're done. Since we don't post any more tasks to ourself, our ref count
   // 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()) {
     Version current_version(current_version_);
     if (current_version.CompareTo(*(extension_->version())) > 0) {
       ReportFailureFromFileThread(
           CrxInstallerError(
               l10n_util::GetStringUTF16(extension_->is_app() ?
                   IDS_APP_CANT_DOWNGRADE_VERSION :
                   IDS_EXTENSION_CANT_DOWNGRADE_VERSION)));
       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
   // exceeds a small constant.  See crbug.com/69693 .
   UMA_HISTOGRAM_CUSTOM_COUNTS(
     "Extensions.CrxInstallDirPathLength",
         install_directory_.value().length(), 0, 500, 100);
 
   FilePath version_dir = extension_file_util::InstallExtension(
       unpacked_extension_root_,
@@ skipping 129 matching lines @@
   // is problematic because they don't know anything about the
   // extension before it is unpacked, so they cannot filter based
   // on the extension.
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_CRX_INSTALLER_DONE,
       content::Source<CrxInstaller>(this),
       content::Details<const Extension>(extension));
 }
 
 }  // namespace extensions