[webkit-changes] [WebKit/WebKit] 4806e6: Regression(264594 at main) Unable to search for fligh...

Chris Dumez noreply at github.com
Sun Jun 25 10:33:39 PDT 2023 

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 4806e6221978ec532ec4430f59549db0658387e4
      https://github.com/WebKit/WebKit/commit/4806e6221978ec532ec4430f59549db0658387e4
  Author: Chris Dumez <cdumez at apple.com>
  Date:   2023-06-25 (Sun, 25 Jun 2023)

  Changed paths:
    A LayoutTests/fast/forms/set-property-expected.txt
    A LayoutTests/fast/forms/set-property.html
    A LayoutTests/fast/html/HTMLCollection-set-property-expected.txt
    A LayoutTests/fast/html/HTMLCollection-set-property.html
    A LayoutTests/fast/html/HTMLOptionsCollection-set-property-expected.txt
    A LayoutTests/fast/html/HTMLOptionsCollection-set-property.html
    M LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/legacy-platform-object/Set-expected.txt
    M LayoutTests/imported/w3c/web-platform-tests/WebIDL/ecmascript-binding/legacy-platform-object/Set.html
    M Source/JavaScriptCore/runtime/JSObject.cpp
    M Source/JavaScriptCore/runtime/JSObject.h
    M Source/JavaScriptCore/runtime/PropertyDescriptor.cpp
    M Source/JavaScriptCore/runtime/PropertyDescriptor.h
    M Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
    M Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterNoIdentifier.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterNoIdentifier.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterThrowingException.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterThrowingException.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterWithIdentifier.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterWithIdentifier.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestLegacyOverrideBuiltIns.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestLegacyOverrideBuiltIns.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterNoIdentifier.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterNoIdentifier.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterThrowingException.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterThrowingException.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterWithIdentifier.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterWithIdentifier.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterNoIdentifier.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterNoIdentifier.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterThrowingException.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterThrowingException.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIdentifier.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIdentifier.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIndexedGetter.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIndexedGetter.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterCallWith.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterCallWith.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterNoIdentifier.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterNoIdentifier.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterWithIdentifier.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterWithIdentifier.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterNoIdentifier.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterNoIdentifier.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterThrowingException.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterThrowingException.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIdentifier.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIdentifier.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIndexedGetter.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIndexedGetter.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIndexedGetterAndSetter.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIndexedGetterAndSetter.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyOverrideBuiltIns.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyOverrideBuiltIns.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyUnforgeableProperties.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyUnforgeableProperties.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyUnforgeablePropertiesAndLegacyOverrideBuiltIns.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyUnforgeablePropertiesAndLegacyOverrideBuiltIns.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestObj.h
    M Source/WebCore/bindings/scripts/test/JS/JSTestPluginInterface.cpp
    M Source/WebCore/bindings/scripts/test/JS/JSTestPluginInterface.h

  Log Message:
  -----------
  Regression(264594 at main) Unable to search for flights on delta.com
https://bugs.webkit.org/show_bug.cgi?id=258471
rdar://110787556

Reviewed by Yusuke Suzuki and Darin Adler.

264594 at main tried to align the behavior of our bindings objects setters / deleters
with the Web IDL specification and with other major browsers. I however made a
slight mistake that impacted objects that have the [LegacyOverrideBuiltIns] Web IDL
extended attribute, such as HTMLFormElement.

Such interfaces are very special because properties returned by the named property
getter override built-in properties. This means that `form.action` returns the
input element by the id/name "action" in the delta case, instead of the built-in
form action attribute. We were getting this part right. However, the behavior when
setting the action property wasn't quite right. We're supposed to set the built-in
action property and we would fail to do so.

The source of the bug is that [[Set]] is supposed to call LegacyPlatformObjectGetOwnProperty()
with `ignoreNamedProperties=true` and then call OrdinarySetWithOwnDescriptor() with the
descriptor that was returned [1]. Because we should ignore named properties, it should
return the slot of the built-in `form.action` property, not the slot for the input
Element with the name/id that is "action".

However, we were calling JSC's ordinarySetSlow() which would call the standard
getOwnPropertySlot(), which would not ignore the named properties.

To address the issue, I moved the getOwnPropertySlot() implementation of our
bindings object to a separate LegacyPlatformObjectGetOwnProperty() function,
which takes a ignoreNamedProperties parameter to decide we should should
ignore named properties or not, as per the specification [2].

I also split part of JSC's ordinarySetSlow() into a new ordinarySetWithOwnDescriptor()
function which implements OrdinarySetWithOwnDescriptor() in the EcmaScript spec [3].
ordinarySetSlow() is an implementation of EcmaScript's OrdinarySet(), which is meant
to call OrdinarySetWithOwnDescriptor() [4] so we are now more aligned with the spec.

