[ContentChangeObserver] Introduce fixed duration content observation
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Mar 2019 00:43:41 +0000 (00:43 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Mar 2019 00:43:41 +0000 (00:43 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195295
<rdar://problem/48579913>

Reviewed by Simon Fraser.

Source/WebCore:

Some pages have a runloop-like scheduling setup where the content triggering change happens at a nested timer firing.
This patch helps finding cases like that using a 32ms long fixed window. Currently nested timers get dropped on the floor and
we stop observing for content changes before they even get fired.

Test: fast/events/touch/ios/visibility-change-happens-on-timer-hops.html

* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::ContentChangeObserver):
(WebCore::ContentChangeObserver::startContentObservationForDuration):
(WebCore::ContentChangeObserver::stopDurationBasedContentObservation):
(WebCore::ContentChangeObserver::hasDeterminateState const):
(WebCore::ContentChangeObserver::adjustObservedState):
* page/ios/ContentChangeObserver.h:
(WebCore::ContentChangeObserver::isObservingContentChanges const):
(WebCore::ContentChangeObserver::hasPendingActivity const):

Source/WebKit:

* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::handleSyntheticClick):

LayoutTests:

* fast/events/touch/ios/visibility-change-happens-on-timer-hops-expected.txt: Added.
* fast/events/touch/ios/visibility-change-happens-on-timer-hops.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/events/touch/ios/visibility-change-happens-on-timer-hops-expected.txt [new file with mode: 0644]
LayoutTests/fast/events/touch/ios/visibility-change-happens-on-timer-hops.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/ios/ContentChangeObserver.cpp
Source/WebCore/page/ios/ContentChangeObserver.h
Source/WebKit/ChangeLog
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index 8e4a37d..d36954e 100644 (file)
@@ -1,3 +1,14 @@
+2019-03-04  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Introduce fixed duration content observation
+        https://bugs.webkit.org/show_bug.cgi?id=195295
+        <rdar://problem/48579913>
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/touch/ios/visibility-change-happens-on-timer-hops-expected.txt: Added.
+        * fast/events/touch/ios/visibility-change-happens-on-timer-hops.html: Added.
+
 2019-03-04  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Native text selection UI is incorrectly suppressed in Microsoft Visio
diff --git a/LayoutTests/fast/events/touch/ios/visibility-change-happens-on-timer-hops-expected.txt b/LayoutTests/fast/events/touch/ios/visibility-change-happens-on-timer-hops-expected.txt
new file mode 100644 (file)
index 0000000..66db4c9
--- /dev/null
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is not shown below.
+
diff --git a/LayoutTests/fast/events/touch/ios/visibility-change-happens-on-timer-hops.html b/LayoutTests/fast/events/touch/ios/visibility-change-happens-on-timer-hops.html
new file mode 100644 (file)
index 0000000..6077028
--- /dev/null
@@ -0,0 +1,66 @@
+<html>
+<head>
+<title>This tests the case when nested timers trigger visible content change</title>
+<script src="../../../../resources/basic-gestures.js"></script>
+<style>
+#tapthis {
+    width: 400px;
+    height: 400px;
+    border: 1px solid green;
+}
+
+#becomesVisible {
+    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 not shown below.</div>
+<div id=becomesVisible></div>
+<pre id=result></pre>
+<script>
+tapthis.addEventListener("mouseover", function( event ) {
+    // 1. Install a short timer on hover
+    setTimeout(function() {
+        // 2. Trigger some non-visibility style change with forcing offsetHeight.
+        becomesVisible.style.marginLeft = "5px";
+        document.body.offsetHeight;
+
+        // 3. Install a nested timer with visibility change.
+        setTimeout(function() {
+            becomesVisible.style.visibility = "visible";
+            document.body.offsetHeight;
+            if (window.testRunner)
+                setTimeout(testRunner.notifyDone(), 0);
+        }, 10);
+    }, 0);
+}, false);
+
+becomesVisible.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked";
+}, false);
+</script>
+</body>
+</html>
index 47b64f7..5899a53 100644 (file)
@@ -1,3 +1,27 @@
+2019-03-04  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Introduce fixed duration content observation
+        https://bugs.webkit.org/show_bug.cgi?id=195295
+        <rdar://problem/48579913>
+
+        Reviewed by Simon Fraser.
+
+        Some pages have a runloop-like scheduling setup where the content triggering change happens at a nested timer firing.
+        This patch helps finding cases like that using a 32ms long fixed window. Currently nested timers get dropped on the floor and
+        we stop observing for content changes before they even get fired.
+
+        Test: fast/events/touch/ios/visibility-change-happens-on-timer-hops.html
+
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::ContentChangeObserver::ContentChangeObserver):
+        (WebCore::ContentChangeObserver::startContentObservationForDuration):
+        (WebCore::ContentChangeObserver::stopDurationBasedContentObservation):
+        (WebCore::ContentChangeObserver::hasDeterminateState const):
+        (WebCore::ContentChangeObserver::adjustObservedState):
+        * page/ios/ContentChangeObserver.h:
+        (WebCore::ContentChangeObserver::isObservingContentChanges const):
+        (WebCore::ContentChangeObserver::hasPendingActivity const):
+
 2019-03-04  Wenson Hsieh  <wenson_hsieh@apple.com>
 
         Native text selection UI is incorrectly suppressed in Microsoft Visio
