[webkit-reviews] review granted: [Bug 191319] Add experimental support for a `supported-color-schemes` CSS property : [Attachment 354256] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Timothy Hatcher <timothy at apple.com>'s
request for review:
Bug 191319: Add experimental support for a `supported-color-schemes` CSS
property
https://bugs.webkit.org/show_bug.cgi?id=191319

Attachment 354256: Patch

https://bugs.webkit.org/attachment.cgi?id=354256&action=review




--- Comment #18 from Dean Jackson <dino at apple.com> ---
Comment on attachment 354256
  --> https://bugs.webkit.org/attachment.cgi?id=354256
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=354256&action=review

> Source/WebCore/dom/Document.cpp:3598
> +	       supportedColorSchemes = { };

Is this needed since you already guarded for auto being the only member of the
list?

> Source/WebCore/inspector/InspectorOverlay.cpp:183
> +    GraphicsContextStateSaver stateSaver(context);

Why was this added?

> Source/WebCore/rendering/style/RenderStyleConstants.h:1027
> +static const size_t ColorSchemesBits = 2;

Shame we can't do a static_assert on these :(

> Source/WebCore/rendering/style/StyleSupportedColorSchemes.h:40
> +	   : m_colorSchemes(other.m_colorSchemes),
m_allowsTransformations(other.m_allowsTransformations) { }

Do we usually put all this on one line?

> Source/WebCore/rendering/style/StyleSupportedColorSchemes.h:43
> +	   : m_colorSchemes(colorSchemes),
m_allowsTransformations(allowsTransformations) { }

Ditto?