put() in our generated bindings now calls legacyPlatformObjectGetOwnProperty() when
it exists and calls JSC's ordinarySetWithOwnDescriptor() with it.

[1] https://webidl.spec.whatwg.org/#legacy-platform-object-set
[2] https://webidl.spec.whatwg.org/#LegacyPlatformObjectGetOwnProperty
[3] https://tc39.es/ecma262/multipage/ordinary-and-exotic-objects-behaviours.html#sec-ordinarysetwithowndescriptor
[4] https://tc39.es/ecma262/multipage/ordinary-and-exotic-objects-behaviours.html#sec-ordinaryset

* LayoutTests/fast/forms/set-property-expected.txt: Added.
* LayoutTests/fast/forms/set-property.html: Added.
* LayoutTests/fast/html/HTMLCollection-set-property-expected.txt: Added.
* LayoutTests/fast/html/HTMLCollection-set-property.html: Added.
* LayoutTests/fast/html/HTMLOptionsCollection-set-property-expected.txt: Added.
* LayoutTests/fast/html/HTMLOptionsCollection-set-property.html: Added.
Extend test coverage. I have verified that all these tests are passing in Chrome
and Firefox.

* Source/JavaScriptCore/runtime/JSObject.cpp:
(JSC::ordinarySetSlow):
(JSC::ordinarySetWithOwnDescriptor):
(JSC::JSObject::getOwnPropertyDescriptor):
(JSC::WeakCustomGetterOrSetterHashTranslator::hash): Deleted.
(JSC::WeakCustomGetterOrSetterHashTranslator::equal): Deleted.
(JSC::createCustomGetterFunction): Deleted.
(JSC::createCustomSetterFunction): Deleted.
* Source/JavaScriptCore/runtime/JSObject.h:
* Source/JavaScriptCore/runtime/PropertyDescriptor.cpp:
(JSC::WeakCustomGetterOrSetterHashTranslator::hash):
(JSC::WeakCustomGetterOrSetterHashTranslator::equal):
(JSC::createCustomSetterFunction):
(JSC::createCustomGetterFunction):
(JSC::PropertyDescriptor::setPropertySlot):
* Source/JavaScriptCore/runtime/PropertyDescriptor.h:
* Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:
(GenerateNamedGetterLambda):
(GenerateGetOwnPropertySlot):
(GenerateGetOwnPropertySlotByIndex):
(GeneratePut):
(GenerateHeader):
* Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.cpp:
(WebCore::JSTestEventTarget::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestEventTarget::getOwnPropertySlot):
(WebCore::JSTestEventTarget::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterNoIdentifier.cpp:
(WebCore::JSTestIndexedSetterNoIdentifier::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestIndexedSetterNoIdentifier::getOwnPropertySlot):
(WebCore::JSTestIndexedSetterNoIdentifier::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterNoIdentifier.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterThrowingException.cpp:
(WebCore::JSTestIndexedSetterThrowingException::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestIndexedSetterThrowingException::getOwnPropertySlot):
(WebCore::JSTestIndexedSetterThrowingException::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterThrowingException.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterWithIdentifier.cpp:
(WebCore::JSTestIndexedSetterWithIdentifier::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestIndexedSetterWithIdentifier::getOwnPropertySlot):
(WebCore::JSTestIndexedSetterWithIdentifier::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestIndexedSetterWithIdentifier.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestLegacyOverrideBuiltIns.cpp:
(WebCore::JSTestLegacyOverrideBuiltIns::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestLegacyOverrideBuiltIns::getOwnPropertySlot):
(WebCore::JSTestLegacyOverrideBuiltIns::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestLegacyOverrideBuiltIns.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterNoIdentifier.cpp:
(WebCore::JSTestNamedAndIndexedSetterNoIdentifier::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedAndIndexedSetterNoIdentifier::getOwnPropertySlot):
(WebCore::JSTestNamedAndIndexedSetterNoIdentifier::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterNoIdentifier.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterThrowingException.cpp:
(WebCore::JSTestNamedAndIndexedSetterThrowingException::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedAndIndexedSetterThrowingException::getOwnPropertySlot):
(WebCore::JSTestNamedAndIndexedSetterThrowingException::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterThrowingException.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterWithIdentifier.cpp:
(WebCore::JSTestNamedAndIndexedSetterWithIdentifier::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedAndIndexedSetterWithIdentifier::getOwnPropertySlot):
(WebCore::JSTestNamedAndIndexedSetterWithIdentifier::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedAndIndexedSetterWithIdentifier.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterNoIdentifier.cpp:
(WebCore::JSTestNamedDeleterNoIdentifier::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedDeleterNoIdentifier::getOwnPropertySlot):
(WebCore::JSTestNamedDeleterNoIdentifier::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterNoIdentifier.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterThrowingException.cpp:
(WebCore::JSTestNamedDeleterThrowingException::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedDeleterThrowingException::getOwnPropertySlot):
(WebCore::JSTestNamedDeleterThrowingException::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterThrowingException.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIdentifier.cpp:
(WebCore::JSTestNamedDeleterWithIdentifier::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedDeleterWithIdentifier::getOwnPropertySlot):
(WebCore::JSTestNamedDeleterWithIdentifier::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIdentifier.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIndexedGetter.cpp:
(WebCore::JSTestNamedDeleterWithIndexedGetter::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedDeleterWithIndexedGetter::getOwnPropertySlot):
(WebCore::JSTestNamedDeleterWithIndexedGetter::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedDeleterWithIndexedGetter.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterCallWith.cpp:
(WebCore::JSTestNamedGetterCallWith::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedGetterCallWith::getOwnPropertySlot):
(WebCore::JSTestNamedGetterCallWith::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterCallWith.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterNoIdentifier.cpp:
(WebCore::JSTestNamedGetterNoIdentifier::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedGetterNoIdentifier::getOwnPropertySlot):
(WebCore::JSTestNamedGetterNoIdentifier::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterNoIdentifier.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterWithIdentifier.cpp:
(WebCore::JSTestNamedGetterWithIdentifier::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedGetterWithIdentifier::getOwnPropertySlot):
(WebCore::JSTestNamedGetterWithIdentifier::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedGetterWithIdentifier.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterNoIdentifier.cpp:
(WebCore::JSTestNamedSetterNoIdentifier::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedSetterNoIdentifier::getOwnPropertySlot):
(WebCore::JSTestNamedSetterNoIdentifier::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterNoIdentifier.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterThrowingException.cpp:
(WebCore::JSTestNamedSetterThrowingException::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedSetterThrowingException::getOwnPropertySlot):
(WebCore::JSTestNamedSetterThrowingException::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterThrowingException.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIdentifier.cpp:
(WebCore::JSTestNamedSetterWithIdentifier::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedSetterWithIdentifier::getOwnPropertySlot):
(WebCore::JSTestNamedSetterWithIdentifier::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIdentifier.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIndexedGetter.cpp:
(WebCore::JSTestNamedSetterWithIndexedGetter::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedSetterWithIndexedGetter::getOwnPropertySlot):
(WebCore::JSTestNamedSetterWithIndexedGetter::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIndexedGetter.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIndexedGetterAndSetter.cpp:
(WebCore::JSTestNamedSetterWithIndexedGetterAndSetter::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedSetterWithIndexedGetterAndSetter::getOwnPropertySlot):
(WebCore::JSTestNamedSetterWithIndexedGetterAndSetter::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithIndexedGetterAndSetter.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyOverrideBuiltIns.cpp:
(WebCore::JSTestNamedSetterWithLegacyOverrideBuiltIns::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedSetterWithLegacyOverrideBuiltIns::getOwnPropertySlot):
(WebCore::JSTestNamedSetterWithLegacyOverrideBuiltIns::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyOverrideBuiltIns.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyUnforgeableProperties.cpp:
(WebCore::JSTestNamedSetterWithLegacyUnforgeableProperties::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedSetterWithLegacyUnforgeableProperties::getOwnPropertySlot):
(WebCore::JSTestNamedSetterWithLegacyUnforgeableProperties::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyUnforgeableProperties.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyUnforgeablePropertiesAndLegacyOverrideBuiltIns.cpp:
(WebCore::JSTestNamedSetterWithLegacyUnforgeablePropertiesAndLegacyOverrideBuiltIns::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestNamedSetterWithLegacyUnforgeablePropertiesAndLegacyOverrideBuiltIns::getOwnPropertySlot):
(WebCore::JSTestNamedSetterWithLegacyUnforgeablePropertiesAndLegacyOverrideBuiltIns::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestNamedSetterWithLegacyUnforgeablePropertiesAndLegacyOverrideBuiltIns.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp:
(WebCore::JSTestObj::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestObj::getOwnPropertySlot):
(WebCore::JSTestObj::put):
* Source/WebCore/bindings/scripts/test/JS/JSTestObj.h:
* Source/WebCore/bindings/scripts/test/JS/JSTestPluginInterface.cpp:
(WebCore::JSTestPluginInterface::legacyPlatformObjectGetOwnProperty):
(WebCore::JSTestPluginInterface::getOwnPropertySlot):
* Source/WebCore/bindings/scripts/test/JS/JSTestPluginInterface.h:

Canonical link: https://commits.webkit.org/265501@main

More information about the webkit-changes mailing list