[webkit-reviews] review granted: [Bug 222159] Avoid tree traversals to look for form and canvas elements : [Attachment 421490] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Ryosuke Niwa
<rniwa at webkit.org>'s request for review:
Bug 222159: Avoid tree traversals to look for form and canvas elements
https://bugs.webkit.org/show_bug.cgi?id=222159

Attachment 421490: Patch

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




--- Comment #5 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 421490
  --> https://bugs.webkit.org/attachment.cgi?id=421490
Patch

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

> Source/WebCore/dom/Element.cpp:2404
> +    if (init.delegatesFocus)
> +	   setDelegatesFocusToShadowRoot();

Would be nice to split these changes into another patch.

> Source/WebCore/dom/Node.h:927
> +    if (states.contains(AncestorState::Canvas))

I'd prefer a blank line above this.

> Source/WebCore/html/HTMLFormElement.cpp:915
> +    if (UNLIKELY(is<HTMLFormElement>(startElement))) {
> +	   auto* parentElement = startElement.parentElement();
> +	   if (!parentElement ||
!parentElement->inclusiveAncestorStates().contains(AncestorState::Form)) {
> +	       ASSERT(!ancestorsOfType<HTMLFormElement>(startElement).first());
> +	       return nullptr;
> +	   }
> +    }

Why don't we add firstAncestorsOfType<> and use it everywhere we do
ancestorsOfType<>().first()?