Changeset 207342 in webkit

Timestamp:

Oct 14, 2016, 9:17:47 AM (8 years ago)

Author:

Ryan Haddad

Message:

Unreviewed, rolling out r207317.
​https://bugs.webkit.org/show_bug.cgi?id=163448

This change caused an existing LayoutTest to fail on all
platforms (Requested by ryanhaddad on #webkit).

Reverted changeset:

"CSS parsing should use Color not RGBA32"
​https://bugs.webkit.org/show_bug.cgi?id=163423
​http://trac.webkit.org/changeset/207317

Patch by Commit Queue <​commit-queue@webkit.org> on 2016-10-14

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• WebCore.xcodeproj/project.pbxproj (modified) (4 diffs)
• css/CSSPrimitiveValue.cpp (modified) (7 diffs)
• css/CSSPrimitiveValue.h (modified) (4 diffs)
• css/CSSPrimitiveValueMappings.h (modified) (1 diff)
• css/CSSValuePool.cpp (modified) (3 diffs)
• css/CSSValuePool.h (modified) (3 diffs)
• css/StyleResolver.cpp (modified) (1 diff)
• css/parser/CSSParser.cpp (modified) (13 diffs)
• css/parser/CSSParser.h (modified) (3 diffs)
• css/parser/CSSParserFastPaths.cpp (modified) (4 diffs)
• css/parser/CSSPropertyParserHelpers.cpp (modified) (1 diff)
• css/parser/SVGCSSParser.cpp (modified) (2 diffs)
• editing/EditingStyle.cpp (modified) (4 diffs)
• editing/cocoa/HTMLConverter.mm (modified) (2 diffs)
• html/HTMLBodyElement.cpp (modified) (1 diff)
• html/canvas/CanvasGradient.cpp (modified) (3 diffs)
• html/canvas/CanvasRenderingContext2D.cpp (modified) (4 diffs)
• html/canvas/CanvasStyle.cpp (modified) (2 diffs)
• html/canvas/CanvasStyle.h (modified) (1 diff)
• platform/graphics/Color.h (modified) (2 diffs)
• platform/graphics/ColorHash.h (deleted)
• svg/SVGColor.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-10-14  Commit Queue  <commit-queue@webkit.org>
+
+        Unreviewed, rolling out r207317.
+        https://bugs.webkit.org/show_bug.cgi?id=163448
+
+        This change caused an existing LayoutTest to fail on all
+        platforms (Requested by ryanhaddad on #webkit).
+
+        Reverted changeset:
+
+        "CSS parsing should use Color not RGBA32"
+        https://bugs.webkit.org/show_bug.cgi?id=163423
+        http://trac.webkit.org/changeset/207317
+
 2016-10-14  Antti Koivisto  <antti@apple.com>
 

trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

@@ -1298 +1298 @@
                 2EF1BFEB121C9F4200C27627 /* FileStream.h in Headers */ = {isa = PBXBuildFile; fileRef = 2EF1BFE9121C9F4200C27627 /* FileStream.h */; settings = {ATTRIBUTES = (Private, ); }; };
                 2EF1BFF9121CB0CE00C27627 /* FileStreamClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 2EF1BFF8121CB0CE00C27627 /* FileStreamClient.h */; settings = {ATTRIBUTES = (Private, ); }; };
-                3103B7DF1DB01567008BB890 /* ColorHash.h in Headers */ = {isa = PBXBuildFile; fileRef = 3103B7DE1DB01556008BB890 /* ColorHash.h */; settings = {ATTRIBUTES = (Private, ); }; };
                 3106036F14327D2E00ABF4BA /* WebKitCSSFilterValue.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3106036C14327D2E00ABF4BA /* WebKitCSSFilterValue.cpp */; };
                 3106037014327D2E00ABF4BA /* WebKitCSSFilterValue.h in Headers */ = {isa = PBXBuildFile; fileRef = 3106036D14327D2E00ABF4BA /* WebKitCSSFilterValue.h */; settings = {ATTRIBUTES = (Private, ); }; };
@@ -8263 +8262 @@
                 2EF1BFE9121C9F4200C27627 /* FileStream.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FileStream.h; sourceTree = "<group>"; };
                 2EF1BFF8121CB0CE00C27627 /* FileStreamClient.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = FileStreamClient.h; sourceTree = "<group>"; };
-                3103B7DE1DB01556008BB890 /* ColorHash.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = ColorHash.h; sourceTree = "<group>"; };
                 3106036C14327D2E00ABF4BA /* WebKitCSSFilterValue.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WebKitCSSFilterValue.cpp; sourceTree = "<group>"; };
                 3106036D14327D2E00ABF4BA /* WebKitCSSFilterValue.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebKitCSSFilterValue.h; sourceTree = "<group>"; };
@@ -20737 +20735 @@
                                 B27535380B053814002CE64F /* Color.cpp */,
                                 B27535390B053814002CE64F /* Color.h */,
-                                3103B7DE1DB01556008BB890 /* ColorHash.h */,
                                 31DCDF411DA1C45400EA5B93 /* ExtendedColor.cpp */,
                                 31DCDF421DA1C45400EA5B93 /* ExtendedColor.h */,
@@ -25390 +25387 @@
                                 FDA15ECE12B03F61003A583A /* JSScriptProcessorNode.h in Headers */,
                                 CED06AD11C77754800FDFAF1 /* JSSecurityPolicyViolationEvent.h in Headers */,
-                                3103B7DF1DB01567008BB890 /* ColorHash.h in Headers */,
                                 9BDA64D81B975CF2009C4387 /* JSShadowRoot.h in Headers */,
                                 CD9DE17B17AAC75B00EA386D /* JSSourceBuffer.h in Headers */,

trunk/Source/WebCore/css/CSSPrimitiveValue.cpp

