[ContentChangeObserver] Check for pending style recalcs at the end of each timer...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Mar 2019 22:48:54 +0000 (22:48 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 1 Mar 2019 22:48:54 +0000 (22:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195220
<rdar://problem/48518979>

Reviewed by Simon Fraser.

Source/WebCore:

didScheduleStyleRecalc callback was introduced to see if a style recalc is scheduled while firing the DOM timer. However it does not handle the case
when in addition to this style recalc scheduling, something later (though during the same timer firing) triggers a sync style recalc.
Let's just check if we've got a pending style recalc when the DOM timer comes back.

Test: fast/events/touch/ios/style-recalc-schedule-and-force-relalc.html

* dom/Document.cpp:
(WebCore::Document::scheduleStyleRecalc):
* page/ios/ContentChangeObserver.cpp:
(WebCore::hasPendingStyleRecalc):
(WebCore::ContentChangeObserver::startObservingDOMTimerExecute):
(WebCore::ContentChangeObserver::stopObservingDOMTimerExecute):
(WebCore::ContentChangeObserver::startObservingContentChanges):
(WebCore::ContentChangeObserver::didScheduleStyleRecalc): Deleted.
* page/ios/ContentChangeObserver.h:
(WebCore::ContentChangeObserver::startObservingStyleRecalcScheduling): Deleted.
(WebCore::ContentChangeObserver::stopObservingStyleRecalcScheduling): Deleted.
(WebCore::ContentChangeObserver::isObservingStyleRecalcScheduling const): Deleted.

LayoutTests:

* fast/events/touch/ios/style-recalc-schedule-and-force-relalc-expected.txt: Added.
* fast/events/touch/ios/style-recalc-schedule-and-force-relalc.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242290 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/events/touch/ios/style-recalc-schedule-and-force-relalc-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/style-recalc-schedule-and-force-relalc.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/page/ios/ContentChangeObserver.cpp
Source/WebCore/page/ios/ContentChangeObserver.h

index 4c60e0c..ae86250 100644 (file)
@@ -1,3 +1,14 @@
+2019-03-01  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Check for pending style recalcs at the end of each timer run.
+        https://bugs.webkit.org/show_bug.cgi?id=195220
+        <rdar://problem/48518979>
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/touch/ios/style-recalc-schedule-and-force-relalc-expected.txt: Added.
+        * fast/events/touch/ios/style-recalc-schedule-and-force-relalc.html: Added.
+
 2019-03-01  John Wilander  <wilander@apple.com>
 
         Resource Load Statistics: Further restrict client-side cookie persistence after cross-site navigations with link decoration
diff --git a/LayoutTests/fast/events/touch/ios/style-recalc-schedule-and-force-relalc-expected.txt b/LayoutTests/fast/events/touch/ios/style-recalc-schedule-and-force-relalc-expected.txt
new file mode 100644 (file)
index 0000000..677e3a3
--- /dev/null
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is shown below.
+clicked
diff --git a/LayoutTests/fast/events/touch/ios/style-recalc-schedule-and-force-relalc.html b/LayoutTests/fast/events/touch/ios/style-recalc-schedule-and-force-relalc.html
new file mode 100644 (file)
index 0000000..8d53c6e
--- /dev/null
@@ -0,0 +1,66 @@
+<html>
+<head>
+<title>This test that we trigger hover when the content change starts as async but it turns into a sync style recalc.</title>
+<script src="../../../../resources/basic-gestures.js"></script>
+<style>
+#tapthis {
+    width: 400px;
+    height: 400px;
+    border: 1px solid green;
+}
+
+#staysHidden {
+    visibility: hidden;
+    width: 100px;
+    height: 100px;
+    background-color: green;
+}
+</style>
+<script>
+async function test() {
+    if (!window.testRunner || !testRunner.runUIScript)
+        return;
+
+    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 'clicked' text is shown below.</div>
+<div id=staysHidden></div>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mouseover", function( event ) {
+    // 1. Install a timer on hover
+    setTimeout(function() {
+        // 2. Trigger some non-forcing (non-visibility) style change with forcing offsetHeight.
+        staysHidden.style.marginLeft = "5px";
+
+        document.body.offsetHeight;
+    }, 0);
+    // 3. Install a longer, empty timer.
+    setTimeout(function() {
+    }, 5);
+}, false);
+
+staysHidden.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked";
+    setTimeout(function() {
+        if (window.testRunner)
+            testRunner.notifyDone();
+    }, 0);
+}, false);
+</script>
+</body>
+</html>
index 0eac3da..3650ccd 100644 (file)
@@ -1,3 +1,30 @@
+2019-03-01  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Check for pending style recalcs at the end of each timer run.
+        https://bugs.webkit.org/show_bug.cgi?id=195220
+        <rdar://problem/48518979>
+
+        Reviewed by Simon Fraser.
+
+        didScheduleStyleRecalc callback was introduced to see if a style recalc is scheduled while firing the DOM timer. However it does not handle the case
+        when in addition to this style recalc scheduling, something later (though during the same timer firing) triggers a sync style recalc.
+        Let's just check if we've got a pending style recalc when the DOM timer comes back.
+
+        Test: fast/events/touch/ios/style-recalc-schedule-and-force-relalc.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::scheduleStyleRecalc):
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::hasPendingStyleRecalc):
+        (WebCore::ContentChangeObserver::startObservingDOMTimerExecute):
+        (WebCore::ContentChangeObserver::stopObservingDOMTimerExecute):
+        (WebCore::ContentChangeObserver::startObservingContentChanges):
+        (WebCore::ContentChangeObserver::didScheduleStyleRecalc): Deleted.
+        * page/ios/ContentChangeObserver.h:
+        (WebCore::ContentChangeObserver::startObservingStyleRecalcScheduling): Deleted.
+        (WebCore::ContentChangeObserver::stopObservingStyleRecalcScheduling): Deleted.
+        (WebCore::ContentChangeObserver::isObservingStyleRecalcScheduling const): Deleted.
+
 2019-03-01  John Wilander  <wilander@apple.com>
 
         Resource Load Statistics: Further restrict client-side cookie persistence after cross-site navigations with link decoration
