Fix handling of minimized panel count

Increments and decrements of minimized panel count in the docking strip would not balance out in certain scenarios, e.g.: 
- if you add a panel that is already minimized (the count was incremented before the panel is added, sometimes causing a DCHECK to fire) 
- if you minimize a panel that is in "temporary layout" 
- if a panel's expansion state is changed when it is not in the docked strip (that is very hard to guard against, since a panel can jump into the overflow strip as a side effect of many operations) 

Changed the handling of minimized panel count to be more bullet-proof; added a test 

BUG=113682
TEST=PanelOverflowBrowserTest::AddMinimizedTillOverflow()

Review URL: http://codereview.chromium.org/9699036

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@126966 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/panels/docked_panel_strip.cc

@@ -180,6 +180,7 @@ void DockedPanelStrip::InsertExistingPanelAtKnownPosition(Panel* panel) {
     if (x > (*iter)->GetBounds().x())
       break;
   panels_.insert(iter, panel);
+  UpdateMinimizedPanelCount();
 
   // This will automatically update all affected panels due to the insertion.
   if (iter != panels_.end())
@@ -195,7 +196,6 @@ void DockedPanelStrip::InsertExistingPanelAtDefaultPosition(
   int width = restored_size.width();
 
   int x = FitPanelWithWidth(width);
-
   if (update_bounds) {
     Panel::ExpansionState expansion_state_to_restore;
     if (panel->expansion_state() == Panel::EXPANDED) {
@@ -208,7 +208,6 @@ void DockedPanelStrip::InsertExistingPanelAtDefaultPosition(
         expansion_state_to_restore = Panel::MINIMIZED;
         height = Panel::kMinimizedPanelHeight;
       }
-      IncrementMinimizedPanels();
     }
     int y =
         GetBottomPositionForExpansionState(expansion_state_to_restore) - height;
@@ -221,6 +220,7 @@ void DockedPanelStrip::InsertExistingPanelAtDefaultPosition(
   }
 
   panels_.push_back(panel);
+  UpdateMinimizedPanelCount();
 }
 
 int DockedPanelStrip::GetMaxPanelWidth() const {
@@ -244,9 +244,6 @@ void DockedPanelStrip::RemovePanel(Panel* panel) {
   DCHECK_EQ(this, panel->panel_strip());
   panel->SetPanelStrip(NULL);
 
-  if (panel->expansion_state() != Panel::EXPANDED)
-    DecrementMinimizedPanels();
-
   if (panel->has_temporary_layout()) {
     panels_in_temporary_layout_.erase(panel);
     return;
@@ -287,6 +284,9 @@ void DockedPanelStrip::RemovePanel(Panel* panel) {
 
     RefreshLayout();
   }
+
+  if (panel->expansion_state() != Panel::EXPANDED)
+    UpdateMinimizedPanelCount();
 }
 
 bool DockedPanelStrip::CanShowPanelAsActive(const Panel* panel) const {
@@ -447,26 +447,23 @@ void DockedPanelStrip::EndDraggingPanelWithinStrip(Panel* panel, bool aborted) {
 void DockedPanelStrip::OnPanelExpansionStateChanged(Panel* panel) {
   gfx::Size size = panel->restored_size();
   Panel::ExpansionState expansion_state = panel->expansion_state();
-  Panel::ExpansionState old_state = panel->old_expansion_state();
   switch (expansion_state) {
     case Panel::EXPANDED:
-      if (old_state == Panel::TITLE_ONLY || old_state == Panel::MINIMIZED)
-        DecrementMinimizedPanels();
+
       break;
     case Panel::TITLE_ONLY:
       size.set_height(panel->TitleOnlyHeight());
-      if (old_state == Panel::EXPANDED)
-        IncrementMinimizedPanels();
+
       break;
     case Panel::MINIMIZED:
       size.set_height(Panel::kMinimizedPanelHeight);
-      if (old_state == Panel::EXPANDED)
-        IncrementMinimizedPanels();
+
       break;
     default:
       NOTREACHED();
       break;
   }
+  UpdateMinimizedPanelCount();
 
   int bottom = GetBottomPositionForExpansionState(expansion_state);
   gfx::Rect bounds = panel->GetBounds();
@@ -517,18 +514,21 @@ bool DockedPanelStrip::IsPanelMinimized(const Panel* panel) const {
   return panel->expansion_state() != Panel::EXPANDED;
 }
 
-void DockedPanelStrip::IncrementMinimizedPanels() {
+void DockedPanelStrip::UpdateMinimizedPanelCount() {
+  int prev_minimized_panel_count = minimized_panel_count_;
+  minimized_panel_count_ = 0;
+  for (Panels::const_iterator panel_iter = panels_.begin();
+        panel_iter != panels_.end(); ++panel_iter) {
+    if ((*panel_iter)->expansion_state() != Panel::EXPANDED)
       minimized_panel_count_++;
-  if (minimized_panel_count_ == 1)
-    panel_manager_->mouse_watcher()->AddObserver(this);
-  DCHECK_LE(minimized_panel_count_, num_panels());
-}
+    }
 
-void DockedPanelStrip::DecrementMinimizedPanels() {
-  minimized_panel_count_--;
-  DCHECK_GE(minimized_panel_count_, 0);
-  if (minimized_panel_count_ == 0)
+  if (prev_minimized_panel_count == 0 && minimized_panel_count_ > 0)
+    panel_manager_->mouse_watcher()->AddObserver(this);
+  else if (prev_minimized_panel_count > 0 &&  minimized_panel_count_ == 0)
     panel_manager_->mouse_watcher()->RemoveObserver(this);
+
+  DCHECK_LE(minimized_panel_count_, num_panels());
 }
 
 void DockedPanelStrip::ResizePanelWindow(

chrome/browser/ui/panels/docked_panel_strip.h

@@ -107,6 +107,8 @@ class DockedPanelStrip : public PanelStrip,
   int num_temporary_layout_panels() const {
     return panels_in_temporary_layout_.size();
   }
+
+  int minimized_panel_count() const {return minimized_panel_count_; }
 #endif
 
  private:
@@ -135,8 +137,7 @@ class DockedPanelStrip : public PanelStrip,
   void InsertExistingPanelAtDefaultPosition(Panel* panel, bool refresh_bounds);
 
   // Keep track of the minimized panels to control mouse watching.
-  void IncrementMinimizedPanels();
-  void DecrementMinimizedPanels();
+  void UpdateMinimizedPanelCount();
 
   // Help functions to drag the given panel.
   void DragLeft(Panel* dragging_panel);

chrome/browser/ui/panels/panel.cc

@@ -52,8 +52,7 @@ Panel::Panel(Browser* browser, const gfx::Size& requested_size)
       auto_resizable_(false),
       always_on_top_(false),
       in_preview_mode_(false),
-      expansion_state_(EXPANDED),
-      old_expansion_state_(EXPANDED) {
+      expansion_state_(EXPANDED) {
 }
 
 Panel::~Panel() {
@@ -178,7 +177,6 @@ void  Panel::SetPanelStrip(PanelStrip* new_strip) {
 void Panel::SetExpansionState(ExpansionState new_state) {
   if (expansion_state_ == new_state)
     return;
-  old_expansion_state_ = expansion_state_;
   expansion_state_ = new_state;
 
   manager()->OnPanelExpansionStateChanged(this);

chrome/browser/ui/panels/panel.h

@@ -226,7 +226,6 @@ class Panel : public BrowserWindow,
   void SetPanelStrip(PanelStrip* new_strip);
 
   ExpansionState expansion_state() const { return expansion_state_; }
-  ExpansionState old_expansion_state() const { return old_expansion_state_; }
   const gfx::Size& min_size() const { return min_size_; }
   const gfx::Size& max_size() const { return max_size_; }
   bool auto_resizable() const { return auto_resizable_; }
@@ -336,7 +335,6 @@ class Panel : public BrowserWindow,
   NativePanel* native_panel_;  // Weak, owns us.
 
   ExpansionState expansion_state_;
-  ExpansionState old_expansion_state_;
 
   content::NotificationRegistrar registrar_;
 

chrome/browser/ui/panels/panel_manager.cc

@@ -169,17 +169,33 @@ void PanelManager::EndDragging(bool cancelled) {
 }
 
 void PanelManager::OnPanelExpansionStateChanged(Panel* panel) {
+  // For panels outside of the docked strip changing state is a no-op.
+  // But since this method may be called for panels in other strips
+  // we need to check this condition.
+  if (panel->panel_strip() == docked_strip_.get())
     docked_strip_->OnPanelExpansionStateChanged(panel);
+
 }
 
 void PanelManager::OnWindowAutoResized(Panel* panel,
                                        const gfx::Size& preferred_window_size) {
   DCHECK(auto_sizing_enabled_);
+  // Even though overflow panels are always minimized, we need
+  // to keep track of their size to put them back into the
+  // docked strip when they fit. So the docked panel strip manages
+  // the size of panels for the overflow strip as well.
+  if (panel->panel_strip() == overflow_strip_.get())
     docked_strip_->ResizePanelWindow(panel, preferred_window_size);
+  else
+    panel->panel_strip()->ResizePanelWindow(panel, preferred_window_size);
 }
 
 void PanelManager::ResizePanel(Panel* panel, const gfx::Size& new_size) {
+  // See the comment in OnWindowAutoResized()
+  if (panel->panel_strip() == overflow_strip_.get())
     docked_strip_->ResizePanelWindow(panel, new_size);
+  else
+    panel->panel_strip()->ResizePanelWindow(panel, new_size);
   panel->SetAutoResizable(false);
 }
 

chrome/browser/ui/panels/panel_overflow_browsertest.cc

@@ -354,6 +354,46 @@ IN_PROC_BROWSER_TEST_F(PanelOverflowBrowserTest,
   PanelManager::GetInstance()->CloseAll();
 }
 
+IN_PROC_BROWSER_TEST_F(PanelOverflowBrowserTest, AddMinimizedTillOverflow) {
+  PanelManager* panel_manager = PanelManager::GetInstance();
+  DockedPanelStrip* docked_strip = panel_manager->docked_strip();
+  OverflowPanelStrip* overflow_strip = panel_manager->overflow_strip();
+
+  EXPECT_EQ(0, panel_manager->num_panels());
+  EXPECT_EQ(0, docked_strip->num_panels());
+  EXPECT_EQ(0, overflow_strip->num_panels());
+
+  CreatePanelParams params("Test", gfx::Rect(0, 0, 100, 100), SHOW_AS_INACTIVE);
+
+  // We want our panels to be uninitialized at the beginning so that
+  // we test minimizing panels in temp layout and its effect on
+  // the minimized panel count.
+  params.wait_for_fully_created = false;
+
+  unsigned int num_panels_to_add = 10;
+  unsigned int num_panels = 0;
+  for (; num_panels < num_panels_to_add; ++num_panels) {
+    Panel* panel = CreatePanelWithParams(params);
+    panel->SetExpansionState(Panel::MINIMIZED);
+
+    panel->panel_strip()->RemovePanel(panel);
+    EXPECT_EQ(NULL, panel->panel_strip());
+    panel->set_has_temporary_layout(false);
+
+    docked_strip->AddPanel(panel, PanelStrip::DEFAULT_POSITION);
+    EXPECT_EQ(Panel::MINIMIZED, panel->expansion_state());
+    EXPECT_EQ(false, panel->has_temporary_layout());
+  }
+
+  EXPECT_EQ(num_panels_to_add, num_panels);
+  EXPECT_EQ(0, docked_strip->num_temporary_layout_panels());
+  EXPECT_EQ(num_panels, static_cast<unsigned int>
+                (docked_strip->minimized_panel_count() +
+                 overflow_strip->num_panels()));
+
+  PanelManager::GetInstance()->CloseAll();
+}
+
 IN_PROC_BROWSER_TEST_F(PanelOverflowBrowserTest,
                        CreatePanelOnDelayedOverflow) {
   // Create 2 big panels.