@@ -351 +351 @@
 }
 
-CSSPrimitiveValue::CSSPrimitiveValue(const Color& color)
+CSSPrimitiveValue::CSSPrimitiveValue(RGBA32 color)
     : CSSValue(PrimitiveClass)
 {
     m_primitiveUnitType = CSS_RGBCOLOR;
-    m_value.color = new Color(color);
+    m_value.rgbcolor = color;
 }
 
@@ -575 +575 @@
         m_value.fontFamily = nullptr;
         break;
-    case CSS_RGBCOLOR:
-        ASSERT(m_value.color);
-        delete m_value.color;
-        m_value.color = nullptr;
-        break;
     case CSS_NUMBER:
     case CSS_PARSER_INTEGER:
@@ -611 +606 @@
     case CSS_FR:
     case CSS_IDENT:
+    case CSS_RGBCOLOR:
     case CSS_UNKNOWN:
     case CSS_UNICODE_RANGE:
@@ -1033 +1029 @@
 
     // FIMXE: This should not return a new object for each invocation.
-    return RGBColor::create(m_value.color->rgb());
+    return RGBColor::create(m_value.rgbcolor);
 }
 
@@ -1180 +1176 @@
         return getLengthRepeatValue()->cssText();
 #endif
+    case CSS_RGBCOLOR:
     case CSS_PARSER_HEXCOLOR: {
-        RGBA32 rgb;
-        Color::parseHexColor((String)m_value.string, rgb);
-        return Color(rgb).cssText();
-    }
-    case CSS_RGBCOLOR:
-        return color().cssText();
+        RGBA32 rgbColor = m_value.rgbcolor;
+        if (m_primitiveUnitType == CSS_PARSER_HEXCOLOR)
+            Color::parseHexColor((String)m_value.string, rgbColor);
+        return Color(rgbColor).cssText();
+    }
     case CSS_PAIR:
         return getPairValue()->cssText();
@@ -1356 +1352 @@
         break;
     case CSS_RGBCOLOR:
-        result = CSSPrimitiveValue::create(*m_value.color);
+        result = CSSPrimitiveValue::createColor(m_value.rgbcolor);
         break;
     case CSS_DIMENSION:
@@ -1435 +1431 @@
 #endif
     case CSS_RGBCOLOR:
-        return color() == other.color();
+        return m_value.rgbcolor == other.m_value.rgbcolor;
     case CSS_PAIR:
         return m_value.pair && other.m_value.pair && m_value.pair->equals(*other.m_value.pair);

trunk/Source/WebCore/css/CSSPrimitiveValue.h

@@ -254 +254 @@
     static Ref<CSSPrimitiveValue> createParserOperator(int parserOperator) { return adoptRef(*new CSSPrimitiveValue(parserOperator)); }
 
+    static Ref<CSSPrimitiveValue> createColor(unsigned rgbValue) { return adoptRef(*new CSSPrimitiveValue(rgbValue)); }
     static Ref<CSSPrimitiveValue> create(double value, UnitTypes type) { return adoptRef(*new CSSPrimitiveValue(value, type)); }
     static Ref<CSSPrimitiveValue> create(const String& value, UnitTypes type) { return adoptRef(*new CSSPrimitiveValue(value, type)); }
@@ -342 +343 @@
 
     WEBCORE_EXPORT Counter* getCounterValue(ExceptionCode&) const;
-    Counter* getCounterValue() const { return m_primitiveUnitType != CSS_COUNTER ? nullptr : m_value.counter; }
+    Counter* getCounterValue() const { return m_primitiveUnitType != CSS_COUNTER ? 0 : m_value.counter; }
 
     WEBCORE_EXPORT Rect* getRectValue(ExceptionCode&) const;
-    Rect* getRectValue() const { return m_primitiveUnitType != CSS_RECT ? nullptr : m_value.rect; }
+    Rect* getRectValue() const { return m_primitiveUnitType != CSS_RECT ? 0 : m_value.rect; }
 
     Quad* getQuadValue(ExceptionCode&) const;
-    Quad* getQuadValue() const { return m_primitiveUnitType != CSS_QUAD ? nullptr : m_value.quad; }
+    Quad* getQuadValue() const { return m_primitiveUnitType != CSS_QUAD ? 0 : m_value.quad; }
 
 #if ENABLE(CSS_SCROLL_SNAP)
     LengthRepeat* getLengthRepeatValue(ExceptionCode&) const;
-    LengthRepeat* getLengthRepeatValue() const { return m_primitiveUnitType != CSS_LENGTH_REPEAT ? nullptr : m_value.lengthRepeat; }
+    LengthRepeat* getLengthRepeatValue() const { return m_primitiveUnitType != CSS_LENGTH_REPEAT ? 0 : m_value.lengthRepeat; }
 #endif
 
     WEBCORE_EXPORT RefPtr<RGBColor> getRGBColorValue(ExceptionCode&) const;
-    const Color& color() const { ASSERT(m_primitiveUnitType == CSS_RGBCOLOR); return *m_value.color; }
+    RGBA32 getRGBA32Value() const { return m_primitiveUnitType != CSS_RGBCOLOR ? 0 : m_value.rgbcolor; }
 
     Pair* getPairValue(ExceptionCode&) const;
@@ -404 +405 @@
     // FIXME: int vs. unsigned overloading is too subtle to distinguish the color and operator cases.
     CSSPrimitiveValue(int parserOperator);
-    CSSPrimitiveValue(const Color&);
+    CSSPrimitiveValue(unsigned color); // RGB value
     CSSPrimitiveValue(const Length&);
     CSSPrimitiveValue(const Length&, const RenderStyle&);
@@ -464 +465 @@
         LengthRepeat* lengthRepeat;
 #endif
-        const Color* color;
+        unsigned rgbcolor;
         Pair* pair;
         DashboardRegion* region;

