Commit d2f3749c authored by Dana Fried's avatar Dana Fried Committed by Commit Bot

Do not use borders when setting padding on tab close button.

This is a start at eliminating redundant layouts and memory allocations
during layout in the tabstrip. Instead of a border, we use
kInternalPaddingKey with a pre-allocated gfx::Insets. This is the same
approach used by ToolbarButton and will also help if tabs are switched
to using a layout manager.

Ultimately, we want to eliminate both allocation of border objets as
well as all layout invalidations during layout.

Bug: 1121681
Change-Id: Icf7ac6c93eda5ccf80639ab95b2e0c705c1c5d45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2375741
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: default avatarPeter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801572}
parent 43098a24
......@@ -348,8 +348,7 @@ void Tab::Layout() {
const int left = std::min(after_title_padding, close_x);
const int bottom = height() - close_button_size - top;
const int right = std::max(0, width() - (close_x + close_button_size));
close_button_->SetBorder(
views::CreateEmptyBorder(top, left, bottom, right));
close_button_->SetButtonPadding(gfx::Insets(top, left, bottom, right));
close_button_->SetBoundsRect(
{gfx::Point(close_x - left, 0), close_button_->GetPreferredSize()});
}
......
......@@ -27,6 +27,7 @@
#include "ui/views/controls/highlight_path_generator.h"
#include "ui/views/layout/layout_provider.h"
#include "ui/views/rect_based_targeting_utils.h"
#include "ui/views/view_class_properties.h"
#if defined(USE_AURA)
#include "ui/aura/env.h"
......@@ -73,6 +74,10 @@ TabCloseButton::TabCloseButton(views::ButtonListener* listener,
std::make_unique<views::CircleHighlightPathGenerator>(gfx::Insets());
ring_highlight_path->set_use_contents_bounds(true);
focus_ring()->SetPathGenerator(std::move(ring_highlight_path));
// Always have a value on this property so we can modify it directly without
// a heap allocation.
SetProperty(views::kInternalPaddingKey, gfx::Insets());
}
TabCloseButton::~TabCloseButton() {}
......@@ -90,6 +95,10 @@ void TabCloseButton::SetIconColors(SkColor foreground_color,
color_utils::GetColorWithMaxContrast(background_color));
}
void TabCloseButton::SetButtonPadding(const gfx::Insets& padding) {
*GetProperty(views::kInternalPaddingKey) = padding;
}
const char* TabCloseButton::GetClassName() const {
return "TabCloseButton";
}
......@@ -130,12 +139,13 @@ void TabCloseButton::OnGestureEvent(ui::GestureEvent* event) {
event->SetHandled();
}
gfx::Insets TabCloseButton::GetInsets() const {
return ImageButton::GetInsets() + *GetProperty(views::kInternalPaddingKey);
}
gfx::Size TabCloseButton::CalculatePreferredSize() const {
int width = GetGlyphSize();
gfx::Size size(width, width);
gfx::Insets insets = GetInsets();
size.Enlarge(insets.width(), insets.height());
return size;
const int glyph_size = GetGlyphSize();
return gfx::Size(glyph_size, glyph_size) + GetInsets().size();
}
void TabCloseButton::PaintButtonContents(gfx::Canvas* canvas) {
......
......@@ -39,6 +39,13 @@ class TabCloseButton : public views::ImageButton,
// theme changes.
void SetIconColors(SkColor foreground_color, SkColor background_color);
// Sets the desired padding around the icon. Only the icon is a target for
// mouse clicks, but the entire button is a target for touch events, since the
// button itself is small. Note that this is cheaper than, for example,
// installing a new EmptyBorder every time we want to change the padding
// around the icon.
void SetButtonPadding(const gfx::Insets& padding);
// views::ImageButton:
const char* GetClassName() const override;
View* GetTooltipHandlerForPoint(const gfx::Point& point) override;
......@@ -46,6 +53,7 @@ class TabCloseButton : public views::ImageButton,
void OnMouseReleased(const ui::MouseEvent& event) override;
void OnMouseMoved(const ui::MouseEvent& event) override;
void OnGestureEvent(ui::GestureEvent* event) override;
gfx::Insets GetInsets() const override;
protected:
// views::ImageButton:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment