Occasional debug assertion under IntlNumberFormat::initializeNumberFo…

…rmat when running Speedometer 3 https://bugs.webkit.org/show_bug.cgi?id=265113 rdar://118619451 Reviewed by Chris Dumez. Grab the JSC API lock before calling into `JSObject::hasProperty()` when checking for the presence of the `tableauPrep` property. * LayoutTests/fast/events/key-event-with-quirks-enabled-expected.txt: Added. * LayoutTests/fast/events/key-event-with-quirks-enabled.html: Added. * Source/WebCore/bindings/js/JSLocalDOMWindowCustom.cpp: (WebCore::JSLocalDOMWindow::getOwnPropertySlot): (WebCore::JSLocalDOMWindow::getOwnPropertySlotByIndex): Add a new debug assertion when reading properties off of the window object without having the JS API lock; without the fix above, the assertion is hit in the new layout test, where site-specific quirks are enabled. * Source/WebCore/page/Quirks.cpp: (WebCore::Quirks::needsDisableDOMPasteAccessQuirk const): Canonical link: https://commits.webkit.org/270961@main
WebKit · Nov 20, 2023 · a6eac11 · a6eac11
1 parent ac61808
commit a6eac11
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 0 deletions.
diff --git a/LayoutTests/fast/events/key-event-with-quirks-enabled-expected.txt b/LayoutTests/fast/events/key-event-with-quirks-enabled-expected.txt
@@ -0,0 +1,12 @@
+Tests that basic key events work, and do not trigger assertions when quirks are enabled.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Received keydown
+PASS Received keyup
+PASS receivedKeyUp became true
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/events/key-event-with-quirks-enabled.html b/LayoutTests/fast/events/key-event-with-quirks-enabled.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ NeedsSiteSpecificQuirks=true ] -->
+<html>
+<head>
+    <script src="../../resources/js-test.js"></script>
+    <script src="../../resources/ui-helper.js"></script>
+</head>
+<body>
+<input id="textField"></input>
+<script>
+jsTestIsAsync = true;
+receivedKeyUp = false;
+
+addEventListener("load", async () => {
+    description("Tests that basic key events work, and do not trigger assertions when quirks are enabled.");
+
+    let textField = document.getElementById("textField");
+    textField.addEventListener("keydown", () => testPassed("Received keydown"));
+    textField.addEventListener("keyup", () => {
+        testPassed("Received keyup");
+        receivedKeyUp = true;
+    });
+
+    await UIHelper.activateElementAndWaitForInputSession(textField);
+    await UIHelper.keyDown("a");
+
+    await shouldBecomeEqual("receivedKeyUp", "true");
+    finishJSTest();
+});
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/bindings/js/JSLocalDOMWindowCustom.cpp b/Source/WebCore/bindings/js/JSLocalDOMWindowCustom.cpp
@@ -198,6 +198,7 @@ bool JSLocalDOMWindow::getOwnPropertySlot(JSObject* object, JSGlobalObject* lexi
 
     auto* thisObject = jsCast<JSLocalDOMWindow*>(object);
 
+    ASSERT(lexicalGlobalObject->vm().currentThreadIsHoldingAPILock());
     // Construct3 assumes that the presence of OffscreenCanvas implies
     // that WebGL will always be available, which isn't true yet.
     // Disable OffscreenCanvas when the Construct3 library is present
@@ -245,6 +246,7 @@ bool JSLocalDOMWindow::getOwnPropertySlotByIndex(JSObject* object, JSGlobalObjec
     // Indexed getters take precendence over regular properties, so caching would be invalid.
     slot.disableCaching();
 
+    ASSERT(lexicalGlobalObject->vm().currentThreadIsHoldingAPILock());
     // These are also allowed cross-origin, so come before the access check.
     if (frame && index < frame->tree().scopedChildCount()) {
         // FIXME: <rdar://118263337> LocalDOMWindow::length needs to include RemoteFrames.

diff --git a/Source/WebCore/page/Quirks.cpp b/Source/WebCore/page/Quirks.cpp
@@ -62,6 +62,7 @@
 #include "UserContentTypes.h"
 #include "UserScript.h"
 #include "UserScriptTypes.h"
+#include <JavaScriptCore/JSLock.h>
 
 #if PLATFORM(COCOA)
 #include <wtf/cocoa/RuntimeApplicationChecksCocoa.h>
@@ -1613,6 +1614,8 @@ bool Quirks::needsDisableDOMPasteAccessQuirk() const
         auto* globalObject = m_document->globalObject();
         if (!globalObject)
             return false;
+
+        JSC::JSLockHolder lock(globalObject->vm());
         auto tableauPrepProperty = JSC::Identifier::fromString(globalObject->vm(), "tableauPrep"_s);
         return globalObject->hasProperty(globalObject, tableauPrepProperty);
     }();