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

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/browser_process.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
 #include "chrome/browser/net/url_request_mock_util.h"
@@ skipping 133 matching lines @@
     std::vector<gfx::Rect> test_begin_bounds = GetAllPanelBounds();
     std::vector<gfx::Rect> expected_bounds = test_begin_bounds;
     std::vector<Panel::ExpansionState> expected_expansion_states(
         panels.size(), Panel::EXPANDED);
     std::vector<NativePanelTesting*> native_panels_testing(panels.size());
     for (size_t i = 0; i < panels.size(); ++i) {
       native_panels_testing[i] =
           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(
           panels[index]->GetBounds().origin());
       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() +
           test_begin_bounds[index].height() - Panel::kMinimizedPanelHeight);
       expected_expansion_states[index] = Panel::MINIMIZED;
       EXPECT_EQ(expected_bounds, GetAllPanelBounds());
       EXPECT_EQ(expected_expansion_states, GetAllPanelExpansionStates());
     }
 
     // Setup bounds and expansion states for minimized and titlebar-only
@@ skipping 343 matching lines @@
 
   // Test with three panels.
   CreatePanelWithBounds("PanelTest1", gfx::Rect(0, 0, 100, 100));
   CreatePanelWithBounds("PanelTest2", gfx::Rect(0, 0, 110, 110));
   CreatePanelWithBounds("PanelTest3", gfx::Rect(0, 0, 120, 120));
   TestMinimizeRestore();
 
   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);
 
   // Test with three panels.
   Panel* panel1 = CreatePanel("PanelTest1");
   Panel* panel2 = CreatePanel("PanelTest2");
   Panel* panel3 = CreatePanel("PanelTest3");
   EXPECT_FALSE(panel1->IsMinimized());
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
 
   scoped_ptr<NativePanelTesting> test_panel1(
       NativePanelTesting::Create(panel1->native_panel()));
   scoped_ptr<NativePanelTesting> test_panel2(
       NativePanelTesting::Create(panel2->native_panel()));
   scoped_ptr<NativePanelTesting> test_panel3(
       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);
   EXPECT_TRUE(panel1->IsMinimized());
   EXPECT_TRUE(panel2->IsMinimized());
   EXPECT_TRUE(panel3->IsMinimized());
   test_panel1->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
   EXPECT_FALSE(panel1->IsMinimized());
   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);
   EXPECT_FALSE(panel1->IsMinimized());
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
 
   // Click on the single minimized panel. Verify all are restored.
   panel1->Minimize();
   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);
   EXPECT_FALSE(panel1->IsMinimized());
   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);
   EXPECT_FALSE(panel1->IsMinimized());
   EXPECT_FALSE(panel2->IsMinimized());
   EXPECT_FALSE(panel3->IsMinimized());
 
-  // Draw attention to a panel. Verify minimize all applies without
-  // affecting draw attention state.
-  panel1->FlashFrame(true);
-  EXPECT_TRUE(panel1->IsDrawingAttention());
-  test_panel3->PressLeftMouseButtonTitlebar(panel3->GetBounds().origin(),
-                                            panel::APPLY_TO_ALL);
-  test_panel3->ReleaseMouseButtonTitlebar(panel::APPLY_TO_ALL);
+  // 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());
-  EXPECT_EQ(Panel::TITLE_ONLY, panel1->expansion_state());
+
+  test_panel2->PressLeftMouseButtonTitlebar(panel3->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());
+  EXPECT_TRUE(panel1->IsDrawingAttention());
 
   // Restore all panels by clicking on the minimized panel that is drawing
-  // attention. Verify restore all applies without affecting draw attention
-  // state.
+  // 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());
+
   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();
 }
 
 #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
@@ skipping 278 matching lines @@
 
   panel->Close();
 }
 
 IN_PROC_BROWSER_TEST_F(PanelBrowserTest, DrawAttentionResetOnActivate) {
   CreatePanelParams params("Initially active", gfx::Rect(), SHOW_AS_ACTIVE);
   Panel* panel = CreatePanelWithParams(params);
   scoped_ptr<NativePanelTesting> native_panel_testing(
       NativePanelTesting::Create(panel->native_panel()));
 
-  // Activate the panel.
+  // Deactivate the panel.
   panel->Deactivate();
   WaitForPanelActiveState(panel, SHOW_AS_INACTIVE);
 
   panel->FlashFrame(true);
   EXPECT_TRUE(panel->IsDrawingAttention());
   MessageLoop::current()->RunAllPending();
   EXPECT_TRUE(native_panel_testing->VerifyDrawingAttention());
 
   // Test that the attention is cleared when panel gets focus.
   panel->Activate();
   MessageLoop::current()->RunAllPending();
   WaitForPanelActiveState(panel, SHOW_AS_ACTIVE);
   EXPECT_FALSE(panel->IsDrawingAttention());
   EXPECT_FALSE(native_panel_testing->VerifyDrawingAttention());
 
   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");
 
-IN_PROC_BROWSER_TEST_F(PanelBrowserTest, MAYBE_DrawAttentionResetOnClick) {
+  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.
+
+  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(
       NativePanelTesting::Create(panel->native_panel()));
 
   panel->FlashFrame(true);
   EXPECT_TRUE(panel->IsDrawingAttention());
   MessageLoop::current()->RunAllPending();
   EXPECT_TRUE(native_panel_testing->VerifyDrawingAttention());
 
   // Test that the attention is cleared when panel gets focus.
   native_panel_testing->PressLeftMouseButtonTitlebar(
       panel->GetBounds().origin());
   native_panel_testing->ReleaseMouseButtonTitlebar();
 
   MessageLoop::current()->RunAllPending();
   WaitForPanelActiveState(panel, SHOW_AS_ACTIVE);
   EXPECT_FALSE(panel->IsDrawingAttention());
   EXPECT_FALSE(native_panel_testing->VerifyDrawingAttention());
 
   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);
   Panel* panel = CreatePanelWithParams(params);
   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());
 
   panel->Activate();
   MessageLoop::current()->RunAllPending();
   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();
 }
 
 IN_PROC_BROWSER_TEST_F(PanelBrowserTest, FocusLostOnMinimize) {
   CreatePanelParams params("Initially Active", gfx::Rect(), SHOW_AS_ACTIVE);
   Panel* panel = CreatePanelWithParams(params);
   EXPECT_EQ(Panel::EXPANDED, panel->expansion_state());
 
@@ skipping 630 matching lines @@
   // position when tall panel brings up its titlebar.
   CloseWindowAndWait(panel1->browser());
   EXPECT_EQ(balloon_bottom_after_tall_panel_titlebar_up,
             GetBalloonBottomPosition(balloon));
 
   // Closing the remaining tall panel should move the notification balloon back
   // to its original position.
   CloseWindowAndWait(panel2->browser());
   EXPECT_EQ(original_balloon_bottom, GetBalloonBottomPosition(balloon));
 }