Attachment 373476 Details for Bug 199506 – Patch

[patch] Patch

bug-199506-20190704205407.patch (text/plain), 6.65 KB, created by zalan on 2019-07-04 20:54:12 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-07-04 20:54:12 PDT

Size: 6.65 KB

patch

obsolete

>Subversion Revision: 247143
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 3fa7fce12486106396578cff4134fedb140c4e1c..67dec25f505840f050d38764b2f8536388f9b3f6 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-03  Simon Fraser  <simon.fraser@apple.com>
> 
>         Some layers inside overflow:scroll don't move back to correct positions when the contents are shrunk
>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 12a91e384f0ff92798b72dda850044946dfdcbc5..f88682d2bb8ef152a8049db940f59d7b65012e2d 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-03  Simon Fraser  <simon.fraser@apple.com>
> 
>         Some layers inside overflow:scroll don't move back to correct positions when the contents are shrunk
>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..85b817572adce4b0763890e4864013a787170325
>--- /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