[mac] App shims

chrome/app/chrome_main_app_mode_mac.mm

Index: chrome/app/chrome_main_app_mode_mac.mm
diff --git a/chrome/app/chrome_main_app_mode_mac.mm b/chrome/app/chrome_main_app_mode_mac.mm
index d102fc8f591e667bc0b2e00a5d67391d30f41729..1c0d2afdd40249d33858fd675c24f3000dfee2ee 100644
--- a/chrome/app/chrome_main_app_mode_mac.mm
+++ b/chrome/app/chrome_main_app_mode_mac.mm
@@ -7,20 +7,177 @@
 // passing the appropriate data. This is the entry point into the framework for
 // those app bundles.
 
-#include "base/basictypes.h"
-#include "base/files/file_path.h"
+#import <Cocoa/Cocoa.h>
+
+#include "base/at_exit.h"
 #include "base/logging.h"
-#include "base/mac/bundle_locations.h"
-#include "base/mac/foundation_util.h"
+#include "base/threading/thread.h"
 #include "base/mac/mac_logging.h"
 #include "base/mac/mac_util.h"
 #include "base/mac/scoped_nsautorelease_pool.h"
+#include "base/message_loop.h"
+#include "base/path_service.h"
 #include "base/sys_string_conversions.h"
-#include "chrome/browser/shell_integration.h"
-#include "chrome/common/chrome_constants.h"
-#include "chrome/common/chrome_paths_internal.h"
+#include "chrome/common/app_shim_messages.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/mac/app_mode_common.h"
+#include "ipc/ipc_channel_proxy.h"
+#include "ipc/ipc_listener.h"
+#include "ipc/ipc_message.h"
+
+const app_mode::ChromeAppModeInfo* g_info;
+base::Thread* g_io_thread = NULL;

[jeremy, 2013/03/11 06:49:59]

Wrap these in an anon namespace?

[jeremya, 2013/03/11 08:42:26]

Done.

+
+class AppShimController : public IPC::Listener {

[jeremy, 2013/03/11 06:49:59]

Class level comment + method comments.

[jeremya, 2013/03/11 08:42:26]

Done.

+ public:
+  AppShimController();
+
+  void Init();
+
+ private:
+  virtual bool OnMessageReceived(const IPC::Message& message);

[jeremy, 2013/03/11 06:49:59]

Add an
// IPC::Listener implementation.

comment

[jeremya, 2013/03/11 08:42:26]

Done.

+  virtual void OnChannelError();
+
+  void OnLaunchAppDone(bool success);
+  void OnDidActivateApplicationNotification(NSNotification* notification);

[jeremy, 2013/03/11 06:49:59]

Where is this implemented?

[jeremya, 2013/03/11 08:42:26]

Ghost of a future patch. Removed.

+
+  IPC::ChannelProxy* channel_;
+
+  DISALLOW_COPY_AND_ASSIGN(AppShimController);
+};
+
+AppShimController::AppShimController() {

[jeremy, 2013/03/11 06:49:59]

channel_(NULL)

[jeremya, 2013/03/11 08:42:26]

Done.

+}
+
+void AppShimController::Init() {
+  base::FilePath user_data_dir;
+  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);

[jeremy, 2013/03/11 06:49:59]

What do you do if this fails?

[jeremya, 2013/03/11 08:42:26]

Let's just die. Didn't realise it could fail.

+  base::FilePath socket_path =
+      user_data_dir.Append(app_mode::kAppShimSocketName);
+  IPC::ChannelHandle handle(socket_path.value());
+  channel_ = new IPC::ChannelProxy(handle, IPC::Channel::MODE_NAMED_CLIENT,
+      this, g_io_thread->message_loop_proxy());

[jeremy, 2013/03/11 06:49:59]

DCHECK(g_io_thread) somewhere ?

[jeremya, 2013/03/11 08:42:26]

Done.

+
+  channel_->Send(new AppShimHostMsg_LaunchApp(
+      g_info->profile_dir.value(), g_info->app_mode_id));
+}
+
+bool AppShimController::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(AppShimController, message)
+    IPC_MESSAGE_HANDLER(AppShimMsg_LaunchApp_Done, OnLaunchAppDone)
+

[jeremy, 2013/03/11 06:49:59]

remove NL

[jeremya, 2013/03/11 08:42:26]

Done.

