Commit a903b7b1 authored by Mason Freed's avatar Mason Freed Committed by Commit Bot

Remove magic number for Mac focus ring alpha

Prior to this CL, there was a magic 166 alpha applied to the
system focus ring color (the Accent Color setting). This has
now been pulled into native_theme.

Along the way, I also added a Color::operator SkColor() to
convert blink::Color to SkColor, and used it in gradient.cc,
along with here in layout_theme_mac.mm. I'm sure there are
other such converters sprinkled around which could be
migrated, but this was all I found.

Quick note on testing: this change will be best tested by [1]
once it lands.

[1] https://chromium-review.googlesource.com/c/chromium/src/+/2150765

Fixed: 1108169
Change-Id: I54ac4e60a1df1b39123721adffda9f39aac94fe4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2317294
Auto-Submit: Mason Freed <masonfreed@chromium.org>
Reviewed-by: default avatarElly Fong-Jones <ellyjones@chromium.org>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792223}
parent 272076f3
......@@ -133,7 +133,8 @@ SkColor AccessibilityFocusHighlight::GetHighlightColor() {
if (theme_color == SK_ColorTRANSPARENT || use_default_color_for_testing_)
return default_color_;
return theme_color;
return browser_view_->GetNativeTheme()->FocusRingColorForBaseColor(
theme_color);
}
void AccessibilityFocusHighlight::CreateOrUpdateLayer(gfx::Rect node_bounds) {
......@@ -256,10 +257,6 @@ void AccessibilityFocusHighlight::OnPaintLayer(
const ui::PaintContext& context) {
ui::PaintRecorder recorder(context, layer_->size());
SkColor highlight_color = GetHighlightColor();
#if defined(OS_MACOSX)
// Match blink::LayoutThemeMacRefresh::FocusRingColor()
highlight_color = SkColorSetA(highlight_color, 166);
#endif
cc::PaintFlags original_flags;
original_flags.setAntiAlias(true);
......
......@@ -362,13 +362,12 @@ Color LayoutThemeMacRefresh::FocusRingColor() const {
// TODO(crbug.com/929098) Need to pass an appropriate color scheme here.
WebColorScheme color_scheme = ComputedStyle::InitialStyle().UsedColorScheme();
Color keyboard_focus_indicator =
GetSystemColor(MacSystemColorID::kKeyboardFocusIndicator, color_scheme);
// Take the RGB values from the keyboard_focus_indicator color, but use a
// different alpha value to avoid having a color too light.
SkColor keyboard_focus_indicator = SkColor(
GetSystemColor(MacSystemColorID::kKeyboardFocusIndicator, color_scheme));
Color focus_ring =
Color(keyboard_focus_indicator.Red(), keyboard_focus_indicator.Green(),
keyboard_focus_indicator.Blue(), /*alpha=*/166);
ui::NativeTheme::GetInstanceForWeb()->FocusRingColorForBaseColor(
keyboard_focus_indicator);
if (!HasCustomFocusRingColor())
return focus_ring;
// Use the custom focus ring color when the system accent color wasn't
......
......@@ -33,6 +33,7 @@
#include "third_party/blink/renderer/platform/wtf/math_extras.h"
#include "third_party/blink/renderer/platform/wtf/text/string_builder.h"
#include "third_party/blink/renderer/platform/wtf/text/string_view.h"
#include "third_party/skia/include/core/SkColor.h"
namespace blink {
......@@ -261,6 +262,10 @@ bool Color::SetNamedColor(const String& name) {
return found_color;
}
Color::operator SkColor() const {
return SkColorSetARGB(Alpha(), Red(), Green(), Blue());
}
Color Color::Light() const {
// Hardcode this common case for speed.
if (color_ == kBlack)
......
......@@ -31,6 +31,8 @@
#include "third_party/blink/renderer/platform/wtf/forward.h"
#include "third_party/blink/renderer/platform/wtf/text/unicode.h"
typedef uint32_t SkColor;
namespace blink {
class Color;
......@@ -120,6 +122,8 @@ class PLATFORM_EXPORT Color {
void GetRGBA(double& r, double& g, double& b, double& a) const;
void GetHSL(double& h, double& s, double& l) const;
explicit operator SkColor() const;
Color Light() const;
Color Dark() const;
......
......@@ -85,11 +85,6 @@ void Gradient::SortStopsIfNecessary() {
std::stable_sort(stops_.begin(), stops_.end(), CompareStops);
}
// FIXME: This would be more at home as Color::operator SkColor.
static inline SkColor MakeSkColor(const Color& c) {
return SkColorSetARGB(c.Alpha(), c.Red(), c.Green(), c.Blue());
}
// Collect sorted stop position and color information into the pos and colors
// buffers, ensuring stops at both 0.0 and 1.0.
// TODO(fmalita): theoretically Skia should provide the same 0.0/1.0 padding
......@@ -108,18 +103,18 @@ void Gradient::FillSkiaStops(ColorBuffer& colors, OffsetBuffer& pos) const {
pos.push_back(WebCoreDoubleToSkScalar(0));
if (color_filter_) {
colors.push_back(
color_filter_->filterColor(MakeSkColor(stops_.front().color)));
color_filter_->filterColor(SkColor(stops_.front().color)));
} else {
colors.push_back(MakeSkColor(stops_.front().color));
colors.push_back(SkColor(stops_.front().color));
}
}
for (const auto& stop : stops_) {
pos.push_back(WebCoreDoubleToSkScalar(stop.stop));
if (color_filter_)
colors.push_back(color_filter_->filterColor(MakeSkColor(stop.color)));
colors.push_back(color_filter_->filterColor(SkColor(stop.color)));
else
colors.push_back(MakeSkColor(stop.color));
colors.push_back(SkColor(stop.color));
}
// Copy the last stop to 1.0 if needed. See comment above about this float
......
......@@ -203,6 +203,10 @@ SkColor NativeTheme::GetSystemButtonPressedColor(SkColor base_color) const {
return base_color;
}
SkColor NativeTheme::FocusRingColorForBaseColor(SkColor base_color) const {
return base_color;
}
float NativeTheme::GetBorderRadiusForPart(Part part,
float width,
float height,
......
......@@ -471,6 +471,9 @@ class NATIVE_THEME_EXPORT NativeTheme {
// pressed states.
virtual SkColor GetSystemButtonPressedColor(SkColor base_color) const;
// Assign the focus-ring-appropriate alpha value to the provided base_color.
virtual SkColor FocusRingColorForBaseColor(SkColor base_color) const;
protected:
explicit NativeTheme(bool should_only_use_dark_colors);
virtual ~NativeTheme();
......
......@@ -100,6 +100,17 @@ NativeThemeAura* NativeThemeAura::web_instance() {
return s_native_theme_for_web.get();
}
SkColor NativeThemeAura::FocusRingColorForBaseColor(SkColor base_color) const {
#if defined(OS_MACOSX)
DCHECK(features::IsFormControlsRefreshEnabled());
// On Mac OSX, the system Accent Color setting is darkened a bit
// for better contrast.
return SkColorSetA(base_color, 166);
#else
return base_color;
#endif // OS_MACOSX
}
void NativeThemeAura::PaintMenuPopupBackground(
cc::PaintCanvas* canvas,
const gfx::Size& size,
......
......@@ -24,6 +24,9 @@ class NATIVE_THEME_EXPORT NativeThemeAura : public NativeThemeBase {
static NativeThemeAura* web_instance();
// Overridden from NativeTheme:
SkColor FocusRingColorForBaseColor(SkColor base_color) const override;
// NativeThemeBase:
void PaintMenuPopupBackground(
cc::PaintCanvas* canvas,
......
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