Reunite PanelStrip::AddPanel and its internals.

chrome/browser/ui/panels/panel_strip.cc

Index: chrome/browser/ui/panels/panel_strip.cc
diff --git a/chrome/browser/ui/panels/panel_strip.cc b/chrome/browser/ui/panels/panel_strip.cc
index 03289f1c2a6ba81c00d3936ebec19fbe4017e302..1ee74cdc71120b878b0d0273aa130e87f750e8f6 100644
--- a/chrome/browser/ui/panels/panel_strip.cc
+++ b/chrome/browser/ui/panels/panel_strip.cc
@@ -84,16 +84,10 @@ void PanelStrip::SetDisplayArea(const gfx::Rect& new_area) {
 }
 
 void PanelStrip::AddPanel(Panel* panel) {
-  if (panel->initialized())
-    AddExistingPanel(panel);
-  else
-    AddNewPanel(panel);
-  panels_.push_back(panel);
-}
-
-void PanelStrip::AddNewPanel(Panel* panel) {
-  DCHECK(!panel->initialized());
+  DCHECK_EQ(Panel::EXPANDED, panel->expansion_state());

[jianli, 2011/12/02 01:56:38]

This might not be true if |panel| is overflow panel.

[jennb, 2011/12/02 02:20:37]

Removed check for now as your patch will have logic for this.

 
+  // Always update limits, even for exiting panels, in case the maximums changed
+  // while panel was out of the strip.
   int max_panel_width = GetMaxPanelWidth();
   int max_panel_height = GetMaxPanelHeight();
   panel->SetSizeRange(gfx::Size(kPanelMinWidth, kPanelMinHeight),
@@ -103,58 +97,58 @@ void PanelStrip::AddNewPanel(Panel* panel) {
   int height = restored_size.height();
   int width = restored_size.width();
 
-  if (height == 0 && width == 0) {
-    // Auto resizable is enabled only if no initial size is provided.
-    panel->SetAutoResizable(true);
+  if (panel->initialized()) {
+    // Bump panels in the strip to make room for this panel.
+    int x;
+    while ((x = GetRightMostAvailablePosition() - width) < display_area_.x()) {
+      DCHECK(!panels_.empty());
+      MovePanelToOverflow(panels_.back(), false);
+    }
+    int y = display_area_.bottom() - height;
+    panel->SetPanelBounds(gfx::Rect(x, y, width, height));
   } else {
-    if (height == 0)
-      height = width / kPanelDefaultWidthToHeightRatio;
-    if (width == 0)
-      width = height * kPanelDefaultWidthToHeightRatio;
-  }
+    // Initialize the newly created panel. Does not bump any panels from strip.
+    if (height == 0 && width == 0) {
+      // Auto resizable is enabled only if no initial size is provided.
+      panel->SetAutoResizable(true);
+    } else {
+      if (height == 0)
+        height = width / kPanelDefaultWidthToHeightRatio;
+      if (width == 0)
+        width = height * kPanelDefaultWidthToHeightRatio;
+    }
 
-  // Constrain sizes to limits.
-  if (width < kPanelMinWidth)
-    width = kPanelMinWidth;
-  else if (width > max_panel_width)
-    width = max_panel_width;
-
-  if (height < kPanelMinHeight)
-    height = kPanelMinHeight;
-  else if (height > max_panel_height)
-    height = max_panel_height;
-  panel->set_restored_size(gfx::Size(width, height));
-
-  // Layout the new panel.
-  int y = display_area_.bottom() - height;
-  int x = GetRightMostAvailablePosition() - width;
-
-  // Keep panel visible in the strip even if overlap would occur.
-  // Panel is moved to overflow from the strip after a delay.
-  if (x < display_area_.x()) {
-    x = display_area_.x();
-    MessageLoop::current()->PostDelayedTask(
-        FROM_HERE,
-        base::Bind(&PanelStrip::MovePanelToOverflow,
-                   overflow_action_factory_.GetWeakPtr(),
-                   panel,
-                   true),  // new panel
-        kMoveNewPanelToOverflowDelayMilliseconds);
-  }
-  panel->Initialize(gfx::Rect(x, y, width, height));
-}
+    // Constrain sizes to limits.
+    if (width < kPanelMinWidth)
+      width = kPanelMinWidth;
+    else if (width > max_panel_width)
+      width = max_panel_width;
 
-void PanelStrip::AddExistingPanel(Panel* panel) {
-  gfx::Size restored_size = panel->restored_size();
-  int height = restored_size.height();
-  int width = restored_size.width();
-  int x;
-  while ((x = GetRightMostAvailablePosition() - width) < display_area_.x()) {
-    DCHECK(!panels_.empty());
-    MovePanelToOverflow(panels_.back(), false);
+    if (height < kPanelMinHeight)
+      height = kPanelMinHeight;
+    else if (height > max_panel_height)
+      height = max_panel_height;
+
+    panel->set_restored_size(gfx::Size(width, height));
+    int x = GetRightMostAvailablePosition() - width;
+    int y = display_area_.bottom() - height;
+
+    // Keep panel visible in the strip even if overlap would occur.
+    // Panel is moved to overflow from the strip after a delay.
+    if (x < display_area_.x()) {
+      x = display_area_.x();
+      MessageLoop::current()->PostDelayedTask(
+          FROM_HERE,
+          base::Bind(&PanelStrip::MovePanelToOverflow,
+                     overflow_action_factory_.GetWeakPtr(),

[dcheng, 2011/12/02 01:29:20]

It may be better to remove the WeakPtrFactory for now since we don't need
cancellation semantics.

[jennb, 2011/12/02 02:20:37]

Done.

+                     panel,
+                     true),  // new panel
+          kMoveNewPanelToOverflowDelayMilliseconds);

[jianli, 2011/12/02 01:56:38]

Can you check remove_delays_for_testing_ here and use 0 for this case?

[jennb, 2011/12/02 02:20:37]

Done.

+    }
+    panel->Initialize(gfx::Rect(x, y, width, height));
   }
-  int y = display_area_.bottom() - height;
-  panel->SetPanelBounds(gfx::Rect(x, y, width, height));
+
+  panels_.push_back(panel);
 }
 
 int PanelStrip::GetMaxPanelWidth() const {