+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+
+  return handled;
+}
+
+void AppShimController::OnChannelError() {
+  [NSApp terminate:nil];

[jeremy, 2013/03/11 06:49:59]

LOG(ERROR) ?

[jeremya, 2013/03/11 08:42:26]

Done.

+}
+
+void AppShimController::OnLaunchAppDone(bool success) {
+  if (!success)
+    [NSApp terminate:nil];
+}
+
+//-----------------------------------------------------------------------------
+
+@interface ReplyEventHandler : NSObject {

[jeremy, 2013/03/11 06:49:59]

Can you add a header comment explaining why we need this class?

[jeremya, 2013/03/11 08:42:26]

Done.

+  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 be destroyed once the reply event has been received.
++ (void)pingProcess:(const ProcessSerialNumber&)psn
+            andCall:(base::Callback<void(bool)>)replyFn;
+@end
+
+@interface ReplyEventHandler (PrivateMethods)
+- (id)initWithCallback:(base::Callback<void(bool)>)replyFn;

[jeremy, 2013/03/11 06:49:59]

Add high level comments for methods.

[jeremya, 2013/03/11 08:42:26]

Done.

+- (void)pingProcess:(const ProcessSerialNumber&)psn;
+- (void)message:(NSAppleEventDescriptor*)event
+      withReply:(NSAppleEventDescriptor*)reply;
+- (void)closeWithSuccess:(bool)success;
+@end
+
+@implementation ReplyEventHandler
++ (void)pingProcess:(const ProcessSerialNumber&)psn
+            andCall:(base::Callback<void(bool)>)replyFn {
+  ReplyEventHandler* handler =
+      [[ReplyEventHandler alloc] initWithCallback:replyFn];

[jeremy, 2013/03/11 06:49:59]

Add a comment on where this is released.

[jeremya, 2013/03/11 08:42:26]

Done.

+  [handler pingProcess:psn];
+}
+@end
+
+@implementation ReplyEventHandler (PrivateMethods)
+- (id)initWithCallback:(base::Callback<void(bool)>)replyFn {
+  if ((self = [super init])) {
+    onReply_ = replyFn;
+  }
+  return self;
+}
+
+- (void)pingProcess:(const ProcessSerialNumber&)psn {
+  // 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.
+  NSAppleEventDescriptor* target = [NSAppleEventDescriptor
+      descriptorWithDescriptorType:typeProcessSerialNumber
+                             bytes:&psn
+                            length:sizeof(psn)];
+  NSAppleEventDescriptor* initial_event =
+      [NSAppleEventDescriptor
+          appleEventWithEventClass:app_mode::kAEChromeAppClass
+                           eventID:app_mode::kAEChromeAppPing
+                  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?
+  OSStatus status = AESendMessage(
+      [initial_event aeDesc], &replyEvent_, kAEQueueReply, kAEDefaultTimeout);
+  if (status != noErr) {
+    OSSTATUS_LOG(ERROR, status) << "AESendMessage";
+    [self closeWithSuccess:false];
+  }
+}
+
+- (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
+
+//-----------------------------------------------------------------------------
+
+namespace {
+
+void OnPingChromeReply(bool success) {

[jeremy, 2013/03/11 06:49:59]

Function comment.

[jeremya, 2013/03/11 08:42:26]

Done.

+  if (!success) {
+    [NSApp terminate:nil];
+    return;
+  }
+  AppShimController* controller = new AppShimController;
+  controller->Init();
+}
+
+}  // namespace
 
 extern "C" {
 
@@ -33,6 +190,8 @@ int ChromeAppModeStart(const app_mode::ChromeAppModeInfo* info);
 
 int ChromeAppModeStart(const app_mode::ChromeAppModeInfo* info) {
   base::mac::ScopedNSAutoreleasePool scoped_pool;
+  base::AtExitManager exit_manager;
+  chrome::RegisterPathProvider();
 
   if (info->major_version < app_mode::kCurrentChromeAppModeInfoMajorVersion) {
     RAW_LOG(ERROR, "App Mode Loader too old.");
@@ -43,11 +202,21 @@ int ChromeAppModeStart(const app_mode::ChromeAppModeInfo* info) {
     return 1;
   }
 
+  g_info = info;
+
+  // Launch the IO thread.
+  base::Thread::Options io_thread_options;
+  io_thread_options.message_loop_type = MessageLoop::TYPE_IO;
+  base::Thread *io_thread = new base::Thread("CrAppShimIO");
+  io_thread->StartWithOptions(io_thread_options);
+  g_io_thread = io_thread;
+
+  // Launch Chrome if it isn't already running.
   FSRef app_fsref;
   if (!base::mac::FSRefFromPath(info->chrome_outer_bundle_path.value(),
         &app_fsref)) {
-    PLOG(ERROR) << "base::mac::FSRefFromPath failed for "
-        << info->chrome_outer_bundle_path.value();
+    LOG(ERROR) << "base::mac::FSRefFromPath failed for "
+               << info->chrome_outer_bundle_path.value();
     return 1;
   }
   std::string silent = std::string("--") + switches::kSilentLaunch;
@@ -63,27 +232,23 @@ int ChromeAppModeStart(const app_mode::ChromeAppModeInfo* info) {
     launch_args,
     NULL   // initialEvent
   };
-  NSAppleEventDescriptor* initial_event =
-      [NSAppleEventDescriptor
-          appleEventWithEventClass:app_mode::kAEChromeAppClass
-                           eventID:app_mode::kAEChromeAppLaunch
-                  targetDescriptor:nil
-                          returnID:kAutoGenerateReturnID
-                     transactionID:kAnyTransactionID];
-  NSAppleEventDescriptor* appid_descriptor = [NSAppleEventDescriptor
-      descriptorWithString:base::SysUTF8ToNSString(info->app_mode_id)];
-  [initial_event setParamDescriptor:appid_descriptor
-                         forKeyword:keyDirectObject];
-  NSAppleEventDescriptor* profile_dir_descriptor = [NSAppleEventDescriptor
-      descriptorWithString:base::SysUTF8ToNSString(info->profile_dir.value())];
-  [initial_event setParamDescriptor:profile_dir_descriptor
-                         forKeyword:app_mode::kAEProfileDirKey];
-  ls_parameters.initialEvent = const_cast<AEDesc*>([initial_event aeDesc]);
-  // Send the Apple Event using launch services, launching Chrome if necessary.
-  OSStatus status = LSOpenApplication(&ls_parameters, NULL);
+  ProcessSerialNumber psn;
+  // TODO(jeremya): this opens a new browser window if Chrome is already
+  // running without any windows open.
+  OSStatus status = LSOpenApplication(&ls_parameters, &psn);
   if (status != noErr) {
     OSSTATUS_LOG(ERROR, status) << "LSOpenApplication";
     return 1;
   }
+
+  // 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:base::Bind(&OnPingChromeReply)];
+
+  MessageLoopForUI main_message_loop;
+  main_message_loop.set_thread_name("MainThread");
+  base::PlatformThread::SetName("CrAppShimMain");
+  main_message_loop.Run();
   return 0;
 }