Attachment 372903 Details for Bug 199220 – Patch

[patch] Patch

bug-199220-20190625212610.patch (text/plain), 6.99 KB, created by zalan on 2019-06-25 21:26:11 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-06-25 21:26:11 PDT

Size: 6.99 KB

patch

obsolete

>Subversion Revision: 246791
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4c8fee3fbb9c0ddae2517a9f5609ec77df1a8451..e01f54f1aace940a6b4100748e5dc4906d3ba416 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,18 @@
>+2019-06-25  Zalan Bujtas  <zalan@apple.com>
>+
>+        [ContentChangeObserver] Dispatch synthetic mouse event asynchronously in completePendingSyntheticClickForContentChangeObserver
>+        https://bugs.webkit.org/show_bug.cgi?id=199220
>+        <rdar://problem/51787961>
>+
>+        Reviewed by Simon Fraser.
>+
>+        WebPage::completePendingSyntheticClickForContentChangeObserver should not dispatch mouse events synchronously.
>+        Mouse events, through style updates could destroy the element that initiated this change.
>+        WebPage::handleSyntheticClick() already implements this pattern.
>+
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::completePendingSyntheticClickForContentChangeObserver):
>+
> 2019-06-25  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         Fully rename WebKitGTK+ -> WebKitGTK everywhere
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index 044e11f017876901c81d39124bf68fde38e1cfa0..326c09bf3ae24b2ed49d43bcca9032a044af8d8b 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -714,18 +714,22 @@ void WebPage::completePendingSyntheticClickForContentChangeObserver()
>     if (!m_pendingSyntheticClickNode)
>         return;
>     auto observedContentChange = m_pendingSyntheticClickNode->document().contentChangeObserver().observedContentChange();
>-    // Only dispatch the click if the document didn't get changed by any timers started by the move event.
>-    if (observedContentChange == WKContentNoChange) {
>-        LOG(ContentObservation, "No chage was observed -> click.");
>-        completeSyntheticClick(*m_pendingSyntheticClickNode, m_pendingSyntheticClickLocation, m_pendingSyntheticClickModifiers, WebCore::OneFingerTap, m_pendingSyntheticClickPointerId);
>-    } else {
>-        // Ensure that the mouse is on the most recent content.
>-        dispatchSyntheticMouseMove(m_page->mainFrame(), m_pendingSyntheticClickLocation, m_pendingSyntheticClickModifiers, m_pendingSyntheticClickPointerId);
>-        LOG(ContentObservation, "Observed meaningful visible change -> hover.");
>-    }
>+    callOnMainThread([protectedThis = makeRefPtr(this), targetNode = Ref<Node>(*m_pendingSyntheticClickNode), observedContentChange, location = m_pendingSyntheticClickLocation, modifiers = m_pendingSyntheticClickModifiers, pointerId = m_pendingSyntheticClickPointerId] {
>+        if (protectedThis->m_isClosed || !protectedThis->corePage() || !targetNode->parentNode())
>+            return;
> 
>+        // Only dispatch the click if the document didn't get changed by any timers started by the move event.
>+        if (observedContentChange == WKContentNoChange) {
>+            LOG(ContentObservation, "No chage was observed -> click.");
>+            protectedThis->completeSyntheticClick(targetNode, location, modifiers, WebCore::OneFingerTap, pointerId);
>+        } else {
>+            // Ensure that the mouse is on the most recent content.
>+            dispatchSyntheticMouseMove(protectedThis->corePage()->mainFrame(), location, modifiers, pointerId);
>+            LOG(ContentObservation, "Observed meaningful visible change -> hover.");
>+        }
>+    });
>     m_pendingSyntheticClickNode = nullptr;
>-    m_pendingSyntheticClickLocation = FloatPoint();
>+    m_pendingSyntheticClickLocation = { };
>     m_pendingSyntheticClickModifiers = { };
>     m_pendingSyntheticClickPointerId = 0;
> }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 81a485538cb98518ebb8573c01eb8fd5a2152e79..0ec2eaff982c8ba15829ea49adb81862e25ae6da 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-06-25  Zalan Bujtas  <zalan@apple.com>
>+
>+        [ContentChangeObserver] Dispatch synthetic mouse event asynchronously in completePendingSyntheticClickForContentChangeObserver
>+        https://bugs.webkit.org/show_bug.cgi?id=199220
>+        <rdar://problem/51787961>
>+
>+        Reviewed by Simon Fraser.
>+
>+        * fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-crash-expected.txt: Added.
>+        * fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-crash.html: Added.
>+
> 2019-06-25  Fujii Hironori  <Hironori.Fujii@sony.com>
> 
>         Unreviewed test gardening
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-crash-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-crash-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..3cd69deabafa67dc5d4513d7a8a3b14ddcc79bc9
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-crash-expected.txt
>@@ -0,0 +1,2 @@
>+PASS if no crash or assert.
>+
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-crash.html b/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-crash.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..594ab2aa5756e5651eb7055de1655e762ed5328d
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-crash.html
>@@ -0,0 +1,59 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<title>This tests the case when visible content change happens through transition and the click removes the animated element.</title>
>+<script src="../../../../../resources/basic-gestures.js"></script>
>+<style>
>+#tapthis {
>+    width: 400px;
>+    height: 400px;
>+    border: 1px solid green;
>+}
>+
>+#becomesVisible {
>+    opacity: 0;
>+    width: 100px;
>+    height: 100px;
>+    background-color: green;
>+    transition: opacity 80ms ease-out 0ms;
>+}
>+</style>
>+<script>
>+async function test() {
>+    if (!window.testRunner || !testRunner.runUIScript)
>+        return;
>+    if (window.internals)
>+        internals.settings.setContentChangeObserverEnabled(true);
>+
>+    testRunner.waitUntilDone();
>+    testRunner.dumpAsText();
>+
>+    let rect = tapthis.getBoundingClientRect();
>+    let x = rect.left + rect.width / 2;
>+    let y = rect.top + rect.height / 2;
>+
>+    await tapAtPoint(x, y);
>+}
>+</script>
>+</head>
>+<body onload="test()">
>+<div id=tapthis>PASS if no crash or assert.</div>
>+<div id=becomesVisible></div>
>+<pre id=result></pre>
>+<script>
>+tapthis.addEventListener("mouseover", function( event ) {
>+    becomesVisible.style.opacity = "1";
>+    setTimeout(function() {
>+        document.body.offsetHeight;
>+    }, 50);
>+}, false);
>+
>+tapthis.addEventListener("click", function( event ) {
>+    becomesVisible.remove();
>+    document.body.offsetHeight;
>+    if (window.testRunner)
>+        testRunner.notifyDone();
>+}, false);
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 199220: 372901 | 372903 | 372921