Title: [257966] trunk/Source/WebCore
Revision
257966
Author
da...@apple.com
Date
2020-03-05 20:11:39 -0800 (Thu, 05 Mar 2020)

Log Message

Simplify gradient parsing
https://bugs.webkit.org/show_bug.cgi?id=208417

Reviewed by Anders Carlsson.

- Use Optional<> and invalid Color to represent unspecified positions and colors.
  This is simpler and easier to get right than separate booleans.
- Simplified sorting of stops in legacy gradients to remove extra CSS value
  evaluation and unnecessary "sort in place" technique.
- Rewrote equals functions for CSS gradient value classes. The new pattern is
  to compare all the data members that hold parsed CSS data, handling null
  correctly, since the parser won't set inappropriate ones. The old code had
  complex logic to only compare certain data members, which was unnecessary
  and hard to read to tell if it was correct.
- Added some more use of WTFMove to cut down on reference count churn.

* css/CSSGradientValue.cpp:
(WebCore::CSSGradientValue::image): Removed unneeded call to get().
(WebCore::compareStops): Deleted.
(WebCore::CSSGradientValue::sortStopsIfNeeded): Deleted.
(WebCore::resolveStopColors): Take advantage of the fact that we know because
of parsing rules that the only stops without colors are midpoints to drastically
simplify this function to a trivial loop.
(WebCore::CSSGradientValue::hasColorDerivedFromElement const): Added.
Checks to see if any of the stop colors is derived from the element. The old
code confusingly would store the answer to this in the stop, but only in the
first stop with this property. Computing it without modifying the stop, and
memoizing it in the gradient preserves the same performance characteristics
as before without requiring a boolean in each stop in the stops vector.
(WebCore::CSSGradientValue::gradientWithStylesResolved): Call the new
hasColorDerivedFromElement function instead of having the logic here.
(WebCore::LinearGradientAdapter::normalizeStopsAndEndpointsOutsideRange):
Update since GradientStop now has optional offsets. By the time this
function is called they are all guaranteed to be filled in, so we can
just use the * operator.
(WebCore::RadialGradientAdapter::normalizeStopsAndEndpointsOutsideRange):
Ditto.
(WebCore::ConicGradientAdapter::normalizeStopsAndEndpointsOutsideRange):
Ditto.
(WebCore::CSSGradientValue::computeStops): Moved the sorting of stops for
the deprecated gradients here. Also updated since Gradient::ColorStop
no longer uses "m_" prefixes on its public struct data members. Some
simplification because we no longer need to explicitly set "specified"
to true since it's no longer a separate boolean.
(WebCore::positionFromValue): Handle a null pointer for value by returning
0, which is what the caller was doing explicitly before. Use float
instead of int for some internal computations that were mixing the two
for no good reason.
(WebCore::computeEndPoint): Removed null checks now that positionFromValue
does them for us, turning this into a one-liner.
(WebCore::CSSGradientValue::isCacheable const): Use hasColorDerivedFromElement.
(WebCore::CSSGradientValue::knownToBeOpaque const): Removed unnnecessary
checking the color both before and after when a color filter is involved.
(WebCore::CSSGradientValue::equals const): Added. Shared by all the equals
functions for derived classes.
(WebCore::appendGradientStops): Updated for changes to CSSGradientColorStop.
(WebCore::appendSpaceSeparatedOptionalCSSPtrText): Added template helper
for writing two optional CSS values with a space between.
(WebCore::writeColorStop): Ditto. Also converted to non-member function,
removed unneeded isMidpoint check, use appendSpaceSeparatedOptionalCSSPtrText.
(WebCore::CSSLinearGradientValue::customCSSText const): Call function
members so we don't have to expose CSSGradientValue data members as
protected things that can be accessed by derived classes. Some other
small refactoring, such as getting rid of extra boolean wroteFirstStop.
(WebCore::CSSLinearGradientValue::createGradient): Updated to use
function members instead of protected data members.
(WebCore::CSSLinearGradientValue::equals const): Compare all data
members and use CSSGradientValue::equals, makes this a 1-liner.
(WebCore::CSSRadialGradientValue::customCSSText const): Call function
members as described above and use appendSpaceSeparatedOptionalCSSPtrText.
(WebCore::CSSRadialGradientValue::createGradient): Ditto.
(WebCore::CSSRadialGradientValue::equals const): Compare all data
members and use CSSGradientValue::equals.
(WebCore::CSSConicGradientValue::customCSSText const): Call function
members as described above and use appendSpaceSeparatedOptionalCSSPtrText.
(WebCore::CSSConicGradientValue::createGradient): Ditto.
(WebCore::CSSConicGradientValue::equals const): Compare all data
members and use CSSGradientValue::equals, makes this a 1-liner.

* css/CSSGradientValue.h: Removed unneeded includes and forward declarations.
Renamed CSSGradientColorStop data members to not use m_ prefix since this is
a struct with public data members, and WebKit style says not to do that here.
Removed m_colorIsDerivedFromElement and isMidpoint from CSSGradientColorStop,
m_colorIsDerivedFromElement is now stored in the gradient, not the color stop,
and midpoints are any color stop with null color. Replaced the
CSSGradientValue::stopCount function, which mixed size_t and unsigned types,
with a hasTwoStops function, which is all the caller needs. Converted the
isFixedSize, fixedSize, isPending, and loadSubimages into static member
functions: they don't do any work and so don't need an instance. Removed
the unneeded gradient type argument to the cloning constructors. Removed
m_stopsSorted and added m_hasColorDerivedFromElement and
hasColorDerivedFromElement. Added getter functions that are protected so
the data members themselves can be private. Removed sortStopsIfNeeded
and writeColorStop.

* css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::consumeDeprecatedGradientColorStop):
Updated for CSSGradientColorStop member renaming.
(WebCore::CSSPropertyParserHelpers::consumeDeprecatedGradient): Use
more WTFMove to save a little bit of reference count churn; in some cases
that means moving the setter calls to the end of the function after all
the error checking.
(WebCore::CSSPropertyParserHelpers::consumeGradientColorStops): Ditto.
Removed code to set isMidpoint and the FIXME-NEWPARSER comment that said
it could be removed. Used lambda to cut down on repeated code. Changed
parsing of stops with a second position to repeat the color instead of
relying on later computation to repeat it; this is required so we can
always treat an omitted color as a midpoint.
(WebCore::CSSPropertyParserHelpers::consumeDeprecatedRadialGradient): Ditto.
(WebCore::CSSPropertyParserHelpers::consumeRadialGradient): Ditto.
(WebCore::CSSPropertyParserHelpers::consumeLinearGradient): Ditto.
(WebCore::CSSPropertyParserHelpers::consumeConicGradient): Ditto.

* html/HTMLInputElement.cpp:
(WebCore::autoFillStrongPasswordMaskImage): Updated for the renamed
CSSGradientColorStop members, added a missing call to doneAddingStops,
and use some WTFMove to cut down on reference count churn.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (257965 => 257966)


