Changeset 207442 in webkit


Ignore:
Timestamp:
Oct 17, 2016 5:12:00 PM (8 years ago)
Author:
dino@apple.com
Message:

Allow creation of ExtendedColors and make Color immutable
​https://bugs.webkit.org/show_bug.cgi?id=163557
<rdar://problem/28805360>

Reviewed by Darin Adler and Dave Hyatt.

Source/WebCore:

  1. Support the creation of ExtendedColor objects via the

Color class.

  1. Fix the remaining few places where a Color object is

modified after creation, instead creating a new Color.
Move all the mutation methods into the private section,
making Color now immutable.

Changes to Color are covered by existing tests. Changes
to ExtendedColor are covered by the ExtendedColor API test.

  • css/parser/CSSParser.cpp:

(WebCore::CSSParser::fastParseColor): Return a new named Color.

  • dom/Document.cpp:

(WebCore::Document::resetActiveLinkColor): Set to be the named "red" color.

  • html/HTMLElement.cpp:

(WebCore::HTMLElement::addHTMLColorToStyle): Use the string-based constructor
where possible.

  • page/CaptionUserPreferencesMediaAF.cpp:

(WebCore::CaptionUserPreferencesMediaAF::captionsTextEdgeCSS): No need to
use the string "black" here - we have a constant value.

  • platform/graphics/cairo/GraphicsContextCairo.cpp: Don't use setRGB.
  • platform/graphics/Color.cpp:

(WebCore::findNamedColor): Move this up in the file.
(WebCore::Color::Color): Copy in the code from setNamedColor. Also
add a constructor for ExtendedColor.
(WebCore::Color::serialized): Call ExtendedColor's serializer if necessary.
(WebCore::Color::cssText): Ditto.
(WebCore::Color::setNamedColor): Deleted.
(WebCore::Color::tagAsExtended): Deleted.

  • platform/graphics/Color.h: Add a new constructor.

(WebCore::Color::setRGB): Move to private.

  • platform/graphics/ExtendedColor.cpp:

(WebCore::ExtendedColor::cssText): Implement serializer.

  • platform/graphics/ExtendedColor.h:
  • rendering/style/RenderStyle.cpp:

(WebCore::RenderStyle::colorIncludingFallback): Construct a new Color rather than changing an existing object.

Tools:

API tests for ExtendedColor.

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/WebCore/ExtendedColor.cpp: Added.

(TestWebKitAPI::TEST):
(TestWebKitAPI::makeColor):

