Prepare chrome_main_app_mode_mac.mm for 10.9

chrome/app_shim/chrome_main_app_mode_mac.mm

 // Copyright 2013 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.
 
 // On Mac, one can't make shortcuts with command-line arguments. Instead, we
 // produce small app bundles which locate the Chromium framework and load it,
 // passing the appropriate data. This is the entry point into the framework for
 // those app bundles.
 
 #import <Cocoa/Cocoa.h>
@@ skipping 462 matching lines @@
 // A ReplyEventHandler is a helper class to send an Apple Event to a process
 // and call a callback when the reply returns.
 //
 // This is used to 'ping' the main Chrome process -- once Chrome has sent back
 // an Apple Event reply, it's guaranteed that it has opened the IPC channel
 // that the app shim will connect to.
 @interface ReplyEventHandler : NSObject {
   base::Callback<void(bool)> onReply_;
   AEDesc replyEvent_;
 }
-// Sends an Apple Event to the process identified by |psn|, and calls |replyFn|
-// when the reply is received. Internally this creates a ReplyEventHandler,
-// which will delete itself once the reply event has been received.
-+ (void)pingProcess:(const ProcessSerialNumber&)psn
-            andCall:(base::Callback<void(bool)>)replyFn;
+
+// Sends an Apple Event to the process identified by the outer bundle
+// identifier, and calls |replyFn| when the reply is received. Internally this
+// creates a ReplyEventHandler, which will delete itself once the reply event
+// has been received.
++ (void)pingProcessAndCall:(base::Callback<void(bool)>)replyFn;
+
 @end
 
 @interface ReplyEventHandler (PrivateMethods)
 // Initialise the reply event handler. Doesn't register any handlers until
 // |-pingProcess:| is called. |replyFn| is the function to be called when the
 // Apple Event reply arrives.
 - (id)initWithCallback:(base::Callback<void(bool)>)replyFn;
 
-// Sends an Apple Event ping to the process identified by |psn| and registers
-// to listen for a reply.
-- (void)pingProcess:(const ProcessSerialNumber&)psn;
+// Returns the apple event that should be sent to the process.
+- (NSAppleEventDescriptor*)appleEventToSendToProcess;
+
+// Sends an Apple Event ping to the process identified by the bundle
+// identifier and registers to listen for a reply.
+- (void)pingProcess;
 
 // Called when a response is received from the target process for the ping sent
 // by |-pingProcess:|.
 - (void)message:(NSAppleEventDescriptor*)event
       withReply:(NSAppleEventDescriptor*)reply;
 
 // Calls |onReply_|, passing it |success| to specify whether the ping was
 // successful.
 - (void)closeWithSuccess:(bool)success;
+
 @end
 
 @implementation ReplyEventHandler
-+ (void)pingProcess:(const ProcessSerialNumber&)psn
-            andCall:(base::Callback<void(bool)>)replyFn {
+
++ (void)pingProcessAndCall:(base::Callback<void(bool)>)replyFn {
   // The object will release itself when the reply arrives, or possibly earlier
   // if an unrecoverable error occurs.
   ReplyEventHandler* handler =
       [[ReplyEventHandler alloc] initWithCallback:replyFn];
-  [handler pingProcess:psn];
+  [handler pingProcess];
 }
+
 @end
 
 @implementation ReplyEventHandler (PrivateMethods)
 - (id)initWithCallback:(base::Callback<void(bool)>)replyFn {
   if ((self = [super init])) {
     onReply_ = replyFn;
   }
   return self;
 }
 
-- (void)pingProcess:(const ProcessSerialNumber&)psn {
+- (void)pingProcess {
   // Register the reply listener.
   NSAppleEventManager* em = [NSAppleEventManager sharedAppleEventManager];
   [em setEventHandler:self
           andSelector:@selector(message:withReply:)
         forEventClass:'aevt'
            andEventID:'ansr'];
+
   // Craft the Apple Event to send.
+  NSString* chrome_bundle_id = [base::mac::OuterBundle() bundleIdentifier];

[tapted, 2016/10/26 00:14:48]

nit: I guess this should be `chromeBundleId ` since we're in @implementation
(but initial_event below should be changed too). The file looks consistent apart
from that.

[spqchan, 2016/10/26 00:45:43]

Done.

   NSAppleEventDescriptor* target = [NSAppleEventDescriptor
-      descriptorWithDescriptorType:typeProcessSerialNumber
-                             bytes:&psn
-                            length:sizeof(psn)];
+      descriptorWithDescriptorType:typeApplicationBundleID
+                              data:[chrome_bundle_id
+                                       dataUsingEncoding:NSUTF8StringEncoding]];
+
   NSAppleEventDescriptor* initial_event =
       [NSAppleEventDescriptor
           appleEventWithEventClass:app_mode::kAEChromeAppClass
                            eventID:app_mode::kAEChromeAppPing
                   targetDescriptor:target
                           returnID:kAutoGenerateReturnID
                      transactionID:kAnyTransactionID];
 
   // 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) {
     OSSTATUS_LOG(ERROR, status) << "AESendMessage";
     [self closeWithSuccess:false];
   }
 }
 
