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

Unified Diff: chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc

Issue 83173007: Fixing M32 crasher where closing of a windowed V1 app on a visiting desktop will cause crashes late… (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Addressed Created 7 years, 1 month 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
« no previous file with comments | « no previous file | chrome/browser/ui/ash/launcher/multi_profile_browser_status_monitor.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
diff --git a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
index 87a5e6a867cb8b3665c169fec2e46209b143a0bb..a9037023dd763c5dd9ff46ccba32df063b3f9e02 100644
--- a/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
+++ b/chrome/browser/ui/ash/launcher/chrome_launcher_controller_unittest.cc
@@ -77,15 +77,13 @@ using extensions::UnloadedExtensionInfo;
namespace {
const char* offline_gmail_url = "https://mail.google.com/mail/mu/u";
const char* gmail_url = "https://mail.google.com/mail/u";
+const char* kGmailLaunchURL = "https://mail.google.com/mail/ca";
// As defined in /chromeos/dbus/cryptohome_client.cc.
const char kUserIdHashSuffix[] = "-hash";
// An extension prefix.
const char kCrxAppPrefix[] = "_crx_";
-}
-
-namespace {
// ShelfModelObserver implementation that tracks what messages are invoked.
class TestShelfModelObserver : public ash::ShelfModelObserver {
@@ -314,7 +312,7 @@ class ChromeLauncherControllerTest : public BrowserWithTestWindowTest {
manifest_gmail.SetString(extensions::manifest_keys::kDescription,
"for testing pinned Gmail");
manifest_gmail.SetString(extensions::manifest_keys::kLaunchWebURL,
- "https://mail.google.com/mail/ca");
+ kGmailLaunchURL);
ListValue* list = new ListValue();
list->Append(Value::CreateStringValue("*://mail.google.com/mail/ca"));
manifest_gmail.Set(extensions::manifest_keys::kWebURLs, list);
@@ -721,16 +719,24 @@ class WebContentsDestroyedWatcher : public content::WebContentsObserver {
};
// A V1 windowed application.
-class V1App {
+class V1App : public TestBrowserWindow {
public:
V1App(Profile* profile, const std::string& app_name) {
+ // Create a window.
+ native_window_.reset(new aura::Window(NULL));
+ native_window_->set_id(0);
+ native_window_->SetType(aura::client::WINDOW_TYPE_POPUP);
+ native_window_->Init(ui::LAYER_TEXTURED);
+ native_window_->Show();
+
Browser::CreateParams params = Browser::CreateParams::CreateForApp(
Browser::TYPE_POPUP,
kCrxAppPrefix + app_name,
gfx::Rect(),
profile,
chrome::HOST_DESKTOP_TYPE_ASH);
- browser_.reset(chrome::CreateBrowserWithTestWindowForParams(&params));
+ params.window = this;
+ browser_.reset(new Browser(params));
chrome::AddTabAt(browser_.get(), GURL(), 0, true);
}
@@ -741,10 +747,18 @@ class V1App {
Browser* browser() { return browser_.get(); }
+ // TestBrowserWindow override:
+ virtual gfx::NativeWindow GetNativeWindow() OVERRIDE {
+ return native_window_.get();
+ }
+
private:
// The associated browser with this app.
scoped_ptr<Browser> browser_;
+ // The native window we use.
+ scoped_ptr<aura::Window> native_window_;
+
DISALLOW_COPY_AND_ASSIGN(V1App);
};
@@ -887,10 +901,22 @@ class MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest
}
// Creates a running V1 application.
+ // Note that with the use of the app_tab_helper as done below, this is only
+ // usable with a single v1 application.
V1App* CreateRunningV1App(Profile* profile,
const std::string& app_name,
const std::string& url) {
V1App* v1_app = new V1App(profile, app_name);
+ // Create a new app tab helper and assign it to the launcher so that this
+ // app gets properly detected.
+ // TODO(skuhne): Create a more intelligent app tab helper which is able to
+ // detect all running apps properly.
+ TestAppTabHelperImpl* app_tab_helper = new TestAppTabHelperImpl;
+ app_tab_helper->SetAppID(
+ v1_app->browser()->tab_strip_model()->GetWebContentsAt(0),
+ app_name);
+ SetAppTabHelper(app_tab_helper);
+
NavigateAndCommitActiveTabWithTitle(
v1_app->browser(), GURL(url), ASCIIToUTF16(""));
return v1_app;
@@ -1508,6 +1534,85 @@ TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,
EXPECT_EQ(2, model_->item_count());
}
+// Check edge case where a visiting V1 app gets closed (crbug.com/321374).
+TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,
+ V1CloseOnVisitingDesktop) {
+ // Create a browser item in the LauncherController.
+ InitLauncherController();
+
+ chrome::MultiUserWindowManager* manager =
+ chrome::MultiUserWindowManager::GetInstance();
+
+ // First create an app when the user is active.
+ std::string user2 = "user2";
+ TestingProfile* profile2 = CreateMultiUserProfile(user2);
+ {
+ // Create a "windowed gmail app".
+ scoped_ptr<V1App> v1_app(CreateRunningV1App(
+ profile(),
+ extension_misc::kGmailAppId,
+ kGmailLaunchURL));
+ EXPECT_EQ(3, model_->item_count());
+
+ // Transfer the app to the other screen and switch users.
+ manager->ShowWindowForUser(v1_app->browser()->window()->GetNativeWindow(),
+ user2);
+ EXPECT_EQ(3, model_->item_count());
+ SwitchActiveUser(profile2->GetProfileName());
+ EXPECT_EQ(2, model_->item_count());
+ }
+ // After the app was destroyed, switch back. (which caused already a crash).
+ SwitchActiveUser(profile()->GetProfileName());
+
+ // Create the same app again - which was also causing the crash.
+ EXPECT_EQ(2, model_->item_count());
+ {
+ // Create a "windowed gmail app".
+ scoped_ptr<V1App> v1_app(CreateRunningV1App(
+ profile(),
+ extension_misc::kGmailAppId,
+ kGmailLaunchURL));
+ EXPECT_EQ(3, model_->item_count());
+ }
+ SwitchActiveUser(profile2->GetProfileName());
+ EXPECT_EQ(2, model_->item_count());
+}
+
+// Check edge cases with multi profile V1 apps in the shelf.
+TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,
+ V1AppUpdateOnUserSwitchEdgecases2) {
+ // Create a browser item in the LauncherController.
+ InitLauncherController();
+ TestAppTabHelperImpl* app_tab_helper = new TestAppTabHelperImpl;
+ SetAppTabHelper(app_tab_helper);
+
+ // First test: Create an app when the user is not active.
+ std::string user2 = "user2";
+ TestingProfile* profile2 = CreateMultiUserProfile(user2);
+ SwitchActiveUser(profile2->GetProfileName());
+ {
+ // Create a "windowed gmail app".
+ scoped_ptr<V1App> v1_app(CreateRunningV1App(
+ profile(), extension_misc::kGmailAppId, gmail_url));
+ EXPECT_EQ(2, model_->item_count());
+
+ // However - switching to the user should show it.
+ SwitchActiveUser(profile()->GetProfileName());
+ EXPECT_EQ(3, model_->item_count());
+
+ // Second test: Remove the app when the user is not active and see that it
+ // works.
+ SwitchActiveUser(profile2->GetProfileName());
+ EXPECT_EQ(2, model_->item_count());
+ v1_app.reset();
+ }
+ EXPECT_EQ(2, model_->item_count());
+ SwitchActiveUser(profile()->GetProfileName());
+ EXPECT_EQ(2, model_->item_count());
+ SwitchActiveUser(profile2->GetProfileName());
+ EXPECT_EQ(2, model_->item_count());
+}
+
// Check that activating an item which is on another user's desktop, will bring
// it back.
TEST_F(MultiProfileMultiBrowserShelfLayoutChromeLauncherControllerTest,
« no previous file with comments | « no previous file | chrome/browser/ui/ash/launcher/multi_profile_browser_status_monitor.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698