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

chrome/browser/apps/app_shim/extension_app_shim_handler_mac.cc

 // 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.
 
 #include "chrome/browser/apps/app_shim/extension_app_shim_handler_mac.h"
 
 #include "apps/app_lifetime_monitor_factory.h"
 #include "apps/launcher.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #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"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/mac/app_shim_messages.h"
 #include "components/crx_file/id_util.h"
 #include "content/public/browser/notification_details.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 #include "extensions/browser/app_window/app_window.h"
 #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"
 
 using extensions::AppWindow;
 using extensions::AppWindowRegistry;
 using extensions::ExtensionRegistry;
 
 namespace {
 
@@ skipping 17 matching lines @@
   for (AppWindowList::const_reverse_iterator it = windows.rbegin();
        it != windows.rend();
        ++it) {
     if (hidden)
       (*it)->GetBaseWindow()->HideWithApp();
     else
       (*it)->GetBaseWindow()->ShowWithApp();
   }
 }
 
+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;
 
   std::set<gfx::NativeWindow> native_windows;
   for (AppWindowList::const_iterator it = windows.begin(); it != windows.end();
        ++it) {
     native_windows.insert((*it)->GetNativeWindow());
   }
   // Allow workspace switching. For the browser process, we can reasonably rely
   // on OS X to switch spaces for us and honor relevant user settings. But shims
   // don't have windows, so we have to do it ourselves.
   ui::FocusWindowSet(native_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.
 class EnableViaPrompt : public ExtensionEnableFlowDelegate {
  public:
   EnableViaPrompt(Profile* profile,
                   const std::string& extension_id,
                   const base::Callback<void()>& callback)
       : profile_(profile),
         extension_id_(extension_id),
@@ skipping 127 matching lines @@
     : delegate_(new Delegate),
       weak_factory_(this) {
   // This is instantiated in BrowserProcessImpl::PreMainMessageLoopRun with
   // AppShimHostManager. Since PROFILE_CREATED is not fired until
   // ProfileManager::GetLastUsedProfile/GetLastOpenedProfiles, this should catch
   // notifications for all profiles.
   registrar_.Add(this, chrome::NOTIFICATION_PROFILE_CREATED,
                  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,
     const std::string& app_id) {
   HostMap::iterator it = hosts_.find(make_pair(profile, app_id));
   return it == hosts_.end() ? NULL : it->second;
 }
 
 // static
 void ExtensionAppShimHandler::QuitAppForWindow(AppWindow* app_window) {
   ExtensionAppShimHandler* handler = GetInstance();
   Host* host = handler->FindHost(
       Profile::FromBrowserContext(app_window->browser_context()),
       app_window->extension_id());
   if (host) {
     handler->OnShimQuit(host);
   } else {
     // App shims might be disabled or the shim is still starting up.
     AppWindowRegistry::Get(
         Profile::FromBrowserContext(app_window->browser_context()))
         ->CloseAllAppWindowsForApp(app_window->extension_id());
   }
 }
 
+// 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());
   Host* host = handler->FindHost(profile, app_window->extension_id());
   if (host)
     host->OnAppHide();
   else
     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());
   const std::string& app_id = app_window->extension_id();
   Host* host = handler->FindHost(profile, app_id);
   if (host) {
     handler->OnShimFocus(host,
                          APP_SHIM_FOCUS_NORMAL,
                          std::vector<base::FilePath>());
   } else {
@@ skipping 113 matching lines @@
   }
 
   // TODO(jeremya): Handle the case that launching the app fails. Probably we
   // need to watch for 'app successfully launched' or at least 'background page
   // exists/was created' and time out with failure if we don't see that sign of
   // life within a certain window.
   const extensions::Extension* extension =
       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;
   }
 
   delegate_->EnableExtension(
       profile, app_id,
       base::Bind(&ExtensionAppShimHandler::OnExtensionEnabled,
                  weak_factory_.GetWeakPtr(),
                  host->GetProfilePath(), app_id, files));
 }
 
@@ skipping 32 matching lines @@
   }
 }
 
 void ExtensionAppShimHandler::OnShimFocus(
     Host* host,
     AppShimFocusType focus_type,
     const std::vector<base::FilePath>& files) {
   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)) {
     return;
   }
 
   const extensions::Extension* extension =
       delegate_->GetAppExtension(profile, host->GetAppId());
   if (extension) {
     delegate_->LaunchApp(profile, extension, files);
   } else {
     // Extensions may have been uninstalled or disabled since the shim
     // started.
     host->OnAppClosed();
   }
 }
 
 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) {
   DCHECK(delegate_->ProfileExistsForPath(host->GetProfilePath()));
   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) {
   delegate_.reset(delegate);
 }
 
 void ExtensionAppShimHandler::Observe(
     int type,
     const content::NotificationSource& source,
     const content::NotificationDetails& details) {
@@ skipping 55 matching lines @@
 
   if (hosts_.empty())
     delegate_->MaybeTerminate();
 }
 
 void ExtensionAppShimHandler::OnAppStop(Profile* profile,
                                         const std::string& app_id) {}
 
 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