Prevent title-only panel from fully minimizing when attention is cleared if mouse is in the panel o…

chrome/browser/ui/panels/docked_panel_strip.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 "chrome/browser/ui/panels/docked_panel_strip.h"
 
 #include <algorithm>
 #include <vector>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
 #include "chrome/browser/ui/browser.h"
-#include "chrome/browser/ui/panels/panel_drag_controller.h"
 #include "chrome/browser/ui/panels/panel_manager.h"
 #include "chrome/browser/ui/panels/panel_mouse_watcher.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_source.h"
 
 namespace {
 // Width to height ratio is used to compute the default width or height
 // when only one value is provided.
 const double kPanelDefaultWidthToHeightRatio = 1.62;  // golden ratio
@@ skipping 444 matching lines @@
 }
 
 void DockedPanelStrip::OnPanelAttentionStateChanged(Panel* panel) {
   DCHECK_EQ(this, panel->panel_strip());
   if (panel->IsDrawingAttention()) {
     // Bring up the titlebar to get user's attention.
     if (panel->expansion_state() == Panel::MINIMIZED)
       panel->SetExpansionState(Panel::TITLE_ONLY);
   } else {
     // Maybe bring down the titlebar now that panel is not drawing attention.
-    if (panel->expansion_state() == Panel::TITLE_ONLY && !are_titlebars_up_)
+    if (panel->expansion_state() == Panel::TITLE_ONLY && !are_titlebars_up_
+        && !IsMouseInPanel(panel))

[Andrei, 2012/05/15 22:43:29]

So now if one clicks on such panel and then moves the mouse without releasing
it, the panel will stay in the title-only state, right? Or am I confused here?
Is that something we care about?
I am slightly worried that there is a part of code somewhere that makes an
assumption about panels not being in the title-only state unless they are
drawing attention.

[jennb, 2012/05/15 22:53:54]

If one clicks on a title-only panel and moves the mouse without releasing, that
panel will be the currently dragged panel. Those panels stay in title-only mode
while being dragged.

Panels can be in title-only mode any time the mouse hovers low enough to raise
up minimized panels. Not necessarily drawing attention. I'm confident we kept
that in mind throughout.

After the drag is finished, and the mouse moves away from the bottom of the
screen, any panels that are in title-only mode but not drawing attention will be
reduced back to 4-pixel minimized mode.

[jianli, 2012/05/15 22:59:47]

nit: && should be placed at the end of last line.

[jennb, 2012/05/16 00:47:09]

Done.

       panel->SetExpansionState(Panel::MINIMIZED);
   }
 }
 
+bool DockedPanelStrip::IsMouseInPanel(const Panel* panel) const {
+  if (dragging_panel_current_iterator_ != panels_.end() &&

[jianli, 2012/05/15 22:59:47]

Please comment this since it might be a bit confusing. When we drag up a docked
panel a little bit, the mouse will be indeed outside the panel.

[jennb, 2012/05/16 00:47:09]

Done.

+      *dragging_panel_current_iterator_ == panel)
+    return true;
+
+  const gfx::Point mouse_position =
+      panel_manager_->mouse_watcher()->GetMousePosition();
+  gfx::Rect bounds = panel->GetBounds();
+  return bounds.x() <= mouse_position.x() &&
+      mouse_position.y() <= bounds.right() &&

[jianli, 2012/05/15 22:59:47]

y()?

Can we call gfx::Rect::Contains?

[jennb, 2012/05/16 00:47:09]

Corrected.
Contains does not include the right edge.

+      mouse_position.y() >= bounds.y();
+}
+
 void DockedPanelStrip::OnPanelTitlebarClicked(Panel* panel,
                                               panel::ClickModifier modifier) {
   DCHECK_EQ(this, panel->panel_strip());
   if (!IsPanelMinimized(panel))
     return;
 
   if (modifier == panel::APPLY_TO_ALL)
     RestoreAll();
   else
     RestorePanel(panel);
@@ skipping 135 matching lines @@
 
     gfx::Rect bounds = panel->GetBounds();
     if (bounds.x() <= mouse_x && mouse_x <= bounds.right() &&
         mouse_y >= bounds.y())
       return true;
   }
   return false;
 }
 
 void DockedPanelStrip::BringUpOrDownTitlebars(bool bring_up) {
-  if (are_titlebars_up_ == bring_up)

[jianli, 2012/05/15 22:59:47]

Why do we need to remove this check?

[jennb, 2012/05/16 00:47:09]

We may need to bring down panels that are in title-only mode even if all panels
are not in titlebar-up-hover-mode. E.g. because we chose not to bring down a
panel after attention is cleared because the mouse is in the panel.

-    return;
   are_titlebars_up_ = bring_up;
-
   int task_delay_ms = 0;
 
   // If the auto-hiding bottom bar exists, delay the action until the bottom
   // bar is fully visible or hidden. We do not want both bottom bar and panel
   // titlebar to move at the same time but with different speeds.
   DisplaySettingsProvider* provider =
       panel_manager_->display_settings_provider();
   if (provider->IsAutoHidingDesktopBarEnabled(
           DisplaySettingsProvider::DESKTOP_BAR_ALIGNED_BOTTOM)) {
     DisplaySettingsProvider::DesktopBarVisibility visibility =
@@ skipping 253 matching lines @@
 void DockedPanelStrip::OnPanelActiveStateChanged(Panel* panel) {
   // Refresh layout, but wait till active states settle.
   // This lets us avoid refreshing too many times when one panel loses
   // focus and another gains it.
   ScheduleLayoutRefresh();
 }
 
 bool DockedPanelStrip::HasPanel(Panel* panel) const {
   return find(panels_.begin(), panels_.end(), panel) != panels_.end();
 }