+- (NSAppleEventDescriptor*)appleEventToSendToProcess {
+  // Register the reply listener.
+  NSAppleEventManager* em = [NSAppleEventManager sharedAppleEventManager];
+  [em setEventHandler:self
+          andSelector:@selector(message:withReply:)
+        forEventClass:'aevt'

[tapted, 2016/10/26 00:14:48]

kCoreEventClass ? 

[spqchan, 2016/10/26 00:45:43]

Done.

+           andEventID:'ansr'];

[tapted, 2016/10/26 00:14:48]

kAEAnswer

I guess we should update the literals in closeWithSuccess as well :/

[spqchan, 2016/10/26 00:45:43]

Done.

+  NSAppleEventDescriptor* initial_event = [NSAppleEventDescriptor
+      appleEventWithEventClass:app_mode::kAEChromeAppClass
+                       eventID:app_mode::kAEChromeAppPing
+              targetDescriptor:nil
+                      returnID:kAutoGenerateReturnID
+                 transactionID:kAnyTransactionID];
+  return initial_event;
+}
+
 - (void)message:(NSAppleEventDescriptor*)event
       withReply:(NSAppleEventDescriptor*)reply {
   [self closeWithSuccess:true];
 }
 
 - (void)closeWithSuccess:(bool)success {
   onReply_.Run(success);
   NSAppleEventManager* em = [NSAppleEventManager sharedAppleEventManager];
   [em removeEventHandlerForEventClass:'aevt' andEventID:'ansr'];
   [self release];
 }
+
 @end
 
 //-----------------------------------------------------------------------------
 
 extern "C" {
 
 // |ChromeAppModeStart()| is the point of entry into the framework from the app
 // mode loader. There are cases where the Chromium framework may have changed in
 // a way that is incompatible with an older shim (e.g. change to libc++ library
 // linking). The function name is versioned to provide a way to force shim
@@ skipping 80 matching lines @@
   AppShimController controller;
   base::MessageLoopForUI main_message_loop;
   base::PlatformThread::SetName("CrAppShimMain");
 
   // In tests, launching Chrome does nothing, and we won't get a ping response,
   // so just assume the socket exists.
   if (pid == -1 &&
       !base::CommandLine::ForCurrentProcess()->HasSwitch(
           app_mode::kLaunchedForTest)) {
     // Launch Chrome if it isn't already running.
-    ProcessSerialNumber psn;
     base::CommandLine command_line(base::CommandLine::NO_PROGRAM);
     command_line.AppendSwitch(switches::kSilentLaunch);
 
     // If the shim is the app launcher, pass --show-app-list when starting a new
     // Chrome process to inform startup codepaths and load the correct profile.
     if (info->app_mode_id == app_mode::kAppListModeId) {
       command_line.AppendSwitch(switches::kShowAppList);
     } else {
       command_line.AppendSwitchPath(switches::kProfileDirectory,
                                     info->profile_dir);
     }
 
+    base::Callback<void(bool)> on_ping_chrome_reply = base::Bind(
+        &AppShimController::OnPingChromeReply, base::Unretained(&controller));
+
+    // Self-owned object that will delete itself.
+    ReplyEventHandler* handler =
+        [[ReplyEventHandler alloc] initWithCallback:on_ping_chrome_reply];
+    NSAppleEventDescriptor* descriptor = [handler appleEventToSendToProcess];
+
     base::Process app = base::mac::OpenApplicationWithPath(
-        base::mac::OuterBundlePath(), command_line, NSWorkspaceLaunchDefault);
-
-    // TODO(crbug.com/652563): Do not use deprecated GetProcessForPID. Change
-    // |ReplyEventHandler| to take |pid_t| instead of |ProcessSerialNumber|.
-    if (!app.IsValid() || GetProcessForPID(app.Pid(), &psn) != noErr)
+        base::mac::OuterBundlePath(), command_line, NSWorkspaceLaunchDefault,
+        descriptor);
+    if (!app.IsValid()) {
+      [handler closeWithSuccess:false];
       return 1;
-
-    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];
+    [ReplyEventHandler pingProcessAndCall:on_ping_chrome_reply];

[tapted, 2016/10/26 00:14:48]

Can this line (and the two methods pingProcessAndCall / pingProcess) simply be
removed now? (that was my understanding of how
NSWorkspaceLaunchConfigurationAppleEvent should work..)

[spqchan, 2016/10/26 00:45:43]

I removed it at first, but this runs into a problem where the app shim fails if
you quit Chromium and then run it.

 
     main_message_loop.task_runner()->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.task_runner()->PostTask(
         FROM_HERE,
         base::Bind(&AppShimController::Init, base::Unretained(&controller)));
   }
 
   base::RunLoop().Run();
   return 0;
 }