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"

[jackhou1, 2014/12/12 00:48:29]

Do you still need this #include?

[mitchellj, 2014/12/12 03:46:23]

Acknowledged.

 #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 33 matching lines @@
        ++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::set<Browser*>& browsers) {
+  if (browsers.empty())
+    return false;
+
+  std::set<gfx::NativeWindow> native_windows;
+  for (const Browser* browser : browsers) {

[jackhou1, 2014/12/12 00:48:28]

No braces on one-line for-loop.

[mitchellj, 2014/12/12 03:46:23]

Done.

+    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.
 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 66 matching lines @@
 AppWindowList ExtensionAppShimHandler::Delegate::GetWindows(
     Profile* profile,
     const std::string& extension_id) {
   return AppWindowRegistry::Get(profile)->GetAppWindowsForApp(extension_id);
 }
 
 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(
     Profile* profile,
     const std::string& extension_id,
     const base::Callback<void()>& callback) {
   (new EnableViaPrompt(profile, extension_id, callback))->Run();
 }
 
 void ExtensionAppShimHandler::Delegate::LaunchApp(
@@ skipping 34 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());
+  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,
     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(

[jackhou1, 2014/12/12 00:48:28]

I don't think you need to check is_hosted_app here, because if there's a host,
then we'll want to call OnShimQuit. So you can have this function take a
Profile* and app_id instead.

Also, we need to handle the case when there is no host. Chrome should just close
all the hosted app's windows.

[mitchellj, 2014/12/12 03:46:23]

Done.

+    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
+    handler->SetHostedAppHidden(profile, app_id, true);
+}
+
+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 (!browser->is_app() ||

[jackhou1, 2014/12/12 00:48:29]

Can this ever happen? All the Browser*s in the map should be associated with
this app.

[mitchellj, 2014/12/12 03:46:23]

This is correct. I've removed the first condition in the IF statement.

+        web_app::GetExtensionIdFromApplicationName(browser->app_name()) !=
+            app_id) {
+      continue;
+    }
+
+    if (hidden)
+      browser->window()->Hide();
+    else
+      browser->window()->Show();
+  }
+}
+
 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 {
     FocusWindows(AppWindowRegistry::Get(profile)->GetAppWindowsForApp(app_id));
   }
 }
 
 // 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
 bool ExtensionAppShimHandler::ActivateAndRequestUserAttentionForWindow(
     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) {
     // Bring the window to the front without showing it.
     AppWindowRegistry::Get(profile)->AppWindowActivated(app_window);
     host->OnAppRequestUserAttention(APP_SHIM_ATTENTION_INFORMATIONAL);
     return true;
@@ skipping 98 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())
+    // If it's a hosted app and a tabbed application, let the shim terminate

[jackhou1, 2014/12/12 00:48:29]

s/and a tabbed application/that opens in a tab/

[mitchellj, 2014/12/12 03:46:23]

Acknowledged.

+    // 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;
   }
 
   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;

[jackhou1, 2014/12/12 00:48:28]

Is |items| ever used?

[mitchellj, 2014/12/12 03:46:23]

Oops, forgot to remove it when I changed the signature of FocusHostedAppWindows
and had to refactor the code. Done.

+    AppBrowserMap::iterator it = app_browser_windows_.find(app_id);
+    if (it == app_browser_windows_.end())
+      return;

[jackhou1, 2014/12/12 00:48:28]

Blank line after return.

[mitchellj, 2014/12/12 03:46:23]

Done.

+    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)) {
     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()) {
+    ExtensionAppShimHandler* handler = GetInstance();
+    handler->SetHostedAppHidden(profile, app_id, hidden);

[jackhou1, 2014/12/12 00:48:28]

You can call SetHostedAppHidden directly here since OnShimSetHidden is a
non-static method.

[mitchellj, 2014/12/12 03:46:23]

Done.

+  } 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;
+
+    for (const Browser* browser : it->second) {

[jackhou1, 2014/12/12 00:48:29]

No braces here.

[mitchellj, 2014/12/12 03:46:23]

Done.

+      browser->window()->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, OnBrowserRemoved
+  // 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) {
-  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(); ) {
         // Increment the iterator first as OnAppClosed may call back to
         // OnShimClose and invalidate the iterator.
         HostMap::iterator current = it++;
         if (profile->IsSameProfile(current->first.first)) {
           Host* host = current->second;
           host->OnAppClosed();
         }
       }
       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.
+      if (!browser->is_app())

[jackhou1, 2014/12/12 00:48:29]

This should also check that the Extension is a hosted app.

[mitchellj, 2014/12/12 03:46:23]

Done.

+        return;
+
+      std::string app_id =
+          web_app::GetExtensionIdFromApplicationName(browser->app_name());
+      BrowserSet& browsers = app_browser_windows_[app_id];
+      browsers.insert(browser);
+      if (browsers.size() == 1)
+        OnAppActivated(browser->profile(), app_id);
+
+      break;
+    }
     default: {
       NOTREACHED();  // Unexpected notification.
       break;
     }
   }
 }
 
 void ExtensionAppShimHandler::OnAppStart(Profile* profile,
                                          const std::string& app_id) {}
 
@@ skipping 22 matching lines @@
 
   if (hosts_.empty())
     delegate_->MaybeTerminate();
 }
 
 void ExtensionAppShimHandler::OnAppStop(Profile* profile,
                                         const std::string& app_id) {}
 
 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 OnAppActivited.

[jackhou1, 2014/12/12 00:48:28]

OnAppActivated

[mitchellj, 2014/12/12 03:46:23]

Done.

+// 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) {
+  if (!browser->is_app())
+    return;
+
+  std::string app_id =
+      web_app::GetExtensionIdFromApplicationName(browser->app_name());
+  const extensions::Extension* extension =
+      GetAppExtension(browser->profile(), app_id);
+  if (!extension)
+    return;
+
+  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