Change Panel titlebars to activate the panel on click (rather than minimize).

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 4561a64cc67f8f6fc8d01053a8af90d4959a8aaf..7352c9e50f19260910c723ab14bfb4cbed3edb86 100644
--- a/chrome/browser/ui/panels/panel_browsertest.cc
+++ b/chrome/browser/ui/panels/panel_browsertest.cc
@@ -151,7 +151,7 @@ class PanelBrowserTest : public BasePanelBrowserTest {
           NativePanelTesting::Create(panels[i]->native_panel());
     }
 
-    // Test minimize.
+    // Verify titlebar click does not minimize.
     for (size_t index = 0; index < panels.size(); ++index) {
       // Press left mouse button.  Verify nothing changed.
       native_panels_testing[index]->PressLeftMouseButtonTitlebar(
@@ -159,8 +159,15 @@ class PanelBrowserTest : public BasePanelBrowserTest {
       EXPECT_EQ(expected_bounds, GetAllPanelBounds());
       EXPECT_EQ(expected_expansion_states, GetAllPanelExpansionStates());
 
-      // Release mouse button.  Verify minimized.
+      // Release mouse button.  Verify nothing changed.
       native_panels_testing[index]->ReleaseMouseButtonTitlebar();
+      EXPECT_EQ(expected_bounds, GetAllPanelBounds());
+      EXPECT_EQ(expected_expansion_states, GetAllPanelExpansionStates());
+    }
+
+    // Minimize all panels for next stage in test.
+    for (size_t index = 0; index < panels.size(); ++index) {
+      panels[index]->Minimize();
       expected_bounds[index].set_height(Panel::kMinimizedPanelHeight);
       expected_bounds[index].set_y(
           test_begin_bounds[index].y() +
@@ -524,7 +531,7 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, MinimizeRestoreThreePanels) {
   PanelManager::GetInstance()->CloseAll();
 }
 
-IN_PROC_BROWSER_TEST_F(PanelBrowserTest, ToggleMinimizeAll) {
+IN_PROC_BROWSER_TEST_F(PanelBrowserTest, RestoreAllWithTitlebarClick) {
   // We'll simulate mouse movements for test.
   PanelMouseWatcher* mouse_watcher = new TestPanelMouseWatcher();
   PanelManager::GetInstance()->SetMouseWatcherForTesting(mouse_watcher);
@@ -545,18 +552,23 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, ToggleMinimizeAll) {
       NativePanelTesting::Create(panel3->native_panel()));
 
   // Press and release mouse button on one panel's titlebar using a modifier
-  // to minimize all panels. Nothing changes until mouse is released.
+  // to minimize all panels. Nothing changes.

[jianli, 2012/04/25 18:19:31]

Please update the comment for "minimize all panels". Probably something like:
Clicking titlebar on expanded panel will only activate the panel. Clicking
titlebar with a modifier is same as clicking without a modifier.

[jennb, 2012/04/25 20:34:05]

Edited.

   test_panel2->PressLeftMouseButtonTitlebar(panel2->GetBounds().origin(),
                                             panel::APPLY_TO_ALL);
+  test_panel2->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
   EXPECT_FALSE(panel1->IsMinimized());
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
-  test_panel2->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
+
+  // Minimize all panels. Then click on a panel with apply-all modifier.

[jianli, 2012/04/25 18:19:31]

Better comment like: Clicking titlebar on minimized panel will expand the panel.
Clicking titlebar with a apply-all modifier will expand all panels.

[jennb, 2012/04/25 20:34:05]

Edited.

+  // Verify all panels restored.
+  panel1->Minimize();
+  panel2->Minimize();
+  panel3->Minimize();
   EXPECT_TRUE(panel1->IsMinimized());
   EXPECT_TRUE(panel2->IsMinimized());
   EXPECT_TRUE(panel3->IsMinimized());
 
-  // Press and release on a panel titlebar to restore all panels.
   // Nothing changes until mouse is released.
   test_panel1->PressLeftMouseButtonTitlebar(panel1->GetBounds().origin(),
                                             panel::APPLY_TO_ALL);
@@ -568,24 +580,26 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, ToggleMinimizeAll) {
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
 
-  // Minimize a single panel. Then minimize all panels to verify that apply-all
-  // logic works even if not all panels have the same expansion state.
+  // Minimize a single panel. Then click on non-minimized panel with apply-all
+  // modifier. Verify nothing changes.
   panel1->Minimize();
   EXPECT_TRUE(panel1->IsMinimized());
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
+
   test_panel2->PressLeftMouseButtonTitlebar(panel2->GetBounds().origin(),
                                             panel::APPLY_TO_ALL);
   test_panel2->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
   EXPECT_TRUE(panel1->IsMinimized());
-  EXPECT_TRUE(panel2->IsMinimized());
-  EXPECT_TRUE(panel3->IsMinimized());
+  EXPECT_FALSE(panel2->IsMinimized());
+  EXPECT_FALSE(panel3->IsMinimized());
 
-  // Expand a single panel. Then restore all panels.
-  panel3->Restore();
+  // Minimize another panel. Then restore all panels.
+  panel2->Minimize();
   EXPECT_TRUE(panel1->IsMinimized());
   EXPECT_TRUE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
+
   test_panel2->PressLeftMouseButtonTitlebar(panel2->GetBounds().origin(),
                                             panel::APPLY_TO_ALL);
   test_panel2->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
@@ -598,6 +612,7 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, ToggleMinimizeAll) {
   EXPECT_TRUE(panel1->IsMinimized());
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
+
   test_panel1->PressLeftMouseButtonTitlebar(panel1->GetBounds().origin(),
                                             panel::APPLY_TO_ALL);
   test_panel1->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
@@ -605,25 +620,32 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, ToggleMinimizeAll) {
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
 
-  // Click on the single expanded panel. Verify all are minimized.
+  // Click on the single expanded panel. Verify nothing changes.
   panel1->Minimize();
   panel3->Minimize();
   EXPECT_TRUE(panel1->IsMinimized());
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_TRUE(panel3->IsMinimized());
+
   test_panel2->PressLeftMouseButtonTitlebar(panel2->GetBounds().origin(),
                                             panel::APPLY_TO_ALL);
   test_panel2->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
   EXPECT_TRUE(panel1->IsMinimized());
-  EXPECT_TRUE(panel2->IsMinimized());
+  EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_TRUE(panel3->IsMinimized());
 
   // Hover over a minimized panel and click on the titlebar while it is in
   // title-only mode. Should restore all panels.
+  panel2->Minimize();
+  EXPECT_TRUE(panel1->IsMinimized());
+  EXPECT_TRUE(panel2->IsMinimized());
+  EXPECT_TRUE(panel3->IsMinimized());
+
   MoveMouseAndWaitForExpansionStateChange(panel2, panel2->GetBounds().origin());
   EXPECT_EQ(Panel::TITLE_ONLY, panel1->expansion_state());
   EXPECT_EQ(Panel::TITLE_ONLY, panel2->expansion_state());
   EXPECT_EQ(Panel::TITLE_ONLY, panel3->expansion_state());
+
   test_panel3->PressLeftMouseButtonTitlebar(panel3->GetBounds().origin(),
                                             panel::APPLY_TO_ALL);
   test_panel3->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
@@ -631,29 +653,42 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, ToggleMinimizeAll) {
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
 
-  // Draw attention to a panel. Verify minimize all applies without
-  // affecting draw attention state.
+  // Draw attention to a minimized panel. Verify restore all applies without
+  // affecting draw attention.
+  panel1->Minimize();
+  panel2->Minimize();
+  panel3->Minimize();
+  EXPECT_TRUE(panel1->IsMinimized());
+  EXPECT_TRUE(panel2->IsMinimized());
+  EXPECT_TRUE(panel3->IsMinimized());
+
   panel1->FlashFrame(true);
   EXPECT_TRUE(panel1->IsDrawingAttention());
-  test_panel3->PressLeftMouseButtonTitlebar(panel3->GetBounds().origin(),
+
+  test_panel2->PressLeftMouseButtonTitlebar(panel3->GetBounds().origin(),
                                             panel::APPLY_TO_ALL);
-  test_panel3->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
+  test_panel2->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
+  EXPECT_FALSE(panel1->IsMinimized());
+  EXPECT_FALSE(panel2->IsMinimized());
+  EXPECT_FALSE(panel3->IsMinimized());
+  EXPECT_TRUE(panel1->IsDrawingAttention());
+
+  // Restore all panels by clicking on the minimized panel that is drawing
+  // attention. Verify restore all applies and clears draw attention.
+  panel1->Minimize();
+  panel2->Minimize();
+  panel3->Minimize();
   EXPECT_TRUE(panel1->IsMinimized());
   EXPECT_TRUE(panel2->IsMinimized());
   EXPECT_TRUE(panel3->IsMinimized());
-  EXPECT_TRUE(panel1->IsDrawingAttention());
-  EXPECT_EQ(Panel::TITLE_ONLY, panel1->expansion_state());
 
-  // Restore all panels by clicking on the minimized panel that is drawing
-  // attention. Verify restore all applies without affecting draw attention
-  // state.
   test_panel1->PressLeftMouseButtonTitlebar(panel1->GetBounds().origin(),
                                             panel::APPLY_TO_ALL);
   test_panel1->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
   EXPECT_FALSE(panel1->IsMinimized());
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
-  EXPECT_TRUE(panel1->IsDrawingAttention());
+  EXPECT_FALSE(panel1->IsDrawingAttention());
 
   PanelManager::GetInstance()->CloseAll();
 }
@@ -952,7 +987,7 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, DrawAttentionResetOnActivate) {
   scoped_ptr<NativePanelTesting> native_panel_testing(
       NativePanelTesting::Create(panel->native_panel()));
 
-  // Activate the panel.
+  // Deactivate the panel.
   panel->Deactivate();
   WaitForPanelActiveState(panel, SHOW_AS_INACTIVE);
 
@@ -971,14 +1006,37 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, DrawAttentionResetOnActivate) {
   panel->Close();
 }
 
-// TODO(dimich): try/enable on other platforms.
-#if defined(OS_MACOSX)
-#define MAYBE_DrawAttentionResetOnClick DrawAttentionResetOnClick
-#else
-#define MAYBE_DrawAttentionResetOnClick DISABLED_DrawAttentionResetOnClick
-#endif
+IN_PROC_BROWSER_TEST_F(PanelBrowserTest,
+                       DrawAttentionMinimizedNotResetOnActivate) {
+  // Create 2 panels so we end up with an inactive panel that can
+  // be made to draw attention.
+  Panel* panel1 = CreatePanel("test panel1");
+  Panel* panel2 = CreatePanel("test panel2");
+
+  panel1->Minimize();
+  EXPECT_TRUE(panel1->IsMinimized());
+  panel1->FlashFrame(true);
+  EXPECT_TRUE(panel1->IsDrawingAttention());
+
+  // Simulate panel being activated while minimized. Cannot call
+  // Activate() as that expands the panel.
+  panel1->OnActiveStateChanged(true);
+  EXPECT_TRUE(panel1->IsDrawingAttention());  // Unchanged.
+
+  // Unminimize panel to show that attention would have been cleared
+  // if panel had not been minimized.
+  panel1->Restore();
+  EXPECT_FALSE(panel1->IsMinimized());
+  EXPECT_TRUE(panel1->IsDrawingAttention());  // Unchanged.
 
-IN_PROC_BROWSER_TEST_F(PanelBrowserTest, MAYBE_DrawAttentionResetOnClick) {
+  panel1->OnActiveStateChanged(true);
+  EXPECT_FALSE(panel1->IsDrawingAttention());  // Attention cleared.
+
+  panel1->Close();
+  panel2->Close();
+}
+
+IN_PROC_BROWSER_TEST_F(PanelBrowserTest, DrawAttentionResetOnClick) {
   CreatePanelParams params("Initially Inactive", gfx::Rect(), SHOW_AS_INACTIVE);
   Panel* panel = CreatePanelWithParams(params);
   scoped_ptr<NativePanelTesting> native_panel_testing(
@@ -1002,9 +1060,6 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest, MAYBE_DrawAttentionResetOnClick) {
   panel->Close();
 }
 
-// There was a bug when it was not possible to minimize the panel by clicking
-// on the titlebar right after it was restored and activated. This test verifies
-// it's possible.
 IN_PROC_BROWSER_TEST_F(PanelBrowserTest,
                        MinimizeImmediatelyAfterRestore) {
   CreatePanelParams params("Initially Inactive", gfx::Rect(), SHOW_AS_ACTIVE);
@@ -1012,7 +1067,7 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest,
   scoped_ptr<NativePanelTesting> native_panel_testing(
       NativePanelTesting::Create(panel->native_panel()));
 
-  panel->SetExpansionState(Panel::MINIMIZED);  // this should deactivate.
+  panel->Minimize();  // this should deactivate.
   MessageLoop::current()->RunAllPending();
   WaitForPanelActiveState(panel, SHOW_AS_INACTIVE);
   EXPECT_EQ(Panel::MINIMIZED, panel->expansion_state());
@@ -1022,10 +1077,8 @@ IN_PROC_BROWSER_TEST_F(PanelBrowserTest,
   WaitForPanelActiveState(panel, SHOW_AS_ACTIVE);
   EXPECT_EQ(Panel::EXPANDED, panel->expansion_state());
 
-  // Test that click on the titlebar right after expansion minimizes the Panel.
-  native_panel_testing->PressLeftMouseButtonTitlebar(
-      panel->GetBounds().origin());
-  native_panel_testing->ReleaseMouseButtonTitlebar();
+  // Verify that minimizing a panel right after expansion works.
+  panel->Minimize();
   MessageLoop::current()->RunAllPending();
   EXPECT_EQ(Panel::MINIMIZED, panel->expansion_state());
   panel->Close();