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

Unified Diff: chrome/browser/ui/panels/panel_app_browsertest.cc

Issue 7694014: Reland: Implement correct Panel closing sequence for Mac. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fixed random result of CreateClose unittest caused by using a freed object. Created 9 years, 4 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
« no previous file with comments | « chrome/browser/ui/panels/panel.cc ('k') | chrome/browser/ui/panels/panel_browser_window_cocoa.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/ui/panels/panel_app_browsertest.cc
diff --git a/chrome/browser/ui/panels/panel_app_browsertest.cc b/chrome/browser/ui/panels/panel_app_browsertest.cc
index 85bc6642878debfb067ef2c40cb86e267989f9a4..03f696b1a42d6edc3183ca2accf6c54e47b54cab 100644
--- a/chrome/browser/ui/panels/panel_app_browsertest.cc
+++ b/chrome/browser/ui/panels/panel_app_browsertest.cc
@@ -5,6 +5,7 @@
#include "base/command_line.h"
#include "base/file_path.h"
+#include "base/mac/scoped_nsautorelease_pool.h"
#include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/profiles/profile.h"
@@ -15,6 +16,7 @@
#include "chrome/browser/ui/panels/panel_manager.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
#include "testing/gtest/include/gtest/gtest.h"
class PanelAppBrowserTest : public ExtensionBrowserTest {
@@ -25,18 +27,45 @@ class PanelAppBrowserTest : public ExtensionBrowserTest {
}
void LoadAndLaunchExtension(const char* name) {
- ASSERT_TRUE(LoadExtension(test_data_dir_.AppendASCII(name)));
+ // Opening panels on a Mac causes NSWindowController of the Panel window
+ // to be autoreleased. We need a pool drained after it's done so the test
+ // can close correctly. The NSWindowController of the Panel window controls
+ // lifetime of the Browser object so we want to release it as soon as
+ // possible. In real Chrome, this is done by message pump.
+ // On non-Mac platform, this is an empty class.
+ base::mac::ScopedNSAutoreleasePool autorelease_pool;
+
+ EXPECT_TRUE(LoadExtension(test_data_dir_.AppendASCII(name)));
ExtensionService* service = browser()->profile()->GetExtensionService();
const Extension* extension = service->GetExtensionById(
last_loaded_extension_id_, false);
- ASSERT_TRUE(extension);
+ EXPECT_TRUE(extension);
+
+ size_t browser_count = BrowserList::size();
Browser::OpenApplication(
browser()->profile(),
extension,
- extension_misc::LAUNCH_PANEL, // Override manifest, open in panel.
+ // Overriding manifest to open in a panel.
+ extension_misc::LAUNCH_PANEL,
NEW_WINDOW);
+
+ // Now we have a new browser instance.
+ EXPECT_EQ(browser_count + 1, BrowserList::size());
+ }
+
+ void CloseWindowAndWait(Browser* browser) {
+ // Closing a browser window may involve several async tasks. Need to use
+ // message pump and wait for the notification.
+ size_t browser_count = BrowserList::size();
+ ui_test_utils::WindowedNotificationObserver signal(
+ chrome::NOTIFICATION_BROWSER_CLOSED,
+ Source<Browser>(browser));
+ browser->CloseWindow();
+ signal.Wait();
+ // Now we have one less browser instance.
+ EXPECT_EQ(browser_count - 1, BrowserList::size());
}
};
@@ -46,7 +75,7 @@ IN_PROC_BROWSER_TEST_F(PanelAppBrowserTest, OpenAppInPanel) {
// No Panels initially.
PanelManager* panel_manager = PanelManager::GetInstance();
- EXPECT_EQ(0, panel_manager->num_panels()); // No panels initially.
+ ASSERT_EQ(0, panel_manager->num_panels()); // No panels initially.
LoadAndLaunchExtension("app_with_panel_container");
@@ -67,6 +96,8 @@ IN_PROC_BROWSER_TEST_F(PanelAppBrowserTest, OpenAppInPanel) {
// Now also check that PanelManager has one new Panel under management.
EXPECT_EQ(1, panel_manager->num_panels());
- new_browser->CloseWindow();
+ CloseWindowAndWait(new_browser);
+
EXPECT_EQ(0, panel_manager->num_panels());
+ EXPECT_EQ(1u, BrowserList::size());
}
« no previous file with comments | « chrome/browser/ui/panels/panel.cc ('k') | chrome/browser/ui/panels/panel_browser_window_cocoa.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698