[webkit-reviews] review granted: [Bug 197725] Consider making CSSStyleSheet::rules() just an alias of CSSStyleSheet::cssRules(). : [Attachment 425139] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Tyler Wilcock
<twilco.o at protonmail.com>'s request for review:
Bug 197725: Consider making CSSStyleSheet::rules() just an alias of
CSSStyleSheet::cssRules().
https://bugs.webkit.org/show_bug.cgi?id=197725

Attachment 425139: Patch

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




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 425139
  --> https://bugs.webkit.org/attachment.cgi?id=425139
Patch

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

> Source/WebCore/css/CSSStyleSheet.cpp:268
>  RefPtr<CSSRuleList> CSSStyleSheet::rules()
>  {
> -    if (!canAccessRules())
> -	   return nullptr;
> -    // IE behavior.
> -    auto ruleList = StaticCSSRuleList::create();
> -    unsigned ruleCount = length();
> -    for (unsigned i = 0; i < ruleCount; ++i)
> -	   ruleList->rules().append(item(i));
> -    return ruleList;
> +    return this->cssRules();
>  }

Four thoughts on this:

1) We should merge CSSStyleSheet::cssRulesForBindings with
CSSStyleSheet::rulesForBindings, not just this lower layer.
2) We should use inlining, putting the forwarding in the header, if possible,
and pay no runtime cost at all. So the function body of
CSSStyleSheet::rulesForBindings should be in the header, not in the .cpp file.
3) We should delete CSSStyleSheet::rules entirely.
4) We can consider naming CSSStyleSheet::rulesForBindings to
CSSStyleSheet::rules.