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

chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc

Index: chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc
diff --git a/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc b/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc
index d41bb0547d7dbe5aac3db33bb7ddb09b4e131f53..8215a47a26846a9c2dbc264567d36e48be99f4ff 100644
--- a/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc
+++ b/chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc
@@ -11,11 +11,15 @@
 #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"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/web_applications/web_app_mac.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_metrics.h"
@@ -29,6 +33,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"
@@ -66,6 +71,32 @@ void SetAppHidden(Profile* profile, const std::string& app_id, bool hidden) {
   }
 }
 
+void SetHostedAppHidden(Profile* profile,

[jackhou1, 2014/12/11 02:43:18]

Maybe make this a method of ExtensionAppShimHandler, so that it can just use the
AppBrowserMap.

[mitchellj, 2014/12/12 00:03:45]

Done.

+                        const std::string& app_id,
+                        bool hidden) {
+  const BrowserList* browser_list =
+      BrowserList::GetInstance(chrome::HOST_DESKTOP_TYPE_NATIVE);
+
+  for (BrowserList::const_iterator it = browser_list->begin();

[jackhou1, 2014/12/11 02:43:18]

You can use C++11's range-based for-loops now:

for (const Browser* browser : *browser_list) {

This probably applies to a few other places in this CL too.

[mitchellj, 2014/12/12 00:03:45]

Done.

+       it != browser_list->end(); ++it) {
+    Browser* browser = *it;
+
+    if (!browser->is_app() ||
+        web_app::GetExtensionIdFromApplicationName(browser->app_name()) !=
+            app_id) {
+      continue;
+    }
+
+    TabStripModel* tab_strip = browser->tab_strip_model();
+    for (int index = 0; index < tab_strip->count(); index++) {
+      if (hidden)

[jackhou1, 2014/12/11 02:43:18]

Do you need this loop? It doesn't look like this block does anything with the
index or tab strip.

[mitchellj, 2014/12/12 00:03:45]

Done.

+        browser->window()->Hide();
+      else
+        browser->window()->Show();
+    }
+  }
+}
+
 bool FocusWindows(const AppWindowList& windows) {
   if (windows.empty())
     return false;
@@ -82,6 +113,20 @@ bool FocusWindows(const AppWindowList& windows) {
   return true;
 }
 
+bool FocusHostedAppWindows(std::vector<content::WebContents*>& windows) {

[jackhou1, 2014/12/11 02:43:18]

Just pass in the BrowserSet here. You can get the native window via
BrowserWindow::GetNativeWindow().

[mitchellj, 2014/12/12 00:03:45]

Done.

+  if (windows.empty())
+    return false;
+
+  std::set<gfx::NativeWindow> native_windows;
+  for (std::vector<content::WebContents*>::const_iterator it = windows.begin();
+       it != windows.end(); ++it) {
+    native_windows.insert((*it)->GetTopLevelNativeWindow());
+  }
+
+  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.
@@ -229,9 +274,12 @@ ExtensionAppShimHandler::ExtensionAppShimHandler()
                  content::NotificationService::AllBrowserContextsAndSources());
   registrar_.Add(this, chrome::NOTIFICATION_PROFILE_DESTROYED,
                  content::NotificationService::AllBrowserContextsAndSources());
+  BrowserList::AddObserver(this);
 }
 
-ExtensionAppShimHandler::~ExtensionAppShimHandler() {}
+ExtensionAppShimHandler::~ExtensionAppShimHandler() {
+  BrowserList::RemoveObserver(this);
+}
 
 AppShimHandler::Host* ExtensionAppShimHandler::FindHost(
     Profile* profile,
@@ -256,6 +304,18 @@ void ExtensionAppShimHandler::QuitAppForWindow(AppWindow* app_window) {
   }
 }
 
+// static
+void ExtensionAppShimHandler::QuitHostedAppForWindow(
+    Browser* browser,
+    const extensions::Extension* extension) {
+  ExtensionAppShimHandler* handler = GetInstance();
+  Host* host = handler->FindHost(
+      Profile::FromBrowserContext(browser->profile()), extension->id());
+  if (host && extension->is_hosted_app()) {
+    handler->OnShimQuit(host);
+  }
+}
+
 void ExtensionAppShimHandler::HideAppForWindow(AppWindow* app_window) {
   ExtensionAppShimHandler* handler = GetInstance();
   Profile* profile = Profile::FromBrowserContext(app_window->browser_context());
@@ -266,6 +326,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
+    SetHostedAppHidden(profile, app_id, true);
+}
+
 void ExtensionAppShimHandler::FocusAppForWindow(AppWindow* app_window) {
   ExtensionAppShimHandler* handler = GetInstance();
   Profile* profile = Profile::FromBrowserContext(app_window->browser_context());
@@ -399,9 +469,16 @@ 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())
-      host->OnAppLaunchComplete(APP_SHIM_LAUNCH_DUPLICATE_HOST);
+    // If it's a hosted app and a tabbed application, kill it immediately.

[jackhou1, 2014/12/11 02:43:18]

s/kill it immediately/let the shim terminate immediately/

[mitchellj, 2014/12/12 00:03:45]

Acknowledged.

+    if (extension->is_hosted_app()) {
+      if (extensions::GetLaunchType(extensions::ExtensionPrefs::Get(profile),
+                                    extension) ==
+          extensions::LAUNCH_TYPE_REGULAR) {
+        host->OnAppLaunchComplete(APP_SHIM_LAUNCH_DUPLICATE_HOST);
+      } else {
+        host->OnAppLaunchComplete(APP_SHIM_LAUNCH_SUCCESS);

[jackhou1, 2014/12/11 02:43:18]

Instead of sending APP_SHIM_LAUNCH_SUCCESS here, have OnBrowserAdded call
OnAppActivated (see comment there).

[mitchellj, 2014/12/12 00:03:45]

Done.

+      }
+    }
     return;
   }
 
