Cleanup to keep panel from manipulating its panel strip assignment directly.

chrome/browser/ui/panels/overflow_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/overflow_panel_strip.h"
 
 #include "base/logging.h"
 #include "chrome/browser/ui/panels/docked_panel_strip.h"
 #include "chrome/browser/ui/panels/panel_manager.h"
 #include "chrome/browser/ui/panels/panel_mouse_watcher.h"
@@ skipping 64 matching lines @@
 }
 
 void OverflowPanelStrip::UpdateCurrentWidth() {
   current_display_width_ = are_overflow_titles_shown_ ? kOverflowAreaHoverWidth
                                                       : display_area_.width();
 }
 
 void OverflowPanelStrip::AddPanel(Panel* panel) {
   // TODO(jianli): consider using other container to improve the perf for
   // inserting to the front. http://crbug.com/106222
-  DCHECK_EQ(this, panel->panel_strip());
+  DCHECK_NE(this, panel->panel_strip());
+  panel->SetPanelStrip(this);
+
   // Newly created panels that were temporarily in the panel strip
   // are added to the back of the overflow, whereas panels that are
   // bumped from the panel strip by other panels go to the front
   // of overflow.
   if (panel->has_temporary_layout()) {
     panel->set_has_temporary_layout(false);
     panels_.push_back(panel);
     DoRefresh(panels_.size() - 1, panels_.size() - 1);
   } else {
     panels_.insert(panels_.begin(), panel);
@@ skipping 12 matching lines @@
   // Update the overflow indicator only when the number of overflow panels go
   // beyond the maximum visible limit.
   if (num_panels() > max_visible_panels_) {
     if (!overflow_indicator_.get()) {
       overflow_indicator_.reset(PanelOverflowIndicator::Create());
     }
     UpdateOverflowIndicatorCount();
   }
 }
 
-bool OverflowPanelStrip::RemovePanel(Panel* panel) {
+void OverflowPanelStrip::RemovePanel(Panel* panel) {
+  DCHECK_EQ(this, panel->panel_strip());
   size_t index = 0;
   Panels::iterator iter = panels_.begin();
   for (; iter != panels_.end(); ++iter, ++index)
     if (*iter == panel)
       break;
-  if (iter == panels_.end())
-    return false;
+  DCHECK(iter != panels_.end());
 
   panels_.erase(iter);
   DoRefresh(index, panels_.size() - 1);
 
   if (panels_.empty() && !panel_manager_->is_full_screen())
     panel_manager_->mouse_watcher()->RemoveObserver(this);
 
   // Update the overflow indicator. If the number of overflow panels fall below
   // the maximum visible limit, we do not need the overflow indicator any more.
   if (num_panels() < max_visible_panels_)
     overflow_indicator_.reset();
   else
     UpdateOverflowIndicatorCount();

[Andrei, 2012/03/01 00:08:50]

Same comment about SetPanelStrip() as for the docked strip

-
-  return true;
 }
 
 void OverflowPanelStrip::CloseAll() {
   // Make a copy of the iterator as closing panels can modify the vector.
   Panels panels_copy = panels_;
 
   // Start from the bottom to avoid reshuffling.
   for (Panels::reverse_iterator iter = panels_copy.rbegin();
        iter != panels_copy.rend(); ++iter)
     (*iter)->Close();
 }
 
 void OverflowPanelStrip::ResizePanelWindow(
     Panel* panel, const gfx::Size& preferred_window_size) {
   // Overflow uses its own panel window sizes.
 }
 
 void OverflowPanelStrip::OnPanelAttentionStateChanged(Panel* panel) {
   DCHECK_EQ(this, panel->panel_strip());
   UpdateOverflowIndicatorAttention();
 }
 
 void OverflowPanelStrip::ActivatePanel(Panel* panel) {
   DCHECK_EQ(this, panel->panel_strip());
   // Activating an overflow panel moves it to the docked panel strip.
-  PanelStrip* docked_strip = panel_manager_->docked_strip();
-  panel->MoveToStrip(docked_strip);
-  docked_strip->ActivatePanel(panel);
+  panel_manager_->ChangePanelLayout(panel, PanelStrip::DOCKED);

[Andrei, 2012/03/01 00:08:50]

Ideally making sure a panel CAN be activated before calling this should be the
job of the caller. 
I'll put it this way: changing panel layout for a panel is a system-wide side
effect. Are the callers always prepared and aware of the fact that, by calling
ActivatePanel(), they are making a change that reaches far beyond this panel?

Of course I am not sure how costly it would be to change :)

[jennb, 2012/03/01 00:30:58]

For this activation path, the panel can always be activated. This is how a
minimized panel gets restored to expanded mode, or an overflow panel gets moved
back to the dock strip. One usage of this is when extensions want to give focus
to a panel.

+  panel->panel_strip()->ActivatePanel(panel);
 }
 
 void OverflowPanelStrip::MinimizePanel(Panel* panel) {
   DCHECK_EQ(this, panel->panel_strip());
   // Overflow is already a minimized mode for a panel. Nothing more to do.
 }
 
 void OverflowPanelStrip::RestorePanel(Panel* panel) {
   DCHECK_EQ(this, panel->panel_strip());
-  PanelStrip* docked_strip = panel_manager_->docked_strip();
-  panel->MoveToStrip(docked_strip);
-  docked_strip->RestorePanel(panel);
+  panel_manager_->ChangePanelLayout(panel, PanelStrip::DOCKED);

[Andrei, 2012/03/01 00:08:50]

Same as above.

[jennb, 2012/03/01 00:30:58]

Ditto above; this is always allowed.

+  panel->panel_strip()->RestorePanel(panel);
 }
 
 bool OverflowPanelStrip::CanShowPanelAsActive(const Panel* panel) const {
   // All overflow panels cannot be shown as active.
   return false;
 }
 
 bool OverflowPanelStrip::CanDragPanel(const Panel* panel) const {
   // All overflow panels are not draggable.
   return false;
@@ skipping 204 matching lines @@
   // expanded overflow panels when in full screen mode so no need
   // to detect when mouse hovers over the overflow strip.
   if (is_full_screen)
     panel_manager_->mouse_watcher()->RemoveObserver(this);
   else
     panel_manager_->mouse_watcher()->AddObserver(this);
 
   for (size_t i = 0; i < panels_.size(); ++i)
     panels_[i]->FullScreenModeChanged(is_full_screen);
 }