Bug 221357 – Avoid an ancestor walk in HTMLFormControlElement::computeWillValidate()

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Simon Fraser (smfr)]

Avoid an ancestor walk in HTMLFormControlElement::computeWillValidate()

[Simon Fraser (smfr)]

Created attachment 419180 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 419180 [details]
Patch

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

This is fine but another approach is to create a new node flag like we do for SelfOrAncestorHasDirAuto and set this bit whenever an ancestor has datalist.

> Source/WebCore/dom/Document.h:1331
> +    void addDisabledFieldsetElement() { ++m_disabledFieldsetElementsCount; }

Can we avoid changing the unrelated code?

> Source/WebCore/dom/Document.h:1334
> +    bool hasDataListElements() const { return m_dataListElementCount; }

We should have this in TreeScope instead.
Otherwise, we're gonna have a bad time inside a ShadowRoot.

> Source/WebCore/dom/Element.cpp:2220
> +    auto* page = document().page();
> +    if (page) {

Please define page inside if.

> Source/WebCore/html/HTMLDataListElement.cpp:61
> +    if (parentOfInsertedTree.isInTreeScope())

This isn't right. We're checking that the inserted ancestor's new parent is in a tree scope.
It doesn't mean it's in the document.

Make the change above to move the function to TreeScope and this will be correct
but it's better to check insertionType.treeScopeChanged instead even in that case.

> Source/WebCore/html/HTMLDataListElement.cpp:70
> +    if (oldParentOfRemovedTree.isInTreeScope())
> +        document().decrementDataListElementCount();

Ditto.

> Source/WebCore/html/HTMLDataListElement.cpp:76
> +void HTMLDataListElement::didMoveToNewDocument(Document& oldDocument, Document& newDocument)
> +{

This function override won't be needed once we made all the changes I suggest above.

[Simon Fraser (smfr)]

Created attachment 419212 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 419212 [details]
Patch

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

> Source/WebCore/ChangeLog:13
> +        * dom/Document.h:
> +        (WebCore::Document::addDisabledFieldsetElement):

Update these change log entries?

> Source/WebCore/html/HTMLDataListElement.cpp:64
> +    if (insertionType.treeScopeChanged)
> +        treeScope().incrementDataListElementCount();

We can do this before calling HTMLElement::insertedIntoAncestor.

> Source/WebCore/page/PointerLockController.cpp:106
> -void PointerLockController::elementRemoved(Element& element)
> +void PointerLockController::elementWasRemoved(Element& element)

Or maybe didRemoveElement?

[Simon Fraser (smfr)]

Created attachment 419225 [details]
Patch

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/272358/webkit

[Radar WebKit Bug Importer]

<rdar://problem/73964453>