index 6b6f0ce..c60d5f6 100644 (file)
@@ -1833,10 +1833,6 @@ void Document::scheduleStyleRecalc()
         return;
 
     m_styleRecalcTimer.startOneShot(0_s);
-#if PLATFORM(IOS_FAMILY)
-    if (auto* page = this->page())
-        page->contentChangeObserver().didScheduleStyleRecalc();
-#endif
 
     InspectorInstrumentation::didScheduleStyleRecalculation(*this);
 }
index dec9b0e..0ad2591 100644 (file)
 
 namespace WebCore {
 
+static bool hasPendingStyleRecalc(const Page& page)
+{
+    for (auto* frame = &page.mainFrame(); frame; frame = frame->tree().traverseNext()) {
+        if (auto* document = frame->document()) {
+            if (document->hasPendingStyleRecalc())
+                return true;
+        }
+    }
+    return false;
+}
+
 ContentChangeObserver::ContentChangeObserver(Page& page)
     : m_page(page)
 {
@@ -71,7 +82,6 @@ void ContentChangeObserver::startObservingDOMTimerExecute(const DOMTimer& timer)
     if (!containsObservedDOMTimer(timer))
         return;
     LOG_WITH_STREAM(ContentObservation, stream << "startObservingDOMTimerExecute: start observing (" << &timer << ") timer callback.");
-    startObservingStyleRecalcScheduling();
     m_isObservingContentChanges = true;
 }
 
@@ -79,32 +89,26 @@ void ContentChangeObserver::stopObservingDOMTimerExecute(const DOMTimer& timer)
 {
     if (!containsObservedDOMTimer(timer))
         return;
+    LOG_WITH_STREAM(ContentObservation, stream << "stopObservingDOMTimerExecute: stop observing (" << &timer << ") timer callback.");
+
     removeObservedDOMTimer(timer);
-    stopObservingStyleRecalcScheduling();
     stopObservingContentChanges();
     auto observedContentChange = this->observedContentChange();
+    auto hasPendingStyleRecalc = WebCore::hasPendingStyleRecalc(m_page);
     // Check if the timer callback triggered either a sync or async style update.
-    auto inDeterminedState = observedContentChange == WKContentVisibilityChange || (!countOfObservedDOMTimers() && observedContentChange == WKContentNoChange);  
-
-    LOG_WITH_STREAM(ContentObservation, stream << "stopObservingDOMTimerExecute: stop observing (" << &timer << ") timer callback.");
-    if (inDeterminedState) {
+    auto hasDeterminedState = observedContentChange == WKContentVisibilityChange || (!countOfObservedDOMTimers() && observedContentChange == WKContentNoChange && !hasPendingStyleRecalc);  
+    if (hasDeterminedState) {
         LOG_WITH_STREAM(ContentObservation, stream << "stopObservingDOMTimerExecute: (" << &timer << ") in determined state.");
         m_page.chrome().client().observedContentChange(m_page.mainFrame());
-    } else if (observedContentChange == WKContentIndeterminateChange) {
+        return;
+    }
+    if (hasPendingStyleRecalc) {
         // An async style recalc has been scheduled. Let's observe it.
         LOG_WITH_STREAM(ContentObservation, stream << "stopObservingDOMTimerExecute: (" << &timer << ") wait until next style recalc fires.");
         setShouldObserveStyleRecalc(true);
     }
 }
 
-void ContentChangeObserver::didScheduleStyleRecalc()
-{
-    if (!isObservingStyleRecalcScheduling())
-        return;
-    LOG(ContentObservation, "didScheduleStyleRecalc: register this style recalc schedule and observe when it fires.");
-    setObservedContentChange(WKContentIndeterminateChange);
-}
-
 void ContentChangeObserver::startObservingStyleRecalc()
 {
     if (!shouldObserveStyleRecalc())
@@ -119,8 +123,8 @@ void ContentChangeObserver::stopObservingStyleRecalc()
         return;
     LOG(ContentObservation, "stopObservingStyleRecalc: stop observing style recalc");
     setShouldObserveStyleRecalc(false);
-    auto inDeterminedState = observedContentChange() == WKContentVisibilityChange || !countOfObservedDOMTimers();
-    if (!inDeterminedState) {
+    auto hasDeterminedState = observedContentChange() == WKContentVisibilityChange || !countOfObservedDOMTimers();
+    if (!hasDeterminedState) {
         LOG(ContentObservation, "stopObservingStyleRecalc: can't decided it yet.");
         return;
     }
@@ -154,6 +158,7 @@ void ContentChangeObserver::didContentVisibilityChange()
 
 void ContentChangeObserver::startObservingContentChanges()
 {
+    ASSERT(!hasPendingStyleRecalc(m_page));
     startObservingDOMTimerScheduling();
     resetObservedContentChange();
     clearObservedDOMTimers();
index 67b5f2a..7ab820d 100644 (file)
@@ -45,7 +45,6 @@ public:
     void didInstallDOMTimer(const DOMTimer&, Seconds timeout, bool singleShot);
     void didRemoveDOMTimer(const DOMTimer&);
     void didContentVisibilityChange();
-    void didScheduleStyleRecalc();
     void didSuspendActiveDOMObjects();
     void willDetachPage();
 
@@ -95,10 +94,6 @@ private:
     void removeObservedDOMTimer(const DOMTimer&);
     bool containsObservedDOMTimer(const DOMTimer& timer) const { return m_DOMTimerList.contains(&timer); }
 
-    void startObservingStyleRecalcScheduling() { m_isObservingStyleRecalcScheduling = true; }
-    void stopObservingStyleRecalcScheduling() { m_isObservingStyleRecalcScheduling = false; }
-    bool isObservingStyleRecalcScheduling() const { return m_isObservingStyleRecalcScheduling; }
-
     void setShouldObserveStyleRecalc(bool shouldObserve) { m_shouldObserveStyleRecalc = shouldObserve; }
     bool shouldObserveStyleRecalc() const { return m_shouldObserveStyleRecalc; }
 
@@ -114,7 +109,6 @@ private:
     Page& m_page;
     HashSet<const DOMTimer*> m_DOMTimerList;
     bool m_shouldObserveStyleRecalc { false };
-    bool m_isObservingStyleRecalcScheduling { false };
     bool m_isObservingDOMTimerScheduling { false };
     bool m_isObservingContentChanges { false };
 };