[mac] App shims

chrome/browser/web_applications/app_shim_host_controller.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.
+
+#import <Cocoa/Cocoa.h>
+
+#include "chrome/browser/web_applications/app_shim_host_controller.h"
+
+#include "base/bind.h"
+#include "base/files/file_path.h"
+#include "base/logging.h"
+#include "base/path_service.h"
+#include "base/threading/non_thread_safe.h"
+#include "chrome/browser/browser_process.h"
+#include "chrome/browser/extensions/extension_host.h"
+#include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/extensions/shell_window_registry.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/extensions/application_launch.h"
+#include "chrome/browser/ui/extensions/shell_window.h"
+#include "chrome/common/app_shim_messages.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/mac/app_mode_common.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "ipc/ipc_channel_proxy.h"
+#include "ipc/ipc_listener.h"
+#include "ipc/ipc_message.h"
+
+//-----------------------------------------------------------------------------
+
+class AppShimHost : public IPC::Listener,

[Mark Mentovai, 2013/03/08 16:50:17]

Some short comments about everything in this class, too. Again, the memory
management here is a little tricky, and you should say something about it.

[jeremya, 2013/03/11 02:47:33]

Done.

+                    public content::NotificationObserver,
+                    public base::NonThreadSafe {
+ public:
+  explicit AppShimHost(const IPC::ChannelHandle& channel);
+  virtual ~AppShimHost();
+
+ private:
+  bool OnMessageReceived(const IPC::Message& message);
+  bool LaunchAppImpl(const std::string& app_id, const std::string& profile_dir);
+
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+  void Close();
+
+  void OnLaunchApp(std::string profile, std::string app_id);
+
+  IPC::ChannelProxy* channel_;
+
+  std::string app_id_;
+  Profile* profile_;
+  content::NotificationRegistrar registrar_;
+};
+
+AppShimHost::AppShimHost(const IPC::ChannelHandle& handle)
+    : channel_(NULL), profile_(NULL) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  channel_ = new IPC::ChannelProxy(handle, IPC::Channel::MODE_SERVER, this,
+      BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO));
+}
+
+AppShimHost::~AppShimHost() {
+  DCHECK(CalledOnValidThread());
+  delete channel_;
+  channel_ = NULL;

[Mark Mentovai, 2013/03/08 16:50:17]

It’s not really necessary to zero things out in a destructor.

[jeremya, 2013/03/11 02:47:33]

Done.

+  profile_ = NULL;
+}
+
+bool AppShimHost::OnMessageReceived(const IPC::Message& message) {
+  DCHECK(CalledOnValidThread());
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(AppShimHost, message)
+    IPC_MESSAGE_HANDLER(AppShimHostMsg_LaunchApp, OnLaunchApp)
+
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+
+  return handled;
+}
+
+void AppShimHost::OnLaunchApp(std::string profile_dir, std::string app_id) {
+  bool success = LaunchAppImpl(profile_dir, app_id);
+  channel_->Send(new AppShimMsg_LaunchApp_Done(success));
+}
+
+bool AppShimHost::LaunchAppImpl(const std::string& profile_dir,
+                                const std::string& app_id) {
+  DCHECK(CalledOnValidThread());
+  if (profile_)
+    // Only one app launch message per channel.

[Mark Mentovai, 2013/03/08 16:50:17]

{braces} around multi-line scopes, even if the scope takes multiple lines for a
single statement.

[jeremya, 2013/03/11 02:47:33]

Done.

+    return false;
+  ProfileManager* profileManager = g_browser_process->profile_manager();
+  base::FilePath path(profile_dir);
+  path = profileManager->user_data_dir().Append(path);
+  profile_ = profileManager->GetProfile(path);
+  if (!profile_) {
+    LOG(ERROR) << "Unable to locate a suitable profile for profile directory '"
+               << profile_dir << "' while trying to load app with id '"
+               << app_id << "'.";
+    return false;
+  }
+  extensions::ExtensionSystem* extension_system =
+      extensions::ExtensionSystem::Get(profile_);
+  ExtensionServiceInterface* extension_service =
+      extension_system->extension_service();
+  const extensions::Extension* extension =
+      extension_service->GetExtensionById(
+          app_id, false);
+  if (!extension) {
+    LOG(ERROR) << "Shortcut attempted to launch nonexistent app with id '"
+               << app_id << "'.";
+    return false;
+  }
+  app_id_ = app_id;
+  chrome::AppLaunchParams params(profile_, extension,
+                                          extension_misc::LAUNCH_NONE,

[Mark Mentovai, 2013/03/08 16:50:17]

Weird indentation.

[jeremya, 2013/03/11 02:47:33]

Done.

+                                          NEW_WINDOW);
+  chrome::OpenApplication(params);
+
+  registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED,
+                 content::Source<Profile>(profile_));
+  return true;
+}
+
+void AppShimHost::Close() {
+  DCHECK(CalledOnValidThread());
+  delete this;
+}
+
+void AppShimHost::Observe(int type,
+                          const content::NotificationSource& source,
+                          const content::NotificationDetails& details) {
+  DCHECK(content::Source<Profile>(source).ptr() == profile_);
+  switch (type) {
+    case chrome::NOTIFICATION_EXTENSION_HOST_DESTROYED: {
+      extensions::ExtensionHost* extension_host =
+          content::Details<extensions::ExtensionHost>(details).ptr();
+      if (app_id_ == extension_host->extension_id())
+        Close();
+      break;
+    }
+    default:
+      NOTREACHED() << "Unexpected notification sent.";
+      break;
+  }
+}
+
+//-----------------------------------------------------------------------------
+
+AppShimHostController::AppShimHostController() : factory_(NULL) {
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&AppShimHostController::InitOnFile, AsWeakPtr()));
+}
+
+AppShimHostController::~AppShimHostController() {
+  delete factory_;
+}
+
+void AppShimHostController::InitOnFile() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  base::FilePath user_data_dir;
+  PathService::Get(chrome::DIR_USER_DATA, &user_data_dir);
+  base::FilePath socket_path =
+      user_data_dir.Append(app_mode::kAppShimSocketName);
+  factory_ = new IPC::ChannelFactory(socket_path, this);
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&AppShimHostController::ListenOnIO, AsWeakPtr()));
+}
+
+void AppShimHostController::ListenOnIO() {
+  factory_->Listen();
+}
+
+void CreateAppShimHost(const IPC::ChannelHandle& handle) {

[Mark Mentovai, 2013/03/08 16:50:17]

Anonymous namespace or static.

[jeremya, 2013/03/11 02:47:33]

Done.

+  new AppShimHost(handle);
+}
+
+void AppShimHostController::OnClientConnected(
+    const IPC::ChannelHandle& handle) {
+  // called on IO thread
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&CreateAppShimHost, handle));
+}