Improve panel tests by properly waiting for expected conditions.

chrome/browser/ui/panels/panel_browsertest.cc

 // Copyright (c) 2012 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/bind.h"
 #include "base/utf_string_conversions.h"
 #include "chrome/browser/net/url_request_mock_util.h"
 #include "chrome/browser/prefs/browser_prefs.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 228 matching lines @@
 
     // The below could be separate tests, just adding a TODO here for tracking.
     // TODO(prasadt): Add test for dragging when in titlebar exposed state.
     // TODO(prasadt): Add test in presence of auto hiding task bar.
 
     for (size_t i = 0; i < panels.size(); ++i)
       delete native_panels_testing[i];
   }
 };
 
-// Flaky (sometimes timeout): http://crbug.com/140971
-IN_PROC_BROWSER_TEST_F(PanelBrowserTest, DISABLED_CheckDockedPanelProperties) {
+IN_PROC_BROWSER_TEST_F(PanelBrowserTest, CheckDockedPanelProperties) {
   PanelManager* panel_manager = PanelManager::GetInstance();
   DockedPanelStrip* docked_strip = panel_manager->docked_strip();
 
   // Create 3 docked panels that are in expanded, title-only or minimized states
   // respectively.
   Panel* panel1 = CreatePanelWithBounds("1", gfx::Rect(0, 0, 100, 100));
   Panel* panel2 = CreatePanelWithBounds("2", gfx::Rect(0, 0, 100, 100));
   Panel* panel3 = CreatePanelWithBounds("3", gfx::Rect(0, 0, 100, 100));
   panel2->SetExpansionState(Panel::TITLE_ONLY);
   EXPECT_EQ(Panel::TITLE_ONLY, panel2->expansion_state());
@@ skipping 73 matching lines @@
       display_settings_provider()->GetDisplayArea();
   Panel* panel = CreatePanelWithBounds("BigPanel", work_area);
   gfx::Rect bounds = panel->GetBounds();
   EXPECT_EQ(panel->max_size().width(), bounds.width());
   EXPECT_LT(bounds.width(), work_area.width());
   EXPECT_EQ(panel->max_size().height(), bounds.height());
   EXPECT_LT(bounds.height(), work_area.height());
   panel->Close();
 }
 
-#if defined(OS_LINUX)
-// http://crbug.com/145740
-#define MAYBE_AutoResize FLAKY_AutoResize
-#else
-#define MAYBE_AutoResize AutoResize
-#endif
-IN_PROC_BROWSER_TEST_F(PanelBrowserTest, MAYBE_AutoResize) {
+IN_PROC_BROWSER_TEST_F(PanelBrowserTest, AutoResize) {
   PanelManager* panel_manager = PanelManager::GetInstance();
   panel_manager->enable_auto_sizing(true);
   // Bigger space is needed by this test.
   SetTestingAreas(gfx::Rect(0, 0, 1200, 900), gfx::Rect());
 
   // Create a test panel with web contents loaded.
   CreatePanelParams params("PanelTest1", gfx::Rect(), SHOW_AS_ACTIVE);
   GURL url(ui_test_utils::GetTestUrl(
       FilePath(kTestDir),
       FilePath(FILE_PATH_LITERAL("update-preferred-size.html"))));
   params.url = url;
   Panel* panel = CreatePanelWithParams(params);
 
   // Ensure panel has auto resized to original web content size.
   // The resize will update the docked panel strip.
   content::WindowedNotificationObserver initial_resize(
       chrome::NOTIFICATION_PANEL_STRIP_UPDATED,
       content::NotificationService::AllSources());
   if (panel->GetBounds().height() < panel->TitleOnlyHeight())

[jennb, 2012/09/13 00:45:34]

This test may have been flaky due to GTK returning something other than expected
title height in TitleOnlyHeight() if window has not been fully allocated yet.
Made change in PanelGtk::TitleOnlyHeight() that should help this test actualy
Wait() here.

     initial_resize.Wait();
 
   // Expand the test page. The resize will update the docked panel strip.
   gfx::Rect initial_bounds = panel->GetBounds();
   content::WindowedNotificationObserver enlarge(
       chrome::NOTIFICATION_PANEL_STRIP_UPDATED,
       content::NotificationService::AllSources());
   EXPECT_TRUE(content::ExecuteJavaScript(
       panel->GetWebContents()->GetRenderViewHost(),
       std::wstring(),
@@ skipping 509 matching lines @@
       docked_strip->StartingRightPosition() - initial_starting_right_position,
       right_bar_thickness_delta);
   EXPECT_EQ(docked_strip->display_area().bottom() - bottom_bar_thickness,
             panel->GetBounds().bottom());
   EXPECT_EQ(docked_strip->StartingRightPosition(),
             panel->GetBounds().right());
 
   panel->Close();
 }
 
-#if defined(OS_MACOSX)
-// This test doesn't pass on Snow Leopard (10.6), although it works just
-// fine on Lion (10.7). The problem is not having a real run loop around
-// the window close is at fault, given how window controllers in Chrome
-// autorelease themselves on a -performSelector:withObject:afterDelay:
-#define MAYBE_ActivatePanelOrTabbedWindow DISABLED_ActivatePanelOrTabbedWindow
-#else
-#define MAYBE_ActivatePanelOrTabbedWindow DISABLED_ActivatePanelOrTabbedWindow
-#endif
-IN_PROC_BROWSER_TEST_F(PanelBrowserTest, MAYBE_ActivatePanelOrTabbedWindow) {
-  CreatePanelParams params1("Panel1", gfx::Rect(), SHOW_AS_ACTIVE);
-  Panel* panel1 = CreatePanelWithParams(params1);
-  CreatePanelParams params2("Panel2", gfx::Rect(), SHOW_AS_ACTIVE);
-  Panel* panel2 = CreatePanelWithParams(params2);
+IN_PROC_BROWSER_TEST_F(PanelBrowserTest, ActivatePanelOrTabbedWindow) {
+  Panel* panel1 = CreatePanel("Panel1");

[Dmitry Titov, 2012/09/13 18:19:00]

I wonder why do we need panel1 here?

+  Panel* panel2 = CreatePanel("Panel2");
 
-  ASSERT_FALSE(panel1->IsActive());
-  ASSERT_TRUE(panel2->IsActive());
   // Activate main tabbed window.
   browser()->window()->Activate();
   WaitForPanelActiveState(panel2, SHOW_AS_INACTIVE);
 
   // Activate a panel.
   panel2->Activate();
   WaitForPanelActiveState(panel2, SHOW_AS_ACTIVE);
 
   // Activate the main tabbed window back.
   browser()->window()->Activate();
   WaitForPanelActiveState(panel2, SHOW_AS_INACTIVE);
-  // Close the main tabbed window. That should move focus back to panel.
-  chrome::CloseWindow(browser());
-  WaitForPanelActiveState(panel2, SHOW_AS_ACTIVE);
 
   // Activate another panel.
   panel1->Activate();
   WaitForPanelActiveState(panel1, SHOW_AS_ACTIVE);
   WaitForPanelActiveState(panel2, SHOW_AS_INACTIVE);
 
   // Switch focus between panels.
   panel2->Activate();
   WaitForPanelActiveState(panel2, SHOW_AS_ACTIVE);
   WaitForPanelActiveState(panel1, SHOW_AS_INACTIVE);
 
-  // Close active panel, focus should move to the remaining one.
-  CloseWindowAndWait(panel2);
-  WaitForPanelActiveState(panel1, SHOW_AS_ACTIVE);
-  panel1->Close();
+  PanelManager::GetInstance()->CloseAll();
 }
 
 // TODO(jianli): To be enabled for other platforms.
 #if defined(OS_WIN)
 #define MAYBE_ActivateDeactivateBasic ActivateDeactivateBasic
 #else
 #define MAYBE_ActivateDeactivateBasic DISABLED_ActivateDeactivateBasic
 #endif
 IN_PROC_BROWSER_TEST_F(PanelBrowserTest, MAYBE_ActivateDeactivateBasic) {
   // Create an active panel.
   Panel* panel = CreatePanel("PanelTest");
   scoped_ptr<NativePanelTesting> native_panel_testing(
       CreateNativePanelTesting(panel));
-  EXPECT_TRUE(panel->IsActive());
+
+  WaitForPanelActiveState(panel, SHOW_AS_ACTIVE);  // doublecheck active state
   EXPECT_TRUE(native_panel_testing->VerifyActiveState(true));
 
   // Deactivate the panel.
   panel->Deactivate();
   WaitForPanelActiveState(panel, SHOW_AS_INACTIVE);
-  EXPECT_FALSE(panel->IsActive());
   EXPECT_TRUE(native_panel_testing->VerifyActiveState(false));
 
   // This test does not reactivate the panel because the panel might not be
   // reactivated programmatically once it is deactivated.
 }
 
 IN_PROC_BROWSER_TEST_F(PanelBrowserTest, ActivateDeactivateMultiple) {
   BrowserWindow* tabbed_window = browser()->window();
 
   // Create 4 panels in the following screen layout:
@@ skipping 614 matching lines @@
   ::MSG key_msg = { NULL, WM_KEYDOWN, ui::VKEY_W, 0 };
   content::NativeWebKeyboardEvent key_event(key_msg);
   key_event.modifiers = content::NativeWebKeyboardEvent::ControlKey;
 #else
   content::NativeWebKeyboardEvent key_event;
 #endif
   panel->HandleKeyboardEvent(key_event);
   signal.Wait();
   EXPECT_EQ(0, panel_manager->num_panels());
 }