Support nested currentcolor resolution for canvas

https://bugs.webkit.org/show_bug.cgi?id=274120 Reviewed by Darin Adler. Support for 'currentcolor' in canvas was previously hardcoded to look for the explicit string, which made any nested usage, such as in `color-mix()` broken. To resolve this, we remove all special casing of `currentcolor` in the canvas code (simplifying CanvasStyle considerably) and use the CSSUnresolvedColorResolutionContext and its delegate to resolve it at any nesting level. * LayoutTests/imported/w3c/web-platform-tests/html/canvas/element/fill-and-stroke-styles/2d.fillStyle.colormix.currentcolor-expected.txt: - Update results now that we pass. * Source/WebCore/css/color/CSSUnresolvedColorKeyword.cpp: (WebCore::createColor): - Call the context's getter functions to allow lazy lookup of the color values. * Source/WebCore/css/color/CSSUnresolvedColorResolutionContext.cpp: * Source/WebCore/css/color/CSSUnresolvedColorResolutionContext.h: - Re-work the resolution context to contain a delegate that allows the client to only compute color values if they are needed. The client is still able to use the context without a delegate, and explicitly set color values before hand if they want to. The keyword lookup code now calls the functions on the context, which check for the memoized value and then call the delegate if available and necessary. * Source/WebCore/html/canvas/CanvasGradient.idl: * Source/WebCore/html/canvas/CanvasGradient.cpp: (WebCore::CanvasGradient::addColorStop): - Remove special casing of `currentcolor`, instead relying on the elementless `parseColor()` function doing the right thing by specifying `Color::black` for the resolved value. This also now more closely follows the spec text. - Update IDL to pass in a ScriptExecutionContext, which is now needed by the elementless `parseColor()` function. * Source/WebCore/html/canvas/CanvasRenderingContext2DBase.h: * Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp: (WebCore::CanvasRenderingContext2DBase::setStrokeStyle): (WebCore::CanvasRenderingContext2DBase::setFillStyle): - With CanvasStyle no longer having its own invalid state, setStrokeStyle/setFillStyle get an additional overload with an std::optional<CanvasStyle>, which takes the place of the invalid state. - We also remove all the special casing of `currentcolor` that is no longer needed. (WebCore::CanvasRenderingContext2DBase::setShadowColor): (WebCore::CanvasRenderingContext2DBase::setShadow): - We can call the standard `parseColor()` now, as it supports `currentcolor`. (WebCore::toStyleVariant): - Use the new `CanvasStyle.visit(...)` function rather than checking each type one at a time. * Source/WebCore/html/canvas/CanvasStyle.h: - Removes the Invalid and CurrentColor states. CurrentColor is handled by the Color state now. Invalid was only used briefly by rendering context and has been replaced by a std::optional result from the create functions. By removing these states, the switchOn statements can now have all their states handled without ASSERT_NOT_REACHED(). The RefPtrs have also been replaced by Refs, as these are known non-null. (WebCore::CanvasStyle::visit): Added. - Added a helper to switch over the different internal states, eagerly converting to a String for color, as that is what is expected by the canvas rendering context code. (WebCore::CanvasStyle::color): - Add check that Color is the alternative held by the variant. Previously, this function was only called when all the other types had been ruled out, but could easily have caused a crash if misused. (WebCore::CanvasStyle::isCurrentColor const): Deleted. (WebCore::CanvasStyle::overrideAlpha const): Deleted. (WebCore::CanvasStyle::CanvasStyle): Deleted. (WebCore::CanvasStyle::srgbaColor const): Deleted. - Remove a bunch of unused functions. * Source/WebCore/html/canvas/CanvasStyle.cpp: (WebCore::CanvasStyleColorResolutionDelegate::CanvasStyleColorResolutionDelegate): - Implement a simple delegate for color resolution, moving the existing code for resolving `currentcolor` from below. (WebCore::parseColor): - Bottleneck color parsing in two parse functions, one taking a CanvasBase, one a ScriptExecutionContext. The one taking a CanvasBase further refines itself based on whether the CanvasBase is a canvas element, in which case, it creates a resolution delegate for resolving `currentcolor` based on the element style. All other canvases are treated as elementless, getting a hardcoded `Color::black` for `currentcolor` and utilizing the execution context to determine if system colors are supported. (WebCore::CanvasStyle::createFromString): (WebCore::CanvasStyle::createFromStringWithOverrideAlpha): - Remove special casing for `currentcolor`, calling through to the shared `parseColor()` function. (WebCore::CanvasStyle::applyStrokeColor const): (WebCore::CanvasStyle::applyFillColor const): - Update switchOns for Ref use and removed states. (WebCore::isCurrentColorString): Deleted. (WebCore::currentColor): Deleted. (WebCore::parseColorOrCurrentColor): Deleted. - Remove now unused functions. Canonical link: https://commits.webkit.org/279017@main
WebKit · May 20, 2024 · b6495ee · b6495ee
1 parent a19c25f
commit b6495ee
Show file tree