trunk/Source/WebCore/css/CSSPrimitiveValueMappings.h

@@ -4940 +4940 @@
 }
 
-template<> inline CSSPrimitiveValue::CSSPrimitiveValue(const Color& color)
-    : CSSValue(PrimitiveClass)
-{
-    m_primitiveUnitType = CSS_RGBCOLOR;
-    m_value.color = new Color(color);
-}
-
 template<> inline CSSPrimitiveValue::CSSPrimitiveValue(CSSFontFamily fontFamily)
     : CSSValue(PrimitiveClass)

trunk/Source/WebCore/css/CSSValuePool.cpp

@@ -49 +49 @@
     m_revertValue.construct();
 
-    m_transparentColor.construct(Color(Color::transparent));
-    m_whiteColor.construct(Color(Color::white));
-    m_blackColor.construct(Color(Color::black));
+    m_transparentColor.construct(Color::transparent);
+    m_whiteColor.construct(Color::white);
+    m_blackColor.construct(Color::black);
 
     for (unsigned i = 0; i < numCSSValueKeywords; ++i)
@@ -74 +74 @@
 }
 
-Ref<CSSPrimitiveValue> CSSValuePool::createColorValue(const Color& color)
+Ref<CSSPrimitiveValue> CSSValuePool::createColorValue(unsigned rgbValue)
 {
     // These are the empty and deleted values of the hash table.
-    if (color == Color::transparent)
+    if (rgbValue == Color::transparent)
         return m_transparentColor.get();
-    if (color == Color::white)
+    if (rgbValue == Color::white)
         return m_whiteColor.get();
     // Just because it is common.
-    if (color == Color::black)
+    if (rgbValue == Color::black)
         return m_blackColor.get();
 
@@ -90 +90 @@
         m_colorValueCache.remove(m_colorValueCache.begin());
 
-    ColorValueCache::AddResult entry = m_colorValueCache.add(color, nullptr);
+    ColorValueCache::AddResult entry = m_colorValueCache.add(rgbValue, nullptr);
     if (entry.isNewEntry)
-        entry.iterator->value = CSSPrimitiveValue::create(color);
+        entry.iterator->value = CSSPrimitiveValue::createColor(rgbValue);
     return *entry.iterator->value;
 }

trunk/Source/WebCore/css/CSSValuePool.h

@@ -35 +35 @@
 #include "CSSUnsetValue.h"
 #include "CSSValueKeywords.h"
-#include "ColorHash.h"
 #include <utility>
 #include <wtf/HashMap.h>
@@ -62 +61 @@
     Ref<CSSPrimitiveValue> createIdentifierValue(CSSValueID identifier);
     Ref<CSSPrimitiveValue> createIdentifierValue(CSSPropertyID identifier);
-    Ref<CSSPrimitiveValue> createColorValue(const Color&);
+    Ref<CSSPrimitiveValue> createColorValue(unsigned rgbValue);
     Ref<CSSPrimitiveValue> createValue(double value, CSSPrimitiveValue::UnitTypes);
     Ref<CSSPrimitiveValue> createValue(const String& value, CSSPrimitiveValue::UnitTypes type) { return CSSPrimitiveValue::create(value, type); }
@@ -74 +73 @@
     CSSValuePool();
 
-    typedef HashMap<Color, RefPtr<CSSPrimitiveValue>> ColorValueCache;
+    typedef HashMap<unsigned, RefPtr<CSSPrimitiveValue>> ColorValueCache;
     ColorValueCache m_colorValueCache;
 

trunk/Source/WebCore/css/StyleResolver.cpp

@@ -1832 +1832 @@
 {
     if (value.isRGBColor())
-        return value.color();
+        return Color(value.getRGBA32Value());
 
     const State& state = m_state;

trunk/Source/WebCore/css/parser/CSSParser.cpp

@@ -495 +495 @@
         return declaration.addParsedProperty(CSSProperty(propertyId, WTFMove(value), important)) ? CSSParser::ParseResult::Changed : CSSParser::ParseResult::Unchanged;
     }
-    Color color = CSSParser::fastParseColor(string, strict && string[0] != '#');
-    if (!color.isValid())
+    RGBA32 color;
+    if (!CSSParser::fastParseColor(color, string, strict && string[0] != '#'))
         return CSSParser::ParseResult::Error;
 
@@ -1371 +1371 @@
 }
 
-Color CSSParser::parseColor(const String& string, bool strict)
+// The color will only be changed when string contains a valid CSS color, so callers
+// can set it to a default color and ignore the boolean result.
+bool CSSParser::parseColor(RGBA32& color, const String& string, bool strict)
 {
     if (string.isEmpty())
-        return Color();
+        return false;
 
     // First try creating a color specified by name, rgba(), rgb() or "#" syntax.
-    Color color = fastParseColor(string, strict);
-    if (color.isValid())
-        return color;
+    if (fastParseColor(color, string, strict))
+        return true;
 
     CSSParser parser(HTMLStandardMode);
 
     // In case the fast-path parser didn't understand the color, try the full parser.
-    if (!parser.parseColorFromString(string))
-        return Color();
+    if (!parser.parseColor(string))
+        return false;
 
     CSSValue& value = *parser.m_parsedProperties.first().value();
     if (!is<CSSPrimitiveValue>(value))
-        return Color();
+        return false;
 
     CSSPrimitiveValue& primitiveValue = downcast<CSSPrimitiveValue>(value);
     if (!primitiveValue.isRGBColor())
-        return Color();
-
-    return primitiveValue.color();
-}
-
-bool CSSParser::parseColorFromString(const String& string)
+        return false;
+
+    color = primitiveValue.getRGBA32Value();
+    return true;
+}
+
+bool CSSParser::parseColor(const String& string)
 {
     setupParser("@-webkit-decls{color:", string, "} ");
@@ -1407 +1409 @@
 }
 
