diff options
author | jackhou@chromium.org <jackhou@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-17 00:26:22 +0000 |
---|---|---|
committer | jackhou@chromium.org <jackhou@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-17 00:26:22 +0000 |
commit | 876ec516ddd2566537eefd2e5029d6f228385d08 (patch) | |
tree | 3dd861bd8d9a645a3f91d06ebabd36020811552b /apps/app_shim | |
parent | c9feb5fdfc7df5f0e30710b58b95a887ac27125d (diff) | |
download | chromium_src-876ec516ddd2566537eefd2e5029d6f228385d08.zip chromium_src-876ec516ddd2566537eefd2e5029d6f228385d08.tar.gz chromium_src-876ec516ddd2566537eefd2e5029d6f228385d08.tar.bz2 |
Use ExtensionEnableFlow for shim initiated launches. (Mac)
Currently when launching a shim, if the extension is not enabled, the
shim quits immediately.
This CL uses the ExtensionEnableFlow which will enable terminated apps,
prompt for escalated permissions, or fail if the app is not installed.
BUG=269151
Review URL: https://codereview.chromium.org/113553002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@241084 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'apps/app_shim')
-rw-r--r-- | apps/app_shim/DEPS | 2 | ||||
-rw-r--r-- | apps/app_shim/extension_app_shim_handler_mac.cc | 110 | ||||
-rw-r--r-- | apps/app_shim/extension_app_shim_handler_mac.h | 9 | ||||
-rw-r--r-- | apps/app_shim/extension_app_shim_handler_mac_unittest.cc | 25 |
4 files changed, 130 insertions, 16 deletions
diff --git a/apps/app_shim/DEPS b/apps/app_shim/DEPS index 8d51bf5..6c77a25 100644 --- a/apps/app_shim/DEPS +++ b/apps/app_shim/DEPS @@ -5,6 +5,8 @@ include_rules = [ "+chrome/browser/ui/app_list/app_list_service.h", "+chrome/browser/ui/app_list/app_list_util.h", "+chrome/browser/ui/extensions/application_launch.h", + "+chrome/browser/ui/extensions/extension_enable_flow.h", + "+chrome/browser/ui/extensions/extension_enable_flow_delegate.h", "+chrome/browser/ui/web_applications/web_app_ui.h", "+chrome/browser/ui/webui/ntp/core_app_launcher_handler.h", "+chrome/browser/web_applications/web_app_mac.h", diff --git a/apps/app_shim/extension_app_shim_handler_mac.cc b/apps/app_shim/extension_app_shim_handler_mac.cc index 3c08f2d..fdb639b 100644 --- a/apps/app_shim/extension_app_shim_handler_mac.cc +++ b/apps/app_shim/extension_app_shim_handler_mac.cc @@ -20,6 +20,8 @@ #include "chrome/browser/extensions/extension_system.h" #include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile_manager.h" +#include "chrome/browser/ui/extensions/extension_enable_flow.h" +#include "chrome/browser/ui/extensions/extension_enable_flow_delegate.h" #include "chrome/browser/ui/web_applications/web_app_ui.h" #include "chrome/browser/ui/webui/ntp/core_app_launcher_handler.h" #include "chrome/browser/web_applications/web_app_mac.h" @@ -73,6 +75,48 @@ bool FocusWindows(const ShellWindowList& 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), + callback_(callback) { + } + + virtual ~EnableViaPrompt() { + } + + void Run() { + flow_.reset(new ExtensionEnableFlow(profile_, extension_id_, this)); + flow_->StartForCurrentlyNonexistentWindow( + base::Callback<gfx::NativeWindow(void)>()); + } + + private: + // ExtensionEnableFlowDelegate overrides. + virtual void ExtensionEnableFlowFinished() OVERRIDE { + callback_.Run(); + delete this; + } + + virtual void ExtensionEnableFlowAborted(bool user_initiated) OVERRIDE { + callback_.Run(); + delete this; + } + + Profile* profile_; + std::string extension_id_; + base::Callback<void()> callback_; + scoped_ptr<ExtensionEnableFlow> flow_; + + DISALLOW_COPY_AND_ASSIGN(EnableViaPrompt); +}; + } // namespace namespace apps { @@ -126,6 +170,13 @@ ExtensionAppShimHandler::Delegate::GetAppExtension( return extension && extension->is_platform_app() ? extension : NULL; } +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( Profile* profile, const extensions::Extension* extension, @@ -286,15 +337,6 @@ void ExtensionAppShimHandler::OnProfileLoaded( const std::vector<base::FilePath>& files, Profile* profile) { const std::string& app_id = host->GetAppId(); - // TODO(jackhou): Add some UI for this case and remove the LOG. - const extensions::Extension* extension = - delegate_->GetAppExtension(profile, app_id); - if (!extension) { - LOG(ERROR) << "Attempted to launch nonexistent app with id '" - << app_id << "'."; - host->OnAppLaunchComplete(APP_SHIM_LAUNCH_APP_NOT_FOUND); - return; - } // The first host to claim this (profile, app_id) becomes the main host. // For any others, focus or relaunch the app. @@ -307,16 +349,54 @@ void ExtensionAppShimHandler::OnProfileLoaded( return; } + if (launch_type != APP_SHIM_LAUNCH_NORMAL) { + host->OnAppLaunchComplete(APP_SHIM_LAUNCH_SUCCESS); + return; + } + // 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) + const extensions::Extension* extension = + delegate_->GetAppExtension(profile, app_id); + if (extension) { delegate_->LaunchApp(profile, extension, files); - else - host->OnAppLaunchComplete(APP_SHIM_LAUNCH_SUCCESS); + return; + } + + delegate_->EnableExtension( + profile, app_id, + base::Bind(&ExtensionAppShimHandler::OnExtensionEnabled, + weak_factory_.GetWeakPtr(), + host->GetProfilePath(), app_id, files)); +} + +void ExtensionAppShimHandler::OnExtensionEnabled( + const base::FilePath& profile_path, + const std::string& app_id, + const std::vector<base::FilePath>& files) { + Profile* profile = delegate_->ProfileForPath(profile_path); + if (!profile) + return; + + const extensions::Extension* extension = + delegate_->GetAppExtension(profile, app_id); + if (!extension || !delegate_->ProfileExistsForPath(profile_path)) { + // If !extension, the extension doesn't exist, or was not re-enabled. + // If the profile doesn't exist, it may have been deleted during the enable + // prompt. In this case, NOTIFICATION_PROFILE_DESTROYED may not be fired + // until later, so respond to the host now. + Host* host = FindHost(profile, app_id); + if (host) + host->OnAppLaunchComplete(APP_SHIM_LAUNCH_APP_NOT_FOUND); + return; + } + + delegate_->LaunchApp(profile, extension, files); } + void ExtensionAppShimHandler::OnShimClose(Host* host) { // This might be called when shutting down. Don't try to look up the profile // since profile_manager might not be around. @@ -400,8 +480,10 @@ void ExtensionAppShimHandler::Observe( // 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)) - current->second->OnAppClosed(); + if (profile->IsSameProfile(current->first.first)) { + Host* host = current->second; + host->OnAppClosed(); + } } break; } diff --git a/apps/app_shim/extension_app_shim_handler_mac.h b/apps/app_shim/extension_app_shim_handler_mac.h index 140fc02..c8069ce2 100644 --- a/apps/app_shim/extension_app_shim_handler_mac.h +++ b/apps/app_shim/extension_app_shim_handler_mac.h @@ -55,6 +55,9 @@ class ExtensionAppShimHandler : public AppShimHandler, virtual const extensions::Extension* GetAppExtension( Profile* profile, const std::string& extension_id); + virtual void EnableExtension(Profile* profile, + const std::string& extension_id, + const base::Callback<void()>& callback); virtual void LaunchApp(Profile* profile, const extensions::Extension* extension, const std::vector<base::FilePath>& files); @@ -121,6 +124,12 @@ class ExtensionAppShimHandler : public AppShimHandler, const std::vector<base::FilePath>& files, Profile* profile); + // This is passed to Delegate::EnableViaPrompt for shim-initiated launches + // where the extension is disabled. + void OnExtensionEnabled(const base::FilePath& profile_path, + const std::string& app_id, + const std::vector<base::FilePath>& files); + scoped_ptr<Delegate> delegate_; HostMap hosts_; diff --git a/apps/app_shim/extension_app_shim_handler_mac_unittest.cc b/apps/app_shim/extension_app_shim_handler_mac_unittest.cc index e4f5cb8..4e528f3 100644 --- a/apps/app_shim/extension_app_shim_handler_mac_unittest.cc +++ b/apps/app_shim/extension_app_shim_handler_mac_unittest.cc @@ -23,6 +23,7 @@ typedef ShellWindowRegistry::ShellWindowList ShellWindowList; using ::testing::_; using ::testing::Invoke; using ::testing::Return; +using ::testing::WithArgs; class MockDelegate : public ExtensionAppShimHandler::Delegate { public: @@ -37,6 +38,9 @@ class MockDelegate : public ExtensionAppShimHandler::Delegate { MOCK_METHOD2(GetWindows, ShellWindowList(Profile*, const std::string&)); MOCK_METHOD2(GetAppExtension, const Extension*(Profile*, const std::string&)); + MOCK_METHOD3(EnableExtension, void(Profile*, + const std::string&, + const base::Callback<void()>&)); MOCK_METHOD3(LaunchApp, void(Profile*, const Extension*, @@ -58,6 +62,10 @@ class MockDelegate : public ExtensionAppShimHandler::Delegate { return callbacks_.erase(path); } + void RunCallback(const base::Callback<void()>& callback) { + callback.Run(); + } + private: std::map<base::FilePath, base::Callback<void(Profile*)> > callbacks_; @@ -209,19 +217,32 @@ class ExtensionAppShimHandlerTest : public testing::Test { DISALLOW_COPY_AND_ASSIGN(ExtensionAppShimHandlerTest); }; -TEST_F(ExtensionAppShimHandlerTest, LaunchFailure) { +TEST_F(ExtensionAppShimHandlerTest, LaunchProfileNotFound) { // Bad profile path. EXPECT_CALL(*delegate_, ProfileExistsForPath(profile_path_a_)) .WillOnce(Return(false)) .WillRepeatedly(Return(true)); EXPECT_CALL(host_aa_, OnAppLaunchComplete(APP_SHIM_LAUNCH_PROFILE_NOT_FOUND)); NormalLaunch(&host_aa_); +} + +TEST_F(ExtensionAppShimHandlerTest, LaunchAppNotFound) { + // App not found. + EXPECT_CALL(*delegate_, GetAppExtension(&profile_a_, kTestAppIdA)) + .WillRepeatedly(Return(static_cast<const Extension*>(NULL))); + EXPECT_CALL(*delegate_, EnableExtension(&profile_a_, kTestAppIdA, _)) + .WillOnce(WithArgs<2>(Invoke(delegate_, &MockDelegate::RunCallback))); + EXPECT_CALL(host_aa_, OnAppLaunchComplete(APP_SHIM_LAUNCH_APP_NOT_FOUND)); + NormalLaunch(&host_aa_); +} +TEST_F(ExtensionAppShimHandlerTest, LaunchAppNotEnabled) { // App not found. EXPECT_CALL(*delegate_, GetAppExtension(&profile_a_, kTestAppIdA)) .WillOnce(Return(static_cast<const Extension*>(NULL))) .WillRepeatedly(Return(extension_a_.get())); - EXPECT_CALL(host_aa_, OnAppLaunchComplete(APP_SHIM_LAUNCH_APP_NOT_FOUND)); + EXPECT_CALL(*delegate_, EnableExtension(&profile_a_, kTestAppIdA, _)) + .WillOnce(WithArgs<2>(Invoke(delegate_, &MockDelegate::RunCallback))); NormalLaunch(&host_aa_); } |