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

chrome/browser/extensions/app_host_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/app_host_installer.h"
+#include "chrome/common/extensions/extension.h"
+
+#include "base/callback.h"
+
+#if defined(OS_WIN)
+#include "chrome/browser/extensions/app_host_installer_impl_win.h"
+#endif
+
+namespace extensions {
+
+namespace app_host_installer {
+
+void InstallAppHostIfNecessary(
+    const Extension& extension,
+    const OnAppHostInstallationCompleteCallback& completion_callback) {
+#if defined(OS_WIN)

[benwells, 2012/11/01 23:11:46]

I think this platform specific pattern is a little differnt to the pattern
typically used.

Can you do something like:
- in app_host_installer.cc only have the #if !defined(OS_WIN) case
- in app_host_installer_win.cc have the complete #if defined(OS_WIN) case (note,
no _impl in the file name).

Then you can also move Impl class as it is into an anonymous namespace in the
_win file.

[erikwright (departed), 2012/11/02 01:07:26]

By default, I think x_win.cc will only be built on Windows, but  x.cc will still
be built on all platforms. So we will need to exclude it using .gyp rules on
other platforms (or #ifdef out the implementation in the x.cc file for Windows).

I don't really mind doing that, but it's not exactly the pattern used elsewhere,
either (usually, there is a version for each platform, plus an optional
cross-platform file that is compiled everywhere).

I made an alternate suggestion in app_shortcut_manager.cc . PTAL in your Friday
and if it suits you we'll do that. Otherwise, I prefer #ifdef option above (x.cc
will be #ifdef'd out on Windows, but not excluded in .gyp).

[benwells, 2012/11/02 01:29:25]

Yes, x_win.cc is only built on Windows. You do not need to exclude it using .gyp
rules, just the implementation of the function using #if.

The suggestion is your 'otherwise, i prefer...' option but without the separate
exposed impl class, and with a _win.cc instead of _impl_win.cc. This matches
what is done elsewhere e.g. base\file_util.h, base\process_util.h.

But, I don't mind too much. As we only have one call site now your other
suggestion is good too. If you think there will ever be
InstallAppHostIfNecessary on linux and mac, we should do it like this, otherwise
an #if in the shortcut manager is good.

[huangs, 2012/11/02 18:39:01]

Done.  So:
-- app_host_installer.cc
-- app_host_installer.h
-- app_host_installer_impl_win.cc
-- app_host_installer_impl_win.h
++ app_host_installer_win.cc
++ app_host_installer_win.h

- app_host_installer_win.h is included iff OS_WIN.
- Renamed AppHostInstallerImpl to AppHostInstaller.
- class AppHostsInstaller exposed.
- Main entry is AppHostInstaller::EnsureAppHostInstalled().

+  if (extension.is_platform_app())
+    AppHostInstallerImpl::EnsureAppHostInstalled(completion_callback);
+  else
+    completion_callback.Run(true);
+#else
+  completion_callback.Run(true);
+#endif
+}
+
+}  // namespace app_host_installer
+
+}  // namespace extensions