-Color CSSParser::parseSystemColor(const String& string, Document* document)
+bool CSSParser::parseSystemColor(RGBA32& color, const String& string, Document* document)
 {
     if (!document || !document->page())
-        return Color();
+        return false;
 
     CSSParserString cssColor;
@@ -1416 +1418 @@
     CSSValueID id = cssValueKeywordID(cssColor);
     if (!validPrimitiveValueColor(id))
-        return Color();
-
-    return document->page()->theme().systemColor(id);
+        return false;
+
+    Color parsedColor = document->page()->theme().systemColor(id);
+    if (!parsedColor.isValid())
+        return false;
+
+    color = parsedColor.rgb();
+    return true;
 }
 
@@ -7475 +7482 @@
 
 template <typename CharacterType>
-static inline Color fastParseColorInternal(const CharacterType* characters, unsigned length , bool strict)
+static inline bool fastParseColorInternal(RGBA32& rgb, const CharacterType* characters, unsigned length , bool strict)
 {
     CSSPrimitiveValue::UnitTypes expect = CSSPrimitiveValue::CSS_UNKNOWN;
-
+    
     if (!strict && length >= 3) {
-        RGBA32 rgb;
         if (characters[0] == '#') {
             if (Color::parseHexColor(characters + 1, length - 1, rgb))
-                return Color(rgb);
+                return true;
         } else {
             if (Color::parseHexColor(characters, length, rgb))
-                return Color(rgb);
+                return true;
         }
     }
@@ -7500 +7506 @@
         
         if (!parseColorIntOrPercentage(current, end, ',', expect, red))
-            return Color();
+            return false;
         if (!parseColorIntOrPercentage(current, end, ',', expect, green))
-            return Color();
+            return false;
         if (!parseColorIntOrPercentage(current, end, ',', expect, blue))
-            return Color();
+            return false;
         if (!parseAlphaValue(current, end, ')', alpha))
-            return Color();
+            return false;
         if (current != end)
-            return Color();
-        return Color(makeRGBA(red, green, blue, alpha));
+            return false;
+        rgb = makeRGBA(red, green, blue, alpha);
+        return true;
     }
 
@@ -7520 +7527 @@
         int blue;
         if (!parseColorIntOrPercentage(current, end, ',', expect, red))
-            return Color();
+            return false;
         if (!parseColorIntOrPercentage(current, end, ',', expect, green))
-            return Color();
+            return false;
         if (!parseColorIntOrPercentage(current, end, ')', expect, blue))
-            return Color();
+            return false;
         if (current != end)
-            return Color();
-        return Color(makeRGB(red, green, blue));
-    }
-
-    return Color();
+            return false;
+        rgb = makeRGB(red, green, blue);
+        return true;
+    }
+
+    return false;
 }
 
 template<typename StringType>
-Color CSSParser::fastParseColor(const StringType& name, bool strict)
+bool CSSParser::fastParseColor(RGBA32& rgb, const StringType& name, bool strict)
 {
     unsigned length = name.length();
+    bool parseResult;
 
     if (!length)
-        return Color();
-
-    Color color;
+        return false;
+
     if (name.is8Bit())
-        color = fastParseColorInternal(name.characters8(), length, strict);
+        parseResult = fastParseColorInternal(rgb, name.characters8(), length, strict);
     else
-        color = fastParseColorInternal(name.characters16(), length, strict);
-
-    if (color.isValid())
-        return color;
+        parseResult = fastParseColorInternal(rgb, name.characters16(), length, strict);
+
+    if (parseResult)
+        return true;
 
     // Try named colors.
-    color.setNamedColor(name);
-    return color;
+    Color tc;
+    tc.setNamedColor(name);
+    if (tc.isValid()) {
+        rgb = tc.rgb();
+        return true;
+    }
+    return false;
 }
     
@@ -7668 +7681 @@
 RefPtr<CSSPrimitiveValue> CSSParser::parseColor(CSSParserValue* value)
 {
-    Color color = parseColorFromValue(value ? *value : *m_valueList->current());
-    if (!color.isValid())
+    RGBA32 c = Color::transparent;
+    if (!parseColorFromValue(value ? *value : *m_valueList->current(), c))
         return nullptr;
-    return CSSValuePool::singleton().createColorValue(color);
-}
-
-Color CSSParser::parseColorFromValue(CSSParserValue& value)
+    return CSSValuePool::singleton().createColorValue(c);
+}
+
+bool CSSParser::parseColorFromValue(CSSParserValue& value, RGBA32& c)
 {
     if (inQuirksMode() && value.unit == CSSPrimitiveValue::CSS_NUMBER
@@ -7680 +7693 @@
         String str = String::format("%06d", static_cast<int>((value.fValue+.5)));
         // FIXME: This should be strict parsing for SVG as well.
-        return fastParseColor(str, inStrictMode());
+        if (!fastParseColor(c, str, inStrictMode()))
+            return false;
     } else if (value.unit == CSSPrimitiveValue::CSS_PARSER_HEXCOLOR
         || value.unit == CSSPrimitiveValue::CSS_IDENT
         || (inQuirksMode() && value.unit == CSSPrimitiveValue::CSS_DIMENSION)) {
-        return fastParseColor(value.string, inStrictMode() && value.unit == CSSPrimitiveValue::CSS_IDENT);
+        if (!fastParseColor(c, value.string, inStrictMode() && value.unit == CSSPrimitiveValue::CSS_IDENT))
+            return false;
     } else if (value.unit == CSSParserValue::Function
         && value.function->args
@@ -7691 +7706 @@
         int colorValues[3];
         if (!parseRGBParameters(value, colorValues, false))
-            return Color();
-        return Color(makeRGB(colorValues[0], colorValues[1], colorValues[2]));
+            return false;
+        c = makeRGB(colorValues[0], colorValues[1], colorValues[2]);
     } else if (value.unit == CSSParserValue::Function
         && value.function->args
@@ -7699 +7714 @@
         int colorValues[4];
         if (!parseRGBParameters(value, colorValues, true))
-            return Color();
-        return Color(makeRGBA(colorValues[0], colorValues[1], colorValues[2], colorValues[3]));
+            return false;
+        c = makeRGBA(colorValues[0], colorValues[1], colorValues[2], colorValues[3]);
     } else if (value.unit == CSSParserValue::Function
         && value.function->args
@@ -7707 +7722 @@
         double colorValues[3];
         if (!parseHSLParameters(value, colorValues, false))
-            return Color();
-        return Color(makeRGBAFromHSLA(colorValues[0], colorValues[1], colorValues[2], 1.0));
+            return false;
+        c = makeRGBAFromHSLA(colorValues[0], colorValues[1], colorValues[2], 1.0);
     } else if (value.unit == CSSParserValue::Function
         && value.function->args
@@ -7715 +7730 @@
         double colorValues[4];
         if (!parseHSLParameters(value, colorValues, true))
-            return Color();
-        return Color(makeRGBAFromHSLA(colorValues[0], colorValues[1], colorValues[2], colorValues[3]));
-    }
-
-    return Color();
+            return false;
+        c = makeRGBAFromHSLA(colorValues[0], colorValues[1], colorValues[2], colorValues[3]);
+    } else
+        return false;
+
+    return true;
 }
 

