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

Unified Diff: chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc

Issue 1330423003: [Extensions Toolbar] Protect against crazy bounds (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Final nits Created 5 years, 3 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/views/toolbar/extension_toolbar_menu_view.cc
diff --git a/chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc b/chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc
index 5e70b294d75e57c794d90b8950b01b6535067724..6b6e79324c578d7ae4d43b01858632963f943574 100644
--- a/chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc
+++ b/chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.cc
@@ -21,7 +21,8 @@ ExtensionToolbarMenuView::ExtensionToolbarMenuView(Browser* browser,
WrenchMenu* wrench_menu)
: browser_(browser),
wrench_menu_(wrench_menu),
- container_(NULL),
+ container_(nullptr),
+ max_height_(0),
browser_actions_container_observer_(this),
weak_factory_(this) {
BrowserActionsContainer* main =
@@ -29,7 +30,7 @@ ExtensionToolbarMenuView::ExtensionToolbarMenuView(Browser* browser,
->toolbar()->browser_actions();
container_ = new BrowserActionsContainer(browser_, main);
container_->Init();
- AddChildView(container_);
+ SetContents(container_);
// We Layout() the container here so that we know the number of actions
// that will be visible in ShouldShow().
container_->Layout();
@@ -40,6 +41,14 @@ ExtensionToolbarMenuView::ExtensionToolbarMenuView(Browser* browser,
browser_actions_container_observer_.Add(container_);
browser_actions_container_observer_.Add(main);
}
+
+ // In *very* extreme cases, it's possible that there are so many overflowed
+ // actions, we won't be able to show them all. Cap the height so that the
+ // overflow won't be excessively tall (at 8 icons per row, this allows for
+ // 104 total extensions).
+ const int kMaxOverflowRows = 13;
+ max_height_ = ToolbarActionsBar::IconHeight() * kMaxOverflowRows;
+ ClipHeightTo(0, max_height_);
}
ExtensionToolbarMenuView::~ExtensionToolbarMenuView() {
@@ -51,24 +60,30 @@ bool ExtensionToolbarMenuView::ShouldShow() {
}
gfx::Size ExtensionToolbarMenuView::GetPreferredSize() const {
- return container_->GetPreferredSize();
+ gfx::Size s = views::ScrollView::GetPreferredSize();
+ // views::ScrollView::GetPreferredSize() includes the contents' size, but
+ // not the scrollbar width. Add it in if necessary.
+ if (container_->GetPreferredSize().height() > max_height_)
+ s.Enlarge(GetScrollBarWidth(), 0);
+ return s;
}
int ExtensionToolbarMenuView::GetHeightForWidth(int width) const {
+ // The width passed in here includes the full width of the menu, so we need
+ // to omit the necessary padding.
const views::MenuConfig& menu_config =
static_cast<const views::MenuItemView*>(parent())->GetMenuConfig();
int end_padding = menu_config.arrow_to_edge_padding -
container_->toolbar_actions_bar()->platform_settings().item_spacing;
width -= start_padding() + end_padding;
- int height = container_->GetHeightForWidth(width);
- return height;
+ return views::ScrollView::GetHeightForWidth(width);
}
void ExtensionToolbarMenuView::Layout() {
- gfx::Size sz = GetPreferredSize();
- SetBounds(start_padding() + 1, 0, sz.width(), sz.height());
- container_->SetBounds(0, 0, sz.width(), sz.height());
+ SetPosition(gfx::Point(start_padding(), 0));
+ SizeToPreferredSize();
+ views::ScrollView::Layout();
}
void ExtensionToolbarMenuView::OnBrowserActionDragDone() {
@@ -89,10 +104,9 @@ void ExtensionToolbarMenuView::CloseWrenchMenu() {
int ExtensionToolbarMenuView::start_padding() const {
// We pad enough on the left so that the first icon starts at the same point
- // as the labels. We need to subtract 1 because we want the pixel *before*
- // the label, and we subtract kItemSpacing because there needs to be padding
+ // as the labels. We subtract kItemSpacing because there needs to be padding
// so we can see the drop indicator.
- return views::MenuItemView::label_start() - 1 -
+ return views::MenuItemView::label_start() -
container_->toolbar_actions_bar()->platform_settings().item_spacing;
}
« no previous file with comments | « chrome/browser/ui/views/toolbar/extension_toolbar_menu_view.h ('k') | chrome/browser/ui/views/toolbar/toolbar_view.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698