Reland: Implement correct Panel closing sequence for Mac.

chrome/browser/ui/panels/panel_browsertest.cc

 // Copyright (c) 2011 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.
 
 #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"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/find_bar/find_bar.h"
 #include "chrome/browser/ui/find_bar/find_bar_controller.h"
 #include "chrome/browser/ui/panels/native_panel.h"
 #include "chrome/browser/ui/panels/panel.h"
@@ skipping 22 matching lines @@
     FindBarBridge::disable_animations_during_testing_ = true;
 #endif
   }
 
   virtual void SetUpCommandLine(CommandLine* command_line) {
     command_line->AppendSwitch(switches::kEnablePanels);
   }
 
  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,
                                                    browser()->profile());
     EXPECT_TRUE(panel_browser->is_type_panel());
 
     TabContentsWrapper* tab_contents =
         new TabContentsWrapper(new TestTabContents(browser()->profile(), NULL));
     panel_browser->AddTab(tab_contents, PageTransition::LINK);
 
     Panel* panel = static_cast<Panel*>(panel_browser->window());
     panel->Show();
     MessageLoopForUI::current()->RunAllPending();
 
     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)
     FilePath full_path(FILE_PATH_LITERAL("c:\\"));
 #else
     FilePath full_path(FILE_PATH_LITERAL("/"));
 #endif
     full_path = full_path.Append(path);
     DictionaryValue input_value;
     input_value.SetString(extension_manifest_keys::kVersion, "1.0.0.0");
@@ skipping 68 matching lines @@
         chrome::NOTIFICATION_BROWSER_CLOSED,
         Source<Browser>(panel5->browser()));
     Panel* panel6 = CreatePanel(
         web_app::GenerateApplicationNameFromExtensionId(extension3->id()),
         gfx::Rect(0, 0, 500, 200));
     signal3.Wait();
     signal4.Wait();
     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 {
     DragTestData(int drag_delta_x,
                  int drag_delta_y,
                  bool is_big_delta,
                  bool should_cancel_drag)
         : drag_delta_x(drag_delta_x), drag_delta_y(drag_delta_y),
           is_big_delta(is_big_delta), should_cancel_drag(should_cancel_drag) {}
     int drag_delta_x;
@@ skipping 109 matching lines @@
 
   Panel* panel = CreatePanel("PanelTest", gfx::Rect());
   EXPECT_EQ(1, panel_manager->num_panels());
 
   gfx::Rect bounds = panel->GetBounds();
   EXPECT_GT(bounds.x(), 0);
   EXPECT_GT(bounds.y(), 0);
   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());
 }
 
 IN_PROC_BROWSER_TEST_F(PanelBrowserTest, FindBar) {
   Panel* panel = CreatePanel("PanelTest", gfx::Rect(0, 0, 400, 400));
   Browser* browser = panel->browser();
   browser->ShowFindBar();
   ASSERT_TRUE(browser->GetFindBarController()->find_bar()->IsFindBarVisible());
   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();
 }
 
 #if defined(OS_MACOSX) || defined(OS_CHROMEOS) || defined(TOOLKIT_VIEWS)
 #define MAYBE_DragPanels DISABLED_DragPanels
 #else
 #define MAYBE_DragPanels DragPanels
 #endif
 
 IN_PROC_BROWSER_TEST_F(PanelBrowserTest, MAYBE_DragPanels) {
@@ skipping 187 matching lines @@
   EXPECT_EQ(1, tabbed_browser->tab_count());
   ASSERT_TRUE(tabbed_browser->window()->IsDownloadShelfVisible());
   tabbed_browser->CloseWindow();
 #endif
 
   EXPECT_EQ(1, panel_browser->tab_count());
   ASSERT_FALSE(panel_browser->window()->IsDownloadShelfVisible());
 
   panel_browser->CloseWindow();
 }