trunk/Source/WebCore/css/parser/CSSParser.h

@@ -135 +135 @@
     static ParseResult parseCustomPropertyValue(MutableStyleProperties&, const AtomicString& propertyName, const String&, bool important, const CSSParserContext&, StyleSheetContents* contextStyleSheet);
 
-    static Color parseColor(const String&, bool strict = false);
+    static bool parseColor(RGBA32& color, const String&, bool strict = false);
     static bool isValidSystemColorValue(CSSValueID);
-    static Color parseSystemColor(const String&, Document*);
+    static bool parseSystemColor(RGBA32& color, const String&, Document*);
     static RefPtr<CSSValueList> parseFontFaceValue(const AtomicString&);
     RefPtr<CSSPrimitiveValue> parseValidPrimitive(CSSValueID ident, ValueWithCalculation&);
@@ -279 +279 @@
     bool parseHSLParameters(CSSParserValue&, double* colorValues, bool parseAlpha);
     RefPtr<CSSPrimitiveValue> parseColor(CSSParserValue* = nullptr);
-    Color parseColorFromValue(CSSParserValue&);
+    bool parseColorFromValue(CSSParserValue&, RGBA32&);
     void parseSelector(const String&, CSSSelectorList&);
 
     template<typename StringType>
-    static Color fastParseColor(const StringType&, bool strict);
+    static bool fastParseColor(RGBA32&, const StringType&, bool strict);
 
     bool parseLineHeight(bool important);
@@ -619 +619 @@
     bool parseFontFaceSrcLocal(CSSValueList&);
 
-    bool parseColorFromString(const String&);
+    bool parseColor(const String&);
 
 #if ENABLE(CSS_GRID_LAYOUT)

trunk/Source/WebCore/css/parser/CSSParserFastPaths.cpp

@@ -412 +412 @@
 
 template <typename CharacterType>
-static Color fastParseColorInternal(const CharacterType* characters, unsigned length, bool quirksMode)
+static bool fastParseColorInternal(RGBA32& rgb, const CharacterType* characters, unsigned length, bool quirksMode)
 {
     CSSPrimitiveValue::UnitTypes expect = CSSPrimitiveValue::UnitTypes::CSS_UNKNOWN;
 
-    if (length >= 4 && characters[0] == '#') {
-        // FIXME: Why doesn't this check if the parse worked? Is the fallback black?
-        RGBA32 rgb;
-        Color::parseHexColor(characters + 1, length - 1, rgb);
-        return Color(rgb);
-    }
+    if (length >= 4 && characters[0] == '#')
+        return Color::parseHexColor(characters + 1, length - 1, rgb);
 
     if (quirksMode && (length == 3 || length == 6)) {
-        RGBA32 rgb;
         if (Color::parseHexColor(characters, length, rgb))
-            return Color(rgb);
+            return true;
     }
 
@@ -439 +434 @@
 
         if (!parseColorIntOrPercentage(current, end, ',', expect, red))
-            return Color();
+            return false;
         if (!parseColorIntOrPercentage(current, end, ',', expect, green))
-            return Color();
+            return false;
         if (!parseColorIntOrPercentage(current, end, ',', expect, blue))
-            return Color();
+            return false;
         if (!parseAlphaValue(current, end, ')', alpha))
-            return Color();
+            return false;
         if (current != end)
-            return Color();
-        return Color(makeRGBA(red, green, blue, alpha));
+            return false;
+        rgb = makeRGBA(red, green, blue, alpha);
+        return true;
     }
 
@@ -459 +455 @@
         int blue;
         if (!parseColorIntOrPercentage(current, end, ',', expect, red))
-            return Color();
+            return false;
         if (!parseColorIntOrPercentage(current, end, ',', expect, green))
-            return Color();
+            return false;
         if (!parseColorIntOrPercentage(current, end, ')', expect, blue))
-            return Color();
+            return false;
         if (current != end)
-            return Color();
-        return Color(makeRGB(red, green, blue));
-    }
-
-    return Color();
+            return false;
+        rgb = makeRGB(red, green, blue);
+        return true;
+    }
+
+    return false;
 }
 
@@ -482 +479 @@
     }
 
+    RGBA32 color;
     bool quirksMode = isQuirksModeBehavior(parserMode);
 
     // Fast path for hex colors and rgb()/rgba() colors
