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

chrome/browser/extensions/app_host_installer_win.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_win.h"
+
+#include <windows.h>

[benwells, 2012/10/03 05:08:43]

Some of these includes are in the .h

[huangs, 2012/10/03 20:09:47]

I thought a .cc file should not assume transitive inclusion?  Please confirm,
thanks!

+#include "base/bind.h"
+#include "base/win/object_watcher.h"
+#include "chrome/installer/launcher_support/chrome_launcher_support.h"
+#include "content/public/browser/browser_thread.h"
+
+namespace extensions {
+
+namespace {

[benwells, 2012/10/03 05:08:43]

Keep the anonymous namespace out of the extensions namespace.

[huangs, 2012/10/03 20:09:47]

Done.

+
+class QuickEnableWatcher : public base::win::ObjectWatcher::Delegate {
+ public:
+  QuickEnableWatcher(bool* running, const base::Closure& on_success,
+                     const base::Closure& on_failure)
+      : running_(running),
+        on_success_(on_success),
+        on_failure_(on_failure) {
+  }
+
+  // base::win::ObjectWatcher::Delegate implementation.
+  void OnObjectSignaled(HANDLE object) {
+    DWORD exit_code;
+    ::GetExitCodeProcess(object, &exit_code);
+    if (exit_code == 0)
+      on_success_.Run();
+    else
+      on_failure_.Run();

[erikwright (departed), 2012/10/03 15:58:35]

Use process_util::GetTerminationStatus instead of GetExitCodeProcess and be sure
to check the TerminationStatus.

Only TERMINATION_STATUS_NORMAL_TERMINATION should call success handler. All
others call the failure handler and LOG(ERROR) with the TerminationStatus and
the exit code (if appropriate).

[huangs, 2012/10/03 20:09:47]

Done.  I think TerminationStatus is sufficient; we don't need the exit_code.

+    on_success_.Reset();
+    on_failure_.Reset();
+    *running_ = false;
+  }
+
+ private:
+  bool* running_;
+  base::Closure on_success_;
+  base::Closure on_failure_;
+
+  DISALLOW_COPY_AND_ASSIGN(QuickEnableWatcher);
+};
+}  // namespace

[benwells, 2012/10/03 05:08:43]

Nit: Blank line before end of namespace.

[huangs, 2012/10/03 20:09:47]

Done.

+
+AppHostInstaller::AppHostInstaller()
+    : running_(false), process_(INVALID_HANDLE_VALUE), delegate_(NULL) {

[erikwright (departed), 2012/10/03 15:58:35]

process_ and delegate_ are objects whose default constructors do the right thing
in this case. No need to initialize them.

[huangs, 2012/10/03 20:09:47]

Done.

+}
+
+void AppHostInstaller::EnsureAppHostPresentAndCall(
+    const base::Closure& on_success,
+    const base::Closure& on_failure) {
+  using content::BrowserThread;
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  DCHECK(!running_);

[benwells, 2012/10/03 05:08:43]

Is running_ just for this DCHECK?

[erikwright (departed), 2012/10/03 15:58:35]

It could have been used to allow the AppHostInstaller instance to be re-used,
but I don't think this is worthwhile.

I would just DCHECK(!delegate_.get()) before line 69.

Then make sure the class comments indicate that each instance should only be
used once.

[huangs, 2012/10/03 20:09:47]

I'm worried about the case where user installs app A, and before
quick-enable-app-host runs, installs app B again!  Possible outcomes:

- There is already built-in mechanism that only allows one app install at a
time?  (Don't know).
- We will have block this explicitly.
- Installer is smart enough to handle simultaneous installs?  (Don't know).

We'll need to revisit this later.

[huangs, 2012/10/03 20:09:47]

Done.

+
+  // If installed.
+  if (chrome_launcher_support::IsAppHostPresent()) {
+    on_success.Run();
+  } else {
+    DCHECK(!process_.IsValid());
+    if (FAILED(chrome_launcher_support::LaunchQuickEnableAppHost(&process_))) {
+      on_failure.Run();
+    } else {
+      DCHECK(process_.IsValid());
+      running_ = true;
+      delegate_.reset(
+          new QuickEnableWatcher(&running_, on_success, on_failure));
+      watcher_.StartWatching(process_, delegate_.get());
+    }
+  }
+}
+
+}  // namespace extensions