Reflection for FrozenArray<Element> caching invariant

mrego · mrego · commit 04d104d7176e · 2022-06-02T21:45:04.000Z
https://bugs.webkit.org/show_bug.cgi?id=240563 Reviewed by Ryosuke Niwa. This patch implements the caching layer described in the spec PR for reflection of FrozenArray<T> attributes: whatwg/html#3917 Which fixes the test cases that were checking for: el.ariaDescribedByElements === el.ariaDescribedByElements This patch stores a new JSObject in the JSElement using a PrivateName. Then for each attribute we store the cached JSValue in the JSObject. If the cached JSValue matches the current Vector of Elements that we're going to return, we return the cached JSValue instead. * LayoutTests/imported/w3c/web-platform-tests/dom/nodes/aria-element-reflection.tentative-expected.txt: Update expectations. * LayoutTests/imported/w3c/web-platform-tests/dom/nodes/aria-element-reflection.tentative.html: Add new test cases. * Source/WebCore/accessibility/AriaAttributes.idl: Add CustomGetter for FrozenArray<Element> reflection. * Source/WebCore/bindings/js/JSElementCustom.cpp: (WebCore::getElementsArrayAttribute): New method that implements the caching invariant. (WebCore::JSElement::ariaControlsElements const): Custom getter that calls getElementsArrayAttribute(). (WebCore::JSElement::ariaDescribedByElements const): Ditto. (WebCore::JSElement::ariaDetailsElements const): Ditto. (WebCore::JSElement::ariaFlowToElements const): Ditto. (WebCore::JSElement::ariaLabelledByElements const): Ditto. (WebCore::JSElement::ariaOwnsElements const): Ditto. * Source/WebCore/bindings/js/WebCoreBuiltinNames.h: New built-in PrivateName. Canonical link: https://commits.webkit.org/251237@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295148 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/imported/w3c/web-platform-tests/dom/nodes/aria-element-reflection.tentative-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/dom/nodes/aria-element-reflection.tentative-expected.txt
@@ -50,7 +50,7 @@ PASS Changing the ID of an element causes the content attribute to become out of
 PASS Reparenting an element into a descendant shadow scope hides the element reference.
 PASS Reparenting referenced element cannot cause retargeting of reference.
 PASS Element reference set in invalid scope remains intact throughout move to valid scope.
-FAIL aria-labelledby. assert_equals: check idl attribute caching after parsing expected [Element node <div id="billingElement">Billing</div>, Element node <div id="nameElement">Name</div>] but got [Element node <div id="billingElement">Billing</div>, Element node <div id="nameElement">Name</div>]
+PASS aria-labelledby.
 PASS aria-controls.
 PASS aria-describedby.
 PASS aria-flowto.
@@ -62,4 +62,6 @@ PASS Reparenting.
 PASS Attaching element reference before it's inserted into the DOM.
 PASS Cross-document references and moves.
 PASS Adopting element keeps references.
+PASS Caching invariant different attributes.
+PASS Caching invariant different elements.
 
diff --git a/LayoutTests/imported/w3c/web-platform-tests/dom/nodes/aria-element-reflection.tentative.html b/LayoutTests/imported/w3c/web-platform-tests/dom/nodes/aria-element-reflection.tentative.html
@@ -730,6 +730,54 @@ <h2 id="lightDomHeading" aria-flowto="shadowChild1 shadowChild2">Light DOM Headi
     }, "Adopting element keeps references.");
   </script>
 
