Hosted apps on OS X now act more like a native app.

- Quitting an app shim will close all associated windows.
- Closing all windows will quit the app shim.
- Can focus between Chrome and app shim windows.
- Can hide/show all windows associated with an app shim.

BUG=440651

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

Cr-Commit-Position: refs/heads/master@{#308473}

chrome/browser/app_controller_mac.mm

@@ -75,6 +75,7 @@
 #include "chrome/browser/ui/startup/startup_browser_creator.h"
 #include "chrome/browser/ui/startup/startup_browser_creator_impl.h"
 #include "chrome/browser/ui/user_manager.h"
+#include "chrome/browser/web_applications/web_app_mac.h"
 #include "chrome/common/chrome_paths_internal.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/cloud_print/cloud_print_class_mac.h"
@@ -95,11 +96,14 @@
 #include "content/public/browser/plugin_service.h"
 #include "content/public/browser/user_metrics.h"
 #include "extensions/browser/extension_system.h"
+#include "extensions/browser/extension_registry.h"
 #include "net/base/filename_util.h"
 #include "ui/base/cocoa/focus_window_set.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/base/l10n/l10n_util_mac.h"
 
+using apps::AppShimHandler;
+using apps::ExtensionAppShimHandler;
 using base::UserMetricsAction;
 using content::BrowserContext;
 using content::BrowserThread;
@@ -1216,8 +1220,22 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
   // notifications so we still need to open a new window.
   if (hasVisibleWindows) {
     std::set<NSWindow*> browserWindows;
+    ExtensionAppShimHandler* appShimHandler =
+        g_browser_process->platform_part()
+            ->app_shim_host_manager()
+            ->extension_app_shim_handler();
     for (chrome::BrowserIterator iter; !iter.done(); iter.Next()) {
       Browser* browser = *iter;
+      // When focusing Chrome, don't focus any browser windows associated with
+      // a currently running app shim, so ignore them.
+      if (browser && browser->is_app()) {
+        AppShimHandler::Host* host = appShimHandler->FindHost(
+            browser->profile(),
+            web_app::GetExtensionIdFromApplicationName(browser->app_name()));
+        if (host) {
+          continue;
+        }
+      }
       browserWindows.insert(browser->window()->GetNativeWindow());
     }
     if (!browserWindows.empty()) {

chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc

@@ -11,8 +11,11 @@
 #include "chrome/browser/apps/app_shim/app_shim_host_manager_mac.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
+#include "chrome/browser/extensions/launch_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/ui/browser_list.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/extensions/extension_enable_flow.h"
 #include "chrome/browser/ui/extensions/extension_enable_flow_delegate.h"
@@ -29,6 +32,7 @@
 #include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/browser/app_window/native_app_window.h"
 #include "extensions/browser/extension_host.h"
+#include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/constants.h"
 #include "ui/base/cocoa/focus_window_set.h"
@@ -82,6 +86,18 @@ bool FocusWindows(const AppWindowList& windows) {
   return true;
 }
 
+bool FocusHostedAppWindows(std::set<Browser*>& browsers) {
+  if (browsers.empty())
+    return false;
+
+  std::set<gfx::NativeWindow> native_windows;
+  for (const Browser* browser : browsers)
+    native_windows.insert(browser->window()->GetNativeWindow());
+
+  ui::FocusWindowSet(native_windows);
+  return true;
+}
+
 // Attempts to launch a packaged app, prompting the user to enable it if
 // necessary. The prompt is shown in its own window.
 // This class manages its own lifetime.
@@ -168,13 +184,7 @@ const extensions::Extension*
 ExtensionAppShimHandler::Delegate::GetAppExtension(
     Profile* profile,
     const std::string& extension_id) {
-  ExtensionRegistry* registry = ExtensionRegistry::Get(profile);
-  const extensions::Extension* extension =
-      registry->GetExtensionById(extension_id, ExtensionRegistry::ENABLED);
-  return extension &&
-                 (extension->is_platform_app() || extension->is_hosted_app())
-             ? extension
-             : NULL;
+  return ExtensionAppShimHandler::GetAppExtension(profile, extension_id);
 }
 
 void ExtensionAppShimHandler::Delegate::EnableExtension(
@@ -229,9 +239,14 @@ ExtensionAppShimHandler::ExtensionAppShimHandler()
                  content::NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
                  content::NotificationService::AllBrowserContextsAndSources());
+  registrar_.Add(this, chrome::NOTIFICATION_BROWSER_WINDOW_READY,
+                 content::NotificationService::AllBrowserContextsAndSources());
+  BrowserList::AddObserver(this);
 }
 
-ExtensionAppShimHandler::~ExtensionAppShimHandler() {}
+ExtensionAppShimHandler::~ExtensionAppShimHandler() {
+  BrowserList::RemoveObserver(this);
+}
 
 AppShimHandler::Host* ExtensionAppShimHandler::FindHost(
     Profile* profile,
@@ -240,6 +255,53 @@ AppShimHandler::Host* ExtensionAppShimHandler::FindHost(
   return it == hosts_.end() ? NULL : it->second;
 }
 
+void ExtensionAppShimHandler::SetHostedAppHidden(Profile* profile,
+                                                 const std::string& app_id,
+                                                 bool hidden) {
+  const AppBrowserMap::iterator it = app_browser_windows_.find(app_id);
+  if (it == app_browser_windows_.end())
+    return;
+
+  for (const Browser* browser : it->second) {
+    if (web_app::GetExtensionIdFromApplicationName(browser->app_name()) !=
+        app_id) {
+      continue;
+    }
+
+    if (hidden)
+      browser->window()->Hide();
+    else
+      browser->window()->Show();
+  }
+}
+
+// static
+const extensions::Extension* ExtensionAppShimHandler::GetAppExtension(
+    Profile* profile,
+    const std::string& extension_id) {
+  if (!profile)
+    return NULL;
+
+  ExtensionRegistry* registry = ExtensionRegistry::Get(profile);
+  const extensions::Extension* extension =
+      registry->GetExtensionById(extension_id, ExtensionRegistry::ENABLED);
+  return extension &&
+                 (extension->is_platform_app() || extension->is_hosted_app())
+             ? extension
+             : NULL;
+}
+
+// static
+const extensions::Extension* ExtensionAppShimHandler::GetAppForBrowser(
+    Browser* browser) {
+  if (!browser || !browser->is_app())
+    return NULL;
+
+  return GetAppExtension(
+      browser->profile(),
+      web_app::GetExtensionIdFromApplicationName(browser->app_name()));
+}
+
 // static
 void ExtensionAppShimHandler::QuitAppForWindow(AppWindow* app_window) {
   ExtensionAppShimHandler* handler = GetInstance();
@@ -256,6 +318,18 @@ void ExtensionAppShimHandler::QuitAppForWindow(AppWindow* app_window) {
   }
 }
 
+// static
+void ExtensionAppShimHandler::QuitHostedAppForWindow(
+    Profile* profile,
+    const std::string& app_id) {
+  ExtensionAppShimHandler* handler = GetInstance();
+  Host* host = handler->FindHost(Profile::FromBrowserContext(profile), app_id);
+  if (host)
+    handler->OnShimQuit(host);
+  else
+    handler->CloseBrowsersForApp(app_id);
+}
+
 void ExtensionAppShimHandler::HideAppForWindow(AppWindow* app_window) {
   ExtensionAppShimHandler* handler = GetInstance();
   Profile* profile = Profile::FromBrowserContext(app_window->browser_context());
@@ -266,6 +340,16 @@ void ExtensionAppShimHandler::HideAppForWindow(AppWindow* app_window) {
     SetAppHidden(profile, app_window->extension_id(), true);
 }
 
+void ExtensionAppShimHandler::HideHostedApp(Profile* profile,
+                                            const std::string& app_id) {
+  ExtensionAppShimHandler* handler = GetInstance();
+  Host* host = handler->FindHost(profile, app_id);
+  if (host)
+    host->OnAppHide();
+  else
+    handler->SetHostedAppHidden(profile, app_id, true);
+}
+
 void ExtensionAppShimHandler::FocusAppForWindow(AppWindow* app_window) {
   ExtensionAppShimHandler* handler = GetInstance();
   Profile* profile = Profile::FromBrowserContext(app_window->browser_context());
@@ -368,6 +452,15 @@ ExtensionAppShimHandler* ExtensionAppShimHandler::GetInstance() {
       ->extension_app_shim_handler();
 }
 
+void ExtensionAppShimHandler::CloseBrowsersForApp(const std::string& app_id) {
+  AppBrowserMap::iterator it = app_browser_windows_.find(app_id);
+  if (it == app_browser_windows_.end())
+    return;
+
+  for (const Browser* browser : it->second)
+    browser->window()->Close();
+}
+
 void ExtensionAppShimHandler::OnProfileLoaded(
     Host* host,
     AppShimLaunchType launch_type,
@@ -399,9 +492,14 @@ void ExtensionAppShimHandler::OnProfileLoaded(
       delegate_->GetAppExtension(profile, app_id);
   if (extension) {
     delegate_->LaunchApp(profile, extension, files);
-    // If it's a hosted app, just kill it immediately after opening for now.
-    if (extension->is_hosted_app())
+    // If it's a hosted app that opens in a tab, let the shim terminate
+    // immediately.
+    if (extension->is_hosted_app() &&
+        extensions::GetLaunchType(extensions::ExtensionPrefs::Get(profile),
+                                  extension) ==
+            extensions::LAUNCH_TYPE_REGULAR) {
       host->OnAppLaunchComplete(APP_SHIM_LAUNCH_DUPLICATE_HOST);
+    }
     return;
   }
 
@@ -454,9 +552,19 @@ void ExtensionAppShimHandler::OnShimFocus(
   DCHECK(delegate_->ProfileExistsForPath(host->GetProfilePath()));
   Profile* profile = delegate_->ProfileForPath(host->GetProfilePath());
 
-  const AppWindowList windows =
-      delegate_->GetWindows(profile, host->GetAppId());
-  bool windows_focused = FocusWindows(windows);
+  bool windows_focused;
+  const std::string& app_id = host->GetAppId();
+  if (delegate_->GetAppExtension(profile, app_id)->is_hosted_app()) {
+    AppBrowserMap::iterator it = app_browser_windows_.find(app_id);
+    if (it == app_browser_windows_.end())
+      return;
+
+    windows_focused = FocusHostedAppWindows(it->second);
+  } else {
+    const AppWindowList windows =
+        delegate_->GetWindows(profile, host->GetAppId());
+    windows_focused = FocusWindows(windows);
+  }
 
   if (focus_type == APP_SHIM_FOCUS_NORMAL ||
       (focus_type == APP_SHIM_FOCUS_REOPEN && windows_focused)) {
@@ -478,7 +586,11 @@ void ExtensionAppShimHandler::OnShimSetHidden(Host* host, bool hidden) {
   DCHECK(delegate_->ProfileExistsForPath(host->GetProfilePath()));
   Profile* profile = delegate_->ProfileForPath(host->GetProfilePath());
 
-  SetAppHidden(profile, host->GetAppId(), hidden);
+  const std::string& app_id = host->GetAppId();
+  if (delegate_->GetAppExtension(profile, app_id)->is_hosted_app())
+    SetHostedAppHidden(profile, app_id, hidden);
+  else
+    SetAppHidden(profile, app_id, hidden);
 }
 
 void ExtensionAppShimHandler::OnShimQuit(Host* host) {
@@ -486,14 +598,19 @@ void ExtensionAppShimHandler::OnShimQuit(Host* host) {
   Profile* profile = delegate_->ProfileForPath(host->GetProfilePath());
 
   const std::string& app_id = host->GetAppId();
-  const AppWindowList windows = delegate_->GetWindows(profile, app_id);
-  for (AppWindowRegistry::const_iterator it = windows.begin();
-       it != windows.end();
-       ++it) {
-    (*it)->GetBaseWindow()->Close();
+  if (delegate_->GetAppExtension(profile, app_id)->is_hosted_app())
+    CloseBrowsersForApp(app_id);
+  else {
+    const AppWindowList windows = delegate_->GetWindows(profile, app_id);
+    for (AppWindowRegistry::const_iterator it = windows.begin();
+         it != windows.end(); ++it) {
+      (*it)->GetBaseWindow()->Close();
+    }
   }
   // Once the last window closes, flow will end up in OnAppDeactivated via
   // AppLifetimeMonitor.
+  // Otherwise, once the last window closes for a hosted app, OnBrowserRemoved
+  // will call OnAppDeactivated.
 }
 
 void ExtensionAppShimHandler::set_delegate(Delegate* delegate) {
@@ -504,16 +621,20 @@ void ExtensionAppShimHandler::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
-  Profile* profile = content::Source<Profile>(source).ptr();
-  if (profile->IsOffTheRecord())
-    return;
-
   switch (type) {
     case chrome::NOTIFICATION_PROFILE_CREATED: {
+      Profile* profile = content::Source<Profile>(source).ptr();
+      if (profile->IsOffTheRecord())
+        return;
+
       AppLifetimeMonitorFactory::GetForProfile(profile)->AddObserver(this);
       break;
     }
     case chrome::NOTIFICATION_PROFILE_DESTROYED: {
+      Profile* profile = content::Source<Profile>(source).ptr();
+      if (profile->IsOffTheRecord())
+        return;
+
       AppLifetimeMonitorFactory::GetForProfile(profile)->RemoveObserver(this);
       // Shut down every shim associated with this profile.
       for (HostMap::iterator it = hosts_.begin(); it != hosts_.end(); ) {
@@ -527,6 +648,20 @@ void ExtensionAppShimHandler::Observe(
       }
       break;
     }
+    case chrome::NOTIFICATION_BROWSER_WINDOW_READY: {
+      Browser* browser = content::Source<Browser>(source).ptr();
+      // Don't keep track of browsers that are not associated with an app.
+      const extensions::Extension* extension = GetAppForBrowser(browser);
+      if (!extension)
+        return;
+
+      BrowserSet& browsers = app_browser_windows_[extension->id()];
+      browsers.insert(browser);
+      if (browsers.size() == 1)
+        OnAppActivated(browser->profile(), extension->id());
+
+      break;
+    }
     default: {
       NOTREACHED();  // Unexpected notification.
       break;
@@ -569,4 +704,26 @@ void ExtensionAppShimHandler::OnAppStop(Profile* profile,
 
 void ExtensionAppShimHandler::OnChromeTerminating() {}
 
+// The BrowserWindow may be NULL when this is called.
+// Therefore we listen for the notification
+// chrome::NOTIFICATION_BROWSER_WINDOW_READY and then call OnAppActivated.
+// If this notification is removed, check that OnBrowserAdded is called after
+// the BrowserWindow is ready.
+void ExtensionAppShimHandler::OnBrowserAdded(Browser* browser) {
+}
+
+void ExtensionAppShimHandler::OnBrowserRemoved(Browser* browser) {
+  const extensions::Extension* extension = GetAppForBrowser(browser);
+  if (!extension)
+    return;
+
+  AppBrowserMap::iterator it = app_browser_windows_.find(extension->id());
+  if (it != app_browser_windows_.end()) {
+    BrowserSet& browsers = it->second;
+    browsers.erase(browser);
+    if (browsers.empty())
+      OnAppDeactivated(browser->profile(), extension->id());
+  }
+}
+
 }  // namespace apps

chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h

@@ -6,6 +6,7 @@
 #define CHROME_BROWSER_APPS_APP_SHIM_EXTENSION_APP_SHIM_HANDLER_MAC_H_
 
 #include <map>
+#include <set>
 #include <string>
 #include <vector>
 
@@ -13,6 +14,8 @@
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/apps/app_shim/app_shim_handler_mac.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_list_observer.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "extensions/browser/app_window/app_window_registry.h"
@@ -38,7 +41,8 @@ namespace apps {
 // extension.
 class ExtensionAppShimHandler : public AppShimHandler,
                                 public content::NotificationObserver,
-                                public AppLifetimeMonitor::Observer {
+                                public AppLifetimeMonitor::Observer,
+                                public chrome::BrowserListObserver {
  public:
   class Delegate {
    public:
@@ -72,10 +76,25 @@ class ExtensionAppShimHandler : public AppShimHandler,
 
   AppShimHandler::Host* FindHost(Profile* profile, const std::string& app_id);
 
+  void SetHostedAppHidden(Profile* profile,
+                          const std::string& app_id,
+                          bool hidden);
+
+  static const extensions::Extension* GetAppExtension(
+      Profile* profile,
+      const std::string& extension_id);
+
+  static const extensions::Extension* GetAppForBrowser(Browser* browser);
+
   static void QuitAppForWindow(extensions::AppWindow* app_window);
 
+  static void QuitHostedAppForWindow(Profile* profile,
+                                     const std::string& app_id);
+
   static void HideAppForWindow(extensions::AppWindow* app_window);
 
+  static void HideHostedApp(Profile* profile, const std::string& app_id);
+
   static void FocusAppForWindow(extensions::AppWindow* app_window);
 
   // Brings the window to the front without showing it and instructs the shim to
@@ -115,9 +134,15 @@ class ExtensionAppShimHandler : public AppShimHandler,
                const content::NotificationSource& source,
                const content::NotificationDetails& details) override;
 
+  // chrome::BrowserListObserver overrides;
+  void OnBrowserAdded(Browser* browser) override;
+  void OnBrowserRemoved(Browser* browser) override;
+
  protected:
   typedef std::map<std::pair<Profile*, std::string>, AppShimHandler::Host*>
       HostMap;
+  typedef std::set<Browser*> BrowserSet;
+  typedef std::map<std::string, BrowserSet> AppBrowserMap;
 
   // Exposed for testing.
   void set_delegate(Delegate* delegate);
@@ -128,6 +153,9 @@ class ExtensionAppShimHandler : public AppShimHandler,
   // Helper function to get the instance on the browser process.
   static ExtensionAppShimHandler* GetInstance();
 
+  // Closes all browsers associated with an app.
+  void CloseBrowsersForApp(const std::string& app_id);
+
   // This is passed to Delegate::LoadProfileAsync for shim-initiated launches
   // where the profile was not yet loaded.
   void OnProfileLoaded(Host* host,
@@ -145,6 +173,9 @@ class ExtensionAppShimHandler : public AppShimHandler,
 
   HostMap hosts_;
 
+  // A map of app ids to associated browser windows.
+  AppBrowserMap app_browser_windows_;
+
   content::NotificationRegistrar registrar_;
 
   base::WeakPtrFactory<ExtensionAppShimHandler> weak_factory_;

chrome/browser/ui/cocoa/apps/app_shim_menu_controller_mac.mm

@@ -10,7 +10,10 @@
 #include "chrome/app/chrome_command_ids.h"
 #include "chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h"
 #include "chrome/browser/apps/app_window_registry_util.h"
+#include "chrome/browser/profiles/profile.h"
 #import "chrome/browser/ui/cocoa/apps/native_app_window_cocoa.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_finder.h"
 #include "chrome/grit/generated_resources.h"
 #include "extensions/browser/app_window/app_window.h"
 #include "extensions/common/extension.h"
@@ -295,8 +298,13 @@ void AddDuplicateItem(NSMenuItem* top_level_item,
             window);
 
     const extensions::Extension* extension = NULL;
+    // If there is no corresponding AppWindow, this could be a hosted app, so
+    // check for a browser.
     if (appWindow)
       extension = appWindow->GetExtension();
+    else
+      extension = apps::ExtensionAppShimHandler::GetAppForBrowser(
+          chrome::FindBrowserWithWindow(window));
 
     if (extension)
       [self addMenuItems:extension];
@@ -374,16 +382,32 @@ void AddDuplicateItem(NSMenuItem* top_level_item,
   extensions::AppWindow* appWindow =
       AppWindowRegistryUtil::GetAppWindowForNativeWindowAnyProfile(
           [NSApp keyWindow]);
-  if (appWindow)
+  if (appWindow) {
     apps::ExtensionAppShimHandler::QuitAppForWindow(appWindow);
+  } else {
+    Browser* browser = chrome::FindBrowserWithWindow([NSApp keyWindow]);
+    const extensions::Extension* extension =
+        apps::ExtensionAppShimHandler::GetAppForBrowser(browser);
+    if (extension)
+      apps::ExtensionAppShimHandler::QuitHostedAppForWindow(browser->profile(),
+                                                            extension->id());
+  }
 }
 
 - (void)hideCurrentPlatformApp {
   extensions::AppWindow* appWindow =
       AppWindowRegistryUtil::GetAppWindowForNativeWindowAnyProfile(
           [NSApp keyWindow]);
-  if (appWindow)
+  if (appWindow) {
     apps::ExtensionAppShimHandler::HideAppForWindow(appWindow);
+  } else {
+    Browser* browser = chrome::FindBrowserWithWindow([NSApp keyWindow]);
+    const extensions::Extension* extension =
+        apps::ExtensionAppShimHandler::GetAppForBrowser(browser);
+    if (extension)
+      apps::ExtensionAppShimHandler::HideHostedApp(browser->profile(),
+                                                   extension->id());
+  }
 }
 
 - (void)focusCurrentPlatformApp {

chrome/browser/ui/cocoa/browser_window_cocoa.mm

@@ -123,8 +123,8 @@ void BrowserWindowCocoa::Show() {
   NSWindowAnimationBehavior saved_animation_behavior =
       NSWindowAnimationBehaviorDefault;
   bool did_save_animation_behavior = false;
-  // Turn off swishing when restoring windows.
-  if (is_session_restore &&
+  // Turn off swishing when restoring windows or showing an app.
+  if ((is_session_restore || browser_->is_app()) &&
       [window() respondsToSelector:@selector(animationBehavior)] &&
       [window() respondsToSelector:@selector(setAnimationBehavior:)]) {
     did_save_animation_behavior = true;
@@ -163,7 +163,7 @@ void BrowserWindowCocoa::ShowInactive() {
 }
 
 void BrowserWindowCocoa::Hide() {
-  // Not implemented.
+  [window() orderOut:controller_];
 }
 
 void BrowserWindowCocoa::SetBounds(const gfx::Rect& bounds) {