diff options
13 files changed, 235 insertions, 36 deletions
diff --git a/apps/app_lifetime_monitor.cc b/apps/app_lifetime_monitor.cc index 16323e9..3190e29 100644 --- a/apps/app_lifetime_monitor.cc +++ b/apps/app_lifetime_monitor.cc @@ -81,21 +81,24 @@ void AppLifetimeMonitor::Observe(int type, } void AppLifetimeMonitor::OnAppWindowRemoved(AppWindow* app_window) { - if (!HasVisibleAppWindows(app_window)) + if (!HasOtherVisibleAppWindows(app_window)) NotifyAppDeactivated(app_window->extension_id()); } void AppLifetimeMonitor::OnAppWindowHidden(AppWindow* app_window) { - if (!HasVisibleAppWindows(app_window)) + if (!HasOtherVisibleAppWindows(app_window)) NotifyAppDeactivated(app_window->extension_id()); } -void AppLifetimeMonitor::OnAppWindowShown(AppWindow* app_window) { +void AppLifetimeMonitor::OnAppWindowShown(AppWindow* app_window, + bool was_hidden) { if (app_window->window_type() != AppWindow::WINDOW_TYPE_DEFAULT) return; - if (HasVisibleAppWindows(app_window)) + // The app is being activated if this is the first window to become visible. + if (was_hidden && !HasOtherVisibleAppWindows(app_window)) { NotifyAppActivated(app_window->extension_id()); + } } void AppLifetimeMonitor::Shutdown() { @@ -106,7 +109,8 @@ void AppLifetimeMonitor::Shutdown() { app_window_registry->RemoveObserver(this); } -bool AppLifetimeMonitor::HasVisibleAppWindows(AppWindow* app_window) const { +bool AppLifetimeMonitor::HasOtherVisibleAppWindows( + AppWindow* app_window) const { AppWindowRegistry::AppWindowList windows = AppWindowRegistry::Get(app_window->browser_context()) ->GetAppWindowsForApp(app_window->extension_id()); @@ -114,7 +118,7 @@ bool AppLifetimeMonitor::HasVisibleAppWindows(AppWindow* app_window) const { for (AppWindowRegistry::AppWindowList::const_iterator i = windows.begin(); i != windows.end(); ++i) { - if (!(*i)->is_hidden()) + if (*i != app_window && !(*i)->is_hidden()) return true; } return false; diff --git a/apps/app_lifetime_monitor.h b/apps/app_lifetime_monitor.h index 8f079fe..90ae2a8 100644 --- a/apps/app_lifetime_monitor.h +++ b/apps/app_lifetime_monitor.h @@ -32,9 +32,11 @@ class AppLifetimeMonitor : public KeyedService, public: // Called when the app starts running. virtual void OnAppStart(Profile* profile, const std::string& app_id) {} - // Called when the app becomes active to the user, i.e. it opens a window. + // Called when the app becomes active to the user, i.e. the first window + // becomes visible. virtual void OnAppActivated(Profile* profile, const std::string& app_id) {} - // Called when the app becomes inactive to the user. + // Called when the app becomes inactive to the user, i.e. the last window is + // hidden or closed. virtual void OnAppDeactivated(Profile* profile, const std::string& app_id) { } // Called when the app stops running. @@ -63,12 +65,13 @@ class AppLifetimeMonitor : public KeyedService, // extensions::AppWindowRegistry::Observer overrides: void OnAppWindowRemoved(extensions::AppWindow* app_window) override; void OnAppWindowHidden(extensions::AppWindow* app_window) override; - void OnAppWindowShown(extensions::AppWindow* app_window) override; + void OnAppWindowShown(extensions::AppWindow* app_window, + bool was_hidden) override; // KeyedService overrides: void Shutdown() override; - bool HasVisibleAppWindows(extensions::AppWindow* app_window) const; + bool HasOtherVisibleAppWindows(extensions::AppWindow* app_window) const; void NotifyAppStart(const std::string& app_id); void NotifyAppActivated(const std::string& app_id); diff --git a/chrome/browser/apps/app_shim/app_shim_interactive_uitest_mac.mm b/chrome/browser/apps/app_shim/app_shim_interactive_uitest_mac.mm index 56db665..8df88e1 100644 --- a/chrome/browser/apps/app_shim/app_shim_interactive_uitest_mac.mm +++ b/chrome/browser/apps/app_shim/app_shim_interactive_uitest_mac.mm @@ -5,6 +5,7 @@ #import <Cocoa/Cocoa.h> #include <vector> +#include "apps/app_lifetime_monitor_factory.h" #include "apps/switches.h" #include "base/auto_reset.h" #include "base/callback.h" @@ -150,6 +151,37 @@ class WindowedAppShimLaunchObserver : public apps::AppShimHandler { DISALLOW_COPY_AND_ASSIGN(WindowedAppShimLaunchObserver); }; +class AppLifetimeMonitorObserver : public apps::AppLifetimeMonitor::Observer { + public: + AppLifetimeMonitorObserver(Profile* profile) + : profile_(profile), activated_count_(0), deactivated_count_(0) { + apps::AppLifetimeMonitorFactory::GetForProfile(profile_)->AddObserver(this); + } + virtual ~AppLifetimeMonitorObserver() { + apps::AppLifetimeMonitorFactory::GetForProfile(profile_) + ->RemoveObserver(this); + } + + int activated_count() { return activated_count_; } + int deactivated_count() { return deactivated_count_; } + + protected: + // AppLifetimeMonitor::Observer overrides: + void OnAppActivated(Profile* profile, const std::string& app_id) override { + ++activated_count_; + } + void OnAppDeactivated(Profile* profile, const std::string& app_id) override { + ++deactivated_count_; + } + + private: + Profile* profile_; + int activated_count_; + int deactivated_count_; + + DISALLOW_COPY_AND_ASSIGN(AppLifetimeMonitorObserver); +}; + NSString* GetBundleID(const base::FilePath& shim_path) { base::FilePath plist_path = shim_path.Append("Contents").Append("Info.plist"); NSMutableDictionary* plist = [NSMutableDictionary @@ -164,6 +196,30 @@ bool HasAppShimHost(Profile* profile, const std::string& app_id) { ->FindHost(profile, app_id); } +base::FilePath GetAppShimPath(Profile* profile, + const extensions::Extension* app) { + // Use a WebAppShortcutCreator to get the path. + web_app::WebAppShortcutCreator shortcut_creator( + web_app::GetWebAppDataDirectory(profile->GetPath(), app->id(), GURL()), + web_app::ShortcutInfoForExtensionAndProfile(app, profile), + extensions::FileHandlersInfo()); + return shortcut_creator.GetInternalShortcutPath(); +} + +void UpdateAppAndAwaitShimCreation(Profile* profile, + const extensions::Extension* app, + const base::FilePath& shim_path) { + // Create the internal app shim by simulating an app update. FilePathWatcher + // is used to wait for file operations on the shim to be finished before + // attempting to launch it. Since all of the file operations are done in the + // same event on the FILE thread, everything will be done by the time the + // watcher's callback is executed. + scoped_refptr<WindowedFilePathWatcher> file_watcher = + new WindowedFilePathWatcher(shim_path); + web_app::UpdateAllShortcuts(base::string16(), profile, app); + file_watcher->Wait(); +} + } // namespace // Watches for NSNotifications from the shared workspace. @@ -224,9 +280,11 @@ namespace apps { // Shims require static libraries http://crbug.com/386024. #if defined(COMPONENT_BUILD) #define MAYBE_Launch DISABLED_Launch +#define MAYBE_ShowWindow DISABLED_ShowWindow #define MAYBE_RebuildShim DISABLED_RebuildShim #else #define MAYBE_Launch Launch +#define MAYBE_ShowWindow ShowWindow #define MAYBE_RebuildShim RebuildShim #endif @@ -234,26 +292,12 @@ namespace apps { // These two cases are combined because the time to run the test is dominated // by loading the extension and creating the shim. IN_PROC_BROWSER_TEST_F(AppShimInteractiveTest, MAYBE_Launch) { - // Install the app. const extensions::Extension* app = InstallPlatformApp("minimal"); - // Use a WebAppShortcutCreator to get the path. - web_app::WebAppShortcutCreator shortcut_creator( - web_app::GetWebAppDataDirectory(profile()->GetPath(), app->id(), GURL()), - web_app::ShortcutInfoForExtensionAndProfile(app, profile()), - extensions::FileHandlersInfo()); - base::FilePath shim_path = shortcut_creator.GetInternalShortcutPath(); + base::FilePath shim_path = GetAppShimPath(profile(), app); EXPECT_FALSE(base::PathExists(shim_path)); - // Create the internal app shim by simulating an app update. FilePathWatcher - // is used to wait for file operations on the shim to be finished before - // attempting to launch it. Since all of the file operations are done in the - // same event on the FILE thread, everything will be done by the time the - // watcher's callback is executed. - scoped_refptr<WindowedFilePathWatcher> file_watcher = - new WindowedFilePathWatcher(shim_path); - web_app::UpdateAllShortcuts(base::string16(), profile(), app); - file_watcher->Wait(); + UpdateAppAndAwaitShimCreation(profile(), app, shim_path); ASSERT_TRUE(base::PathExists(shim_path)); NSString* bundle_id = GetBundleID(shim_path); @@ -316,6 +360,125 @@ IN_PROC_BROWSER_TEST_F(AppShimInteractiveTest, MAYBE_Launch) { } } +// Test that the shim's lifetime depends on the visibility of windows. I.e. the +// shim is only active when there are visible windows. +IN_PROC_BROWSER_TEST_F(AppShimInteractiveTest, MAYBE_ShowWindow) { + const extensions::Extension* app = InstallPlatformApp("hidden"); + + base::FilePath shim_path = GetAppShimPath(profile(), app); + EXPECT_FALSE(base::PathExists(shim_path)); + + UpdateAppAndAwaitShimCreation(profile(), app, shim_path); + ASSERT_TRUE(base::PathExists(shim_path)); + NSString* bundle_id = GetBundleID(shim_path); + + // It's impractical to confirm that the shim did not launch by timing out, so + // instead we watch AppLifetimeMonitor::Observer::OnAppActivated. + AppLifetimeMonitorObserver lifetime_observer(profile()); + + // Launch the app. It should create a hidden window, but the shim should not + // launch. + { + ExtensionTestMessageListener launched_listener("Launched", false); + LaunchPlatformApp(app); + EXPECT_TRUE(launched_listener.WaitUntilSatisfied()); + } + extensions::AppWindow* window_1 = GetFirstAppWindow(); + ASSERT_TRUE(window_1); + EXPECT_TRUE(window_1->is_hidden()); + EXPECT_FALSE(HasAppShimHost(profile(), app->id())); + EXPECT_EQ(0, lifetime_observer.activated_count()); + + // Showing the window causes the shim to launch. + { + base::scoped_nsobject<WindowedNSNotificationObserver> ns_observer( + [[WindowedNSNotificationObserver alloc] + initForNotification:NSWorkspaceDidLaunchApplicationNotification + andBundleId:bundle_id]); + WindowedAppShimLaunchObserver observer(app->id()); + window_1->Show(extensions::AppWindow::SHOW_INACTIVE); + [ns_observer wait]; + observer.Wait(); + EXPECT_EQ(1, lifetime_observer.activated_count()); + EXPECT_TRUE(HasAppShimHost(profile(), app->id())); + } + + // Hiding the window causes the shim to quit. + { + base::scoped_nsobject<WindowedNSNotificationObserver> ns_observer( + [[WindowedNSNotificationObserver alloc] + initForNotification:NSWorkspaceDidTerminateApplicationNotification + andBundleId:bundle_id]); + window_1->Hide(); + [ns_observer wait]; + EXPECT_FALSE(HasAppShimHost(profile(), app->id())); + } + + // Launch a second window. It should not launch the shim. + { + ExtensionTestMessageListener launched_listener("Launched", false); + LaunchPlatformApp(app); + EXPECT_TRUE(launched_listener.WaitUntilSatisfied()); + } + const extensions::AppWindowRegistry::AppWindowList& app_windows = + extensions::AppWindowRegistry::Get(profile())->app_windows(); + EXPECT_EQ(2u, app_windows.size()); + extensions::AppWindow* window_2 = app_windows.front(); + EXPECT_NE(window_1, window_2); + ASSERT_TRUE(window_2); + EXPECT_TRUE(window_2->is_hidden()); + EXPECT_FALSE(HasAppShimHost(profile(), app->id())); + EXPECT_EQ(1, lifetime_observer.activated_count()); + + // Showing one of the windows should launch the shim. + { + base::scoped_nsobject<WindowedNSNotificationObserver> ns_observer( + [[WindowedNSNotificationObserver alloc] + initForNotification:NSWorkspaceDidLaunchApplicationNotification + andBundleId:bundle_id]); + WindowedAppShimLaunchObserver observer(app->id()); + window_1->Show(extensions::AppWindow::SHOW_INACTIVE); + [ns_observer wait]; + observer.Wait(); + EXPECT_EQ(2, lifetime_observer.activated_count()); + EXPECT_TRUE(HasAppShimHost(profile(), app->id())); + EXPECT_TRUE(window_2->is_hidden()); + } + + // Showing the other window does nothing. + { + window_2->Show(extensions::AppWindow::SHOW_INACTIVE); + EXPECT_EQ(2, lifetime_observer.activated_count()); + } + + // Showing an already visible window does nothing. + { + window_1->Show(extensions::AppWindow::SHOW_INACTIVE); + EXPECT_EQ(2, lifetime_observer.activated_count()); + } + + // Hiding one window does nothing. + { + AppLifetimeMonitorObserver deactivate_observer(profile()); + window_1->Hide(); + EXPECT_EQ(0, deactivate_observer.deactivated_count()); + } + + // Hiding other window causes the shim to quit. + { + AppLifetimeMonitorObserver deactivate_observer(profile()); + EXPECT_TRUE(HasAppShimHost(profile(), app->id())); + base::scoped_nsobject<WindowedNSNotificationObserver> ns_observer( + [[WindowedNSNotificationObserver alloc] + initForNotification:NSWorkspaceDidTerminateApplicationNotification + andBundleId:bundle_id]); + window_2->Hide(); + [ns_observer wait]; + EXPECT_EQ(1, deactivate_observer.deactivated_count()); + EXPECT_FALSE(HasAppShimHost(profile(), app->id())); + } +} + #if defined(ARCH_CPU_64_BITS) // Tests that a 32 bit shim attempting to launch 64 bit Chrome will eventually diff --git a/chrome/browser/ui/ash/launcher/app_window_launcher_controller.cc b/chrome/browser/ui/ash/launcher/app_window_launcher_controller.cc index 9113e3b..f07f122 100644 --- a/chrome/browser/ui/ash/launcher/app_window_launcher_controller.cc +++ b/chrome/browser/ui/ash/launcher/app_window_launcher_controller.cc @@ -99,7 +99,8 @@ void AppWindowLauncherController::OnAppWindowIconChanged( app_window->app_icon().AsImageSkia()); } -void AppWindowLauncherController::OnAppWindowShown(AppWindow* app_window) { +void AppWindowLauncherController::OnAppWindowShown(AppWindow* app_window, + bool was_hidden) { aura::Window* window = app_window->GetNativeWindow(); if (!ControlsWindow(window)) return; diff --git a/chrome/browser/ui/ash/launcher/app_window_launcher_controller.h b/chrome/browser/ui/ash/launcher/app_window_launcher_controller.h index 8f2ca87..4ff452e 100644 --- a/chrome/browser/ui/ash/launcher/app_window_launcher_controller.h +++ b/chrome/browser/ui/ash/launcher/app_window_launcher_controller.h @@ -51,7 +51,8 @@ class AppWindowLauncherController // Overridden from AppWindowRegistry::Observer: void OnAppWindowIconChanged(extensions::AppWindow* app_window) override; - void OnAppWindowShown(extensions::AppWindow* app_window) override; + void OnAppWindowShown(extensions::AppWindow* app_window, + bool was_hidden) override; void OnAppWindowHidden(extensions::AppWindow* app_window) override; // Overriden from aura::WindowObserver: diff --git a/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.cc b/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.cc index 95c6f41..00e8c73 100644 --- a/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.cc +++ b/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.cc @@ -91,7 +91,8 @@ void MultiProfileAppWindowLauncherController::OnAppWindowAdded( } void MultiProfileAppWindowLauncherController::OnAppWindowShown( - extensions::AppWindow* app_window) { + extensions::AppWindow* app_window, + bool was_hidden) { if (!ControlsWindow(app_window->GetNativeWindow())) return; diff --git a/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.h b/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.h index 9e30906..d998ab6 100644 --- a/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.h +++ b/chrome/browser/ui/ash/launcher/multi_profile_app_window_launcher_controller.h @@ -23,7 +23,8 @@ class MultiProfileAppWindowLauncherController // Overridden from AppWindowRegistry::Observer: void OnAppWindowAdded(extensions::AppWindow* app_window) override; void OnAppWindowRemoved(extensions::AppWindow* app_window) override; - void OnAppWindowShown(extensions::AppWindow* app_window) override; + void OnAppWindowShown(extensions::AppWindow* app_window, + bool was_hidden) override; void OnAppWindowHidden(extensions::AppWindow* app_window) override; private: diff --git a/chrome/test/data/extensions/platform_apps/hidden/empty.html b/chrome/test/data/extensions/platform_apps/hidden/empty.html new file mode 100644 index 0000000..aabcd1b --- /dev/null +++ b/chrome/test/data/extensions/platform_apps/hidden/empty.html @@ -0,0 +1 @@ +<!-- This file intentionally left blank. --> diff --git a/chrome/test/data/extensions/platform_apps/hidden/manifest.json b/chrome/test/data/extensions/platform_apps/hidden/manifest.json new file mode 100644 index 0000000..71a83d3 --- /dev/null +++ b/chrome/test/data/extensions/platform_apps/hidden/manifest.json @@ -0,0 +1,10 @@ +{ + "name": "Platform App Hidden", + "version": "1", + "manifest_version": 2, + "app": { + "background": { + "scripts": ["test.js"] + } + } +} diff --git a/chrome/test/data/extensions/platform_apps/hidden/test.js b/chrome/test/data/extensions/platform_apps/hidden/test.js new file mode 100644 index 0000000..c110227 --- /dev/null +++ b/chrome/test/data/extensions/platform_apps/hidden/test.js @@ -0,0 +1,11 @@ +// Copyright 2014 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. + +chrome.app.runtime.onLaunched.addListener(function() { + chrome.app.window.create('main.html', { + hidden: true, + }, function () { + chrome.test.sendMessage('Launched'); + }); +});
\ No newline at end of file diff --git a/extensions/browser/app_window/app_window.cc b/extensions/browser/app_window/app_window.cc index c7990e1..8ce9cf6 100644 --- a/extensions/browser/app_window/app_window.cc +++ b/extensions/browser/app_window/app_window.cc @@ -656,6 +656,7 @@ void AppWindow::SetContentSizeConstraints(const gfx::Size& min_size, } void AppWindow::Show(ShowType show_type) { + bool was_hidden = is_hidden_ || !has_been_shown_; is_hidden_ = false; if (CommandLine::ForCurrentProcess()->HasSwitch( @@ -676,7 +677,7 @@ void AppWindow::Show(ShowType show_type) { GetBaseWindow()->ShowInactive(); break; } - AppWindowRegistry::Get(browser_context_)->AppWindowShown(this); + AppWindowRegistry::Get(browser_context_)->AppWindowShown(this, was_hidden); has_been_shown_ = true; SendOnWindowShownIfShown(); diff --git a/extensions/browser/app_window/app_window_registry.cc b/extensions/browser/app_window/app_window_registry.cc index 48397f5..572c4b2 100644 --- a/extensions/browser/app_window/app_window_registry.cc +++ b/extensions/browser/app_window/app_window_registry.cc @@ -60,7 +60,8 @@ void AppWindowRegistry::Observer::OnAppWindowRemoved(AppWindow* app_window) { void AppWindowRegistry::Observer::OnAppWindowHidden(AppWindow* app_window) { } -void AppWindowRegistry::Observer::OnAppWindowShown(AppWindow* app_window) { +void AppWindowRegistry::Observer::OnAppWindowShown(AppWindow* app_window, + bool was_shown) { } AppWindowRegistry::Observer::~Observer() { @@ -100,8 +101,9 @@ void AppWindowRegistry::AppWindowHidden(AppWindow* app_window) { FOR_EACH_OBSERVER(Observer, observers_, OnAppWindowHidden(app_window)); } -void AppWindowRegistry::AppWindowShown(AppWindow* app_window) { - FOR_EACH_OBSERVER(Observer, observers_, OnAppWindowShown(app_window)); +void AppWindowRegistry::AppWindowShown(AppWindow* app_window, bool was_hidden) { + FOR_EACH_OBSERVER(Observer, observers_, + OnAppWindowShown(app_window, was_hidden)); } void AppWindowRegistry::RemoveAppWindow(AppWindow* app_window) { diff --git a/extensions/browser/app_window/app_window_registry.h b/extensions/browser/app_window/app_window_registry.h index 7037953..ed2cdc6 100644 --- a/extensions/browser/app_window/app_window_registry.h +++ b/extensions/browser/app_window/app_window_registry.h @@ -44,7 +44,7 @@ class AppWindowRegistry : public KeyedService { // it not visible. virtual void OnAppWindowHidden(AppWindow* app_window); // Called just after a app window was shown. - virtual void OnAppWindowShown(AppWindow* app_window); + virtual void OnAppWindowShown(AppWindow* app_window, bool was_hidden); protected: virtual ~Observer(); @@ -67,7 +67,7 @@ class AppWindowRegistry : public KeyedService { // Called by |app_window| when it is activated. void AppWindowActivated(AppWindow* app_window); void AppWindowHidden(AppWindow* app_window); - void AppWindowShown(AppWindow* app_window); + void AppWindowShown(AppWindow* app_window, bool was_hidden); void RemoveAppWindow(AppWindow* app_window); void AddObserver(Observer* observer); |