+  <div id="cachingInvariantMain"></div>
+  <div id="cachingInvariantElement1"></div>
+  <div id="cachingInvariantElement2"></div>
+  <div id="cachingInvariantElement3"></div>
+  <div id="cachingInvariantElement4"></div>
+  <div id="cachingInvariantElement5"></div>
+
+  <script>
+    test(function(t) {
+      cachingInvariantMain.ariaControlsElements = [cachingInvariantElement1, cachingInvariantElement2];
+      cachingInvariantMain.ariaDescribedByElements = [cachingInvariantElement3, cachingInvariantElement4];
+      cachingInvariantMain.ariaDetailsElements = [cachingInvariantElement5];
+      cachingInvariantMain.ariaFlowToElements = [cachingInvariantElement1, cachingInvariantElement3];
+      cachingInvariantMain.ariaLabelledByElements = [cachingInvariantElement2, cachingInvariantElement4];
+      cachingInvariantMain.ariaOwnsElements = [cachingInvariantElement1, cachingInvariantElement2, cachingInvariantElement3];
+
+      let ariaControlsElementsArray = cachingInvariantMain.ariaControlsElements;
+      let ariaDescribedByElementsArray = cachingInvariantMain.ariaDescribedByElements;
+      let ariaDetailsElementsArray = cachingInvariantMain.ariaDetailsElements;
+      let ariaFlowToElementsArray = cachingInvariantMain.ariaFlowToElements;
+      let ariaLabelledByElementsArray = cachingInvariantMain.ariaLabelledByElements;
+      let ariaOwnsElementsArray = cachingInvariantMain.ariaOwnsElements;
+
+      assert_equals(ariaControlsElementsArray, cachingInvariantMain.ariaControlsElements, "Caching invariant for ariaControlsElements");
+      assert_equals(ariaDescribedByElementsArray, cachingInvariantMain.ariaDescribedByElements, "Caching invariant for ariaDescribedByElements");
+      assert_equals(ariaDetailsElementsArray, cachingInvariantMain.ariaDetailsElements, "Caching invariant for ariaDetailsElements");
+      assert_equals(ariaFlowToElementsArray, cachingInvariantMain.ariaFlowToElements, "Caching invariant for ariaFlowToElements");
+      assert_equals(ariaLabelledByElementsArray, cachingInvariantMain.ariaLabelledByElements, "Caching invariant for ariaLabelledByElements");
+      assert_equals(ariaOwnsElementsArray, cachingInvariantMain.ariaOwnsElements, "Caching invariant for ariaOwnsElements");
+    }, "Caching invariant different attributes.");
+  </script>
+
+  <div id="cachingInvariantMain1"></div>
+  <div id="cachingInvariantMain2"></div>
+
+  <script>
+    test(function(t) {
+      cachingInvariantMain1.ariaDescribedByElements = [cachingInvariantElement1, cachingInvariantElement2];
+      cachingInvariantMain2.ariaDescribedByElements = [cachingInvariantElement3, cachingInvariantElement4];
+
+      let ariaDescribedByElementsArray1 = cachingInvariantMain1.ariaDescribedByElements;
+      let ariaDescribedByElementsArray2 = cachingInvariantMain2.ariaDescribedByElements;
+
+      assert_equals(ariaDescribedByElementsArray1, cachingInvariantMain1.ariaDescribedByElements, "Caching invariant for ariaDescribedByElements in one elemnt");
+      assert_equals(ariaDescribedByElementsArray2, cachingInvariantMain2.ariaDescribedByElements, "Caching invariant for ariaDescribedByElements in onother elemnt");
+    }, "Caching invariant different elements.");
+  </script>
+
   <!-- TODO(chrishall): add additional GC test covering:
        if an element is in an invalid scope but attached to the document, it's
        not GC'd;
