DescriptionFix using theme colors for the download shelf's close button.
The theme world is messy:
- ThemeProvider encapsulates some state like incognito status, but not
other state like whether the active or inactive version of a color
should be used (e.g. COLOR_TAB_TEXT vs. COLOR_BACKGROUND_TAB_TEXT).
- A browser window always(?) has a ThemeProvider, but since some views
try to access theme information when they're not in a window, there
are null checks all over.
- Some views listen for theme callbacks. Others, like HoverCloseButton,
expect to be told to redraw when the theme changes.
- -[TabView iconColor] modifies a theme color by tweaking its alpha and
wants its children to use it. It also returns a hard-coded color that
isn't in the theme at all for "dark" themes. This isn't really
supported by the theme system.
The issue that this CL fixes was the result of HoverCloseButton
expecting to be a child of a TabView and falling back to a default
color. This fix doesn't really change that dependency, but it lets
anyone set the color instead of assuming a TabView parent. It will still
use a default color if nobody tells it otherwise, since it doesn't look
at the theme itself.
A more complex solution would let HoverCloseButton pull a color from the
theme but allow that color to be overridden (by TabView), but that
doesn't seem worth it. I'd be a fan of rethinking details of how we deal
with themes to make issues like this harder to come by.
BUG=656553
Review-Url: https://codereview.chromium.org/2668393003
Cr-Commit-Position: refs/heads/master@{#447826}
Committed: https://chromium.googlesource.com/chromium/src/+/524a361c86c23bb4f5384c1738f786532a296e46
Patch Set 1 #
Total comments: 4
Patch Set 2 : Comment iconColor, move the default icon color into a constant. #
Total comments: 2
Patch Set 3 : Delete more unused includes. #Patch Set 4 : Add @private. #
Messages
Total messages: 24 (18 generated)
|