[mac] Better handling of lost Apple Events during App Shim Startup.

AESendMessage accepts a timeout argument, but it is effectively ignored
by Mac. For asynchronous Apple Events (kAEQueueReply), kAEWantReceipt
must also be set to receive a message on timeout. However,
kAEWantReceipt is "Deprecated and unsupported in Mac OS X".

Also, the "ping" apple event allows an app shim to ensure it doesn't try
to  connect to Chrome until it has finished starting up, when it is the
shim  that has just started Chrome. However, if Chrome is shutting down,
the apple event is lost and the shim process must wait for a long
timeout.

Instead, if the shim itself is not launching Chrome, this CL proceeds 
straight to init. This will either succeed quickly, fail to connect to 
the shim socket, or connect and get disconnected quickly. Both "fail" 
cases are preferable to a timeout. 

For the Chrome-not-running case, we still can not leave zombie processes
around that never get their Apple Event reply. For this case, emulate
the broken Apple Event timeout by posting a DelayedTask to the UI
MessageLoop in the app shim process.

BUG=318013
TEST=With Chrome not running, Start App Launcher via dock icon then
(really quickly) launch the app launcher again. Chrome must still be
shutting down, and the Apple Event is lost. After 1 minute the App
Launcher should work again.

Review URL: https://codereview.chromium.org/68523004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@234494 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 54 insertions, 25 deletions

diff --git a/apps/app_shim/chrome_main_app_mode_mac.mm b/apps/app_shim/chrome_main_app_mode_mac.mm
index 18917f8..2c13ba23 100644
--- a/apps/app_shim/chrome_main_app_mode_mac.mm
+++ b/apps/app_shim/chrome_main_app_mode_mac.mm
@@ -50,6 +50,11 @@
 
 namespace {
 
+// Timeout in seconds to wait for a reply for the initial Apple Event. Note that
+// kAEDefaultTimeout on Mac is "about one minute" according to Apple's
+// documentation, but is no longer supported for asynchronous Apple Events.
+const int kPingChromeTimeoutSeconds = 60;
+
 const app_mode::ChromeAppModeInfo* g_info;
 base::Thread* g_io_thread = NULL;
 
@@ -97,6 +102,10 @@ class AppShimController : public IPC::Listener {
   // was sent, or when the ping fails (if |success| is false).
   void OnPingChromeReply(bool success);
 
+  // Called |kPingChromeTimeoutSeconds| after startup, to allow a timeout on the
+  // ping event to be detected.
+  void OnPingChromeTimeout();
+
   // Connects to Chrome and sends a LaunchApp message.
   void Init();
 
@@ -134,6 +143,7 @@ class AppShimController : public IPC::Listener {
   IPC::ChannelProxy* channel_;
   base::scoped_nsobject<AppShimDelegate> delegate_;
   bool launch_app_done_;
+  bool ping_chrome_reply_received_;
 
   DISALLOW_COPY_AND_ASSIGN(AppShimController);
 };
@@ -141,7 +151,8 @@ class AppShimController : public IPC::Listener {
 AppShimController::AppShimController()
     : channel_(NULL),
       delegate_([[AppShimDelegate alloc] init]),
-      launch_app_done_(false) {
+      launch_app_done_(false),
+      ping_chrome_reply_received_(false) {
   // Since AppShimController is created before the main message loop starts,
   // NSApp will not be set, so use sharedApplication.
   [[NSApplication sharedApplication] setDelegate:delegate_];
@@ -153,6 +164,7 @@ AppShimController::~AppShimController() {
 }
 
 void AppShimController::OnPingChromeReply(bool success) {
+  ping_chrome_reply_received_ = true;
   if (!success) {
     [NSApp terminate:nil];
     return;
@@ -161,6 +173,11 @@ void AppShimController::OnPingChromeReply(bool success) {
   Init();
 }
 
+void AppShimController::OnPingChromeTimeout() {
+  if (!ping_chrome_reply_received_)
+    [NSApp terminate:nil];
+}
+
 void AppShimController::Init() {
   DCHECK(g_io_thread);
 
@@ -476,9 +493,10 @@ void AppShimController::SendSetAppHidden(bool hidden) {
                   targetDescriptor:target
                           returnID:kAutoGenerateReturnID
                      transactionID:kAnyTransactionID];
-  // And away we go.
-  // TODO(jeremya): if we don't care about the contents of the reply, can we
-  // pass NULL for the reply event parameter?
+
+  // Note that AESendMessage effectively ignores kAEDefaultTimeout, because this
+  // call does not pass kAEWantReceipt (which is deprecated and unsupported on
+  // Mac). Instead, rely on OnPingChromeTimeout().
   OSStatus status = AESendMessage(
       [initial_event aeDesc], &replyEvent_, kAEQueueReply, kAEDefaultTimeout);
   if (status != noErr) {
@@ -577,14 +595,14 @@ int ChromeAppModeStart(const app_mode::ChromeAppModeInfo* info) {
       pid = [[existing_chrome objectAtIndex:0] processIdentifier];
   }
 
-  // Launch Chrome if it isn't already running.
-  ProcessSerialNumber psn;
-  if (pid > -1) {
-    OSStatus status = GetProcessForPID(pid, &psn);
-    if (status)
-      return 1;
+  AppShimController controller;
+  base::MessageLoopForUI main_message_loop;
+  main_message_loop.set_thread_name("MainThread");
+  base::PlatformThread::SetName("CrAppShimMain");
 
-  } else {
+  if (pid == -1) {
+    // Launch Chrome if it isn't already running.
+    ProcessSerialNumber psn;
     CommandLine command_line(CommandLine::NO_PROGRAM);
     command_line.AppendSwitch(switches::kSilentLaunch);
     command_line.AppendSwitchPath(switches::kProfileDirectory,
@@ -596,22 +614,33 @@ int ChromeAppModeStart(const app_mode::ChromeAppModeInfo* info) {
                                            &psn);
     if (!success)
       return 1;
-  }
-
-  AppShimController controller;
-  base::Callback<void(bool)> on_ping_chrome_reply =
-      base::Bind(&AppShimController::OnPingChromeReply,
-                 base::Unretained(&controller));
 
-  // This code abuses the fact that Apple Events sent before the process is
-  // fully initialized don't receive a reply until its run loop starts. Once
-  // the reply is received, Chrome will have opened its IPC port, guaranteed.
-  [ReplyEventHandler pingProcess:psn
-                         andCall:on_ping_chrome_reply];
+    base::Callback<void(bool)> on_ping_chrome_reply =
+        base::Bind(&AppShimController::OnPingChromeReply,
+                   base::Unretained(&controller));
+
+    // This code abuses the fact that Apple Events sent before the process is
+    // fully initialized don't receive a reply until its run loop starts. Once
+    // the reply is received, Chrome will have opened its IPC port, guaranteed.
+    [ReplyEventHandler pingProcess:psn
+                           andCall:on_ping_chrome_reply];
+
+    main_message_loop.PostDelayedTask(
+        FROM_HERE,
+        base::Bind(&AppShimController::OnPingChromeTimeout,
+                   base::Unretained(&controller)),
+        base::TimeDelta::FromSeconds(kPingChromeTimeoutSeconds));
+  } else {
+    // Chrome already running. Proceed to init. This could still fail if Chrome
+    // is still starting up or shutting down, but the process will exit quickly,
+    // which is preferable to waiting for the Apple Event to timeout after one
+    // minute.
+    main_message_loop.PostTask(
+        FROM_HERE,
+        base::Bind(&AppShimController::Init,
+                   base::Unretained(&controller)));
+  }
 
-  base::MessageLoopForUI main_message_loop;
-  main_message_loop.set_thread_name("MainThread");
-  base::PlatformThread::SetName("CrAppShimMain");
   main_message_loop.Run();
   return 0;
 }