Hide file tree

Showing 12 changed files with 229 additions and 184 deletions.
diff --git a/...tml/canvas/element/fill-and-stroke-styles/2d.fillStyle.colormix.currentcolor-expected.txt b/...tml/canvas/element/fill-and-stroke-styles/2d.fillStyle.colormix.currentcolor-expected.txt
@@ -2,5 +2,5 @@
 color-mix works as color input with currentcolor
 Actual output:
 
-FAIL color-mix works as color input with currentcolor assert_equals: ctx.fillStyle === 'color(srgb 0.5 0 0.5)' (got #000000[string], expected color(srgb 0.5 0 0.5)[string]) expected "color(srgb 0.5 0 0.5)" but got "#000000"
+PASS color-mix works as color input with currentcolor
 
diff --git a/...tests/html/canvas/offscreen/fill-and-stroke-styles/2d.fillStyle.parse.system-expected.txt b/...tests/html/canvas/offscreen/fill-and-stroke-styles/2d.fillStyle.parse.system-expected.txt
@@ -1,5 +1,5 @@
 2d.fillStyle.parse.system
 
 
-FAIL OffscreenCanvas test: 2d.fillStyle.parse.system assert_regexp_match: expected object "/^#(?!(FF0000|ff0000|f00)$)/" but got "#ff0000"
+PASS OffscreenCanvas test: 2d.fillStyle.parse.system
 
diff --git a/Source/WebCore/css/color/CSSUnresolvedColorKeyword.cpp b/Source/WebCore/css/color/CSSUnresolvedColorKeyword.cpp
@@ -52,15 +52,15 @@ Color createColor(const CSSUnresolvedColorKeyword& unresolved, const CSSUnresolv
 {
     switch (unresolved.valueID) {
     case CSSValueInternalDocumentTextColor:
-        return context.internalDocumentTextColor;
+        return context.internalDocumentTextColor();
     case CSSValueWebkitLink:
-        return context.forVisitedLink == Style::ForVisitedLink::Yes ? context.webkitLinkVisited : context.webkitLink;
+        return context.forVisitedLink == Style::ForVisitedLink::Yes ? context.webkitLinkVisited() : context.webkitLink();
     case CSSValueWebkitActivelink:
-        return context.webkitActiveLink;
+        return context.webkitActiveLink();
     case CSSValueWebkitFocusRingColor:
-        return context.webkitFocusRingColor;
+        return context.webkitFocusRingColor();
     case CSSValueCurrentcolor:
-        return context.currentColor;
+        return context.currentColor();
     default:
         return StyleColor::colorFromKeyword(unresolved.valueID, context.keywordOptions);
     }

diff --git a/Source/WebCore/css/color/CSSUnresolvedColorResolutionContext.cpp b/Source/WebCore/css/color/CSSUnresolvedColorResolutionContext.cpp
@@ -26,15 +26,8 @@
 #include "config.h"
 #include "CSSUnresolvedColorResolutionContext.h"
 
-#include "StyleBuilderState.h"
-
 namespace WebCore {
 
-CSSUnresolvedColorResolutionContext::CSSUnresolvedColorResolutionContext()
-    : forVisitedLink { Style::ForVisitedLink::No }
-{
-}
-
-CSSUnresolvedColorResolutionContext::~CSSUnresolvedColorResolutionContext() = default;
+CSSUnresolvedColorResolutionDelegate::~CSSUnresolvedColorResolutionDelegate() = default;
 
 } // namespace WebCore
diff --git a/Source/WebCore/css/color/CSSUnresolvedColorResolutionContext.h b/Source/WebCore/css/color/CSSUnresolvedColorResolutionContext.h
@@ -26,38 +26,98 @@
 #pragma once
 
 #include "Color.h"
+#include "StyleBuilderState.h"
 #include "StyleColor.h"
 #include <wtf/Forward.h>
 #include <wtf/OptionSet.h>
 
 namespace WebCore {
 
-namespace Style {
-enum class ForVisitedLink : bool;
-}
-
 enum class CSSUnresolvedLightDarkAppearance : bool;
 
-struct CSSUnresolvedColorResolutionContext {
-    CSSUnresolvedColorResolutionContext();
-    ~CSSUnresolvedColorResolutionContext();
-
-    Style::ForVisitedLink forVisitedLink;
+class CSSUnresolvedColorResolutionDelegate {
+public:
+    virtual ~CSSUnresolvedColorResolutionDelegate();
 
     // Colors to use that usually get resolved dynamically using Document & RenderStyle.
-    Color currentColor;              // For CSSValueCurrentcolor
-    Color internalDocumentTextColor; // For CSSValueInternalDocumentTextColor
-    Color webkitLink;                // For CSSValueWebkitLink [Style::ForVisitedLink::No]
-    Color webkitLinkVisited;         // For CSSValueWebkitLink [Style::ForVisitedLink::Yes]
-    Color webkitActiveLink;          // For CSSValueWebkitActivelink
-    Color webkitFocusRingColor;      // For CSSValueWebkitFocusRingColor
+    virtual Color currentColor() const { return { }; }              // For CSSValueCurrentcolor
+    virtual Color internalDocumentTextColor() const { return { }; } // For CSSValueInternalDocumentTextColor
+    virtual Color webkitLink() const { return { }; }                // For CSSValueWebkitLink [Style::ForVisitedLink::No]
+    virtual Color webkitLinkVisited() const { return { }; }         // For CSSValueWebkitLink [Style::ForVisitedLink::Yes]
+    virtual Color webkitActiveLink() const { return { }; }          // For CSSValueWebkitActivelink
+    virtual Color webkitFocusRingColor() const { return { }; }      // For CSSValueWebkitFocusRingColor
+};
+
+struct CSSUnresolvedColorResolutionContext {
+    // Delegate for lazily computing color values.
+    std::unique_ptr<CSSUnresolvedColorResolutionDelegate> delegate = nullptr;
+
+    // Whether links should be resolved to the visited style.
+    Style::ForVisitedLink forVisitedLink = Style::ForVisitedLink::No;
 
     // Options to pass when resolving any other keyword with StyleColor::colorFromKeyword()
-    OptionSet<StyleColorOptions> keywordOptions;
+    OptionSet<StyleColorOptions> keywordOptions = { };
 
     // Appearance used to select from a light-dark() color function.
     // If unset, light-dark() colors will return the invalid Color.
-    std::optional<CSSUnresolvedLightDarkAppearance> appearance;
+    std::optional<CSSUnresolvedLightDarkAppearance> appearance = std::nullopt;
+
+    // Colors are resolved:
+    //   1. Checking if the color is set below, and if it is, returning it.
+    //   2. If a delegate has been set, calling the associated delegate function,
+    //      storing the result below, and returning that color.
+    //   3. Returning the invalid `Color` value.
+    mutable std::optional<Color> resolvedCurrentColor = std::nullopt;
+    mutable std::optional<Color> resolvedInternalDocumentTextColor = std::nullopt;
+    mutable std::optional<Color> resolvedWebkitLink = std::nullopt;
+    mutable std::optional<Color> resolvedWebkitLinkVisited = std::nullopt;
+    mutable std::optional<Color> resolvedWebkitActiveLink = std::nullopt;
+    mutable std::optional<Color> resolvedWebkitFocusRingColor = std::nullopt;
+
+    Color currentColor() const
+    {
+        return resolveColor(resolvedCurrentColor, &CSSUnresolvedColorResolutionDelegate::currentColor);
+    }
+
+    Color internalDocumentTextColor() const
+    {
+        return resolveColor(resolvedInternalDocumentTextColor, &CSSUnresolvedColorResolutionDelegate::internalDocumentTextColor);
+    }
+
+    Color webkitLink() const
+    {
+        return resolveColor(resolvedWebkitLink, &CSSUnresolvedColorResolutionDelegate::webkitLink);
+    }
+
+    Color webkitLinkVisited() const
+    {
+        return resolveColor(resolvedWebkitLinkVisited, &CSSUnresolvedColorResolutionDelegate::webkitLinkVisited);
+    }
+
+    Color webkitActiveLink() const
+    {
+        return resolveColor(resolvedWebkitActiveLink, &CSSUnresolvedColorResolutionDelegate::webkitActiveLink);
+    }
+
+    Color webkitFocusRingColor() const
+    {
+        return resolveColor(resolvedWebkitFocusRingColor, &CSSUnresolvedColorResolutionDelegate::webkitFocusRingColor);
+    }
+
+private:
+    Color resolveColor(std::optional<Color>& existing, Color (CSSUnresolvedColorResolutionDelegate::*resolver)() const) const
+    {
+        if (existing)
+            return *existing;
+
+        if (delegate) {
+            auto resolved = ((*delegate).*resolver)();
+            existing = resolved;
+            return resolved;
+        }
+
+        return { };
+    }
 };
 
 } // namespace WebCore
diff --git a/Source/WebCore/html/canvas/CanvasGradient.cpp b/Source/WebCore/html/canvas/CanvasGradient.cpp
@@ -68,20 +68,12 @@ Ref<CanvasGradient> CanvasGradient::create(const FloatPoint& centerPoint, float
 
 CanvasGradient::~CanvasGradient() = default;
 
-ExceptionOr<void> CanvasGradient::addColorStop(double value, const String& colorString)
+ExceptionOr<void> CanvasGradient::addColorStop(ScriptExecutionContext& scriptExecutionContext, double value, const String& colorString)
 {
     if (!(value >= 0 && value <= 1))
         return Exception { ExceptionCode::IndexSizeError };
 
-    // Treat currentColor as black, as required by the standard.
-    Color color;
-    if (isCurrentColorString(colorString))
-        color = Color::black;
-    else if (m_context)
-        color = parseColor(colorString, m_context->canvasBase());
-    else
-        color = parseColor(colorString);
-
+    auto color = parseColor(colorString, scriptExecutionContext);
     if (!color.isValid())
         return Exception { ExceptionCode::SyntaxError };
 

diff --git a/Source/WebCore/html/canvas/CanvasGradient.h b/Source/WebCore/html/canvas/CanvasGradient.h
@@ -33,6 +33,7 @@ namespace WebCore {
 
 class CanvasRenderingContext;
 class Gradient;
+class ScriptExecutionContext;
 
 class CanvasGradient : public RefCounted<CanvasGradient> {
 public:
@@ -44,7 +45,7 @@ class CanvasGradient : public RefCounted<CanvasGradient> {
     Gradient& gradient() { return m_gradient; }
     const Gradient& gradient() const { return m_gradient; }
 
-    ExceptionOr<void> addColorStop(double value, const String& color);
+    ExceptionOr<void> addColorStop(ScriptExecutionContext&, double value, const String& color);
 
 private:
     CanvasGradient(const FloatPoint& p0, const FloatPoint& p1, CanvasRenderingContext&);

diff --git a/Source/WebCore/html/canvas/CanvasGradient.idl b/Source/WebCore/html/canvas/CanvasGradient.idl
@@ -28,5 +28,5 @@
     Exposed=(Window,Worker),
 ] interface CanvasGradient {
     // opaque object
-    undefined addColorStop(double offset, DOMString color);
+    [CallWith=CurrentScriptExecutionContext] undefined addColorStop(double offset, DOMString color);
 };
diff --git a/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp b/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.cpp
@@ -498,43 +498,38 @@ void CanvasRenderingContext2DBase::restore()
 
 void CanvasRenderingContext2DBase::setStrokeStyle(CanvasStyle style)
 {
-    if (!style.isValid())
-        return;
-
     if (state().strokeStyle.isEquivalentColor(style))
         return;
 
-    if (style.isCurrentColor())
-        style = CanvasStyle(currentColor(canvasBase()).colorWithAlpha(style.overrideAlpha()));
-    else
-        checkOrigin(style.canvasPattern().get());
+    checkOrigin(style.canvasPattern().get());
 
     realizeSaves();
     State& state = modifiableState();
-    state.strokeStyle = style;
+    state.strokeStyle = WTFMove(style);
     GraphicsContext* c = drawingContext();
     if (!c)
         return;
     state.strokeStyle.applyStrokeColor(*c);
     state.unparsedStrokeColor = String();
 }
 
-void CanvasRenderingContext2DBase::setFillStyle(CanvasStyle style)
+void CanvasRenderingContext2DBase::setStrokeStyle(std::optional<CanvasStyle> style)
 {
-    if (!style.isValid())
+    if (!style)
         return;
+    return setStrokeStyle(WTFMove(*style));
+}
 
+void CanvasRenderingContext2DBase::setFillStyle(CanvasStyle style)
+{
     if (state().fillStyle.isEquivalentColor(style))
         return;
 
-    if (style.isCurrentColor())
-        style = CanvasStyle(currentColor(canvasBase()).colorWithAlpha(style.overrideAlpha()));
-    else
-        checkOrigin(style.canvasPattern().get());
+    checkOrigin(style.canvasPattern().get());
 
     realizeSaves();
     State& state = modifiableState();
-    state.fillStyle = style;
+    state.fillStyle = WTFMove(style);
     GraphicsContext* c = drawingContext();
     if (!c)
         return;
@@ -543,6 +538,13 @@ void CanvasRenderingContext2DBase::setFillStyle(CanvasStyle style)
     state.unparsedFillColor = String();
 }
 
+void CanvasRenderingContext2DBase::setFillStyle(std::optional<CanvasStyle> style)
+{
+    if (!style)
+        return;
+    return setFillStyle(WTFMove(*style));
+}
+
 void CanvasRenderingContext2DBase::setLineWidth(double width)
 {
     if (!(std::isfinite(width) && width > 0))
@@ -662,7 +664,7 @@ void CanvasRenderingContext2DBase::setShadowBlur(float blur)
 
 void CanvasRenderingContext2DBase::setShadowColor(const String& colorString)
 {
-    Color color = parseColorOrCurrentColor(colorString, canvasBase());
+    Color color = parseColor(colorString, canvasBase());
     if (!color.isValid())
         return;
     if (state().shadowColor == color)
@@ -1416,7 +1418,7 @@ void CanvasRenderingContext2DBase::setShadow(float width, float height, float bl
 
     Color color = Color::transparentBlack;
     if (!colorString.isNull()) {
-        color = parseColorOrCurrentColor(colorString, canvasBase());
+        color = parseColor(colorString, canvasBase());
         if (!color.isValid())
             return;
     }
@@ -2055,11 +2057,11 @@ template<class T> void CanvasRenderingContext2DBase::fullCanvasCompositedDrawIma
 
 static CanvasRenderingContext2DBase::StyleVariant toStyleVariant(const CanvasStyle& style)
 {
-    if (auto gradient = style.canvasGradient())
-        return gradient;
-    if (auto pattern = style.canvasPattern())
-        return pattern;
-    return style.color();
+    return style.visit(
+        [](const String& string) -> CanvasRenderingContext2DBase::StyleVariant { return string; },
+        [](const Ref<CanvasGradient>& gradient) -> CanvasRenderingContext2DBase::StyleVariant { return gradient.ptr(); },
+        [](const Ref<CanvasPattern>& pattern) -> CanvasRenderingContext2DBase::StyleVariant { return pattern.ptr(); }
+    );
 }
 
 CanvasRenderingContext2DBase::StyleVariant CanvasRenderingContext2DBase::strokeStyle() const
@@ -2069,10 +2071,10 @@ CanvasRenderingContext2DBase::StyleVariant CanvasRenderingContext2DBase::strokeS
 
 void CanvasRenderingContext2DBase::setStrokeStyle(CanvasRenderingContext2DBase::StyleVariant&& style)
 {
-    WTF::switchOn(style,
-        [this] (const String& string) { this->setStrokeColor(string); },
-        [this] (const RefPtr<CanvasGradient>& gradient) { this->setStrokeStyle(CanvasStyle(*gradient)); },
-        [this] (const RefPtr<CanvasPattern>& pattern) { this->setStrokeStyle(CanvasStyle(*pattern)); }
+    WTF::switchOn(WTFMove(style),
+        [this](String&& string) { this->setStrokeColor(WTFMove(string)); },
+        [this](RefPtr<CanvasGradient>&& gradient) { this->setStrokeStyle(CanvasStyle(gradient.releaseNonNull())); },
+        [this](RefPtr<CanvasPattern>&& pattern) { this->setStrokeStyle(CanvasStyle(pattern.releaseNonNull())); }
     );
 }
 
@@ -2083,10 +2085,10 @@ CanvasRenderingContext2DBase::StyleVariant CanvasRenderingContext2DBase::fillSty
 
 void CanvasRenderingContext2DBase::setFillStyle(CanvasRenderingContext2DBase::StyleVariant&& style)
 {
-    WTF::switchOn(style,
-        [this] (const String& string) { this->setFillColor(string); },
-        [this] (const RefPtr<CanvasGradient>& gradient) { this->setFillStyle(CanvasStyle(*gradient)); },
-        [this] (const RefPtr<CanvasPattern>& pattern) { this->setFillStyle(CanvasStyle(*pattern)); }
+    WTF::switchOn(WTFMove(style),
+        [this](String&& string) { this->setFillColor(WTFMove(string)); },
+        [this](RefPtr<CanvasGradient>&& gradient) { this->setFillStyle(CanvasStyle(gradient.releaseNonNull())); },
+        [this](RefPtr<CanvasPattern>&& pattern) { this->setFillStyle(CanvasStyle(pattern.releaseNonNull())); }
     );
 }
 

diff --git a/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.h b/Source/WebCore/html/canvas/CanvasRenderingContext2DBase.h
@@ -402,7 +402,9 @@ class CanvasRenderingContext2DBase : public CanvasRenderingContext, public Canva
     void realizeSavesLoop();
 
     void setStrokeStyle(CanvasStyle);
+    void setStrokeStyle(std::optional<CanvasStyle>);
     void setFillStyle(CanvasStyle);
+    void setFillStyle(std::optional<CanvasStyle>);
 
     ExceptionOr<RefPtr<CanvasPattern>> createPattern(CachedImage&, RenderElement*, bool repeatX, bool repeatY);
     ExceptionOr<RefPtr<CanvasPattern>> createPattern(HTMLImageElement&, bool repeatX, bool repeatY);