Location:
trunk
Files:
1 added
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r207438 r207442  
     12016-10-17  Dean Jackson  <dino@apple.com>
     2
     3        Allow creation of ExtendedColors and make Color immutable
     4        https://bugs.webkit.org/show_bug.cgi?id=163557
     5        <rdar://problem/28805360>
     6
     7        Reviewed by Darin Adler and Dave Hyatt.
     8
     9        1. Support the creation of ExtendedColor objects via the
     10        Color class.
     11
     12        2. Fix the remaining few places where a Color object is
     13        modified after creation, instead creating a new Color.
     14        Move all the mutation methods into the private section,
     15        making Color now immutable.
     16
     17        Changes to Color are covered by existing tests. Changes
     18        to ExtendedColor are covered by the ExtendedColor API test.
     19
     20        * css/parser/CSSParser.cpp:
     21        (WebCore::CSSParser::fastParseColor): Return a new named Color.
     22
     23        * dom/Document.cpp:
     24        (WebCore::Document::resetActiveLinkColor): Set to be the named "red" color.
     25
     26        * html/HTMLElement.cpp:
     27        (WebCore::HTMLElement::addHTMLColorToStyle): Use the string-based constructor
     28        where possible.
     29
     30        * page/CaptionUserPreferencesMediaAF.cpp:
     31        (WebCore::CaptionUserPreferencesMediaAF::captionsTextEdgeCSS): No need to
     32        use the string "black" here - we have a constant value.
     33
     34        * platform/graphics/cairo/GraphicsContextCairo.cpp: Don't use setRGB.
     35
     36        * platform/graphics/Color.cpp:
     37        (WebCore::findNamedColor): Move this up in the file.
     38        (WebCore::Color::Color): Copy in the code from setNamedColor. Also
     39        add a constructor for ExtendedColor.
     40        (WebCore::Color::serialized): Call ExtendedColor's serializer if necessary.
     41        (WebCore::Color::cssText): Ditto.
     42        (WebCore::Color::setNamedColor): Deleted.
     43        (WebCore::Color::tagAsExtended): Deleted.
     44
     45        * platform/graphics/Color.h: Add a new constructor.
     46        (WebCore::Color::setRGB): Move to private.
     47
     48        * platform/graphics/ExtendedColor.cpp:
     49        (WebCore::ExtendedColor::cssText): Implement serializer.
     50        * platform/graphics/ExtendedColor.h:
     51
     52        * rendering/style/RenderStyle.cpp:
     53        (WebCore::RenderStyle::colorIncludingFallback): Construct a new Color rather than changing an existing object.
     54
    1552016-10-17  Simon Fraser  <simon.fraser@apple.com>
    256

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

    r207396 r207442  
    75517551
    75527552    // Try named colors.
    7553     color.setNamedColor(name);
    7554     return color;
     7553    return Color { name };
    75557554}
    75567555   

  • trunk/Source/WebCore/dom/Document.cpp

    r207372 r207442  
    821821void Document::resetActiveLinkColor()
    822822{
    823     m_activeLinkColor.setNamedColor("red");
     823    m_activeLinkColor = Color(255, 0, 0);
    824824}
    825825

  • trunk/Source/WebCore/html/HTMLElement.cpp

    r206944 r207442  
    10381038        return;
    10391039
    1040     // If the string is a named CSS color or a 3/6-digit hex color, use that.
    1041     // We can't use the default Color constructor because it accepts
     1040    Color color;
     1041    // We can't always use the default Color constructor because it accepts
    10421042    // 4/8-digit hex, which conflict with some legacy HTML content using attributes.
    1043 
    1044     Color color;
    1045 
    1046     if ((colorString.length() == 4 || colorString.length() == 7) && colorString[0] == '#')
     1043    if ((colorString.length() != 5 && colorString.length() != 9) || colorString[0] != '#')
    10471044        color = Color(colorString);
    10481045    if (!color.isValid())
    1049         color.setNamedColor(colorString);
    1050     if (!color.isValid())
    1051         color.setRGB(parseColorStringWithCrazyLegacyRules(colorString));
     1046        color = Color(parseColorStringWithCrazyLegacyRules(colorString));
    10521047
    10531048    style.setProperty(propertyID, CSSValuePool::singleton().createColorValue(color.rgb()));

  • trunk/Source/WebCore/page/CaptionUserPreferencesMediaAF.cpp

    r206830 r207442  
    397397    Color color = captionsTextColor(unused);
    398398    if (!color.isValid())
    399         color.setNamedColor("black");
     399        color = Color { Color::black };
    400400    color = captionsEdgeColorForTextColor(color);
    401401

  • trunk/Source/WebCore/platform/graphics/Color.cpp

    r207265 r207442  
    223223}
    224224
     225static inline const NamedColor* findNamedColor(const String& name)
     226{
     227    char buffer[64]; // easily big enough for the longest color name
     228    unsigned length = name.length();
     229    if (length > sizeof(buffer) - 1)
     230        return nullptr;
     231    for (unsigned i = 0; i < length; ++i) {
     232        UChar c = name[i];
     233        if (!c || !WTF::isASCII(c))
     234            return nullptr;
     235        buffer[i] = toASCIILower(static_cast<char>(c));
     236    }
     237    buffer[length] = '\0';
     238    return findColor(buffer, length);
     239}
     240
    225241Color::Color(const String& name)
    226242{
    … …  
    236252        if (valid)
    237253            setRGB(color);
    238     } else
    239         setNamedColor(name);
     254    } else {
     255        if (auto* foundColor = findNamedColor(name))
     256            setRGB(foundColor->ARGBValue);
     257        else
     258            m_colorData.rgbaAndFlags = invalidRGBAColor;
     259    }
    240260}
    241261
    … …  
    268288}
    269289
     290Color::Color(float r, float g, float b, float a, ColorSpace colorSpace)
     291{
     292    auto extendedColorRef = ExtendedColor::create(r, g, b, a, colorSpace);
     293    m_colorData.extendedColor = &extendedColorRef.leakRef();
     294    ASSERT(isExtended());
     295}
     296
    270297Color::~Color()
    271298{
    … …  
    302329String Color::serialized() const
    303330{
     331    if (isExtended())
     332        return asExtended().cssText();
     333
    304334    if (!hasAlpha()) {
    305335        StringBuilder builder;
    … …  
    317347String Color::cssText() const
    318348{
     349    if (isExtended())
     350        return asExtended().cssText();
     351
    319352    StringBuilder builder;
    320353    builder.reserveCapacity(28);
    … …  
    351384        return String::format("#%02X%02X%02X%02X", red(), green(), blue(), alpha());
    352385    return String::format("#%02X%02X%02X", red(), green(), blue());
    353 }
    354 
    355 static inline const NamedColor* findNamedColor(const String& name)
    356 {
    357     char buffer[64]; // easily big enough for the longest color name
    358     unsigned length = name.length();
    359     if (length > sizeof(buffer) - 1)
    360         return 0;
    361     for (unsigned i = 0; i < length; ++i) {
    362         UChar c = name[i];
    363         if (!c || c > 0x7F)
    364             return 0;
    365         buffer[i] = toASCIILower(static_cast<char>(c));
    366     }
    367     buffer[length] = '\0';
    368     return findColor(buffer, length);
    369 }
    370 
    371 void Color::setNamedColor(const String& name)
    372 {
    373     const NamedColor* foundColor = findNamedColor(name);
    374     if (foundColor)
    375         setRGB(foundColor->ARGBValue);
    376     else
    377         m_colorData.rgbaAndFlags = invalidRGBAColor;
    378386}
    379387
    … …  
    625633}
    626634
    627 void Color::tagAsExtended()
    628 {
    629     // FIXME: Is this method necessary? Will colors ever change from RGBA32 to Extended?
    630     // Valid colors should not change type.
    631     ASSERT(!isValid());
    632     m_colorData.rgbaAndFlags &= ~(invalidRGBAColor);
    633 }
    634 
    635635bool Color::isExtended() const
    636636{
    … …  
    638638}
    639639
    640 ExtendedColor* Color::asExtended() const
     640ExtendedColor& Color::asExtended() const
    641641{
    642642    ASSERT(isExtended());
    643     if (!isExtended())
    644         return nullptr;
    645     return m_colorData.extendedColor;
     643    return *m_colorData.extendedColor;
    646644}
    647645

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

    r207361 r207442  
    159159    }
    160160
    161     // FIXME: Add constructor for ExtendedColor type.
     161    // This creates an ExtendedColor.
     162    // FIXME: If the colorSpace is sRGB and the values can all be
     163    // converted exactly to integers, we should make a normal Color.
     164    WEBCORE_EXPORT Color(float r, float g, float b, float a, ColorSpace colorSpace);
    162165
    163166    Color(RGBA, ColorSpace);
    … …  
    182185    WEBCORE_EXPORT String serialized() const;
    183186
    184     String cssText() const;
     187    WEBCORE_EXPORT String cssText() const;
    185188
    186189    // Returns the color serialized as either #RRGGBB or #RRGGBBAA
    187190    // The latter format is not a valid CSS color, and should only be seen in DRT dumps.
    188191    String nameForRenderTreeAsText() const;
    189 
    190     void setNamedColor(const String&);
    191192
    192193    bool isValid() const { return m_colorData.rgbaAndFlags & validRGBAColorBit; }
    … …  
    200201   
    201202    RGBA32 rgb() const { ASSERT(!isExtended()); return static_cast<RGBA32>(m_colorData.rgbaAndFlags >> 32); }
    202     void setRGB(int r, int g, int b) { setRGB(makeRGB(r, g, b)); }
    203     void setRGB(RGBA32);
    204203    uint64_t asUint64() const { return m_colorData.rgbaAndFlags; }
    205204
    … …  
    257256
    258257    WEBCORE_EXPORT bool isExtended() const;
    259     WEBCORE_EXPORT ExtendedColor* asExtended() const;
     258    WEBCORE_EXPORT ExtendedColor& asExtended() const;
    260259
    261260    WEBCORE_EXPORT Color& operator=(const Color&);
    … …  
    265264
    266265private:
     266    void setRGB(int r, int g, int b) { setRGB(makeRGB(r, g, b)); }
     267    void setRGB(RGBA32);
    267268
    268269    // 0x_______00 is an ExtendedColor pointer.
    … …  
    275276
    276277    WEBCORE_EXPORT void tagAsValid();
    277     void tagAsExtended();
    278278
    279279    union {

  • trunk/Source/WebCore/platform/graphics/ExtendedColor.cpp

    r207265 r207442  
    2828
    2929#include "ColorSpace.h"
     30#include <wtf/MathExtras.h>
     31#include <wtf/dtoa.h>
     32#include <wtf/text/StringBuilder.h>
    3033
    3134namespace WebCore {
    … …  
    3639}
    3740
     41String ExtendedColor::cssText() const
     42{
     43    StringBuilder builder;
     44    builder.reserveCapacity(40);
     45    builder.appendLiteral("color(");
     46
     47    switch (m_colorSpace) {
     48    case ColorSpaceSRGB:
     49        builder.appendLiteral("srgb ");
     50        break;
     51    case ColorSpaceDisplayP3:
     52        builder.appendLiteral("display-p3 ");
     53        break;
     54    default:
     55        ASSERT_NOT_REACHED();
     56        return WTF::emptyString();
     57    }
     58
     59    NumberToStringBuffer buffer;
     60    bool shouldTruncateTrailingZeros = true;
     61
     62    builder.append(numberToFixedPrecisionString(red(), 6, buffer, shouldTruncateTrailingZeros));
     63    builder.append(' ');
     64
     65    builder.append(numberToFixedPrecisionString(green(), 6, buffer, shouldTruncateTrailingZeros));
     66    builder.append(' ');
     67
     68    builder.append(numberToFixedPrecisionString(blue(), 6, buffer, shouldTruncateTrailingZeros));
     69    builder.appendLiteral(" / ");
     70
     71    builder.append(numberToFixedPrecisionString(alpha(), 6, buffer, shouldTruncateTrailingZeros));
     72    builder.append(')');
     73
     74    return builder.toString();
    3875}
     76
     77}

  • trunk/Source/WebCore/platform/graphics/ExtendedColor.h

    r207265 r207442  
    3030#include <wtf/Ref.h>
    3131#include <wtf/RefCounted.h>
     32#include <wtf/text/WTFString.h>
    3233
    3334namespace WebCore {
    … …  
    4344
    4445    ColorSpace colorSpace() const { return m_colorSpace; }
     46
     47    WEBCORE_EXPORT String cssText() const;
    4548
    4649private:

  • trunk/Source/WebCore/platform/graphics/cairo/GraphicsContextCairo.cpp

    r207002 r207442  
    541541#if !PLATFORM(GTK)
    542542    // Force the alpha to 50%.  This matches what the Mac does with outline rings.
    543     color.setRGB(makeRGBA(color.red(), color.green(), color.blue(), 127));
     543    color = Color(makeRGBA(color.red(), color.green(), color.blue(), 127));
    544544#else
    545545    UNUSED_PARAM(color);

  • trunk/Source/WebCore/platform/mock/MockRealtimeVideoSource.cpp

    r205348 r207442  
    200200    static const RGBA32 red = 0xffff0000;
    201201    static const RGBA32 green = 0xff008000;
     202    static const RGBA32 cyan = 0xFF00FFFF;
    202203
    203204    IntSize size = this->size();
    … …  
    249250    boxTop += boxSize + 2;
    250251    boxLeft = boxSize;
    251     Color boxColors[] = { Color::white, yellow, Color::cyan, green, magenta, red, blue };
     252    Color boxColors[] = { Color::white, yellow, cyan, green, magenta, red, blue };
    252253    for (unsigned i = 0; i < sizeof(boxColors) / sizeof(boxColors[0]); i++) {
    253254        context.fillRect(FloatRect(boxLeft, boxTop, boxSize + 1, boxSize + 1), boxColors[i]);

  • trunk/Source/WebCore/rendering/style/RenderStyle.cpp

    r206839 r207442  
    16841684    if (!result.isValid()) {
    16851685        if (!visitedLink && (borderStyle == INSET || borderStyle == OUTSET || borderStyle == RIDGE || borderStyle == GROOVE))
    1686             result.setRGB(238, 238, 238);
     1686            result = Color(238, 238, 238);
    16871687        else
    16881688            result = visitedLink ? visitedLinkColor() : color();

  • trunk/Tools/ChangeLog

    r207432 r207442  
     12016-10-17  Dean Jackson  <dino@apple.com>
     2
     3        Allow creation of ExtendedColors and make Color immutable
     4        https://bugs.webkit.org/show_bug.cgi?id=163557
     5        <rdar://problem/28805360>
     6
     7        Reviewed by Darin Adler and Dave Hyatt.
     8
     9        API tests for ExtendedColor.
     10
     11        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     12        * TestWebKitAPI/Tests/WebCore/ExtendedColor.cpp: Added.
     13        (TestWebKitAPI::TEST):
     14        (TestWebKitAPI::makeColor):
     15
    1162016-10-17  JF Bastien  <jfbastien@apple.com>
    217

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r207385 r207442  
    8686                2EFF06D41D8AEDBB0004BB30 /* TestWKWebViewMac.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2EFF06D31D8AEDBB0004BB30 /* TestWKWebViewMac.mm */; };
    8787                2EFF06D71D8AF34A0004BB30 /* WKWebViewCandidateTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2EFF06D61D8AF34A0004BB30 /* WKWebViewCandidateTests.mm */; };
     88                315118101DB1AE4000176304 /* ExtendedColor.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 3151180F1DB1ADD500176304 /* ExtendedColor.cpp */; };
    8889                33BE5AF9137B5AAE00705813 /* MouseMoveAfterCrash_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 33BE5AF8137B5AAE00705813 /* MouseMoveAfterCrash_Bundle.cpp */; };
    8990                33DC8912141955FE00747EF7 /* simple-iframe.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 33DC890E1419539300747EF7 /* simple-iframe.html */; };
    … …  
    823824                2EFF06D31D8AEDBB0004BB30 /* TestWKWebViewMac.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = TestWKWebViewMac.mm; sourceTree = "<group>"; };
    824825                2EFF06D61D8AF34A0004BB30 /* WKWebViewCandidateTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebViewCandidateTests.mm; sourceTree = "<group>"; };
     826                3151180F1DB1ADD500176304 /* ExtendedColor.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ExtendedColor.cpp; sourceTree = "<group>"; };
    825827                333B9CE11277F23100FEFCE3 /* PreventEmptyUserAgent.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PreventEmptyUserAgent.cpp; sourceTree = "<group>"; };
    826828                33BE5AF4137B5A6C00705813 /* MouseMoveAfterCrash.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = MouseMoveAfterCrash.cpp; sourceTree = "<group>"; };
    … …  
    14571459                                260BA57A1B1D2EE2004FA07C /* DFAHelpers.h */,
    14581460                                26F6E1EF1ADC749B00DE696B /* DFAMinimizer.cpp */,
     1461                                3151180F1DB1ADD500176304 /* ExtendedColor.cpp */,
    14591462                                41973B5A1AF2286A006C7B36 /* FileSystem.cpp */,
    14601463                                83B88A331C80056D00BB2418 /* HTMLParserIdioms.cpp */,
    … …  
    24322435                                7CCE7EC61A411A7E00447C4C /* MemoryCachePruneWithinResourceLoadDelegate.mm in Sources */,
    24332436                                7C83E0B71D0A64B800FEBCF3 /* MenuTypesForMouseEvents.cpp in Sources */,
     2437                                315118101DB1AE4000176304 /* ExtendedColor.cpp in Sources */,
    24342438                                51CD1C6C1B38CE4300142CA5 /* ModalAlerts.mm in Sources */,
    24352439                                7C83E0B61D0A64B300FEBCF3 /* ModalAlertsSPI.cpp in Sources */,
Note: See TracChangeset for help on using the changeset viewer.