Attachment 373477 Details for Bug 199506 – Patch

[patch] Patch

bug-199506-20190704210629.patch (text/plain), 6.61 KB, created by zalan on 2019-07-04 21:06:29 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-07-04 21:06:29 PDT

Size: 6.61 KB

patch

obsolete

>Subversion Revision: 247146
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 41cf3035e3191de1e86d54724cb6bd8c23a5591c..94a712e76b6fee490850e61689644d9605e6138b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-07-04  Zalan Bujtas  <zalan@apple.com>
>+
>+        [ContentChangeObserver] didFinishTransition triggers a nested style recalc via isConsideredClickable
>+        https://bugs.webkit.org/show_bug.cgi?id=199506
>+        <rdar://problem/52656221>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        isConsideredClickable() may trigger style update through Node::computeEditability. Let's adjust the observer state in the next runloop. 
>+
>+        Test: fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-and-recursive-update-style.html
>+
>+        * page/ios/ContentChangeObserver.cpp:
>+        (WebCore::ContentChangeObserver::didFinishTransition):
>+        * page/ios/ContentChangeObserver.h:
>+
> 2019-07-04  Zalan Bujtas  <zalan@apple.com>
> 
>         [ContentChangeObserver] Limit mouseOut dispatching after synthetic click to YouTube.com
>diff --git a/Source/WebCore/page/ios/ContentChangeObserver.cpp b/Source/WebCore/page/ios/ContentChangeObserver.cpp
>index 089973cc261bbb19837fa5276dab9cd059db8cdd..2862e5c5722a04cda2b35f83fbc76d6b54e5adf4 100644
>--- a/Source/WebCore/page/ios/ContentChangeObserver.cpp
>+++ b/Source/WebCore/page/ios/ContentChangeObserver.cpp
>@@ -194,11 +194,16 @@ void ContentChangeObserver::didFinishTransition(const Element& element, CSSPrope
>         return;
>     LOG_WITH_STREAM(ContentObservation, stream << "didFinishTransition: transition finished (" << &element << ").");
> 
>-    if (isConsideredHidden(element)) {
>-        adjustObservedState(Event::EndedTransitionButFinalStyleIsNotDefiniteYet);
>-        return;
>-    }
>-    adjustObservedState(isConsideredClickable(element, ElementHadRenderer::Yes) ? Event::CompletedTransitionWithClickableContent : Event::CompletedTransitionWithoutClickableContent);
>+    // isConsideredClickable may trigger style update through Node::computeEditability. Let's adjust the state in the next runloop.
>+    callOnMainThread([weakThis = makeWeakPtr(*this), targetElement = makeWeakPtr(element)] {
>+        if (!weakThis || !targetElement)
>+            return;
>+        if (isConsideredHidden(*targetElement)) {
>+            weakThis->adjustObservedState(Event::EndedTransitionButFinalStyleIsNotDefiniteYet);
>+            return;
>+        }
>+        weakThis->adjustObservedState(isConsideredClickable(*targetElement, ElementHadRenderer::Yes) ? Event::CompletedTransitionWithClickableContent : Event::CompletedTransitionWithoutClickableContent);
>+    });
> }
> 
> void ContentChangeObserver::didRemoveTransition(const Element& element, CSSPropertyID propertyID)
>diff --git a/Source/WebCore/page/ios/ContentChangeObserver.h b/Source/WebCore/page/ios/ContentChangeObserver.h
>index 32da407e6d43e17ce2427aab6d69250e7c3026d6..722740417b2d163bb8cd0ed8a2d54b147e52a2be 100644
>--- a/Source/WebCore/page/ios/ContentChangeObserver.h
>+++ b/Source/WebCore/page/ios/ContentChangeObserver.h
>@@ -43,7 +43,7 @@ class Animation;
> class DOMTimer;
> class Element;
> 
>-class ContentChangeObserver {
>+class ContentChangeObserver : public CanMakeWeakPtr<ContentChangeObserver> {
> public:
>     ContentChangeObserver(Document&);
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index fd255d7b7197b8669f5b0ae9e57ecf49b63a2441..c211526f420ab7a33b4b363040085820ea95306e 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-07-04  Zalan Bujtas  <zalan@apple.com>
>+
>+        [ContentChangeObserver] didFinishTransition triggers a nested style recalc via isConsideredClickable
>+        https://bugs.webkit.org/show_bug.cgi?id=199506
>+        <rdar://problem/52656221>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-and-recursive-update-style-expected.txt: Added.
>+        * fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-and-recursive-update-style.html: Added.
>+
> 2019-07-04  Zalan Bujtas  <zalan@apple.com>
> 
>         [ContentChangeObserver] Limit mouseOut dispatching after synthetic click to YouTube.com
>diff --git a/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-and-recursive-update-style-expected.txt b/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-and-recursive-update-style-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..3cd69deabafa67dc5d4513d7a8a3b14ddcc79bc9
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-and-recursive-update-style-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-and-recursive-update-style.html b/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-and-recursive-update-style.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..c801e824b615059f82c443c399f3347fa2f3408b
>--- /dev/null
>+++ b/LayoutTests/fast/events/touch/ios/content-observation/animation-end-with-visiblity-change-and-recursive-update-style.html
>@@ -0,0 +1,53 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<title>This tests the case when visible content change happens through transition and there's a pending style update.</title>
>+<script src="../../../../../resources/ui-helper.js"></script>
>+<style>
>+
>+#tapThis {
>+    width: 400px;
>+    height: 400px;
>+    border: 1px solid green;
>+    -webkit-user-modify: read-write;
>+}
>+
>+#becomesVisible {
>+    visibility: hidden;
>+    opacity: 0.8;
>+    width: 100px;
>+    height: 100px;
>+    background-color: green;
>+    transition: opacity 1ms;
>+}
>+</style>
>+<script>
>+async function test() {
>+    if (!window.testRunner || !testRunner.runUIScript)
>+        return;
>+    if (window.internals)
>+        internals.settings.setContentChangeObserverEnabled(true);
>+
>+    testRunner.waitUntilDone();
>+    testRunner.dumpAsText();
>+
>+    await UIHelper.activateElement(tapThis);
>+}
>+</script>
>+</head>
>+<body onload="test()">
>+<div id=tapThis>PASS if no crash or assert.</div>
>+<div id=becomesVisible></div>
>+<script>
>+tapThis.addEventListener("touchstart", function( event ) {
>+    becomesVisible.style.visibility = "visible";
>+    becomesVisible.style.opacity = "1";
>+}, false);
>+
>+tapThis.addEventListener("click", function( event ) {
>+    if (window.testRunner)
>+        testRunner.notifyDone();
>+}, false);
>+</script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 199506: 373475 | 373476 | 373477