--- trunk/Source/WebCore/ChangeLog	2020-03-06 03:57:30 UTC (rev 257965)
+++ trunk/Source/WebCore/ChangeLog	2020-03-06 04:11:39 UTC (rev 257966)
@@ -1,3 +1,123 @@
+2020-02-29  Darin Adler  <da...@apple.com>
+
+        Simplify gradient parsing
+        https://bugs.webkit.org/show_bug.cgi?id=208417
+
+        Reviewed by Anders Carlsson.
+
+        - Use Optional<> and invalid Color to represent unspecified positions and colors.
+          This is simpler and easier to get right than separate booleans.
+        - Simplified sorting of stops in legacy gradients to remove extra CSS value
+          evaluation and unnecessary "sort in place" technique.
+        - Rewrote equals functions for CSS gradient value classes. The new pattern is
+          to compare all the data members that hold parsed CSS data, handling null
+          correctly, since the parser won't set inappropriate ones. The old code had
+          complex logic to only compare certain data members, which was unnecessary
+          and hard to read to tell if it was correct.
+        - Added some more use of WTFMove to cut down on reference count churn.
+
+        * css/CSSGradientValue.cpp:
+        (WebCore::CSSGradientValue::image): Removed unneeded call to get().
+        (WebCore::compareStops): Deleted.
+        (WebCore::CSSGradientValue::sortStopsIfNeeded): Deleted.
+        (WebCore::resolveStopColors): Take advantage of the fact that we know because
+        of parsing rules that the only stops without colors are midpoints to drastically
+        simplify this function to a trivial loop.
+        (WebCore::CSSGradientValue::hasColorDerivedFromElement const): Added.
+        Checks to see if any of the stop colors is derived from the element. The old
+        code confusingly would store the answer to this in the stop, but only in the
+        first stop with this property. Computing it without modifying the stop, and
+        memoizing it in the gradient preserves the same performance characteristics
+        as before without requiring a boolean in each stop in the stops vector.
+        (WebCore::CSSGradientValue::gradientWithStylesResolved): Call the new
+        hasColorDerivedFromElement function instead of having the logic here.
+        (WebCore::LinearGradientAdapter::normalizeStopsAndEndpointsOutsideRange):
+        Update since GradientStop now has optional offsets. By the time this
+        function is called they are all guaranteed to be filled in, so we can
+        just use the * operator.
+        (WebCore::RadialGradientAdapter::normalizeStopsAndEndpointsOutsideRange):
+        Ditto.
+        (WebCore::ConicGradientAdapter::normalizeStopsAndEndpointsOutsideRange):
+        Ditto.
+        (WebCore::CSSGradientValue::computeStops): Moved the sorting of stops for
+        the deprecated gradients here. Also updated since Gradient::ColorStop
+        no longer uses "m_" prefixes on its public struct data members. Some
+        simplification because we no longer need to explicitly set "specified"
+        to true since it's no longer a separate boolean.
+        (WebCore::positionFromValue): Handle a null pointer for value by returning
+        0, which is what the caller was doing explicitly before. Use float
+        instead of int for some internal computations that were mixing the two
+        for no good reason.
+        (WebCore::computeEndPoint): Removed null checks now that positionFromValue
+        does them for us, turning this into a one-liner.
+        (WebCore::CSSGradientValue::isCacheable const): Use hasColorDerivedFromElement.
+        (WebCore::CSSGradientValue::knownToBeOpaque const): Removed unnnecessary
+        checking the color both before and after when a color filter is involved.
+        (WebCore::CSSGradientValue::equals const): Added. Shared by all the equals
+        functions for derived classes.
+        (WebCore::appendGradientStops): Updated for changes to CSSGradientColorStop.
+        (WebCore::appendSpaceSeparatedOptionalCSSPtrText): Added template helper
+        for writing two optional CSS values with a space between.
+        (WebCore::writeColorStop): Ditto. Also converted to non-member function,
+        removed unneeded isMidpoint check, use appendSpaceSeparatedOptionalCSSPtrText.
+        (WebCore::CSSLinearGradientValue::customCSSText const): Call function
+        members so we don't have to expose CSSGradientValue data members as
+        protected things that can be accessed by derived classes. Some other
+        small refactoring, such as getting rid of extra boolean wroteFirstStop.
+        (WebCore::CSSLinearGradientValue::createGradient): Updated to use
+        function members instead of protected data members.
+        (WebCore::CSSLinearGradientValue::equals const): Compare all data
+        members and use CSSGradientValue::equals, makes this a 1-liner.
+        (WebCore::CSSRadialGradientValue::customCSSText const): Call function
+        members as described above and use appendSpaceSeparatedOptionalCSSPtrText.
+        (WebCore::CSSRadialGradientValue::createGradient): Ditto.
+        (WebCore::CSSRadialGradientValue::equals const): Compare all data
+        members and use CSSGradientValue::equals.
+        (WebCore::CSSConicGradientValue::customCSSText const): Call function
+        members as described above and use appendSpaceSeparatedOptionalCSSPtrText.
+        (WebCore::CSSConicGradientValue::createGradient): Ditto.
+        (WebCore::CSSConicGradientValue::equals const): Compare all data
+        members and use CSSGradientValue::equals, makes this a 1-liner.
+
+        * css/CSSGradientValue.h: Removed unneeded includes and forward declarations.
+        Renamed CSSGradientColorStop data members to not use m_ prefix since this is
+        a struct with public data members, and WebKit style says not to do that here.
+        Removed m_colorIsDerivedFromElement and isMidpoint from CSSGradientColorStop,
+        m_colorIsDerivedFromElement is now stored in the gradient, not the color stop,
+        and midpoints are any color stop with null color. Replaced the
+        CSSGradientValue::stopCount function, which mixed size_t and unsigned types,
+        with a hasTwoStops function, which is all the caller needs. Converted the
+        isFixedSize, fixedSize, isPending, and loadSubimages into static member
+        functions: they don't do any work and so don't need an instance. Removed
+        the unneeded gradient type argument to the cloning constructors. Removed
+        m_stopsSorted and added m_hasColorDerivedFromElement and
+        hasColorDerivedFromElement. Added getter functions that are protected so
+        the data members themselves can be private. Removed sortStopsIfNeeded
+        and writeColorStop.
+
+        * css/parser/CSSPropertyParserHelpers.cpp:
+        (WebCore::CSSPropertyParserHelpers::consumeDeprecatedGradientColorStop):
+        Updated for CSSGradientColorStop member renaming.
+        (WebCore::CSSPropertyParserHelpers::consumeDeprecatedGradient): Use
+        more WTFMove to save a little bit of reference count churn; in some cases
+        that means moving the setter calls to the end of the function after all
+        the error checking.
+        (WebCore::CSSPropertyParserHelpers::consumeGradientColorStops): Ditto.
+        Removed code to set isMidpoint and the FIXME-NEWPARSER comment that said
+        it could be removed. Used lambda to cut down on repeated code. Changed
+        parsing of stops with a second position to repeat the color instead of
+        relying on later computation to repeat it; this is required so we can
+        always treat an omitted color as a midpoint.
+        (WebCore::CSSPropertyParserHelpers::consumeDeprecatedRadialGradient): Ditto.
+        (WebCore::CSSPropertyParserHelpers::consumeRadialGradient): Ditto.
+        (WebCore::CSSPropertyParserHelpers::consumeLinearGradient): Ditto.
+        (WebCore::CSSPropertyParserHelpers::consumeConicGradient): Ditto.
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::autoFillStrongPasswordMaskImage): Updated for the renamed
+        CSSGradientColorStop members, added a missing call to doneAddingStops,
+        and use some WTFMove to cut down on reference count churn.
+
 2020-03-05  Said Abou-Hallawa  <sabouhall...@apple.com>
 
         Remove the optimization for discarding no operation DisplayList items between Save and Restore items

Modified: trunk/Source/WebCore/css/CSSGradientValue.cpp (257965 => 257966)


--- trunk/Source/WebCore/css/CSSGradientValue.cpp	2020-03-06 03:57:30 UTC (rev 257965)
+++ trunk/Source/WebCore/css/CSSGradientValue.cpp	2020-03-06 04:11:39 UTC (rev 257966)
@@ -63,34 +63,16 @@
     }
     auto newImage = GradientImage::create(createGradient(*this, renderer, size), size);
     if (cacheable)
-        saveCachedImageForSize(size, newImage.get());
+        saveCachedImageForSize(size, newImage);
     return newImage;
 }
 
-// Should only ever be called for deprecated gradients.
-static inline bool compareStops(const CSSGradientColorStop& a, const CSSGradientColorStop& b)
-{
-    double aVal = a.m_position->doubleValue(CSSUnitType::CSS_NUMBER);
-    double bVal = b.m_position->doubleValue(CSSUnitType::CSS_NUMBER);
-
-    return aVal < bVal;
-}
-
-void CSSGradientValue::sortStopsIfNeeded()
-{
-    ASSERT(m_gradientType == CSSDeprecatedLinearGradient || m_gradientType == CSSDeprecatedRadialGradient);
-    if (!m_stopsSorted) {
-        if (m_stops.size())
-            std::stable_sort(m_stops.begin(), m_stops.end(), compareStops);
-        m_stopsSorted = true;
-    }
-}
-
 struct GradientStop {
     Color color;
-    float offset { 0 };
-    bool specified { false };
-    bool isMidpoint { false };
+    Optional<float> offset;
+
+    bool isSpecified() const { return offset.hasValue(); }
+    bool isMidpoint() const { return !color.isValid(); }
 };
 
 static inline Ref<CSSGradientValue> clone(CSSGradientValue& value)
@@ -103,35 +85,31 @@
     return downcast<CSSConicGradientValue>(value).clone();
 }
 
