Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(58)

Unified Diff: chrome/browser/ui/panels/overflow_panel_strip.cc

Issue 9560002: Cleanup to keep panel from manipulating its panel strip assignment directly. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Created 8 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/browser/ui/panels/overflow_panel_strip.cc
diff --git a/chrome/browser/ui/panels/overflow_panel_strip.cc b/chrome/browser/ui/panels/overflow_panel_strip.cc
index 934d71857f9359290249de2ee8269942428b3651..3f41812f5fabdf829cd36313e453775d0e52e493 100644
--- a/chrome/browser/ui/panels/overflow_panel_strip.cc
+++ b/chrome/browser/ui/panels/overflow_panel_strip.cc
@@ -82,7 +82,9 @@ void OverflowPanelStrip::UpdateCurrentWidth() {
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
@@ -115,14 +117,14 @@ void OverflowPanelStrip::AddPanel(Panel* panel) {
}
}
-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);
@@ -136,8 +138,6 @@ bool OverflowPanelStrip::RemovePanel(Panel* panel) {
overflow_indicator_.reset();
else
UpdateOverflowIndicatorCount();
Andrei 2012/03/01 00:08:50 Same comment about SetPanelStrip() as for the dock
-
- return true;
}
void OverflowPanelStrip::CloseAll() {
@@ -163,9 +163,8 @@ void OverflowPanelStrip::OnPanelAttentionStateChanged(Panel* panel) {
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 befor
jennb 2012/03/01 00:30:58 For this activation path, the panel can always be
+ panel->panel_strip()->ActivatePanel(panel);
}
void OverflowPanelStrip::MinimizePanel(Panel* panel) {
@@ -175,9 +174,8 @@ void OverflowPanelStrip::MinimizePanel(Panel* panel) {
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 {

Powered by Google App Engine
This is Rietveld 408576698