Reland: Implement correct Panel closing sequence for Mac.

chrome/browser/ui/panels/panel_browsertest.cc

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 4f5ff116af4779804c8442d135859bdf0d0f8c4d..2a45b17d632f698914dc409a73c2a67ec48156fc 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 wan to release it as soon as

[jennb, 2011/08/19 21:46:12]

typo: wan

[Dmitry Titov, 2011/08/22 18:34:56]

Done.

+    // 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)
@@ -150,8 +172,8 @@ class PanelBrowserTest : public InProcessBrowserTest {
     ASSERT_EQ(2, panel_manager->num_panels());
     EXPECT_LT(panel6->GetBounds().right(), panel1->GetBounds().x());
 
-    panel1->Close();
-    panel6->Close();
+    CloseWindowAndWait(panel1->browser());

[jennb, 2011/08/19 21:46:12]

Test doesn't care when close actually happens so no need to wait for these 2
closes.

[Dmitry Titov, 2011/08/22 18:34:56]

Done. Put panel->Close() back.

+    CloseWindowAndWait(panel6->browser());
   }
 
   struct DragTestData {
@@ -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());

[jennb, 2011/08/19 21:46:12]

Wait not needed if we remove panel from panel manager immediately.

[Dmitry Titov, 2011/08/22 18:34:56]

See comment at the top.

+
   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();
 }