Release assert in Document::updateLayout() via HTMLTextAreaElement::c…

…hildrenChanged

https://bugs.webkit.org/show_bug.cgi?id=224471

Reviewed by Ryosuke Niwa.

This patch is based on a set of patches made by Sergio Villar Senin
and Gabriel Nava Marino.

Executing some editing commands in a text area might force the recomputation of things
like caret or the visible selection position and extent. Under some circumstances (like
when the text area has no content and children) we might end up trying to update layout
when it is not safe as a side effect of updating the caret.

In order to fix that, we can switch to a model in which we update the selection asynchronously
in the case of having a non-user triggered change. That way we don't do it inside
a restricted layout scope.

The App Highlight restoration code had to be tuned as well (when restoring and scrolling to reveal
a Quick Note on iOS MacOS). It uses TemporarySelectionChange to select and scroll to reveal the highlight
range; the code assumed that this scrolling happens synchronously, since it reverts the selection to
the original range at the end of `AppHighlightStorage::attemptToRestoreHighlightAndScroll` when the
TemporarySelectionChange falls out of scope. That is however no longer the case. Actually no scrolling
happened after this patch because we end up only scheduling the appearance update timer before setting
the selection back to the original state with `SelectionRevealMode::DoNotReveal`. Since this only happens
when the user interacts in the Notes app, it seems sensible to consider it as a user triggered event.

Layout Tests

Moved some tests out of the text-based-repaint.js model because selection is now updated
and revealed asynchronously in the case of non-user triggered changes. The problem is that
by the time the repaint rects are queried the update has not happened yet. That's why
the tests were modified so that we wait until the repaint happens.

Same situation for other tests that do not involve repaint rects but trigger accesibility
tree updates. We must ensure that we let the notification be thrown before checking whether or
not has been emited. As it's done asynchronously we must let the main thread run before checking.

Last but not least, some of the tests are using setTimeout() instead of requestAnimationFrame()
because the results with the latter were not as reliable under stress/debug conditions.

* Source/WebCore/Modules/highlight/AppHighlightStorage.cpp:
(WebCore::AppHighlightStorage::attemptToRestoreHighlightAndScroll):
* Source/WebCore/editing/Editor.cpp:
(WebCore::TemporarySelectionChange::setSelection):
* Source/WebCore/editing/Editor.h:
* Source/WebCore/editing/FrameSelection.cpp:
(WebCore::FrameSelection::setSelection):
(WebCore::FrameSelection::updateSelectionAppearanceNow):
(WebCore::FrameSelection::absoluteCaretBounds):
(WebCore::FrameSelection::setCaretVisibility):
(WebCore::FrameSelection::selectionBounds):
(WebCore::FrameSelection::revealSelection):
(WebCore::updateSelectionByUpdatingLayoutOrStyle): Deleted.
(WebCore::FrameSelection::selectionBounds const): Deleted.
* Source/WebCore/editing/FrameSelection.h:
* Source/WebCore/page/EventHandler.cpp:
(WebCore::setSelectionIfNeeded):
* Source/WebCore/page/Page.cpp:
(WebCore::Page::doAfterUpdateRendering):
* LayoutTests/accessibility/mac/selection-boundary-userinfo.html:
* LayoutTests/accessibility/mac/selection-change-userinfo.html:
* LayoutTests/accessibility/mac/selection-value-changes-for-aria-textbox.html:
* LayoutTests/editing/selection-with-absolute-positioned-empty-content.html:
* LayoutTests/fast/forms/textarea-scrolled-endline-caret.html:
* LayoutTests/fast/repaint/selection-gap-absolute-child-expected.txt:
* LayoutTests/fast/repaint/selection-gap-absolute-child.html:
* LayoutTests/fast/repaint/selection-gap-fixed-child.html:
* LayoutTests/fast/repaint/selection-gap-flipped-absolute-child-expected.txt:
* LayoutTests/fast/repaint/selection-gap-flipped-absolute-child.html:
* LayoutTests/fast/repaint/selection-gap-transformed-absolute-child-expected.txt:
* LayoutTests/fast/repaint/selection-gap-transformed-absolute-child.html:
* LayoutTests/fast/repaint/selection-gap-transformed-fixed-child-expected.txt:
* LayoutTests/fast/repaint/selection-gap-transformed-fixed-child.html:
* LayoutTests/fast/repaint/selection-paint-invalidation-expected.txt:
* LayoutTests/fast/repaint/selection-ruby-rl-expected.txt:
* LayoutTests/fast/repaint/selection-ruby-rl.html:
* LayoutTests/fast/repaint/text-selection-overflow-hidden-expected.txt:
* LayoutTests/fast/repaint/text-selection-overflow-hidden.html:
* LayoutTests/fast/text/incorrect-deselection-across-multiple-elements.html:
* LayoutTests/platform/gtk/TestExpectations:
* LayoutTests/platform/gtk/fast/repaint/selection-ruby-rl-expected.txt: Copied from LayoutTests/fast/repaint/selection-ruby-rl-expected.txt.
* LayoutTests/platform/gtk/fast/repaint/text-selection-overflow-hidden-expected.txt:
* LayoutTests/platform/mac-wk1/TestExpectations:
* LayoutTests/platform/mac-wk1/accessibility/mac/focus-setting-selection-syncronizing-not-clearing-expected.txt: Added.
* LayoutTests/platform/mac-wk1/fast/repaint/4776765-expected.txt: Added.
* LayoutTests/platform/mac-wk1/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-005-expected.txt:
* LayoutTests/platform/mac/TestExpectations:
* LayoutTests/platform/win/fast/repaint/4776765-expected.txt: Added.
* LayoutTests/platform/win/fast/repaint/selection-ruby-rl-expected.txt:
* LayoutTests/platform/win/fast/repaint/text-selection-overflow-hidden-expected.txt:

Canonical link: https://commits.webkit.org/250778@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@294523 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/accessibility/mac/selection-boundary-userinfo.html

@@ -162,20 +162,31 @@
         var addedNotification = webArea.addNotificationListener(notificationCallback);
         shouldBe("addedNotification", "true");
 
-        textbox = document.getElementById("textbox");
-        textbox.focus();
-
-        move(true);
-
-        textbox = document.getElementById("input");
-        textbox.focus();
+        function waitForAnimationFrame() {
+            return new Promise(function(resolve, reject) {
+                requestAnimationFrame(resolve);
+            });
+        }
 
-        move(false);
+        function focusElement(elementId) {
+            element = document.getElementById(elementId);
+            element.focus();
+            return waitForAnimationFrame();
+        }
 
-        textbox = document.getElementById("textarea");
-        textbox.focus();
+        function moveAsync(isVertical) {
+            move(isVertical);
+            return waitForAnimationFrame();
+        }
 
-        move(true);
+        (async function () {
+            await focusElement("textbox");
+            await moveAsync(true);
+            await focusElement("input");
+            await moveAsync(false);
+            await focusElement("textarea");
+            await moveAsync(true);
+        })();
     }
 
 </script>

LayoutTests/accessibility/mac/selection-change-userinfo.html

@@ -195,19 +195,87 @@
         shouldBe("addedNotification", "true");
 
         textbox = document.getElementById("textbox");
-        textbox.focus();
 
         // Trigger selection changes.
         var s = window.getSelection();
-        s.setPosition(textbox, 0);
-        for (var i in gran) {
-            s.modify("move", "forward", gran[i]);
-            s.modify("move", "backward", gran[i]);
+
+        function focusTextBox() {
+            return new Promise(function(resolve, reject) {
+                setTimeout(() => {
+                    textbox.focus();
+                    resolve();
+                }, 0);
+            });
+        }
+
+        function moveSelection(direction, granularity) {
+            return new Promise(function(resolve, reject) {
+                setTimeout(function() {
+                    s.modify("move", direction, granularity);
+                    resolve();
+                }, 0);
+            });
         }
-        for (var i in gran) {
-            s.setPosition(textbox, 0);
-            s.modify("extend", "forward", gran[i]);
+
+        function extendSelection(granularity) {
+            return new Promise(function(resolve, reject) {
+                setTimeout(function() {
+                    s.modify("extend", "forward", granularity);
+                    resolve();
+                }, 0);
+            });
         }
+
+        function resetPosition() {
+            return new Promise(function(resolve, reject) {
+                setTimeout(function() {
+                    s = window.getSelection();
+                    s.setPosition(textbox, 0);
+                    resolve();
+                }, 0);
+            });
+        }
+
+        (async function() {
+            await focusTextBox();
+            await resetPosition();
+            await moveSelection("forward", gran[0]);
+            await moveSelection("backward", gran[0]);
+            await moveSelection("forward", gran[1]);
+            await moveSelection("backward", gran[1]);
+            await moveSelection("forward", gran[2]);
+            await moveSelection("backward", gran[2]);
+            await moveSelection("forward", gran[3]);
+            await moveSelection("backward", gran[3]);
+            await moveSelection("forward", gran[4]);
+            await moveSelection("backward", gran[4]);
+            await moveSelection("forward", gran[5]);
+            await moveSelection("backward", gran[5]);
+            await moveSelection("forward", gran[6]);
+            await moveSelection("backward", gran[6]);
+            await moveSelection("forward", gran[7]);
+            await moveSelection("backward", gran[7]);
+            await moveSelection("forward", gran[8]);
+            await moveSelection("backward", gran[8]);
+            await resetPosition();
+            await extendSelection(gran[0]);
+            await resetPosition();
+            await extendSelection(gran[1]);
+            await resetPosition();
+            await extendSelection(gran[2]);
+            await resetPosition();
+            await extendSelection(gran[3]);
+            await resetPosition();
+            await extendSelection(gran[4]);
+            await resetPosition();
+            await extendSelection(gran[5]);
+            await resetPosition();
+            await extendSelection(gran[6]);
+            await resetPosition();
+            await extendSelection(gran[7]);
+            await resetPosition();
+            await extendSelection(gran[8]);
+          })();
     }
 
 </script>

LayoutTests/accessibility/mac/selection-value-changes-for-aria-textbox.html

@@ -43,12 +43,17 @@
         // Trigger selection changes.
         var s = window.getSelection();
         s.setPosition(textbox, 0);
-        for (var k = 0; k < 3; k++) {
+        requestAnimationFrame(() => {
             s.modify("move", "forward", "character");
-        }
-
-        // Trigger value change.
-        document.execCommand("InsertText", false, "hello ");
+            requestAnimationFrame(() => {
+                s.modify("move", "forward", "character");
+                requestAnimationFrame(() => {
+                    s.modify("move", "forward", "character");
+                    // Trigger value change.
+                    document.execCommand("InsertText", false, "hello ");
+                });
+            });
+        });
     }
 
 </script>

