Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1987)

Unified Diff: apps/app_shim/extension_app_shim_handler_mac.cc

Issue 17481002: Restructure ExtensionAppShimHandler testing. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Address comments Created 7 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: apps/app_shim/extension_app_shim_handler_mac.cc
diff --git a/apps/app_shim/extension_app_shim_handler_mac.cc b/apps/app_shim/extension_app_shim_handler_mac.cc
index 21edc8a502e04e2374095ca19ae04ff4bde28feb..eea475c5511262ee57d5de1a6b5c7aeb42a914e9 100644
--- a/apps/app_shim/extension_app_shim_handler_mac.cc
+++ b/apps/app_shim/extension_app_shim_handler_mac.cc
@@ -26,26 +26,11 @@
#include "content/public/browser/notification_source.h"
#include "ui/base/cocoa/focus_window_set.h"
-namespace {
+namespace apps {
typedef extensions::ShellWindowRegistry::ShellWindowList ShellWindowList;
-ShellWindowList GetWindows(Profile* profile, const std::string& extension_id) {
- return extensions::ShellWindowRegistry::Get(profile)->
- GetShellWindowsForApp(extension_id);
-}
-
-const extensions::Extension* GetExtension(Profile* profile,
- const std::string& extension_id) {
- return extensions::ExtensionSystem::Get(profile)->extension_service()->
- GetExtensionById(extension_id, false);
-}
-
-} // namespace
-
-namespace apps {
-
-bool ExtensionAppShimHandler::ProfileManagerFacade::ProfileExistsForPath(
+bool ExtensionAppShimHandler::Delegate::ProfileExistsForPath(
const base::FilePath& path) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
// Check for the profile name in the profile info cache to ensure that we
@@ -55,15 +40,48 @@ bool ExtensionAppShimHandler::ProfileManagerFacade::ProfileExistsForPath(
return cache.GetIndexOfProfileWithPath(full_path) != std::string::npos;
}
-Profile* ExtensionAppShimHandler::ProfileManagerFacade::ProfileForPath(
+Profile* ExtensionAppShimHandler::Delegate::ProfileForPath(
const base::FilePath& path) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
base::FilePath full_path = profile_manager->user_data_dir().Append(path);
return profile_manager->GetProfile(full_path);
}
+ShellWindowList ExtensionAppShimHandler::Delegate::GetWindows(
+ Profile* profile,
+ const std::string& extension_id) {
+ return extensions::ShellWindowRegistry::Get(profile)->
+ GetShellWindowsForApp(extension_id);
+}
+
+const extensions::Extension*
+ExtensionAppShimHandler::Delegate::GetAppExtension(
+ Profile* profile,
+ const std::string& extension_id) {
+ ExtensionService* extension_service =
+ extensions::ExtensionSystem::Get(profile)->extension_service();
+ DCHECK(extension_service);
+ const extensions::Extension* extension =
+ extension_service->GetExtensionById(extension_id, false);
+ return extension && extension->is_platform_app() ? extension : NULL;
+}
+
+void ExtensionAppShimHandler::Delegate::LaunchApp(
+ Profile* profile,
+ const extensions::Extension* extension) {
+ chrome::OpenApplication(
+ chrome::AppLaunchParams(profile, extension, NEW_FOREGROUND_TAB));
+}
+
+void ExtensionAppShimHandler::Delegate::LaunchShim(
+ Profile* profile,
+ const extensions::Extension* extension) {
+ web_app::MaybeLaunchShortcut(
+ web_app::ShortcutInfoForExtensionAndProfile(extension, profile));
+}
+
ExtensionAppShimHandler::ExtensionAppShimHandler()
- : profile_manager_facade_(new ProfileManagerFacade) {
+ : delegate_(new Delegate) {
// This is instantiated in BrowserProcessImpl::PreMainMessageLoopRun with
// AppShimHostManager. Since PROFILE_CREATED is not fired until
// ProfileManager::GetLastUsedProfile/GetLastOpenedProfiles, this should catch
@@ -81,7 +99,7 @@ bool ExtensionAppShimHandler::OnShimLaunch(Host* host,
const base::FilePath& profile_path = host->GetProfilePath();
DCHECK(!profile_path.empty());
- if (!profile_manager_facade_->ProfileExistsForPath(profile_path)) {
+ if (!delegate_->ProfileExistsForPath(profile_path)) {
// User may have deleted the profile this shim was originally created for.
// TODO(jackhou): Add some UI for this case and remove the LOG.
LOG(ERROR) << "Requested directory is not a known profile '"
@@ -89,7 +107,7 @@ bool ExtensionAppShimHandler::OnShimLaunch(Host* host,
return false;
}
- Profile* profile = profile_manager_facade_->ProfileForPath(profile_path);
+ Profile* profile = delegate_->ProfileForPath(profile_path);
const std::string& app_id = host->GetAppId();
if (!extensions::Extension::IdIsValid(app_id)) {
@@ -98,8 +116,28 @@ bool ExtensionAppShimHandler::OnShimLaunch(Host* host,
}
// TODO(jackhou): Add some UI for this case and remove the LOG.
- if (!LaunchApp(profile, app_id, launch_type))
+ const extensions::Extension* extension =
+ delegate_->GetAppExtension(profile, app_id);
+ if (!extension) {
+ LOG(ERROR) << "Attempted to launch nonexistent app with id '"
+ << app_id << "'.";
return false;
+ }
+
+ // If the shim was launched in response to a window appearing, but the window
+ // is closed by the time the shim process launched, return false to close the
+ // shim.
+ if (launch_type == APP_SHIM_LAUNCH_REGISTER_ONLY &&
+ delegate_->GetWindows(profile, app_id).empty()) {
+ return false;
+ }
+
+ // 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.
+ if (launch_type == APP_SHIM_LAUNCH_NORMAL)
+ delegate_->LaunchApp(profile, extension);
// The first host to claim this (profile, app_id) becomes the main host.
// For any others, focus the app and return false.
@@ -112,10 +150,8 @@ bool ExtensionAppShimHandler::OnShimLaunch(Host* host,
}
void ExtensionAppShimHandler::OnShimClose(Host* host) {
- DCHECK(profile_manager_facade_->ProfileExistsForPath(
- host->GetProfilePath()));
- Profile* profile =
- profile_manager_facade_->ProfileForPath(host->GetProfilePath());
+ DCHECK(delegate_->ProfileExistsForPath(host->GetProfilePath()));
+ Profile* profile = delegate_->ProfileForPath(host->GetProfilePath());
HostMap::iterator it = hosts_.find(make_pair(profile, host->GetAppId()));
// Any hosts other than the main host will still call OnShimClose, so ignore
@@ -125,12 +161,11 @@ void ExtensionAppShimHandler::OnShimClose(Host* host) {
}
void ExtensionAppShimHandler::OnShimFocus(Host* host) {
- DCHECK(profile_manager_facade_->ProfileExistsForPath(
- host->GetProfilePath()));
- Profile* profile =
- profile_manager_facade_->ProfileForPath(host->GetProfilePath());
+ DCHECK(delegate_->ProfileExistsForPath(host->GetProfilePath()));
+ Profile* profile = delegate_->ProfileForPath(host->GetProfilePath());
- const ShellWindowList windows = GetWindows(profile, host->GetAppId());
+ const ShellWindowList windows =
+ delegate_->GetWindows(profile, host->GetAppId());
std::set<gfx::NativeWindow> native_windows;
for (ShellWindowList::const_iterator it = windows.begin();
it != windows.end(); ++it) {
@@ -140,43 +175,19 @@ void ExtensionAppShimHandler::OnShimFocus(Host* host) {
}
void ExtensionAppShimHandler::OnShimQuit(Host* host) {
- DCHECK(profile_manager_facade_->ProfileExistsForPath(
- host->GetProfilePath()));
- Profile* profile =
- profile_manager_facade_->ProfileForPath(host->GetProfilePath());
+ DCHECK(delegate_->ProfileExistsForPath(host->GetProfilePath()));
+ Profile* profile = delegate_->ProfileForPath(host->GetProfilePath());
- const ShellWindowList windows = GetWindows(profile, host->GetAppId());
+ const ShellWindowList windows =
+ delegate_->GetWindows(profile, host->GetAppId());
for (extensions::ShellWindowRegistry::const_iterator it = windows.begin();
it != windows.end(); ++it) {
(*it)->GetBaseWindow()->Close();
}
}
-void ExtensionAppShimHandler::set_profile_manager_facade(
- ProfileManagerFacade* profile_manager_facade) {
- profile_manager_facade_.reset(profile_manager_facade);
-}
-
-bool ExtensionAppShimHandler::LaunchApp(Profile* profile,
- const std::string& app_id,
- AppShimLaunchType launch_type) {
- const extensions::Extension* extension = GetExtension(profile, app_id);
- if (!extension) {
- LOG(ERROR) << "Attempted to launch nonexistent app with id '"
- << app_id << "'.";
- return false;
- }
-
- if (launch_type == APP_SHIM_LAUNCH_REGISTER_ONLY)
- return !GetWindows(profile, app_id).empty();
-
- // 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.
- chrome::OpenApplication(chrome::AppLaunchParams(
- profile, extension, NEW_FOREGROUND_TAB));
- return true;
+void ExtensionAppShimHandler::set_delegate(Delegate* delegate) {
+ delegate_.reset(delegate);
}
void ExtensionAppShimHandler::Observe(
@@ -213,15 +224,15 @@ void ExtensionAppShimHandler::OnAppStart(Profile* profile,
void ExtensionAppShimHandler::OnAppActivated(Profile* profile,
const std::string& app_id) {
- const extensions::Extension* extension = GetExtension(profile, app_id);
- if (!extension || !extension->is_platform_app())
+ const extensions::Extension* extension =
+ delegate_->GetAppExtension(profile, app_id);
+ if (!extension)
return;
- if (hosts_.count(make_pair(profile, extension->id())))
+ if (hosts_.count(make_pair(profile, app_id)))
return;
- web_app::MaybeLaunchShortcut(
- web_app::ShortcutInfoForExtensionAndProfile(extension, profile));
+ delegate_->LaunchShim(profile, extension);
}
void ExtensionAppShimHandler::OnAppDeactivated(Profile* profile,
« no previous file with comments | « apps/app_shim/extension_app_shim_handler_mac.h ('k') | apps/app_shim/extension_app_shim_handler_mac_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698