index 8b1cfe2..b079559 100644 (file)
@@ -38,9 +38,26 @@ namespace WebCore {
 
 ContentChangeObserver::ContentChangeObserver(Document& document)
     : m_document(document)
+    , m_contentObservationTimer([this] { stopDurationBasedContentObservation(); })
 {
 }
 
+void ContentChangeObserver::startContentObservationForDuration(Seconds duration)
+{
+    if (hasVisibleChangeState())
+        return;
+    LOG_WITH_STREAM(ContentObservation, stream << "startContentObservationForDuration: start observing the content for " << duration.milliseconds() << "ms");
+    adjustObservedState(Event::StartedFixedObservationTimeWindow);
+    m_contentObservationTimer.startOneShot(duration);
+}
+
+void ContentChangeObserver::stopDurationBasedContentObservation()
+{
+    LOG_WITH_STREAM(ContentObservation, stream << "stopDurationBasedContentObservation: stop duration based content observing ");
+    adjustObservedState(Event::EndedFixedObservationTimeWindow);
+    notifyContentChangeIfNeeded();
+}
+
 void ContentChangeObserver::didInstallDOMTimer(const DOMTimer& timer, Seconds timeout, bool singleShot)
 {
     if (m_document.activeDOMObjectsAreSuspended())
@@ -179,7 +196,7 @@ bool ContentChangeObserver::hasDeterminateState() const
 {
     if (hasVisibleChangeState())
         return true;
-    return observedContentChange() == WKContentNoChange && !hasObservedDOMTimer() && !m_document.hasPendingStyleRecalc();
+    return observedContentChange() == WKContentNoChange && !hasPendingActivity();
 }
 
 void ContentChangeObserver::adjustObservedState(Event event)
@@ -189,14 +206,16 @@ void ContentChangeObserver::adjustObservedState(Event event)
         setHasNoChangeState();
         break;
     case Event::InstalledDOMTimer:
+    case Event::StartedFixedObservationTimeWindow:
         // Expecting a timer fire. Promote to an indeterminate state.
         ASSERT(!hasVisibleChangeState());
         setHasIndeterminateState();
         break;
     case Event::RemovedDOMTimer:
     case Event::StyleRecalcFinished:
+    case Event::EndedFixedObservationTimeWindow:
         // Demote to "no change" when there's no pending activity anymore.
-        if (observedContentChange() == WKContentIndeterminateChange && !hasObservedDOMTimer() && !m_document.hasPendingStyleRecalc())
+        if (observedContentChange() == WKContentIndeterminateChange && !hasPendingActivity())
             setHasNoChangeState();
         break;
     case Event::ContentVisibilityChanged:
index 23d1250..170f2e7 100644 (file)
@@ -27,6 +27,7 @@
 
 #if PLATFORM(IOS_FAMILY)
 
+#include "Timer.h"
 #include "WKContentObservation.h"
 
 namespace WebCore {
@@ -38,6 +39,7 @@ class ContentChangeObserver {
 public:
     ContentChangeObserver(Document&);
 
+    WEBCORE_EXPORT void startContentObservationForDuration(Seconds duration);
     WEBCORE_EXPORT WKContentChange observedContentChange() const;
 
     void didInstallDOMTimer(const DOMTimer&, Seconds timeout, bool singleShot);
@@ -102,7 +104,7 @@ private:
     void setShouldObserveNextStyleRecalc(bool);
     bool isObservingStyleRecalc() const { return m_isObservingStyleRecalc; }
 
-    bool isObservingContentChanges() const { return m_domTimerisBeingExecuted || m_styleRecalcIsBeingExecuted; }
+    bool isObservingContentChanges() const { return m_domTimerisBeingExecuted || m_styleRecalcIsBeingExecuted || m_contentObservationTimer.isActive(); }
 
     void clearObservedDOMTimers() { m_DOMTimerList.clear(); }
     void clearTimersAndReportContentChange();
@@ -117,16 +119,23 @@ private:
 
     void notifyContentChangeIfNeeded();
 
+    void stopDurationBasedContentObservation();
+
+    bool hasPendingActivity() const { return hasObservedDOMTimer() || m_document.hasPendingStyleRecalc() || m_contentObservationTimer.isActive(); }
+
     enum class Event {
         ContentObservationStarted,
         InstalledDOMTimer,
         RemovedDOMTimer,
         StyleRecalcFinished,
-        ContentVisibilityChanged
+        ContentVisibilityChanged,
+        StartedFixedObservationTimeWindow,
+        EndedFixedObservationTimeWindow
     };
     void adjustObservedState(Event);
 
     Document& m_document;
+    Timer m_contentObservationTimer;
     HashSet<const DOMTimer*> m_DOMTimerList;
     bool m_isObservingStyleRecalc { false };
     bool m_styleRecalcIsBeingExecuted { false };
index 6188332..51635c2 100644 (file)
@@ -1,3 +1,14 @@
+2019-03-04  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Introduce fixed duration content observation
+        https://bugs.webkit.org/show_bug.cgi?id=195295
+        <rdar://problem/48579913>
+
+        Reviewed by Simon Fraser.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::handleSyntheticClick):
+
 2019-03-04  Truitt Savell  <tsavell@apple.com>
 
         Unreviewed, rolling out r242396.
index 42ee757..0105768 100644 (file)
@@ -552,6 +552,9 @@ void WebPage::handleSyntheticClick(Node& nodeRespondingToClick, const WebCore::F
         mainframe.document()->updateStyleIfNeeded();
     }
 
+    const Seconds observationDuration = 32_ms;
+    respondingDocument.contentChangeObserver().startContentObservationForDuration(observationDuration);
+
     m_pendingSyntheticClickNode = nullptr;
     m_pendingSyntheticClickLocation = FloatPoint();
     m_pendingSyntheticClickModifiers = { };