-    Color color;
+    bool parseResult;
     if (string.is8Bit())
-        color = fastParseColorInternal(string.characters8(), string.length(), quirksMode);
+        parseResult = fastParseColorInternal(color, string.characters8(), string.length(), quirksMode);
     else
-        color = fastParseColorInternal(string.characters16(), string.length(), quirksMode);
-    if (!color.isValid())
+        parseResult = fastParseColorInternal(color, string.characters16(), string.length(), quirksMode);
+    if (!parseResult)
         return nullptr;
     return CSSValuePool::singleton().createColorValue(color);

trunk/Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp

@@ -510 +510 @@
     if (!parseHexColor(range, color, acceptQuirkyColors) && !parseColorFunction(range, color))
         return nullptr;
-    return CSSPrimitiveValue::create(Color(color));
+    return CSSPrimitiveValue::createColor(color);
 }
 

trunk/Source/WebCore/css/parser/SVGCSSParser.cpp

@@ -132 +132 @@
                 parsedValue = SVGPaint::createColor(RenderTheme::defaultTheme()->systemColor(id));
             else if (valueWithCalculation.value().unit == CSSPrimitiveValue::CSS_URI) {
+                RGBA32 c = Color::transparent;
                 if (m_valueList->next()) {
-                    Color color = parseColorFromValue(*m_valueList->current());
-                    if (color.isValid())
-                        parsedValue = SVGPaint::createURIAndColor(valueWithCalculation.value().string, color);
+                    if (parseColorFromValue(*m_valueList->current(), c))
+                        parsedValue = SVGPaint::createURIAndColor(valueWithCalculation.value().string, c);
                     else if (m_valueList->current()->id == CSSValueNone)
                         parsedValue = SVGPaint::createURIAndNone(valueWithCalculation.value().string);
@@ -287 +287 @@
 RefPtr<SVGPaint> CSSParser::parseSVGPaint()
 {
-    Color color = parseColorFromValue(*m_valueList->current());
-    if (!color.isValid())
+    RGBA32 c = Color::transparent;
+    if (!parseColorFromValue(*m_valueList->current(), c))
         return nullptr;
-    return SVGPaint::createColor(color);
+    return SVGPaint::createColor(Color(c));
 }
 
 RefPtr<SVGColor> CSSParser::parseSVGColor()
 {
-    Color color = parseColorFromValue(*m_valueList->current());
-    if (!color.isValid())
+    RGBA32 c = Color::transparent;
+    if (!parseColorFromValue(*m_valueList->current(), c))
         return nullptr;
-    return SVGColor::createFromColor(color);
+    return SVGColor::createFromColor(Color(c));
 }
 

trunk/Source/WebCore/editing/EditingStyle.cpp

@@ -383 +383 @@
 }
 
-static Color cssValueToColor(CSSValue* colorValue)
+static RGBA32 cssValueToRGBA(CSSValue* colorValue)
 {
     if (!is<CSSPrimitiveValue>(colorValue))
@@ -390 +390 @@
     CSSPrimitiveValue& primitiveColor = downcast<CSSPrimitiveValue>(*colorValue);
     if (primitiveColor.isRGBColor())
-        return primitiveColor.color();
+        return primitiveColor.getRGBA32Value();
     
-    return CSSParser::parseColor(colorValue->cssText());
+    RGBA32 rgba = 0;
+    CSSParser::parseColor(rgba, colorValue->cssText());
+    return rgba;
 }
 
 template<typename T>
-static inline Color textColorFromStyle(T& style)
-{
-    return cssValueToColor(extractPropertyValue(style, CSSPropertyColor).get());
+static inline RGBA32 textColorFromStyle(T& style)
+{
+    return cssValueToRGBA(extractPropertyValue(style, CSSPropertyColor).get());
 }
 
 template<typename T>
-static inline Color backgroundColorFromStyle(T& style)
-{
-    return cssValueToColor(extractPropertyValue(style, CSSPropertyBackgroundColor).get());
-}
-
-static inline Color rgbaBackgroundColorInEffect(Node* node)
-{
-    return cssValueToColor(backgroundColorInEffect(node).get());
+static inline RGBA32 backgroundColorFromStyle(T& style)
+{
+    return cssValueToRGBA(extractPropertyValue(style, CSSPropertyBackgroundColor).get());
+}
+
+static inline RGBA32 rgbaBackgroundColorInEffect(Node* node)
+{
+    return cssValueToRGBA(backgroundColorInEffect(node).get());
 }
 
@@ -1095 +1097 @@
 
     if (hasTransparentBackgroundColor(m_mutableStyle.get())
-        || cssValueToColor(m_mutableStyle->getPropertyCSSValue(CSSPropertyBackgroundColor).get()) == rgbaBackgroundColorInEffect(position.containerNode()))
+        || cssValueToRGBA(m_mutableStyle->getPropertyCSSValue(CSSPropertyBackgroundColor).get()) == rgbaBackgroundColorInEffect(position.containerNode()))
         m_mutableStyle->removeProperty(CSSPropertyBackgroundColor);
 
@@ -1839 +1841 @@
     CSSPrimitiveValue& value = downcast<CSSPrimitiveValue>(*cssValue);
     if (value.isRGBColor())
-        return value.color().hasAlpha();
+        return !alphaChannel(value.getRGBA32Value());
     return value.getValueID() == CSSValueTransparent;
 }

trunk/Source/WebCore/editing/cocoa/HTMLConverter.mm

@@ -990 +990 @@
     if (RefPtr<CSSValue> value = computedStylePropertyForElement(element, propertyId)) {
         if (is<CSSPrimitiveValue>(*value) && downcast<CSSPrimitiveValue>(*value).isRGBColor())
-            return normalizedColor(Color(downcast<CSSPrimitiveValue>(*value).color().rgb()), propertyId == CSSPropertyColor);
+            return normalizedColor(Color(downcast<CSSPrimitiveValue>(*value).getRGBA32Value()), propertyId == CSSPropertyColor);
     }
 
@@ -996 +996 @@
     if (RefPtr<CSSValue> value = inlineStylePropertyForElement(element, propertyId)) {
         if (is<CSSPrimitiveValue>(*value) && downcast<CSSPrimitiveValue>(*value).isRGBColor())
-            return normalizedColor(Color(downcast<CSSPrimitiveValue>(*value).color().rgb()), propertyId == CSSPropertyColor);
+            return normalizedColor(Color(downcast<CSSPrimitiveValue>(*value).getRGBA32Value()), propertyId == CSSPropertyColor);
         if (value->isInheritedValue())
             inherit = true;

trunk/Source/WebCore/html/HTMLBodyElement.cpp

@@ -159 +159 @@
                 document().resetActiveLinkColor();
         } else {
-            Color color = CSSParser::parseColor(value, !document().inQuirksMode());
-            if (color.isValid()) {
+            RGBA32 color;
+            if (CSSParser::parseColor(color, value, !document().inQuirksMode())) {
                 if (name == linkAttr)
                     document().setLinkColor(color);

trunk/Source/WebCore/html/canvas/CanvasGradient.cpp

@@ -51 +51 @@
 }
 
-void CanvasGradient::addColorStop(float value, const String& colorString, ExceptionCode& ec)
+void CanvasGradient::addColorStop(float value, const String& color, ExceptionCode& ec)
 {
     if (!(value >= 0 && value <= 1.0f)) {
@@ -58 +58 @@
     }
 
-    Color color = parseColorOrCurrentColor(colorString, 0 /*canvas*/);
-    if (!color.isValid()) {
+    RGBA32 rgba = 0;
+    if (!parseColorOrCurrentColor(rgba, color, 0 /*canvas*/)) {
 #if ENABLE(DASHBOARD_SUPPORT)
         if (!m_dashboardCompatibilityMode)
@@ -69 +69 @@
     }
 
-    m_gradient->addColorStop(value, color);
+    m_gradient->addColorStop(value, Color(rgba));
 }
 

trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp

@@ -405 +405 @@
 
     if (style.isCurrentColor()) {
-        if (style.hasOverrideAlpha()) {
-            // FIXME: Should not use RGBA32 here.
-            style = CanvasStyle(colorWithOverrideAlpha(currentColor(canvas()).rgb(), style.overrideAlpha()));
-        } else
+        if (style.hasOverrideAlpha())
+            style = CanvasStyle(colorWithOverrideAlpha(currentColor(canvas()), style.overrideAlpha()));
+        else
             style = CanvasStyle(currentColor(canvas()));
     } else
@@ -432 +431 @@
 
     if (style.isCurrentColor()) {
-        if (style.hasOverrideAlpha()) {
-            // FIXME: Should not use RGBA32 here.
-            style = CanvasStyle(colorWithOverrideAlpha(currentColor(canvas()).rgb(), style.overrideAlpha()));
-        } else
+        if (style.hasOverrideAlpha())
+            style = CanvasStyle(colorWithOverrideAlpha(currentColor(canvas()), style.overrideAlpha()));
+        else
             style = CanvasStyle(currentColor(canvas()));
     } else
@@ -581 +579 @@
 }
 
-void CanvasRenderingContext2D::setShadowColor(const String& colorString)
-{
-    Color color = parseColorOrCurrentColor(colorString, canvas());
-    if (!color.isValid())
-        return;
-    if (state().shadowColor == color)
-        return;
-    realizeSaves();
-    modifiableState().shadowColor = color;
+void CanvasRenderingContext2D::setShadowColor(const String& color)
+{
+    RGBA32 rgba;
+    if (!parseColorOrCurrentColor(rgba, color, canvas()))
+        return;
+    if (state().shadowColor == rgba)
+        return;
+    realizeSaves();
+    modifiableState().shadowColor = rgba;
     applyShadow();
 }
@@ -1251 +1249 @@
 }
 
-void CanvasRenderingContext2D::setShadow(float width, float height, float blur, const String& colorString, Optional<float> alpha)
-{
-    Color color = parseColorOrCurrentColor(colorString, canvas());
-    if (!colorString.isNull() && !color.isValid())
-        return;
-    if (!color.isValid())
-        color = Color::transparent;
-    // FIXME: Should not use RGBA32 here.
-    setShadow(FloatSize(width, height), blur, colorWithOverrideAlpha(color.rgb(), alpha));
+void CanvasRenderingContext2D::setShadow(float width, float height, float blur, const String& color, Optional<float> alpha)
+{
+    RGBA32 rgba = Color::transparent;
+    if (!color.isNull() && !parseColorOrCurrentColor(rgba, color, canvas()))
+        return;
+    setShadow(FloatSize(width, height), blur, colorWithOverrideAlpha(rgba, alpha));
 }
 

trunk/Source/WebCore/html/canvas/CanvasStyle.cpp

@@ -44 +44 @@
 namespace WebCore {
 
-static bool isCurrentColorString(const String& colorString)
-{
-    return equalLettersIgnoringASCIICase(colorString, "currentcolor");
-}
-
-static Color parseColor(const String& colorString, Document* document = nullptr)
-{
-    Color color = CSSParser::parseColor(colorString);
-    if (color.isValid())
-        return color;
-
-    return CSSParser::parseSystemColor(colorString, document);
-}
-
-Color currentColor(HTMLCanvasElement* canvas)
+enum ColorParseResult { ParsedRGBA, ParsedCurrentColor, ParsedSystemColor, ParseFailed };
+
+static ColorParseResult parseColor(RGBA32& parsedColor, const String& colorString, Document* document = nullptr)
+{
+    if (equalLettersIgnoringASCIICase(colorString, "currentcolor"))
+        return ParsedCurrentColor;
+    if (CSSParser::parseColor(parsedColor, colorString))
+        return ParsedRGBA;
+    if (CSSParser::parseSystemColor(parsedColor, colorString, document))
+        return ParsedSystemColor;
+    return ParseFailed;
+}
+
+RGBA32 currentColor(HTMLCanvasElement* canvas)
 {
     if (!canvas || !canvas->inDocument() || !canvas->inlineStyle())
         return Color::black;
-    Color color = CSSParser::parseColor(canvas->inlineStyle()->getPropertyValue(CSSPropertyColor));
-    if (!color.isValid())
-        return Color::black;
-    return color;
-}
-
-Color parseColorOrCurrentColor(const String& colorString, HTMLCanvasElement* canvas)
-{
-    if (isCurrentColorString(colorString))
-        return currentColor(canvas);
-
-    return parseColor(colorString, canvas ? &canvas->document() : nullptr);
+    RGBA32 rgba = Color::black;
+    CSSParser::parseColor(rgba, canvas->inlineStyle()->getPropertyValue(CSSPropertyColor));
+    return rgba;
+}
+
+bool parseColorOrCurrentColor(RGBA32& parsedColor, const String& colorString, HTMLCanvasElement* canvas)
+{
+    ColorParseResult parseResult = parseColor(parsedColor, colorString, canvas ? &canvas->document() : 0);
+    switch (parseResult) {
+    case ParsedRGBA:
+    case ParsedSystemColor:
+        return true;
+    case ParsedCurrentColor:
+        parsedColor = currentColor(canvas);
+        return true;
+    case ParseFailed:
+        return false;
+    default:
+        ASSERT_NOT_REACHED();
+        return false;
+    }
 }
 
@@ -126 +134 @@
 }
 
-CanvasStyle CanvasStyle::createFromString(const String& colorString, Document* document)
-{
-    if (isCurrentColorString(colorString))
+CanvasStyle CanvasStyle::createFromString(const String& color, Document* document)
+{
+    RGBA32 rgba;
+    ColorParseResult parseResult = parseColor(rgba, color, document);
+    switch (parseResult) {
+    case ParsedRGBA:
+    case ParsedSystemColor:
+        return CanvasStyle(rgba);
+    case ParsedCurrentColor:
         return CanvasStyle(ConstructCurrentColor);
-
-    Color color = parseColor(colorString, document);
-    if (color.isValid())
-        return CanvasStyle(color);
-
-    return CanvasStyle();
-}
-
-CanvasStyle CanvasStyle::createFromStringWithOverrideAlpha(const String& colorString, float alpha)
-{
-    if (isCurrentColorString(colorString))
+    case ParseFailed:
+        return CanvasStyle();
+    default:
+        ASSERT_NOT_REACHED();
+        return CanvasStyle();
+    }
+}
+
+CanvasStyle CanvasStyle::createFromStringWithOverrideAlpha(const String& color, float alpha)
+{
+    RGBA32 rgba;
+    ColorParseResult parseResult = parseColor(rgba, color);
+    switch (parseResult) {
+    case ParsedRGBA:
+        return CanvasStyle(colorWithOverrideAlpha(rgba, alpha));
+    case ParsedCurrentColor:
         return CanvasStyle(CurrentColorWithOverrideAlpha, alpha);
-
-    Color color = parseColor(colorString);
-    if (color.isValid())
-        return CanvasStyle(colorWithOverrideAlpha(color.rgb(), alpha));
-
-    return CanvasStyle();
+    case ParseFailed:
+        return CanvasStyle();
+    default:
+        ASSERT_NOT_REACHED();
+        return CanvasStyle();
+    }
 }
 

trunk/Source/WebCore/html/canvas/CanvasStyle.h

@@ -114 +114 @@
     };
 
-    Color currentColor(HTMLCanvasElement*);
-    Color parseColorOrCurrentColor(const String& colorString, HTMLCanvasElement*);
+    RGBA32 currentColor(HTMLCanvasElement*);
+    bool parseColorOrCurrentColor(RGBA32& parsedColor, const String& colorString, HTMLCanvasElement*);
 
     inline CanvasStyle::CanvasStyle()

trunk/Source/WebCore/platform/graphics/Color.h

@@ -142 +142 @@
     explicit Color(const char*);
 
-    explicit Color(WTF::HashTableDeletedValueType)
-    {
-        m_colorData.rgbaAndFlags = 0xfffffffffffffffd;
-        ASSERT(!isExtended());
-    }
-
-    bool isHashTableDeletedValue() const
-    {
-        return m_colorData.rgbaAndFlags == 0xfffffffffffffffd;
-    }
-
-    explicit Color(WTF::HashTableEmptyValueType)
-    {
-        m_colorData.rgbaAndFlags = 0xffffffffffffffb;
-        ASSERT(!isExtended());
-    }
-
     // FIXME: Add constructor for ExtendedColor type.
 
@@ -202 +185 @@
     void setRGB(int r, int g, int b) { setRGB(makeRGB(r, g, b)); }
     void setRGB(RGBA32);
-    uint64_t asUint64() const { return m_colorData.rgbaAndFlags; }
 
     WEBCORE_EXPORT void getRGBA(float& r, float& g, float& b, float& a) const;

trunk/Source/WebCore/svg/SVGColor.cpp

@@ -52 +52 @@
     // FIXME: Move this out of the SVGColor class?
     // FIXME: Is it really OK to do stripWhitespace here instead of stripLeadingAndTrailingHTMLSpaces?
-    return CSSParser::parseColor(colorString.stripWhiteSpace());
+    RGBA32 color;
+    if (CSSParser::parseColor(color, colorString.stripWhiteSpace()))
+        return color;
+    return Color();
 }