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

Unified Diff: chrome/browser/ui/panels/panel_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
Index: chrome/browser/ui/panels/panel_browsertest.cc
diff --git a/chrome/browser/ui/panels/panel_browsertest.cc b/chrome/browser/ui/panels/panel_browsertest.cc
index 2e88fc3e44d7ac7dae3ec1b0774f89cd3312f2a3..acca37b504a90b3551aaba62fd4087534fd2220c 100644
--- a/chrome/browser/ui/panels/panel_browsertest.cc
+++ b/chrome/browser/ui/panels/panel_browsertest.cc
@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "base/command_line.h"
+#include "base/mac/scoped_nsautorelease_pool.h"
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/prefs/pref_service.h"
#include "chrome/browser/profiles/profile.h"
@@ -45,6 +46,14 @@ class PanelBrowserTest : public InProcessBrowserTest {
protected:
Panel* CreatePanel(const std::string& name, const gfx::Rect& bounds) {
+ // 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;
+
Browser* panel_browser = Browser::CreateForApp(Browser::TYPE_PANEL,
name,
bounds,
@@ -62,6 +71,19 @@ class PanelBrowserTest : public InProcessBrowserTest {
return panel;
}
+ 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());
+ }
+
// Creates a testing extension.
scoped_refptr<Extension> CreateExtension(const FilePath::StringType& path) {
#if defined(OS_WIN)
@@ -281,7 +303,8 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, CreatePanel) {
EXPECT_GT(bounds.width(), 0);
EXPECT_GT(bounds.height(), 0);
- panel->Close();
+ CloseWindowAndWait(panel->browser());
+
EXPECT_EQ(0, panel_manager->num_panels());
}
@@ -293,14 +316,7 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, FindBar) {
panel->Close();
}
-// TODO(jianli): Investigate and enable it for Mac.
-#ifdef OS_MACOSX
-#define MAYBE_CreatePanelOnOverflow DISABLED_CreatePanelOnOverflow
-#else
-#define MAYBE_CreatePanelOnOverflow CreatePanelOnOverflow
-#endif
-
-IN_PROC_BROWSER_TEST_F(PanelBrowserTest, MAYBE_CreatePanelOnOverflow) {
+IN_PROC_BROWSER_TEST_F(PanelBrowserTest, CreatePanelOnOverflow) {
TestCreatePanelOnOverflow();
}

Powered by Google App Engine
This is Rietveld 408576698