-template<typename Function>
-void resolveStopColors(Vector<CSSGradientColorStop, 2>& stops, Function&& colorResolveFunction)
+template<typename Function> void resolveStopColors(Vector<CSSGradientColorStop, 2>& stops, Function&& colorResolveFunction)
 {
-    for (size_t i = 0; i < stops.size(); ++i) {
-        auto& stop = stops[i];
-        if (stop.isMidpoint)
-            continue;
-        if (stop.m_color)
-            stop.m_resolvedColor = colorResolveFunction(*stop.m_color);
-        else if (i) {
-            auto& previousStop = stops[i - 1];
-            ASSERT(previousStop.m_color);
-            stop.m_color = previousStop.m_color;
-            stop.m_resolvedColor = previousStop.m_resolvedColor;
+    for (auto& stop : stops) {
+        if (stop.color)
+            stop.resolvedColor = colorResolveFunction(*stop.color);
+    }
+}
+
+bool CSSGradientValue::hasColorDerivedFromElement() const
+{
+    if (!m_hasColorDerivedFromElement) {
+        m_hasColorDerivedFromElement = false;
+        for (auto& stop : m_stops) {
+            if (stop.color && Style::BuilderState::isColorFromPrimitiveValueDerivedFromElement(*stop.color)) {
+                m_hasColorDerivedFromElement = true;
+                break;
+            }
         }
     }
+    return *m_hasColorDerivedFromElement;
 }
 
 Ref<CSSGradientValue> CSSGradientValue::gradientWithStylesResolved(Style::BuilderState& builderState)
 {
-    bool colorIsDerivedFromElement = false;
-    for (auto& stop : m_stops) {
-        if (!stop.isMidpoint && stop.m_color && Style::BuilderState::isColorFromPrimitiveValueDerivedFromElement(*stop.m_color)) {
-            stop.m_colorIsDerivedFromElement = true;
-            colorIsDerivedFromElement = true;
-            break;
-        }
-    }
-    auto result = colorIsDerivedFromElement ? clone(*this) : makeRef(*this);
+    auto result = hasColorDerivedFromElement() ? clone(*this) : makeRef(*this);
     resolveStopColors(result->m_stops, [&](const CSSPrimitiveValue& colorValue) {
         return builderState.colorFromPrimitiveValue(colorValue);
     });
@@ -152,7 +130,7 @@
         : m_data(data)
     {
     }
-    
+
     float gradientLength() const
     {
         auto gradientSize = m_data.point0 - m_data.point1;
@@ -162,13 +140,13 @@
 
     void normalizeStopsAndEndpointsOutsideRange(Vector<GradientStop>& stops)
     {
-        float firstOffset = stops.first().offset;
-        float lastOffset = stops.last().offset;
+        float firstOffset = *stops.first().offset;
+        float lastOffset = *stops.last().offset;
         if (firstOffset != lastOffset) {
             float scale = lastOffset - firstOffset;
 
             for (auto& stop : stops)
-                stop.offset = (stop.offset - firstOffset) / scale;
+                stop.offset = (*stop.offset - firstOffset) / scale;
 
             auto p0 = m_data.point0;
             auto p1 = m_data.point1;
@@ -209,13 +187,13 @@
 
         // Rather than scaling the points < 0, we truncate them, so only scale according to the largest point.
         float firstOffset = 0;
-        float lastOffset = stops.last().offset;
+        float lastOffset = *stops.last().offset;
         float scale = lastOffset - firstOffset;
 
         // Reset points below 0 to the first visible color.
         size_t firstZeroOrGreaterIndex = numStops;
         for (size_t i = 0; i < numStops; ++i) {
-            if (stops[i].offset >= 0) {
+            if (*stops[i].offset >= 0) {
                 firstZeroOrGreaterIndex = i;
                 break;
             }
@@ -222,9 +200,9 @@
         }
 
         if (firstZeroOrGreaterIndex > 0) {
-            if (firstZeroOrGreaterIndex < numStops && stops[firstZeroOrGreaterIndex].offset > 0) {
-                float prevOffset = stops[firstZeroOrGreaterIndex - 1].offset;
-                float nextOffset = stops[firstZeroOrGreaterIndex].offset;
+            if (firstZeroOrGreaterIndex < numStops && *stops[firstZeroOrGreaterIndex].offset > 0) {
+                float prevOffset = *stops[firstZeroOrGreaterIndex - 1].offset;
+                float nextOffset = *stops[firstZeroOrGreaterIndex].offset;
 
                 float interStopProportion = -prevOffset / (nextOffset - prevOffset);
                 // FIXME: when we interpolate gradients using premultiplied colors, this should do premultiplication.
@@ -243,7 +221,7 @@
         }
 
         for (auto& stop : stops)
-            stop.offset /= scale;
+            *stop.offset /= scale;
 
         m_data.startRadius *= scale;
         m_data.endRadius *= scale;
@@ -264,7 +242,7 @@
         
         size_t firstZeroOrGreaterIndex = numStops;
         for (size_t i = 0; i < numStops; ++i) {
-            if (stops[i].offset >= 0) {
+            if (*stops[i].offset >= 0) {
                 firstZeroOrGreaterIndex = i;
                 break;
             }
@@ -271,9 +249,9 @@
         }
 
         if (firstZeroOrGreaterIndex > 0) {
-            if (firstZeroOrGreaterIndex < numStops && stops[firstZeroOrGreaterIndex].offset > 0) {
-                float prevOffset = stops[firstZeroOrGreaterIndex - 1].offset;
-                float nextOffset = stops[firstZeroOrGreaterIndex].offset;
+            if (firstZeroOrGreaterIndex < numStops && *stops[firstZeroOrGreaterIndex].offset > 0) {
+                float prevOffset = *stops[firstZeroOrGreaterIndex - 1].offset;
+                float nextOffset = *stops[firstZeroOrGreaterIndex].offset;
                 
                 float interStopProportion = -prevOffset / (nextOffset - prevOffset);
                 // FIXME: when we interpolate gradients using premultiplied colors, this should do premultiplication.
@@ -293,7 +271,7 @@
 
         size_t lastOneOrLessIndex = numStops;
         for (int i = numStops - 1; i >= 0; --i) {
-            if (stops[i].offset <= 1) {
+            if (*stops[i].offset <= 1) {
                 lastOneOrLessIndex = i;
                 break;
             }
@@ -300,9 +278,9 @@
         }
         
         if (lastOneOrLessIndex < numStops - 1) {
-            if (lastOneOrLessIndex < numStops && stops[lastOneOrLessIndex].offset < 1) {
-                float prevOffset = stops[lastOneOrLessIndex].offset;
-                float nextOffset = stops[lastOneOrLessIndex + 1].offset;
+            if (lastOneOrLessIndex < numStops && *stops[lastOneOrLessIndex].offset < 1) {
+                float prevOffset = *stops[lastOneOrLessIndex].offset;
+                float nextOffset = *stops[lastOneOrLessIndex + 1].offset;
                 
                 float interStopProportion = (1 - prevOffset) / (nextOffset - prevOffset);
                 // FIXME: when we interpolate gradients using premultiplied colors, this should do premultiplication.
@@ -326,24 +304,26 @@
 Gradient::ColorStopVector CSSGradientValue::computeStops(GradientAdapter& gradientAdapter, const CSSToLengthConversionData& conversionData, const RenderStyle& style, float maxLengthForRepeat)
 {
     if (m_gradientType == CSSDeprecatedLinearGradient || m_gradientType == CSSDeprecatedRadialGradient) {
-        sortStopsIfNeeded();
-
         Gradient::ColorStopVector result;
         result.reserveInitialCapacity(m_stops.size());
 
         for (auto& stop : m_stops) {
             float offset;
-            if (stop.m_position->isPercentage())
-                offset = stop.m_position->floatValue(CSSUnitType::CSS_PERCENTAGE) / 100;
+            if (stop.position->isPercentage())
+                offset = stop.position->floatValue(CSSUnitType::CSS_PERCENTAGE) / 100;
             else
-                offset = stop.m_position->floatValue(CSSUnitType::CSS_NUMBER);
+                offset = stop.position->floatValue(CSSUnitType::CSS_NUMBER);
 
-            Color color = stop.m_resolvedColor;
+            Color color = stop.resolvedColor;
             if (style.hasAppleColorFilter())
                 style.appleColorFilter().transformColor(color);
             result.uncheckedAppend({ offset, color });
         }
 
+        std::stable_sort(result.begin(), result.end(), [] (const Gradient::ColorStop& a, const Gradient::ColorStop& b) {
+            return a.offset < b.offset;
+        });
+
         return result;
     }
 
@@ -355,16 +335,14 @@
     for (size_t i = 0; i < numStops; ++i) {
         auto& stop = m_stops[i];
 
-        stops[i].isMidpoint = stop.isMidpoint;
-
-        Color color = stop.m_resolvedColor;
+        Color color = stop.resolvedColor;
         if (style.hasAppleColorFilter())
             style.appleColorFilter().transformColor(color);
 
         stops[i].color = color;
 
-        if (stop.m_position) {
-            auto& positionValue = *stop.m_position;
+        if (stop.position) {
+            auto& positionValue = *stop.position;
             if (positionValue.isPercentage())
                 stops[i].offset = positionValue.floatValue(CSSUnitType::CSS_PERCENTAGE) / 100;
             else if (positionValue.isLength() || positionValue.isViewportPercentageLength() || positionValue.isCalculatedPercentageWithLength()) {
@@ -382,35 +360,31 @@
                 ASSERT_NOT_REACHED();
                 stops[i].offset = 0;
             }
-            stops[i].specified = true;
         } else {
             // If the first color-stop does not have a position, its position defaults to 0%.
             // If the last color-stop does not have a position, its position defaults to 100%.
-            if (!i) {
+            if (!i)
                 stops[i].offset = 0;
-                stops[i].specified = true;
-            } else if (numStops > 1 && i == numStops - 1) {
+            else if (numStops > 1 && i == numStops - 1)
                 stops[i].offset = 1;
-                stops[i].specified = true;
-            }
         }
 
         // If a color-stop has a position that is less than the specified position of any
         // color-stop before it in the list, its position is changed to be equal to the
         // largest specified position of any color-stop before it.
-        if (stops[i].specified && i > 0) {
+        if (stops[i].isSpecified() && i > 0) {
             size_t prevSpecifiedIndex;
             for (prevSpecifiedIndex = i - 1; prevSpecifiedIndex; --prevSpecifiedIndex) {
-                if (stops[prevSpecifiedIndex].specified)
+                if (stops[prevSpecifiedIndex].isSpecified())
                     break;
             }
 
-            if (stops[i].offset < stops[prevSpecifiedIndex].offset)
+            if (*stops[i].offset < *stops[prevSpecifiedIndex].offset)
                 stops[i].offset = stops[prevSpecifiedIndex].offset;
         }
     }
 
-    ASSERT(stops[0].specified && stops[numStops - 1].specified);
+    ASSERT(stops[0].isSpecified() && stops[numStops - 1].isSpecified());
 
     // If any color-stop still does not have a position, then, for each run of adjacent
     // color-stops without positions, set their positions so that they are evenly spaced
@@ -420,15 +394,15 @@
         bool inUnspecifiedRun = false;
 
         for (size_t i = 0; i < numStops; ++i) {
-            if (!stops[i].specified && !inUnspecifiedRun) {
+            if (!stops[i].isSpecified() && !inUnspecifiedRun) {
                 unspecifiedRunStart = i;
                 inUnspecifiedRun = true;
-            } else if (stops[i].specified && inUnspecifiedRun) {
+            } else if (stops[i].isSpecified() && inUnspecifiedRun) {
                 size_t unspecifiedRunEnd = i;
 
                 if (unspecifiedRunStart < unspecifiedRunEnd) {
-                    float lastSpecifiedOffset = stops[unspecifiedRunStart - 1].offset;
-                    float nextSpecifiedOffset = stops[unspecifiedRunEnd].offset;
+                    float lastSpecifiedOffset = *stops[unspecifiedRunStart - 1].offset;
+                    float nextSpecifiedOffset = *stops[unspecifiedRunEnd].offset;
                     float delta = (nextSpecifiedOffset - lastSpecifiedOffset) / (unspecifiedRunEnd - unspecifiedRunStart + 1);
 
                     for (size_t j = unspecifiedRunStart; j < unspecifiedRunEnd; ++j)
@@ -454,7 +428,7 @@
     // Midpoints that coincide with color stops are treated specially since they don't require
     // extra stops and generate hard lines.
     for (size_t x = 1; x < stops.size() - 1;) {
-        if (!stops[x].isMidpoint) {
+        if (!stops[x].isMidpoint()) {
             ++x;
             continue;
         }
@@ -464,9 +438,9 @@
         Color color1 = stops[x - 1].color;
         Color color2 = stops[x + 1].color;
         // Likewise find the position of previous and next color stop.
-        float offset1 = stops[x - 1].offset;
-        float offset2 = stops[x + 1].offset;
-        float offset = stops[x].offset;
+        float offset1 = *stops[x - 1].offset;
+        float offset2 = *stops[x + 1].offset;
+        float offset = *stops[x].offset;
 
         // Check if everything coincides or the midpoint is exactly in the middle.
         // If so, ignore the midpoint.
@@ -479,7 +453,6 @@
         if (offset1 == offset) {
             // Morph the midpoint to a regular stop with the color of the next color stop.
             stops[x].color = color2;
-            stops[x].isMidpoint = false;
             continue;
         }
 
@@ -487,7 +460,6 @@
         if (offset2 == offset) {
             // Morph the midpoint to a regular stop with the color of the previous color stop.
             stops[x].color = color1;
-            stops[x].isMidpoint = false;
             continue;
         }
 
@@ -508,7 +480,7 @@
         }
         // calculate colors
         for (size_t y = 0; y < 9; ++y) {
-            float relativeOffset = (newStops[y].offset - offset1) / (offset2 - offset1);
+            float relativeOffset = (*newStops[y].offset - offset1) / (offset2 - offset1);
             float multiplier = std::pow(relativeOffset, std::log(.5f) / std::log(midpoint));
             // FIXME: Why not premultiply here?
             newStops[y].color = blend(color1, color2, multiplier, false /* do not premultiply */);
@@ -527,7 +499,7 @@
     if (m_repeating && numStops > 1) {
         // If the difference in the positions of the first and last color-stops is 0,
         // the gradient defines a solid-color image with the color of the last color-stop in the rule.
-        float gradientRange = stops.last().offset - stops.first().offset;
+        float gradientRange = *stops.last().offset - *stops.first().offset;
         if (!gradientRange) {
             stops.first().offset = 0;
             stops.first().color = stops.last().color;
@@ -540,7 +512,7 @@
             size_t originalFirstStopIndex = 0;
 
             // Work backwards from the first, adding stops until we get one before 0.
-            float firstOffset = stops[0].offset;
+            float firstOffset = *stops[0].offset;
             if (firstOffset > 0) {
                 float currOffset = firstOffset;
                 size_t srcStopOrdinal = originalNumStops - 1;
@@ -554,13 +526,13 @@
                         break;
 
                     if (srcStopOrdinal)
-                        currOffset -= stops[originalFirstStopIndex + srcStopOrdinal].offset - stops[originalFirstStopIndex + srcStopOrdinal - 1].offset;
+                        currOffset -= *stops[originalFirstStopIndex + srcStopOrdinal].offset - *stops[originalFirstStopIndex + srcStopOrdinal - 1].offset;
                     srcStopOrdinal = (srcStopOrdinal + originalNumStops - 1) % originalNumStops;
                 }
             }
 
             // Work forwards from the end, adding stops until we get one after 1.
-            float lastOffset = stops[stops.size() - 1].offset;
+            float lastOffset = *stops[stops.size() - 1].offset;
             if (lastOffset < maxExtent) {
                 float currOffset = lastOffset;
                 size_t srcStopOrdinal = 0;
@@ -573,7 +545,7 @@
                     if (currOffset > maxExtent)
                         break;
                     if (srcStopOrdinal < originalNumStops - 1)
-                        currOffset += stops[srcStopIndex + 1].offset - stops[srcStopIndex].offset;
+                        currOffset += *stops[srcStopIndex + 1].offset - *stops[srcStopIndex].offset;
                     srcStopOrdinal = (srcStopOrdinal + 1) % originalNumStops;
                 }
             }
@@ -581,13 +553,13 @@
     }
 
     // If the gradient goes outside the 0-1 range, normalize it by moving the endpoints, and adjusting the stops.
-    if (stops.size() > 1 && (stops.first().offset < 0 || stops.last().offset > 1))
+    if (stops.size() > 1 && (*stops.first().offset < 0 || *stops.last().offset > 1))
         gradientAdapter.normalizeStopsAndEndpointsOutsideRange(stops);
     
     Gradient::ColorStopVector result;
     result.reserveInitialCapacity(stops.size());
     for (auto& stop : stops)
-        result.uncheckedAppend({ stop.offset, stop.color });
+        result.uncheckedAppend({ *stop.offset, stop.color });
 
     return result;
 }
@@ -594,34 +566,34 @@
 
 static float positionFromValue(const CSSPrimitiveValue* value, const CSSToLengthConversionData& conversionData, const FloatSize& size, bool isHorizontal)
 {
-    int origin = 0;
-    int sign = 1;
-    int edgeDistance = isHorizontal ? size.width() : size.height();
-    
+    if (!value)
+        return 0;
+
+    float origin = 0;
+    float sign = 1;
+    float edgeDistance = isHorizontal ? size.width() : size.height();
+
     // In this case the center of the gradient is given relative to an edge in the
     // form of: [ top | bottom | right | left ] [ <percentage> | <length> ].
     if (value->isPair()) {
         CSSValueID originID = value->pairValue()->first()->valueID();
-        value = value->pairValue()->second();
-        
         if (originID == CSSValueRight || originID == CSSValueBottom) {
             // For right/bottom, the offset is relative to the far edge.
             origin = edgeDistance;
             sign = -1;
         }
+        value = value->pairValue()->second();
     }
-    
+
     if (value->isNumber())
         return origin + sign * value->floatValue() * conversionData.zoom();
-    
+
     if (value->isPercentage())
-        return origin + sign * value->floatValue() / 100.f * edgeDistance;
+        return origin + sign * value->floatValue() / 100 * edgeDistance;
 
-    if (value->isCalculatedPercentageWithLength()) {
-        Ref<CalculationValue> calculationValue { value->cssCalcValue()->createCalculationValue(conversionData) };
-        return origin + sign * calculationValue->evaluate(edgeDistance);
-    }
-    
+    if (value->isCalculatedPercentageWithLength())
+        return origin + sign * value->cssCalcValue()->createCalculationValue(conversionData)->evaluate(edgeDistance);
+
     switch (value->valueID()) {
     case CSSValueTop:
         ASSERT(!isHorizontal);
@@ -631,10 +603,10 @@
         return 0;
     case CSSValueBottom:
         ASSERT(!isHorizontal);
-        return size.height();
+        return edgeDistance;
     case CSSValueRight:
         ASSERT(isHorizontal);
-        return size.width();
+        return edgeDistance;
     case CSSValueCenter:
         return origin + sign * .5f * edgeDistance;
     default:
@@ -644,32 +616,20 @@
     return origin + sign * value->computeLength<float>(conversionData);
 }
 
-FloatPoint CSSGradientValue::computeEndPoint(CSSPrimitiveValue* horizontal, CSSPrimitiveValue* vertical, const CSSToLengthConversionData& conversionData, const FloatSize& size)
+// Resolve points/radii to front end values.
+static FloatPoint computeEndPoint(const CSSPrimitiveValue* horizontal, const CSSPrimitiveValue* vertical, const CSSToLengthConversionData& conversionData, const FloatSize& size)
 {
-    FloatPoint result;
-
-    if (horizontal)
-        result.setX(positionFromValue(horizontal, conversionData, size, true));
-
-    if (vertical)
-        result.setY(positionFromValue(vertical, conversionData, size, false));
-
-    return result;
+    return { positionFromValue(horizontal, conversionData, size, true), positionFromValue(vertical, conversionData, size, false) };
 }
 
 bool CSSGradientValue::isCacheable() const
 {
+    if (hasColorDerivedFromElement())
+        return false;
     for (auto& stop : m_stops) {
-        if (stop.m_colorIsDerivedFromElement)
+        if (stop.position && stop.position->isFontRelativeLength())
             return false;
-
-        if (!stop.m_position)
-            continue;
-
-        if (stop.m_position->isFontRelativeLength())
-            return false;
     }
-
     return true;
 }
 
@@ -676,54 +636,63 @@
 bool CSSGradientValue::knownToBeOpaque(const RenderElement& renderer) const
 {
     bool hasColorFilter = renderer.style().hasAppleColorFilter();
-
     for (auto& stop : m_stops) {
-        if (hasColorFilter) {
-            Color stopColor = stop.m_resolvedColor;
-            renderer.style().appleColorFilter().transformColor(stopColor);
-            if (!stopColor.isOpaque())
-                return false;
-        }
-
-        if (!stop.m_resolvedColor.isOpaque())
+        Color color = stop.resolvedColor;
+        if (hasColorFilter)
+            renderer.style().appleColorFilter().transformColor(color);
+        if (!color.isOpaque())
             return false;
     }
     return true;
 }
 
+bool CSSGradientValue::equals(const CSSGradientValue& other) const
+{
+    return compareCSSValuePtr(m_firstX, other.m_firstX)
+        && compareCSSValuePtr(m_firstY, other.m_firstY)
+        && compareCSSValuePtr(m_secondX, other.m_secondX)
+        && compareCSSValuePtr(m_secondY, other.m_secondY)
+        && m_stops == other.m_stops
+        && m_gradientType == other.m_gradientType
+        && m_repeating == other.m_repeating;
+}
+
 static void appendGradientStops(StringBuilder& builder, const Vector<CSSGradientColorStop, 2>& stops)
 {
     for (auto& stop : stops) {
-        double position = stop.m_position->doubleValue(CSSUnitType::CSS_NUMBER);
+        double position = stop.position->doubleValue(CSSUnitType::CSS_NUMBER);
         if (!position)
-            builder.append(", from(", stop.m_color->cssText(), ')');
+            builder.append(", from(", stop.color->cssText(), ')');
         else if (position == 1)
-            builder.append(", to(", stop.m_color->cssText(), ')');
+            builder.append(", to(", stop.color->cssText(), ')');
         else
-            builder.append(", color-stop(", position, ", ", stop.m_color->cssText(), ')');
+            builder.append(", color-stop(", position, ", ", stop.color->cssText(), ')');
     }
 }
 
-void CSSGradientValue::writeColorStop(StringBuilder& builder, const CSSGradientColorStop& stop) const
+template<typename T, typename U> static void appendSpaceSeparatedOptionalCSSPtrText(StringBuilder& builder, const T& a, const U& b)
 {
-    if (!stop.isMidpoint && stop.m_color)
-        builder.append(stop.m_color->cssText());
+    if (a && b)
+        builder.append(a->cssText(), ' ', b->cssText());
+    else if (a)
+        builder.append(a->cssText());
+    else if (b)
+        builder.append(b->cssText());
+}
 
-    if (stop.m_position) {
-        if (!stop.isMidpoint)
-            builder.append(' ');
-        builder.append(stop.m_position->cssText());
-    }
+static void writeColorStop(StringBuilder& builder, const CSSGradientColorStop& stop)
+{
+    appendSpaceSeparatedOptionalCSSPtrText(builder, stop.color, stop.position);
 }
 
 String CSSLinearGradientValue::customCSSText() const
 {
     StringBuilder result;
-    if (m_gradientType == CSSDeprecatedLinearGradient) {
-        result.append("-webkit-gradient(linear, ", m_firstX->cssText(), ' ', m_firstY->cssText(), ", ", m_secondX->cssText(), ' ', m_secondY->cssText());
-        appendGradientStops(result, m_stops);
-    } else if (m_gradientType == CSSPrefixedLinearGradient) {
-        if (m_repeating)
+    if (gradientType() == CSSDeprecatedLinearGradient) {
+        result.append("-webkit-gradient(linear, ", firstX()->cssText(), ' ', firstY()->cssText(), ", ", secondX()->cssText(), ' ', secondY()->cssText());
+        appendGradientStops(result, stops());
+    } else if (gradientType() == CSSPrefixedLinearGradient) {
+        if (isRepeating())
             result.appendLiteral("-webkit-repeating-linear-gradient(");
         else
             result.appendLiteral("-webkit-linear-gradient(");
@@ -730,21 +699,15 @@
 
         if (m_angle)
             result.append(m_angle->cssText());
-        else {
-            if (m_firstX && m_firstY)
-                result.append(m_firstX->cssText(), ' ', m_firstY->cssText());
-            else if (m_firstX)
-                result.append(m_firstX->cssText());
-            else if (m_firstY)
-                result.append(m_firstY->cssText());
-        }
+        else
+            appendSpaceSeparatedOptionalCSSPtrText(result, firstX(), firstY());
 
-        for (auto& stop : m_stops) {
+        for (auto& stop : stops()) {
             result.appendLiteral(", ");
             writeColorStop(result, stop);
         }
     } else {
-        if (m_repeating)
+        if (isRepeating())
             result.appendLiteral("repeating-linear-gradient(");
         else
             result.appendLiteral("linear-gradient(");
@@ -754,25 +717,16 @@
         if (m_angle && m_angle->computeDegrees() != 180) {
             result.append(m_angle->cssText());
             wroteSomething = true;
-        } else if ((m_firstX || m_firstY) && !(!m_firstX && m_firstY && m_firstY->valueID() == CSSValueBottom)) {
+        } else if ((firstX() || firstY()) && !(!firstX() && firstY() && firstY()->valueID() == CSSValueBottom)) {
             result.appendLiteral("to ");
-            if (m_firstX && m_firstY)
-                result.append(m_firstX->cssText(), ' ', m_firstY->cssText());
-            else if (m_firstX)
-                result.append(m_firstX->cssText());
-            else
-                result.append(m_firstY->cssText());
+            appendSpaceSeparatedOptionalCSSPtrText(result, firstX(), firstY());
             wroteSomething = true;
         }
 
-        if (wroteSomething)
-            result.appendLiteral(", ");
-
-        bool wroteFirstStop = false;
-        for (auto& stop : m_stops) {
-            if (wroteFirstStop)
+        for (auto& stop : stops()) {
+            if (wroteSomething)
                 result.appendLiteral(", ");
-            wroteFirstStop = true;
+            wroteSomething = true;
             writeColorStop(result, stop);
         }
     }
@@ -859,44 +813,44 @@
     FloatPoint secondPoint;
     if (m_angle) {
         float angle = m_angle->floatValue(CSSUnitType::CSS_DEG);
-        endPointsFromAngle(angle, size, firstPoint, secondPoint, m_gradientType);
+        endPointsFromAngle(angle, size, firstPoint, secondPoint, gradientType());
     } else {
-        switch (m_gradientType) {
+        switch (gradientType()) {
         case CSSDeprecatedLinearGradient:
-            firstPoint = computeEndPoint(m_firstX.get(), m_firstY.get(), conversionData, size);
-            if (m_secondX || m_secondY)
-                secondPoint = computeEndPoint(m_secondX.get(), m_secondY.get(), conversionData, size);
+            firstPoint = computeEndPoint(firstX(), firstY(), conversionData, size);
+            if (secondX() || secondY())
+                secondPoint = computeEndPoint(secondX(), secondY(), conversionData, size);
             else {
-                if (m_firstX)
+                if (firstX())
                     secondPoint.setX(size.width() - firstPoint.x());
-                if (m_firstY)
+                if (firstY())
                     secondPoint.setY(size.height() - firstPoint.y());
             }
             break;
         case CSSPrefixedLinearGradient:
-            firstPoint = computeEndPoint(m_firstX.get(), m_firstY.get(), conversionData, size);
-            if (m_firstX)
+            firstPoint = computeEndPoint(firstX(), firstY(), conversionData, size);
+            if (firstX())
                 secondPoint.setX(size.width() - firstPoint.x());
-            if (m_firstY)
+            if (firstY())
                 secondPoint.setY(size.height() - firstPoint.y());
             break;
         case CSSLinearGradient:
-            if (m_firstX && m_firstY) {
+            if (firstX() && firstY()) {
                 // "Magic" corners, so the 50% line touches two corners.
                 float rise = size.width();
                 float run = size.height();
-                if (m_firstX && m_firstX->valueID() == CSSValueLeft)
+                if (firstX() && firstX()->valueID() == CSSValueLeft)
                     run *= -1;
-                if (m_firstY && m_firstY->valueID() == CSSValueBottom)
+                if (firstY() && firstY()->valueID() == CSSValueBottom)
                     rise *= -1;
                 // Compute angle, and flip it back to "bearing angle" degrees.
                 float angle = 90 - rad2deg(atan2(rise, run));
-                endPointsFromAngle(angle, size, firstPoint, secondPoint, m_gradientType);
-            } else if (m_firstX || m_firstY) { 
-                secondPoint = computeEndPoint(m_firstX.get(), m_firstY.get(), conversionData, size);
-                if (m_firstX)
+                endPointsFromAngle(angle, size, firstPoint, secondPoint, gradientType());
+            } else if (firstX() || firstY()) {
+                secondPoint = computeEndPoint(firstX(), firstY(), conversionData, size);
+                if (firstX())
                     firstPoint.setX(size.width() - secondPoint.x());
-                if (m_firstY)
+                if (firstY())
                     firstPoint.setY(size.height() - secondPoint.y());
             } else
                 secondPoint.setY(size.height());
@@ -917,37 +871,7 @@
 
 bool CSSLinearGradientValue::equals(const CSSLinearGradientValue& other) const
 {
-    if (m_gradientType == CSSDeprecatedLinearGradient)
-        return other.m_gradientType == m_gradientType
-            && compareCSSValuePtr(m_firstX, other.m_firstX)
-            && compareCSSValuePtr(m_firstY, other.m_firstY)
-            && compareCSSValuePtr(m_secondX, other.m_secondX)
-            && compareCSSValuePtr(m_secondY, other.m_secondY)
-            && m_stops == other.m_stops;
-
-    if (m_gradientType != other.m_gradientType)
-        return false;
-
-    if (m_repeating != other.m_repeating)
-        return false;
-
-    if (m_angle)
-        return compareCSSValuePtr(m_angle, other.m_angle) && m_stops == other.m_stops;
-
-    if (other.m_angle)
-        return false;
-
-    bool equalXandY = false;
-    if (m_firstX && m_firstY)
-        equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && compareCSSValuePtr(m_firstY, other.m_firstY);
-    else if (m_firstX)
-        equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && !other.m_firstY;
-    else if (m_firstY)
-        equalXandY = compareCSSValuePtr(m_firstY, other.m_firstY) && !other.m_firstX;
-    else
-        equalXandY = !other.m_firstX && !other.m_firstY;
-
-    return equalXandY && m_stops == other.m_stops;
+    return CSSGradientValue::equals(other) && compareCSSValuePtr(m_angle, other.m_angle);
 }
 
 String CSSRadialGradientValue::customCSSText() const
@@ -954,22 +878,18 @@
 {
     StringBuilder result;
 
-    if (m_gradientType == CSSDeprecatedRadialGradient) {
-        result.append("-webkit-gradient(radial, ", m_firstX->cssText(), ' ', m_firstY->cssText(), ", ", m_firstRadius->cssText(),
-            ", ", m_secondX->cssText(), ' ', m_secondY->cssText(), ", ", m_secondRadius->cssText());
-        appendGradientStops(result, m_stops);
-    } else if (m_gradientType == CSSPrefixedRadialGradient) {
-        if (m_repeating)
+    if (gradientType() == CSSDeprecatedRadialGradient) {
+        result.append("-webkit-gradient(radial, ", firstX()->cssText(), ' ', firstY()->cssText(), ", ", m_firstRadius->cssText(),
+            ", ", secondX()->cssText(), ' ', secondY()->cssText(), ", ", m_secondRadius->cssText());
+        appendGradientStops(result, stops());
+    } else if (gradientType() == CSSPrefixedRadialGradient) {
+        if (isRepeating())
             result.appendLiteral("-webkit-repeating-radial-gradient(");
         else
             result.appendLiteral("-webkit-radial-gradient(");
 
-        if (m_firstX && m_firstY)
-            result.append(m_firstX->cssText(), ' ', m_firstY->cssText());
-        else if (m_firstX)
-            result.append(m_firstX->cssText());
-        else if (m_firstY)
-            result.append(m_firstY->cssText());
+        if (firstX() || firstY())
+            appendSpaceSeparatedOptionalCSSPtrText(result, firstX(), firstY());
         else
             result.appendLiteral("center");
 
@@ -986,12 +906,12 @@
         } else if (m_endHorizontalSize && m_endVerticalSize)
             result.append(", ", m_endHorizontalSize->cssText(), ' ', m_endVerticalSize->cssText());
 
-        for (auto& stop : m_stops) {
+        for (auto& stop : stops()) {
             result.appendLiteral(", ");
             writeColorStop(result, stop);
         }
     } else {
-        if (m_repeating)
+        if (isRepeating())
             result.appendLiteral("repeating-radial-gradient(");
         else
             result.appendLiteral("radial-gradient(");
@@ -1019,16 +939,11 @@
             wroteSomething = true;
         }
 
-        if (m_firstX || m_firstY) {
+        if (firstX() || firstY()) {
             if (wroteSomething)
                 result.append(' ');
             result.appendLiteral("at ");
-            if (m_firstX && m_firstY)
-                result.append(m_firstX->cssText(), ' ', m_firstY->cssText());
-            else if (m_firstX)
-                result.append(m_firstX->cssText());
-            else
-                result.append(m_firstY->cssText());
+            appendSpaceSeparatedOptionalCSSPtrText(result, firstX(), firstY());
             wroteSomething = true;
         }
 
@@ -1036,7 +951,7 @@
             result.appendLiteral(", ");
 
         bool wroteFirstStop = false;
-        for (auto& stop : m_stops) {
+        for (auto& stop : stops()) {
             if (wroteFirstStop)
                 result.appendLiteral(", ");
             wroteFirstStop = true;
@@ -1143,16 +1058,16 @@
 
     CSSToLengthConversionData conversionData(&renderer.style(), renderer.document().documentElement()->renderStyle(), &renderer.view());
 
-    FloatPoint firstPoint = computeEndPoint(m_firstX.get(), m_firstY.get(), conversionData, size);
-    if (!m_firstX)
+    FloatPoint firstPoint = computeEndPoint(firstX(), firstY(), conversionData, size);
+    if (!firstX())
         firstPoint.setX(size.width() / 2);
-    if (!m_firstY)
+    if (!firstY())
         firstPoint.setY(size.height() / 2);
 
-    FloatPoint secondPoint = computeEndPoint(m_secondX.get(), m_secondY.get(), conversionData, size);
-    if (!m_secondX)
+    FloatPoint secondPoint = computeEndPoint(secondX(), secondY(), conversionData, size);
+    if (!secondX())
         secondPoint.setX(size.width() / 2);
-    if (!m_secondY)
+    if (!secondY())
         secondPoint.setY(size.height() / 2);
 
     float firstRadius = 0;
@@ -1266,7 +1181,7 @@
 
     // computeStops() only uses maxExtent for repeating gradients.
     float maxExtent = 0;
-    if (m_repeating) {
+    if (isRepeating()) {
         FloatPoint corner;
         maxExtent = distanceToFarthestCorner(secondPoint, size, corner);
     }
@@ -1282,59 +1197,18 @@
 
 bool CSSRadialGradientValue::equals(const CSSRadialGradientValue& other) const
 {
-    if (m_gradientType == CSSDeprecatedRadialGradient)
-        return other.m_gradientType == m_gradientType
-            && compareCSSValuePtr(m_firstX, other.m_firstX)
-            && compareCSSValuePtr(m_firstY, other.m_firstY)
-            && compareCSSValuePtr(m_secondX, other.m_secondX)
-            && compareCSSValuePtr(m_secondY, other.m_secondY)
-            && compareCSSValuePtr(m_firstRadius, other.m_firstRadius)
-            && compareCSSValuePtr(m_secondRadius, other.m_secondRadius)
-            && m_stops == other.m_stops;
-
-    if (m_gradientType != other.m_gradientType)
-        return false;
-
-    if (m_repeating != other.m_repeating)
-        return false;
-
-    bool equalXandY = false;
-    if (m_firstX && m_firstY)
-        equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && compareCSSValuePtr(m_firstY, other.m_firstY);
-    else if (m_firstX)
-        equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && !other.m_firstY;
-    else if (m_firstY)
-        equalXandY = compareCSSValuePtr(m_firstY, other.m_firstY) && !other.m_firstX;
-    else
-        equalXandY = !other.m_firstX && !other.m_firstY;
-
-    if (!equalXandY)
-        return false;
-
-    bool equalShape = true;
-    bool equalSizingBehavior = true;
-    bool equalHorizontalAndVerticalSize = true;
-
-    if (m_shape)
-        equalShape = compareCSSValuePtr(m_shape, other.m_shape);
-    else if (m_sizingBehavior)
-        equalSizingBehavior = compareCSSValuePtr(m_sizingBehavior, other.m_sizingBehavior);
-    else if (m_endHorizontalSize && m_endVerticalSize)
-        equalHorizontalAndVerticalSize = compareCSSValuePtr(m_endHorizontalSize, other.m_endHorizontalSize) && compareCSSValuePtr(m_endVerticalSize, other.m_endVerticalSize);
-    else {
-        equalShape = !other.m_shape;
-        equalSizingBehavior = !other.m_sizingBehavior;
-        equalHorizontalAndVerticalSize = !other.m_endHorizontalSize && !other.m_endVerticalSize;
-    }
-    return equalShape && equalSizingBehavior && equalHorizontalAndVerticalSize && m_stops == other.m_stops;
+    return CSSGradientValue::equals(other)
+        && compareCSSValuePtr(m_shape, other.m_shape)
+        && compareCSSValuePtr(m_sizingBehavior, other.m_sizingBehavior)
+        && compareCSSValuePtr(m_endHorizontalSize, other.m_endHorizontalSize)
+        && compareCSSValuePtr(m_endVerticalSize, other.m_endVerticalSize);
 }
 
-
 String CSSConicGradientValue::customCSSText() const
 {
     StringBuilder result;
 
-    if (m_repeating)
+    if (isRepeating())
         result.appendLiteral("repeating-conic-gradient(");
     else
         result.appendLiteral("conic-gradient(");
@@ -1346,10 +1220,11 @@
         wroteSomething = true;
     }
 
-    if (m_firstX && m_firstY) {
+    if (firstX() && firstY()) {
         if (wroteSomething)
             result.append(' ');
-        result.append("at ", m_firstX->cssText(), ' ', m_firstY->cssText());
+        result.appendLiteral("at ");
+        appendSpaceSeparatedOptionalCSSPtrText(result, firstX(), firstY());
         wroteSomething = true;
     }
 
@@ -1357,7 +1232,7 @@
         result.appendLiteral(", ");
 
     bool wroteFirstStop = false;
-    for (auto& stop : m_stops) {
+    for (auto& stop : stops()) {
         if (wroteFirstStop)
             result.appendLiteral(", ");
         wroteFirstStop = true;
@@ -1374,10 +1249,10 @@
 
     CSSToLengthConversionData conversionData(&renderer.style(), renderer.document().documentElement()->renderStyle(), &renderer.view());
 
-    FloatPoint centerPoint = computeEndPoint(m_firstX.get(), m_firstY.get(), conversionData, size);
-    if (!m_firstX)
+    FloatPoint centerPoint = computeEndPoint(firstX(), firstY(), conversionData, size);
+    if (!firstX())
         centerPoint.setX(size.width() / 2);
-    if (!m_firstY)
+    if (!firstY())
         centerPoint.setY(size.height() / 2);
 
     float angleRadians = 0;
@@ -1395,23 +1270,7 @@
 
 bool CSSConicGradientValue::equals(const CSSConicGradientValue& other) const
 {
-    if (m_repeating != other.m_repeating)
-        return false;
-
-    if (!compareCSSValuePtr(m_angle, other.m_angle))
-        return false;
-
-    bool equalXandY = false;
-    if (m_firstX && m_firstY)
-        equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && compareCSSValuePtr(m_firstY, other.m_firstY);
-    else if (m_firstX)
-        equalXandY = compareCSSValuePtr(m_firstX, other.m_firstX) && !other.m_firstY;
-    else if (m_firstY)
-        equalXandY = compareCSSValuePtr(m_firstY, other.m_firstY) && !other.m_firstX;
-    else
-        equalXandY = !other.m_firstX && !other.m_firstY;
-
-    return equalXandY && m_stops == other.m_stops;
+    return CSSGradientValue::equals(other) && compareCSSValuePtr(m_angle, other.m_angle);
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/css/CSSGradientValue.h (257965 => 257966)


--- trunk/Source/WebCore/css/CSSGradientValue.h	2020-03-06 03:57:30 UTC (rev 257965)
+++ trunk/Source/WebCore/css/CSSGradientValue.h	2020-03-06 04:11:39 UTC (rev 257966)
@@ -28,13 +28,9 @@
 #include "CSSImageGeneratorValue.h"
 #include "CSSPrimitiveValue.h"
 #include "Gradient.h"
-#include <wtf/Vector.h>
 
 namespace WebCore {
 
-class FloatPoint;
-class StyleResolver;
-
 namespace Style {
 class BuilderState;
 }
@@ -51,47 +47,38 @@
 enum CSSGradientRepeat { NonRepeating, Repeating };
 
 struct CSSGradientColorStop {
-    RefPtr<CSSPrimitiveValue> m_position; // percentage or length
-    RefPtr<CSSPrimitiveValue> m_color;
-    Color m_resolvedColor;
-    bool m_colorIsDerivedFromElement = false;
-    bool isMidpoint = false;
-    bool operator==(const CSSGradientColorStop& other) const
-    {
-        return compareCSSValuePtr(m_color, other.m_color)
-            && compareCSSValuePtr(m_position, other.m_position);
-    }
+    RefPtr<CSSPrimitiveValue> color;
+    RefPtr<CSSPrimitiveValue> position; // percentage or length
+    Color resolvedColor;
 };
 
+inline bool operator==(const CSSGradientColorStop& a, const CSSGradientColorStop& b)
+{
+    return compareCSSValuePtr(a.color, b.color) && compareCSSValuePtr(a.position, b.position);
+}
+
 class CSSGradientValue : public CSSImageGeneratorValue {
 public:
-    RefPtr<Image> image(RenderElement&, const FloatSize&);
-
-    void setFirstX(RefPtr<CSSPrimitiveValue>&& val) { m_firstX = WTFMove(val); }
-    void setFirstY(RefPtr<CSSPrimitiveValue>&& val) { m_firstY = WTFMove(val); }
-    void setSecondX(RefPtr<CSSPrimitiveValue>&& val) { m_secondX = WTFMove(val); }
-    void setSecondY(RefPtr<CSSPrimitiveValue>&& val) { m_secondY = WTFMove(val); }
-
-    void addStop(const CSSGradientColorStop& stop) { m_stops.append(stop); }
+    void setFirstX(RefPtr<CSSPrimitiveValue>&& value) { m_firstX = WTFMove(value); }
+    void setFirstY(RefPtr<CSSPrimitiveValue>&& value) { m_firstY = WTFMove(value); }
+    void setSecondX(RefPtr<CSSPrimitiveValue>&& value) { m_secondX = WTFMove(value); }
+    void setSecondY(RefPtr<CSSPrimitiveValue>&& value) { m_secondY = WTFMove(value); }
+    void addStop(CSSGradientColorStop&& stop) { m_stops.append(WTFMove(stop)); }
     void doneAddingStops() { m_stops.shrinkToFit(); }
+    bool hasAtLeastTwoStops() const { return m_stops.size() >= 2; }
+    void resolveRGBColors();
 
-    unsigned stopCount() const { return m_stops.size(); }
-
-    void sortStopsIfNeeded();
-
-    bool isRepeating() const { return m_repeating; }
-
     CSSGradientType gradientType() const { return m_gradientType; }
 
-    bool isFixedSize() const { return false; }
-    FloatSize fixedSize(const RenderElement&) const { return FloatSize(); }
-
-    bool isPending() const { return false; }
+    RefPtr<Image> image(RenderElement&, const FloatSize&);
     bool knownToBeOpaque(const RenderElement&) const;
 
-    void loadSubimages(CachedResourceLoader&, const ResourceLoaderOptions&) { }
+    static constexpr bool isFixedSize() { return false; }
+    static FloatSize fixedSize(const RenderElement&) { return FloatSize(); }
+    static constexpr bool isPending() { return false; }
+    static void loadSubimages(CachedResourceLoader&, const ResourceLoaderOptions&) { }
+
     Ref<CSSGradientValue> gradientWithStylesResolved(Style::BuilderState&);
-    void resolveRGBColors();
 
 protected:
     CSSGradientValue(ClassType classType, CSSGradientRepeat repeat, CSSGradientType gradientType)
@@ -101,7 +88,7 @@
     {
     }
 
-    CSSGradientValue(const CSSGradientValue& other, ClassType classType, CSSGradientType gradientType)
+    CSSGradientValue(const CSSGradientValue& other, ClassType classType)
         : CSSImageGeneratorValue(classType)
         , m_firstX(other.m_firstX)
         , m_firstY(other.m_firstY)
@@ -108,34 +95,36 @@
         , m_secondX(other.m_secondX)
         , m_secondY(other.m_secondY)
         , m_stops(other.m_stops)
-        , m_stopsSorted(other.m_stopsSorted)
-        , m_gradientType(gradientType)
+        , m_gradientType(other.m_gradientType)
         , m_repeating(other.m_repeating)
+        , m_hasColorDerivedFromElement(other.m_hasColorDerivedFromElement)
     {
     }
 
-    template<typename GradientAdapter>
-    Gradient::ColorStopVector computeStops(GradientAdapter&, const CSSToLengthConversionData&, const RenderStyle&, float maxLengthForRepeat);
+    template<typename GradientAdapter> Gradient::ColorStopVector computeStops(GradientAdapter&, const CSSToLengthConversionData&, const RenderStyle&, float maxLengthForRepeat);
 
-    // Resolve points/radii to front end values.
-    FloatPoint computeEndPoint(CSSPrimitiveValue*, CSSPrimitiveValue*, const CSSToLengthConversionData&, const FloatSize&);
+    auto firstX() const { return m_firstX.get(); }
+    auto firstY() const { return m_firstY.get(); }
+    auto secondX() const { return m_secondX.get(); }
+    auto secondY() const { return m_secondY.get(); }
+    auto& stops() const { return m_stops; }
+    bool isRepeating() const { return m_repeating; }
 
+    bool equals(const CSSGradientValue&) const;
+
+private:
+    bool hasColorDerivedFromElement() const;
     bool isCacheable() const;
-    
-    void writeColorStop(StringBuilder&, const CSSGradientColorStop&) const;
 
-    // Points. Some of these may be null.
     RefPtr<CSSPrimitiveValue> m_firstX;
     RefPtr<CSSPrimitiveValue> m_firstY;
-
     RefPtr<CSSPrimitiveValue> m_secondX;
     RefPtr<CSSPrimitiveValue> m_secondY;
-
-    // Stops
     Vector<CSSGradientColorStop, 2> m_stops;
-    bool m_stopsSorted { false };
     CSSGradientType m_gradientType;
     bool m_repeating { false };
+
+    mutable Optional<bool> m_hasColorDerivedFromElement;
 };
 
 class CSSLinearGradientValue final : public CSSGradientValue {
@@ -145,7 +134,7 @@
         return adoptRef(*new CSSLinearGradientValue(repeat, gradientType));
     }
 
-    void setAngle(Ref<CSSPrimitiveValue>&& val) { m_angle = WTFMove(val); }
+    void setAngle(Ref<CSSPrimitiveValue>&& value) { m_angle = WTFMove(value); }
 
     String customCSSText() const;
 
@@ -166,7 +155,7 @@
     }
 
     CSSLinearGradientValue(const CSSLinearGradientValue& other)
-        : CSSGradientValue(other, LinearGradientClass, other.gradientType())
+        : CSSGradientValue(other, LinearGradientClass)
         , m_angle(other.m_angle)
     {
     }
@@ -188,14 +177,14 @@
 
     String customCSSText() const;
 
-    void setFirstRadius(RefPtr<CSSPrimitiveValue>&& val) { m_firstRadius = WTFMove(val); }
-    void setSecondRadius(RefPtr<CSSPrimitiveValue>&& val) { m_secondRadius = WTFMove(val); }
+    void setFirstRadius(RefPtr<CSSPrimitiveValue>&& value) { m_firstRadius = WTFMove(value); }
+    void setSecondRadius(RefPtr<CSSPrimitiveValue>&& value) { m_secondRadius = WTFMove(value); }
 
-    void setShape(RefPtr<CSSPrimitiveValue>&& val) { m_shape = WTFMove(val); }
-    void setSizingBehavior(RefPtr<CSSPrimitiveValue>&& val) { m_sizingBehavior = WTFMove(val); }
+    void setShape(RefPtr<CSSPrimitiveValue>&& value) { m_shape = WTFMove(value); }
+    void setSizingBehavior(RefPtr<CSSPrimitiveValue>&& value) { m_sizingBehavior = WTFMove(value); }
 
-    void setEndHorizontalSize(RefPtr<CSSPrimitiveValue>&& val) { m_endHorizontalSize = WTFMove(val); }
-    void setEndVerticalSize(RefPtr<CSSPrimitiveValue>&& val) { m_endVerticalSize = WTFMove(val); }
+    void setEndHorizontalSize(RefPtr<CSSPrimitiveValue>&& value) { m_endHorizontalSize = WTFMove(value); }
+    void setEndVerticalSize(RefPtr<CSSPrimitiveValue>&& value) { m_endVerticalSize = WTFMove(value); }
 
     // Create the gradient for a given size.
     Ref<Gradient> createGradient(RenderElement&, const FloatSize&);
@@ -209,7 +198,7 @@
     }
 
     CSSRadialGradientValue(const CSSRadialGradientValue& other)
-        : CSSGradientValue(other, RadialGradientClass, other.gradientType())
+        : CSSGradientValue(other, RadialGradientClass)
         , m_firstRadius(other.m_firstRadius)
         , m_secondRadius(other.m_secondRadius)
         , m_shape(other.m_shape)
@@ -248,7 +237,7 @@
 
     String customCSSText() const;
 
-    void setAngle(RefPtr<CSSPrimitiveValue>&& val) { m_angle = WTFMove(val); }
+    void setAngle(RefPtr<CSSPrimitiveValue>&& value) { m_angle = WTFMove(value); }
 
     // Create the gradient for a given size.
     Ref<Gradient> createGradient(RenderElement&, const FloatSize&);
@@ -256,13 +245,13 @@
     bool equals(const CSSConicGradientValue&) const;
 
 private:
-    CSSConicGradientValue(CSSGradientRepeat repeat)
+    explicit CSSConicGradientValue(CSSGradientRepeat repeat)
         : CSSGradientValue(ConicGradientClass, repeat, CSSConicGradient)
     {
     }
 
     CSSConicGradientValue(const CSSConicGradientValue& other)
-        : CSSGradientValue(other, ConicGradientClass, other.gradientType())
+        : CSSGradientValue(other, ConicGradientClass)
         , m_angle(other.m_angle)
     {
     }

Modified: trunk/Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp (257965 => 257966)


--- trunk/Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp	2020-03-06 03:57:30 UTC (rev 257965)
+++ trunk/Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp	2020-03-06 04:11:39 UTC (rev 257966)
@@ -1021,9 +1021,9 @@
             return false;
     }
 
-    stop.m_position = CSSValuePool::singleton().createValue(position, CSSUnitType::CSS_NUMBER);
-    stop.m_color = consumeDeprecatedGradientStopColor(args, cssParserMode);
-    return stop.m_color && args.atEnd();
+    stop.position = CSSValuePool::singleton().createValue(position, CSSUnitType::CSS_NUMBER);
+    stop.color = consumeDeprecatedGradientStopColor(args, cssParserMode);
+    return stop.color && args.atEnd();
 }
 
 static RefPtr<CSSValue> consumeDeprecatedGradient(CSSParserTokenRange& args, CSSParserMode cssParserMode)
@@ -1041,11 +1041,11 @@
     auto point = consumeDeprecatedGradientPoint(args, true);
     if (!point)
         return nullptr;
-    result->setFirstX(point.copyRef());
+    result->setFirstX(WTFMove(point));
     point = consumeDeprecatedGradientPoint(args, false);
     if (!point)
         return nullptr;
-    result->setFirstY(point.copyRef());
+    result->setFirstY(WTFMove(point));
 
     if (!consumeCommaIncludingWhitespace(args))
         return nullptr;
@@ -1055,17 +1055,17 @@
         auto radius = consumeNumber(args, ValueRangeNonNegative);
         if (!radius || !consumeCommaIncludingWhitespace(args))
             return nullptr;
-        downcast<CSSRadialGradientValue>(result.get())->setFirstRadius(radius.copyRef());
+        downcast<CSSRadialGradientValue>(result.get())->setFirstRadius(WTFMove(radius));
     }
 
     point = consumeDeprecatedGradientPoint(args, true);
     if (!point)
         return nullptr;
-    result->setSecondX(point.copyRef());
+    result->setSecondX(WTFMove(point));
     point = consumeDeprecatedGradientPoint(args, false);
     if (!point)
         return nullptr;
-    result->setSecondY(point.copyRef());
+    result->setSecondY(WTFMove(point));
 
     // For radial gradients only, we now expect the second radius.
     if (isDeprecatedRadialGradient) {
@@ -1074,7 +1074,7 @@
         auto radius = consumeNumber(args, ValueRangeNonNegative);
         if (!radius)
             return nullptr;
-        downcast<CSSRadialGradientValue>(result.get())->setSecondRadius(radius.copyRef());
+        downcast<CSSRadialGradientValue>(result.get())->setSecondRadius(WTFMove(radius));
     }
 
     CSSGradientColorStop stop;
@@ -1081,7 +1081,7 @@
     while (consumeCommaIncludingWhitespace(args)) {
         if (!consumeDeprecatedGradientColorStop(args, stop, cssParserMode))
             return nullptr;
-        result->addStop(stop);
+        result->addStop(WTFMove(stop));
     }
 
     result->doneAddingStops();
@@ -1088,63 +1088,54 @@
     return result;
 }
 
-static bool consumeGradientColorStops(CSSParserTokenRange& range, CSSParserMode cssParserMode, CSSGradientValue& gradient)
+static bool consumeGradientColorStops(CSSParserTokenRange& range, CSSParserMode mode, CSSGradientValue& gradient)
 {
     bool supportsColorHints = gradient.gradientType() == CSSLinearGradient || gradient.gradientType() == CSSRadialGradient || gradient.gradientType() == CSSConicGradient;
     
-    bool isConicGradient = gradient.gradientType() == CSSConicGradient;
+    auto consumeStopPosition = [&] {
+        return gradient.gradientType() == CSSConicGradient
+            ? consumeAngleOrPercent(range, mode, ValueRangeAll, UnitlessQuirk::Forbid)
+            : consumeLengthOrPercent(range, mode, ValueRangeAll);
+    };
 
     // The first color stop cannot be a color hint.
     bool previousStopWasColorHint = true;
     do {
-        CSSGradientColorStop stop;
-        stop.m_color = consumeColor(range, cssParserMode);
-        // Two hints in a row are not allowed.
-        if (!stop.m_color && (!supportsColorHints || previousStopWasColorHint))
+        CSSGradientColorStop stop { consumeColor(range, mode), consumeStopPosition(), { } };
+        if (!stop.color && !stop.position)
             return false;
-        
-        previousStopWasColorHint = !stop.m_color;
-        
-        // FIXME-NEWPARSER: This boolean could be removed. Null checking color would be sufficient.
-        stop.isMidpoint = !stop.m_color;
 
-        if (isConicGradient)
-            stop.m_position = consumeAngleOrPercent(range, cssParserMode, ValueRangeAll, UnitlessQuirk::Forbid);
-        else
-            stop.m_position = consumeLengthOrPercent(range, cssParserMode, ValueRangeAll);
-        
-        if (!stop.m_color && !stop.m_position)
+        // Two color hints in a row are not allowed.
+        if (!stop.color && (!supportsColorHints || previousStopWasColorHint))
             return false;
+        previousStopWasColorHint = !stop.color;
 
-        gradient.addStop(stop);
-
-        if (!stop.m_color || !stop.m_position)
-            continue;
-
-        CSSGradientColorStop secondStop;
-        if (isConicGradient)
-            secondStop.m_position = consumeAngleOrPercent(range, cssParserMode, ValueRangeAll, UnitlessQuirk::Forbid);
-        else
-            secondStop.m_position = consumeLengthOrPercent(range, cssParserMode, ValueRangeAll);
-        
-        if (secondStop.m_position)
-            gradient.addStop(secondStop);
-        
+        // Stops with both a color and a position can have a second position, which shares the same color.
+        if (stop.color && stop.position) {
+            if (auto secondPosition = consumeStopPosition()) {
+                gradient.addStop(CSSGradientColorStop { stop });
+                stop.position = WTFMove(secondPosition);
+            }
+        }
+        gradient.addStop(WTFMove(stop));
     } while (consumeCommaIncludingWhitespace(range));
 
-    gradient.doneAddingStops();
-
     // The last color stop cannot be a color hint.
     if (previousStopWasColorHint)
         return false;
 
-    // Must have 2 or more stops to be valid.
-    return gradient.stopCount() >= 2;
+    // Must have two or more stops to be valid.
+    if (!gradient.hasAtLeastTwoStops())
+        return false;
+
+    gradient.doneAddingStops();
+    return true;
 }
 
 static RefPtr<CSSValue> consumeDeprecatedRadialGradient(CSSParserTokenRange& args, CSSParserMode cssParserMode, CSSGradientRepeat repeating)
 {
-    RefPtr<CSSRadialGradientValue> result = CSSRadialGradientValue::create(repeating, CSSPrefixedRadialGradient);
+    auto result = CSSRadialGradientValue::create(repeating, CSSPrefixedRadialGradient);
+
     RefPtr<CSSPrimitiveValue> centerX;
     RefPtr<CSSPrimitiveValue> centerY;
     consumeOneOrTwoValuedPosition(args, cssParserMode, UnitlessQuirk::Forbid, centerX, centerY);
@@ -1151,17 +1142,10 @@
     if ((centerX || centerY) && !consumeCommaIncludingWhitespace(args))
         return nullptr;
 
-    result->setFirstX(centerX.copyRef());
-    result->setFirstY(centerY.copyRef());
-    result->setSecondX(centerX.copyRef());
-    result->setSecondY(centerY.copyRef());
-
     auto shape = consumeIdent<CSSValueCircle, CSSValueEllipse>(args);
     auto sizeKeyword = consumeIdent<CSSValueClosestSide, CSSValueClosestCorner, CSSValueFarthestSide, CSSValueFarthestCorner, CSSValueContain, CSSValueCover>(args);
     if (!shape)
         shape = consumeIdent<CSSValueCircle, CSSValueEllipse>(args);
-    result->setShape(shape.copyRef());
-    result->setSizingBehavior(sizeKeyword.copyRef());
 
     // Or, two lengths or percentages
     if (!shape && !sizeKeyword) {
@@ -1172,16 +1156,24 @@
             if (!verticalSize)
                 return nullptr;
             consumeCommaIncludingWhitespace(args);
-            result->setEndHorizontalSize(horizontalSize.copyRef());
-            result->setEndVerticalSize(verticalSize.copyRef());
+            result->setEndHorizontalSize(WTFMove(horizontalSize));
+            result->setEndVerticalSize(WTFMove(verticalSize));
         }
     } else {
         consumeCommaIncludingWhitespace(args);
     }
-    if (!consumeGradientColorStops(args, cssParserMode, *result))
+
+    if (!consumeGradientColorStops(args, cssParserMode, result))
         return nullptr;
 
-    return result;
+    result->setFirstX(centerX.copyRef());
+    result->setFirstY(centerY.copyRef());
+    result->setSecondX(WTFMove(centerX));
+    result->setSecondY(WTFMove(centerY));
+    result->setShape(WTFMove(shape));
+    result->setSizingBehavior(WTFMove(sizeKeyword));
+
+    return WTFMove(result);
 }
 
 static RefPtr<CSSValue> consumeRadialGradient(CSSParserTokenRange& args, CSSParserMode cssParserMode, CSSGradientRepeat repeating)
@@ -1242,11 +1234,6 @@
         || (verticalSize && verticalSize->isCalculatedPercentageWithLength()))
         return nullptr;
 
-    result->setShape(shape.copyRef());
-    result->setSizingBehavior(sizeKeyword.copyRef());
-    result->setEndHorizontalSize(horizontalSize.copyRef());
-    result->setEndVerticalSize(verticalSize.copyRef());
-
     RefPtr<CSSPrimitiveValue> centerX;
     RefPtr<CSSPrimitiveValue> centerY;
     if (args.peek().id() == CSSValueAt) {
@@ -1267,6 +1254,12 @@
         return nullptr;
     if (!consumeGradientColorStops(args, cssParserMode, *result))
         return nullptr;
+
+    result->setShape(WTFMove(shape));
+    result->setSizingBehavior(WTFMove(sizeKeyword));
+    result->setEndHorizontalSize(WTFMove(horizontalSize));
+    result->setEndVerticalSize(WTFMove(verticalSize));
+
     return result;
 }
 
@@ -1290,8 +1283,8 @@
             endX = consumeIdent<CSSValueLeft, CSSValueRight>(args);
         }
 
-        result->setFirstX(endX.copyRef());
-        result->setFirstY(endY.copyRef());
+        result->setFirstX(WTFMove(endX));
+        result->setFirstY(WTFMove(endY));
     } else {
         expectComma = false;
     }
@@ -1314,10 +1307,10 @@
             auto angle = consumeAngle(args, context.mode, UnitlessQuirk::Forbid);
             if (!angle)
                 return nullptr;
-            result->setAngle(angle.releaseNonNull());
+            result->setAngle(WTFMove(angle));
             expectComma = true;
         }
-        
+
         if (consumeIdent<CSSValueAt>(args)) {
             RefPtr<CSSPrimitiveValue> centerX;
             RefPtr<CSSPrimitiveValue> centerY;
@@ -1324,14 +1317,14 @@
             consumePosition(args, context.mode, UnitlessQuirk::Forbid, PositionSyntax::Position, centerX, centerY);
             if (!(centerX && centerY))
                 return nullptr;
-            
+
             result->setFirstX(centerX.copyRef());
             result->setFirstY(centerY.copyRef());
-            
+
             // Right now, conic gradients have the same start and end centers.
-            result->setSecondX(centerX.copyRef());
-            result->setSecondY(centerY.copyRef());
-    
+            result->setSecondX(WTFMove(centerX));
+            result->setSecondY(WTFMove(centerY));
+
             expectComma = true;
         }
     }

Modified: trunk/Source/WebCore/html/HTMLInputElement.cpp (257965 => 257966)


--- trunk/Source/WebCore/html/HTMLInputElement.cpp	2020-03-06 03:57:30 UTC (rev 257965)
+++ trunk/Source/WebCore/html/HTMLInputElement.cpp	2020-03-06 04:11:39 UTC (rev 257966)
@@ -2055,19 +2055,19 @@
 static Ref<CSSLinearGradientValue> autoFillStrongPasswordMaskImage()
 {
     CSSGradientColorStop firstStop;
-    firstStop.m_color = CSSValuePool::singleton().createColorValue(Color::black);
-    firstStop.m_position = CSSValuePool::singleton().createValue(50, CSSUnitType::CSS_PERCENTAGE);
+    firstStop.color = CSSValuePool::singleton().createColorValue(Color::black);
+    firstStop.position = CSSValuePool::singleton().createValue(50, CSSUnitType::CSS_PERCENTAGE);
 
     CSSGradientColorStop secondStop;
-    secondStop.m_color = CSSValuePool::singleton().createColorValue(Color::transparent);
-    secondStop.m_position = CSSValuePool::singleton().createValue(100, CSSUnitType::CSS_PERCENTAGE);
+    secondStop.color = CSSValuePool::singleton().createColorValue(Color::transparent);
+    secondStop.position = CSSValuePool::singleton().createValue(100, CSSUnitType::CSS_PERCENTAGE);
 
     auto gradient = CSSLinearGradientValue::create(CSSGradientRepeat::NonRepeating, CSSGradientType::CSSLinearGradient);
     gradient->setAngle(CSSValuePool::singleton().createValue(90, CSSUnitType::CSS_DEG));
-    gradient->addStop(firstStop);
-    gradient->addStop(secondStop);
+    gradient->addStop(WTFMove(firstStop));
+    gradient->addStop(WTFMove(secondStop));
+    gradient->doneAddingStops();
     gradient->resolveRGBColors();
-
     return gradient;
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to