@@ -454,9 +531,30 @@ 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()) {
+    std::vector<content::WebContents*> items;
+    AppBrowserMap::iterator it = app_browser_windows_.find(app_id);
+    if (it == app_browser_windows_.end())
+      return;
+
+    BrowserSet& browsers = it->second;
+    for (BrowserSet::const_iterator it = browsers.begin(); it != browsers.end();
+         ++it) {
+      Browser* browser = *it;
+      TabStripModel* tab_strip = browser->tab_strip_model();
+      for (int index = 0; index < tab_strip->count(); index++) {
+        content::WebContents* web_contents = tab_strip->GetWebContentsAt(index);
+        items.push_back(web_contents);
+      }
+    }
+    windows_focused = FocusHostedAppWindows(items);
+  } 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 +576,12 @@ 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 +589,33 @@ 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()) {
+    std::vector<content::WebContents*> items;
+    AppBrowserMap::iterator it = app_browser_windows_.find(app_id);
+    if (it == app_browser_windows_.end())
+      return;
+
+    const BrowserSet& browsers = it->second;
+    for (BrowserSet::const_iterator it = browsers.begin(); it != browsers.end();
+         ++it) {
+      Browser* browser = *it;
+      TabStripModel* tab_strip = browser->tab_strip_model();

[jackhou1, 2014/12/11 02:43:18]

I think you can just call BrowserWindow::Close().

[mitchellj, 2014/12/12 00:03:45]

Done.

+      for (int index = 0; index < tab_strip->count(); index++) {
+        content::WebContents* web_contents = tab_strip->GetWebContentsAt(index);
+        web_contents->Close();
+      }
+    }
+  } 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, OnBrowserRevmoed

[jackhou1, 2014/12/11 02:43:18]

s/Revmoed/Removed/

[mitchellj, 2014/12/12 00:03:45]

Done.

+  // will call OnAppDeactivated.
 }
 
 void ExtensionAppShimHandler::set_delegate(Delegate* delegate) {
@@ -569,4 +691,30 @@ void ExtensionAppShimHandler::OnAppStop(Profile* profile,
 
 void ExtensionAppShimHandler::OnChromeTerminating() {}
 
+void ExtensionAppShimHandler::OnBrowserAdded(Browser* browser) {
+  // Don't keep track of browsers that are not associated with an app.
+  if (!browser->is_app())
+    return;
+
+  std::string app_id =
+      web_app::GetExtensionIdFromApplicationName(browser->app_name());
+  BrowserSet& browsers = app_browser_windows_[app_id];
+  browsers.insert(browser);

[jackhou1, 2014/12/11 02:43:18]

You should make this call OnAppActivated if it is the first browser being added
for an app. OnAppActivated will ensure that the shim gets launched when you
start the app from chrome:apps.

[mitchellj, 2014/12/12 00:03:45]

Done. I ran into a problem where calling OnAppActivated immediately tries to
focus the browser windows. However, browser sends the OnBrowserAdded message
when BrowserWindow may still be NULL. So I had to move the code in
OnBrowserAdded to the ExtensionAppShimHandler::Observer function to listen for
when the BrowserWindow is created instead.

+}
+
+void ExtensionAppShimHandler::OnBrowserRemoved(Browser* browser) {
+  if (!browser->is_app())

[jackhou1, 2014/12/11 02:43:18]

I think we should be consistent about checking both Browser:is_app and
Extension::is_hosted_app, in case they are not equivalent.

[mitchellj, 2014/12/12 00:03:45]

Done.

+    return;
+
+  std::string app_id =
+      web_app::GetExtensionIdFromApplicationName(browser->app_name());
+  AppBrowserMap::iterator it = app_browser_windows_.find(app_id);
+  if (it != app_browser_windows_.end()) {
+    BrowserSet& browsers = it->second;
+    browsers.erase(browser);
+    if (browsers.empty())
+      OnAppDeactivated(browser->profile(), app_id);
+  }
+}
+
 }  // namespace apps