ValidityState object should be the same on each access (JS object get…

…ting GC'd incorrectly) https://bugs.webkit.org/show_bug.cgi?id=33733 Reviewed by Darin Adler. Fixed the bug by making the form associated element an opaque root of ValidityState. * LayoutTests/fast/forms/ValidityState-gc-expected.txt: Added. * LayoutTests/fast/forms/ValidityState-gc.html: Added. * Source/WebCore/html/HTMLFormControlElement.h: * Source/WebCore/html/HTMLObjectElement.h: * Source/WebCore/html/ValidityState.h: (WebCore::ValidityState::element): (WebCore::ValidityState::opaqueRootConcurrently): * Source/WebCore/html/ValidityState.idl: Canonical link: https://commits.webkit.org/251480@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@295475 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Jun 11, 2022 · d8ee674 · d8ee674
1 parent 33dc230
commit d8ee674
Show file tree

Hide file tree

Showing 6 changed files with 76 additions and 2 deletions.
diff --git a/LayoutTests/fast/forms/ValidityState-gc-expected.txt b/LayoutTests/fast/forms/ValidityState-gc-expected.txt
@@ -0,0 +1,12 @@
+This tests that ValidityState survives garbage collection.
+
+<button></button>: PASS
+<fieldset></fieldset>: PASS
+<input type="text">: PASS
+<input type="radio">: PASS
+<input type="checkbox">: PASS
+<object></object>: PASS
+<output></output>: PASS
+<select></select>: PASS
+<textarea></textarea>: PASS
+
diff --git a/LayoutTests/fast/forms/ValidityState-gc.html b/LayoutTests/fast/forms/ValidityState-gc.html
@@ -0,0 +1,52 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p>This tests that ValidityState survives garbage collection.</p>
+<div id="test-cases">
+<button></button>
+<fieldset></fieldset>
+<input type="text">
+<input type="radio">
+<input type="checkbox">
+<object></object>
+<output></output>
+<select></select>
+<textarea></textarea>
+</div>
+<pre id="result"></pre>
+<script>
+function gc()
+{
+    if (window.GCController)
+        return GCController.collect();
+
+    for (var i = 0; i < 1000000; i++) // > force garbage collection (FF requires about 9K allocations before a collect)
+        var s = new String('');
+}
+
+const testCases = Array.from(document.getElementById('test-cases').children);
+for (const child of testCases)
+    child.validity.foo = 'PASS';
+
+gc();
+
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+}
+
+setTimeout(function () {
+    gc();
+    setTimeout(function () {
+        gc();
+        for (const child of testCases)
+            result.textContent += `${child.outerHTML}: ${child.validity.foo || 'FAIL'}\n`;
+        document.getElementById('test-cases').style.display = 'none';
+        if (window.testRunner)
+            testRunner.notifyDone();
+    }, 0);
+}, 0);
+
+</script>
+</body>
+</html>
diff --git a/Source/WebCore/html/HTMLFormControlElement.h b/Source/WebCore/html/HTMLFormControlElement.h
@@ -181,8 +181,10 @@ class HTMLFormControlElement : public LabelableElement, public FormAssociatedEle
     void startDelayingUpdateValidity() { ++m_delayedUpdateValidityCount; }
     void endDelayingUpdateValidity();
 
+    // These functions can be called concurrently for ValidityState.
     HTMLElement& asHTMLElement() final { return *this; }
     const HTMLFormControlElement& asHTMLElement() const final { return *this; }
+
     FormNamedItem* asFormNamedItem() final { return this; }
     FormAssociatedElement* asFormAssociatedElement() final { return this; }
 

diff --git a/Source/WebCore/html/HTMLObjectElement.h b/Source/WebCore/html/HTMLObjectElement.h
@@ -93,6 +93,8 @@ class HTMLObjectElement final : public HTMLPlugInImageElement, public FormAssoci
 
     FormNamedItem* asFormNamedItem() final { return this; }
     FormAssociatedElement* asFormAssociatedElement() final { return this; }
+
+    // These functions can be called concurrently for ValidityState.
     HTMLObjectElement& asHTMLElement() final { return *this; }
     const HTMLObjectElement& asHTMLElement() const final { return *this; }
 

diff --git a/Source/WebCore/html/ValidityState.h b/Source/WebCore/html/ValidityState.h
@@ -1,7 +1,7 @@
 /*
  * This file is part of the WebKit project.
  *
- * Copyright (C) 2013 Apple Inc. All rights reserved.
+ * Copyright (C) 2013-2022 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -31,6 +31,9 @@ namespace WebCore {
 // but that would have a small runtime cost, and no significant benefit. We'd prefer to implement this
 // as a typedef of FormAssociatedElement, but that would require changes to bindings generation.
 class ValidityState : public FormAssociatedElement {
+public:
+    Element* element() { return &asHTMLElement(); }
+    Node* opaqueRootConcurrently() { return &asHTMLElement(); }
 };
 
 inline ValidityState* FormAssociatedElement::validity()

diff --git a/Source/WebCore/html/ValidityState.idl b/Source/WebCore/html/ValidityState.idl
@@ -2,6 +2,7 @@
  * This file is part of the WebKit project.
  *
  * Copyright (C) 2009 Michelangelo De Simone <micdesim@gmail.com>
+ * Copyright (C) 2013-2022 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
@@ -22,7 +23,9 @@
 
 [
     SkipVTableValidation,
-    Exposed=Window
+    Exposed=Window,
+    GenerateIsReachable=ImplElementRoot,
+    GenerateAddOpaqueRoot=opaqueRootConcurrently
 ] interface ValidityState {
     readonly attribute boolean         valueMissing;
     readonly attribute boolean         typeMismatch;