LayoutTests/editing/selection-with-absolute-positioned-empty-content.html

@@ -12,15 +12,26 @@
 <div><a id=foobar style="color: black" href="">select this text<span></span></a> but not this</div>
 <pre id=result></pre>
 <script>
-if (window.internals)
-  internals.startTrackingRepaints();
-if (window.testRunner)
-  testRunner.dumpAsText();
-
-window.getSelection().selectAllChildren(foobar);
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
 
-if (window.internals) {
-  result.innerText = internals.repaintRectsAsText();
-  internals.stopTrackingRepaints();
+function timerAfterRAF(callback) {
+    return requestAnimationFrame(() => setTimeout(callback, 0));
 }
-</script>
+
+timerAfterRAF(() => {
+    if (window.internals)
+        internals.startTrackingRepaints();
+    window.getSelection().selectAllChildren(foobar);
+    timerAfterRAF(() => {
+        if (window.internals) {
+            result.innerText = internals.repaintRectsAsText();
+            internals.stopTrackingRepaints();
+        }
+        if (window.testRunner)
+            testRunner.notifyDone();
+    });
+});
+</script>

LayoutTests/fast/forms/textarea-scrolled-endline-caret.html

@@ -15,13 +15,17 @@
     ta.focus();
     // click
     if (window.eventSender) {
-        eventSender.mouseMoveTo(90, 20);
-        eventSender.mouseDown();
-        eventSender.mouseUp();
-        if (ta.selectionEnd == 17)
-            res.innerHTML = "Test Succeeded";
-        else
-            res.innerHTML = "Test Failed: caret is at " + ta.selectionEnd;
+        testRunner.waitUntilDone();
+        requestAnimationFrame(() => {
+            eventSender.mouseMoveTo(90, 20);
+            eventSender.mouseDown();
+            eventSender.mouseUp();
+            if (ta.selectionEnd == 17)
+                res.innerHTML = "Test Succeeded";
+            else
+                res.innerHTML = "Test Failed: caret is at " + ta.selectionEnd;
+            testRunner.notifyDone();
+        });
     } else {
         res.innerHTML = "Test can't run without event sender (part of DumpRenderTree). "
             + "To test manually, click at the middle of the line marked 9 and check that the caret appears after the 9.";

LayoutTests/fast/repaint/selection-gap-absolute-child-expected.txt

@@ -2,7 +2,4 @@ Bug 111000: Selection gaps don't repaint correctly with transforms
 This tests that absolute elements are invalidated correctly. The box will be competely green if the selected area was invalidated correctly.
 
 
-(repaint rects
-  (rect 0 0 100 100)
-)
-
+(repaint rects (rect 0 0 100 100) )

LayoutTests/fast/repaint/selection-gap-absolute-child.html

@@ -1,11 +1,27 @@
 <!doctype html>
 <head>
-    <script src="resources/text-based-repaint.js"></script>
     <script>
-    function repaintTest()
+    if (window.testRunner) {
+        testRunner.dumpAsText(false);
+        testRunner.waitUntilDone();
+    }
+    function runRepaintTest()
     {
+        if (window.internals)
+            internals.startTrackingRepaints();
+
         var target = document.getElementById("target");
         getSelection().setBaseAndExtent(target, 0, target.nextSibling, 0);
+
+        setTimeout(function() {
+            if (window.internals) {
+                document.querySelector('#repaints').innerHTML = window.internals.repaintRectsAsText();
+                internals.stopTrackingRepaints();
+            }
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }, 0);
     }
     </script>
     <style>
@@ -21,4 +37,5 @@
     <div>
         <div id="target" style="background-color: red; width: 100px; height: 100px; position: absolute;"><br/></div><br/>
     </div>
+    <div id="repaints"></div>
 </body>

LayoutTests/fast/repaint/selection-gap-fixed-child-expected.txt

@@ -2,7 +2,4 @@ Bug 111000: Selection gaps don't repaint correctly with transforms
 This tests that fixed elements are invalidated correctly. The box will be competely green if the selected area was invalidated correctly.
 
 
-(repaint rects
-  (rect 0 0 100 100)
-)
-
+(repaint rects (rect 0 0 100 100) )

LayoutTests/fast/repaint/selection-gap-fixed-child.html

@@ -1,11 +1,26 @@
 <!doctype html>
 <head>
-    <script src="resources/text-based-repaint.js"></script>
     <script>
-    function repaintTest()
+    if (window.testRunner) {
+        testRunner.dumpAsText(false);
+        testRunner.waitUntilDone();
+    }
+    function runRepaintTest()
     {
+        if (window.internals)
+            internals.startTrackingRepaints();
+
         var target = document.getElementById("target");
         getSelection().setBaseAndExtent(target, 0, target.nextSibling, 0);
+
+        setTimeout(function() {
+            if (window.internals) {
+                document.querySelector('#repaints').innerHTML = window.internals.repaintRectsAsText();
+                internals.stopTrackingRepaints();
+            }
+            if (window.testRunner)
+                testRunner.notifyDone();
+        });
     }
     </script>
     <style>
@@ -21,4 +36,5 @@
     <div>
         <div id="target" style="background-color: red; width: 100px; height: 100px; position: fixed;"><br/></div><br/>
     </div>
+    <div id="repaints"></div>
 </body>

LayoutTests/fast/repaint/selection-gap-flipped-absolute-child-expected.txt

@@ -2,7 +2,4 @@ Bug 111000: Selection gaps don't repaint correctly with transforms
 This tests that absolute elements that get flipped are invalidated correctly. The box will be competely green if the selected area was invalidated correctly.
 
 
-(repaint rects
-  (rect 0 0 100 100)
-)
-
+(repaint rects (rect 0 0 100 100) )

LayoutTests/fast/repaint/selection-gap-flipped-absolute-child.html

@@ -1,11 +1,27 @@
 <!doctype html>
 <head>
-    <script src="resources/text-based-repaint.js"></script>
     <script>
-    function repaintTest()
+    if (window.testRunner) {
+        testRunner.dumpAsText(false);
+        testRunner.waitUntilDone();
+    }
+    function runRepaintTest()
     {
+        if (window.internals)
+            internals.startTrackingRepaints();
+
         var target = document.getElementById("target");
         getSelection().setBaseAndExtent(target, 0, target.nextSibling, 0);
+
+        setTimeout(function() {
+            if (window.internals) {
+                document.querySelector('#repaints').innerHTML = window.internals.repaintRectsAsText();
+                internals.stopTrackingRepaints();
+            }
+
+            if (window.testRunner)
+                testRunner.notifyDone();
+        }, 0);
     }
     </script>
     <style>
@@ -21,4 +37,5 @@
     <div style="-webkit-writing-mode: vertical-rl">
         <div id="target" style="background-color: red; width: 100px; height: 100px; position: absolute;"><br/></div><br/>
     </div>
+    <div id="repaints"></div>
 </body>

LayoutTests/fast/repaint/selection-gap-transformed-absolute-child-expected.txt

@@ -2,7 +2,4 @@ Bug 111000: Selection gaps don't repaint correctly with transforms
 This tests that absolute elements that get transformed are invalidated correctly. The box will be completely green if the selected area was invalidated correctly.
 
 
-(repaint rects
-  (rect 50 50 100 100)
-)
-
+(repaint rects (rect 50 50 100 100) )