Bug 121018 – Move <style scoped> code behind ENABLE(STYLE_SCOPED)

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Andreas Kling]

Move <style scoped> code behind ENABLE(STYLE_SCOPED)

[Andreas Kling]

Created attachment 211006 [details]
Patch

[Andreas Kling]

Created attachment 211007 [details]
Patch

[Darin Adler]

Comment on attachment 211007 [details]
Patch

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

r=me as long as it doesn’t break the build

> Source/WebCore/css/StyleResolver.cpp:469
> +#if ENABLE(STYLE_SCOPED)
>      if (parent->hasScopedHTMLStyleChild())
>          return 0;
> +#endif

The way I’d prefer to do this is to have hasScopedHTMLStyleChild be defined but be an inline function that returns false.

> Source/WebCore/css/StyleResolver.cpp:662
> +#if ENABLE(STYLE_SCOPED)
>      if (element->hasScopedHTMLStyleChild())
>          return false;
> +#endif

Ditto.

> Source/WebCore/css/StyleResolver.cpp:749
> +#if ENABLE(STYLE_SCOPED)
>      if (state.styledElement()->hasScopedHTMLStyleChild())
>          return 0;
> +#endif

Ditto.

> Source/WebCore/dom/Node.h:591
> +#if ENABLE(STYLE_SCOPED)
>      virtual void registerScopedHTMLStyleChild();
>      virtual void unregisterScopedHTMLStyleChild();
>      size_t numberOfScopedHTMLStyleChildren() const;
> +#endif

Bizarre to have this all in Node!

> Source/WebCore/dom/Node.h:637
> +#if ENABLE(STYLE_SCOPED)
>          HasScopedHTMLStyleChildFlag = 1 << 22,
> +#endif

Not sure this is needed.

> Source/WebCore/dom/ShadowRoot.h:75
> +#if ENABLE(STYLE_SCOPED)
>      virtual void registerScopedHTMLStyleChild() OVERRIDE;
>      virtual void unregisterScopedHTMLStyleChild() OVERRIDE;
> +#endif

At least mark these final?

> Source/WebCore/html/HTMLStyleElement.cpp:82
> +#if ENABLE(STYLE_SCOPED)
>      else if (name == scopedAttr && ContextFeatures::styleScopedEnabled(&document()))
>          scopedAttributeChanged(!value.isNull());
> +#endif

Are we keeping this ContextFeatures thing at all? Who is using it in WebKit?

> Source/WebCore/html/HTMLStyleElement.cpp:187
> +bool HTMLStyleElement::scoped() const
> +{
> +    return fastHasAttribute(scopedAttr) && ContextFeatures::styleScopedEnabled(&document());
> +}
> +
> +void HTMLStyleElement::setScoped(bool scopedValue)
> +{
> +    setBooleanAttribute(scopedAttr, scopedValue);
> +}

These should be generated by the bindings rather than writing explicit C++ code.

> Source/WebCore/html/HTMLStyleElement.cpp:213
> +#if ENABLE(STYLE_SCOPED)
>          if (m_scopedStyleRegistrationState == NotRegistered && (scoped() || isInShadowTree()))
>              registerWithScopingNode(scoped());
> +#endif

Lame how this calls scoped() twice. That’s not necessarily a fast function.

[Build Bot]

Comment on attachment 211007 [details]
Patch

Attachment 211007 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/1728263

[Build Bot]

Comment on attachment 211007 [details]
Patch

Attachment 211007 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1735162

[Build Bot]

Comment on attachment 211007 [details]
Patch

Attachment 211007 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/1846072

[Andreas Kling]

Committed r155931: <http://trac.webkit.org/changeset/155931>