diff --git a/Source/WebCore/accessibility/AriaAttributes.idl b/Source/WebCore/accessibility/AriaAttributes.idl
@@ -33,27 +33,27 @@ interface mixin AriaAttributes {
     [Reflect=aria_colcount]         attribute DOMString? ariaColCount;
     [Reflect=aria_colindex]         attribute DOMString? ariaColIndex;
     [Reflect=aria_colspan]          attribute DOMString? ariaColSpan;
-    [Reflect=aria_controls, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaControlsElements;
+    [CustomGetter, Reflect=aria_controls, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaControlsElements;
     [Reflect=aria_current]          attribute DOMString? ariaCurrent;
-    [Reflect=aria_describedby, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaDescribedByElements;
-    [Reflect=aria_details, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaDetailsElements;
+    [CustomGetter, Reflect=aria_describedby, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaDescribedByElements;
+    [CustomGetter, Reflect=aria_details, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaDetailsElements;
     [Reflect=aria_disabled]         attribute DOMString? ariaDisabled;
     [Reflect=aria_errormessage, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute Element? ariaErrorMessageElement;
     [Reflect=aria_expanded]         attribute DOMString? ariaExpanded;
-    [Reflect=aria_flowto, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaFlowToElements;
+    [CustomGetter, Reflect=aria_flowto, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaFlowToElements;
     [Reflect=aria_haspopup]         attribute DOMString? ariaHasPopup;
     [Reflect=aria_hidden]           attribute DOMString? ariaHidden;
     [Reflect=aria_invalid]          attribute DOMString? ariaInvalid;
     [Reflect=aria_keyshortcuts]     attribute DOMString? ariaKeyShortcuts;
     [Reflect=aria_label]            attribute DOMString? ariaLabel;
-    [Reflect=aria_labelledby, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaLabelledByElements;
+    [CustomGetter, Reflect=aria_labelledby, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaLabelledByElements;
     [Reflect=aria_level]            attribute DOMString? ariaLevel;
     [Reflect=aria_live]             attribute DOMString? ariaLive;
     [Reflect=aria_modal]            attribute DOMString? ariaModal;
     [Reflect=aria_multiline]        attribute DOMString? ariaMultiLine;
     [Reflect=aria_multiselectable]  attribute DOMString? ariaMultiSelectable;
     [Reflect=aria_orientation]      attribute DOMString? ariaOrientation;
-    [Reflect=aria_owns, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaOwnsElements;
+    [CustomGetter, Reflect=aria_owns, EnabledBySetting=AriaReflectionForElementReferencesEnabled] attribute FrozenArray<Element>? ariaOwnsElements;
     [Reflect=aria_placeholder]      attribute DOMString? ariaPlaceholder;
     [Reflect=aria_posinset]         attribute DOMString? ariaPosInSet;
     [Reflect=aria_pressed]          attribute DOMString? ariaPressed;
diff --git a/Source/WebCore/bindings/js/JSElementCustom.cpp b/Source/WebCore/bindings/js/JSElementCustom.cpp
@@ -35,13 +35,17 @@
 #include "HTMLNames.h"
 #include "JSAttr.h"
 #include "JSDOMBinding.h"
+#include "JSDOMConvertInterface.h"
+#include "JSDOMConvertNullable.h"
+#include "JSDOMConvertSequences.h"
 #include "JSHTMLElementWrapperFactory.h"
 #include "JSMathMLElementWrapperFactory.h"
 #include "JSNodeList.h"
 #include "JSSVGElementWrapperFactory.h"
 #include "MathMLElement.h"
 #include "NodeList.h"
 #include "SVGElement.h"
+#include "WebCoreJSClientData.h"
 
 
 namespace WebCore {
@@ -81,4 +85,62 @@ JSValue toJSNewlyCreated(JSGlobalObject*, JSDOMGlobalObject* globalObject, Ref<E
     return createNewElementWrapper(globalObject, WTFMove(element));
 }
 
+static JSValue getElementsArrayAttribute(JSGlobalObject& lexicalGlobalObject, const JSElement& thisObject, const QualifiedName& attributeName)
+{
+    auto& vm = JSC::getVM(&lexicalGlobalObject);
+    auto throwScope = DECLARE_THROW_SCOPE(vm);
+
+    JSObject* cachedObject = nullptr;
+    JSValue cachedObjectValue = thisObject.getDirect(vm, builtinNames(vm).cachedAttrAssociatedElementsPrivateName());
+    if (cachedObjectValue)
+        cachedObject = asObject(cachedObjectValue);
+    else {
+        cachedObject = constructEmptyObject(vm, thisObject.globalObject()->nullPrototypeObjectStructure());
+        const_cast<JSElement&>(thisObject).putDirect(vm, builtinNames(vm).cachedAttrAssociatedElementsPrivateName(), cachedObject);
+    }
+
+    std::optional<Vector<RefPtr<Element>>> elements = thisObject.wrapped().getElementsArrayAttribute(attributeName);
+    auto propertyName = PropertyName(Identifier::fromString(vm, attributeName.toString()));
+    JSValue cachedValue = cachedObject->getDirect(vm, propertyName);
+    if (!cachedValue.isEmpty()) {
+        std::optional<Vector<RefPtr<Element>>> cachedElements = convert<IDLNullable<IDLFrozenArray<IDLInterface<Element>>>>(lexicalGlobalObject, cachedValue);
+        if (elements == cachedElements)
+            return cachedValue;
+    }
+
+    JSValue elementsValue = toJS<IDLNullable<IDLFrozenArray<IDLInterface<Element>>>>(lexicalGlobalObject, *thisObject.globalObject(), throwScope, elements);
+    cachedObject->putDirect(vm, propertyName, elementsValue);
+    return elementsValue;
+}
+
+JSValue JSElement::ariaControlsElements(JSGlobalObject& lexicalGlobalObject) const
+{
+    return getElementsArrayAttribute(lexicalGlobalObject, *this, WebCore::HTMLNames::aria_controlsAttr);
+}
+
+JSValue JSElement::ariaDescribedByElements(JSGlobalObject& lexicalGlobalObject) const
+{
+    return getElementsArrayAttribute(lexicalGlobalObject, *this, WebCore::HTMLNames::aria_describedbyAttr);
+}
+
+JSValue JSElement::ariaDetailsElements(JSGlobalObject& lexicalGlobalObject) const
+{
+    return getElementsArrayAttribute(lexicalGlobalObject, *this, WebCore::HTMLNames::aria_detailsAttr);
+}
+
+JSValue JSElement::ariaFlowToElements(JSGlobalObject& lexicalGlobalObject) const
+{
+    return getElementsArrayAttribute(lexicalGlobalObject, *this, WebCore::HTMLNames::aria_flowtoAttr);
+}
+
+JSValue JSElement::ariaLabelledByElements(JSGlobalObject& lexicalGlobalObject) const
+{
+    return getElementsArrayAttribute(lexicalGlobalObject, *this, WebCore::HTMLNames::aria_labelledbyAttr);
+}
+
+JSValue JSElement::ariaOwnsElements(JSGlobalObject& lexicalGlobalObject) const
+{
+    return getElementsArrayAttribute(lexicalGlobalObject, *this, WebCore::HTMLNames::aria_ownsAttr);
+}
+
 } // namespace WebCore
diff --git a/Source/WebCore/bindings/js/WebCoreBuiltinNames.h b/Source/WebCore/bindings/js/WebCoreBuiltinNames.h
@@ -432,6 +432,7 @@ namespace WebCore {
     macro(blur) \
     macro(body) \
     macro(byobRequest) \
+    macro(cachedAttrAssociatedElements) \
     macro(caches) \
     macro(cancel) \
     macro(cancelAlgorithm) \
