[webkit-reviews] review denied: [Bug 128904] [GTK] ASSERTION FAILED: hasClass() : [Attachment 224360] Patch

[bugzilla-daemon at webkit.org]

Anders Carlsson <andersca at apple.com> has denied Piotr Grad
<p.grad at samsung.com>'s request for review:
Bug 128904: [GTK] ASSERTION FAILED: hasClass()
https://bugs.webkit.org/show_bug.cgi?id=128904

Attachment 224360: Patch
https://bugs.webkit.org/attachment.cgi?id=224360&action=review

------- Additional Comments from Anders Carlsson <andersca at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=224360&action=review


> Source/WebCore/platform/gtk/RenderThemeGtk.cpp:114
>  static bool nodeHasClass(Node* node, const char* className)

I think this should take a const Node& since it can never be null (You'd have
to change the call sites as well). The same thing is true for nodeHasPseudo but
that doesn't have to be in this patch.

> Source/WebCore/platform/gtk/RenderThemeGtk.cpp:116
> -    return node->isElementNode() ?
toElement(node)->classNames().contains(className) : false;
> +    return (node->isElementNode() && toElement(node)->hasClass()) ?
toElement(node)->classNames().contains(className) : false;

I think this would look better with early returns, something like:

if (!node.isElementNode())
    return false;

const Element& element = toElement(node);
if (!element.hasClass())
    return false;

return element.classNames().contains(className);