Title: [242321] trunk
Revision
242321
Author
[email protected]
Date
2019-03-02 16:15:11 -0800 (Sat, 02 Mar 2019)

Log Message

[ContentChangeObserver] Introduce ContentChangeObserver::adjustObservedState
https://bugs.webkit.org/show_bug.cgi?id=195244
<rdar://problem/48536737>

Reviewed by Simon Fraser.

Source/WebCore:

Move state change handling code to adjustObservedState() and introduce signalContentChangeIfNeeded() to
let the client know about the state change (or lack of state change).

Test: fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html

* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::didInstallDOMTimer):
(WebCore::ContentChangeObserver::didRemoveDOMTimer):
(WebCore::ContentChangeObserver::stopObservingDOMTimerExecute):
(WebCore::ContentChangeObserver::stopObservingStyleRecalc):
(WebCore::ContentChangeObserver::clearTimersAndReportContentChange):
(WebCore::ContentChangeObserver::didContentVisibilityChange):
(WebCore::ContentChangeObserver::addObservedDOMTimer):
(WebCore::ContentChangeObserver::removeObservedDOMTimer):
(WebCore::ContentChangeObserver::setShouldObserveStyleRecalc):
(WebCore::ContentChangeObserver::adjustObservedState):
(WebCore::ContentChangeObserver::signalContentChangeIfNeeded):
* page/ios/ContentChangeObserver.h:
(WebCore::ContentChangeObserver::isObservingDOMTimerScheduling const):
(WebCore::ContentChangeObserver::addObservedDOMTimer): Deleted.
(WebCore::ContentChangeObserver::setShouldObserveStyleRecalc): Deleted.

LayoutTests:

* fast/events/touch/ios/visibility-change-happens-at-the-second-timer-expected.txt: Added.
* fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (242320 => 242321)


--- trunk/LayoutTests/ChangeLog	2019-03-03 00:02:34 UTC (rev 242320)
+++ trunk/LayoutTests/ChangeLog	2019-03-03 00:15:11 UTC (rev 242321)
@@ -1,3 +1,14 @@
+2019-03-02  Zalan Bujtas  <[email protected]>
+
+        [ContentChangeObserver] Introduce ContentChangeObserver::adjustObservedState
+        https://bugs.webkit.org/show_bug.cgi?id=195244
+        <rdar://problem/48536737>
+
+        Reviewed by Simon Fraser.
+
+        * fast/events/touch/ios/visibility-change-happens-at-the-second-timer-expected.txt: Added.
+        * fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html: Added.
+
 2019-03-02  Wenson Hsieh  <[email protected]>
 
         [iOS] Programmatic paste access should be granted when copying and pasting within the same origin

Added: trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer-expected.txt (0 => 242321)


--- trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer-expected.txt	2019-03-03 00:15:11 UTC (rev 242321)
@@ -0,0 +1,2 @@
+PASS if 'clicked' text is not shown below.
+

Added: trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html (0 => 242321)


--- trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html	2019-03-03 00:15:11 UTC (rev 242321)
@@ -0,0 +1,65 @@
+<html>
+<head>
+<title>This tests the case when the second timer triggers visible content change</title>
+<script src=""
+<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 2 timers on hover
+    setTimeout(function() {
+        // 2. Trigger some non-visibility style change with forcing offsetHeight.
+        becomesVisible.style.marginLeft = "5px";
+        document.body.offsetHeight;
+    }, 0);
+    // 3. Install a longer timer with visibility change.
+    setTimeout(function() {
+        becomesVisible.style.visibility = "visible";
+        document.body.offsetHeight;
+        if (window.testRunner)
+            setTimeout(testRunner.notifyDone(), 0);
+    }, 20);
+}, false);
+
+becomesVisible.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked hidden";
+}, false);
+
+tapthis.addEventListener("click", function( event ) {   
+    result.innerHTML = "clicked";
+}, false);
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (242320 => 242321)


--- trunk/Source/WebCore/ChangeLog	2019-03-03 00:02:34 UTC (rev 242320)
+++ trunk/Source/WebCore/ChangeLog	2019-03-03 00:15:11 UTC (rev 242321)
@@ -1,5 +1,35 @@
 2019-03-02  Zalan Bujtas  <[email protected]>
 
+        [ContentChangeObserver] Introduce ContentChangeObserver::adjustObservedState
+        https://bugs.webkit.org/show_bug.cgi?id=195244
+        <rdar://problem/48536737>
+
+        Reviewed by Simon Fraser.
+
+        Move state change handling code to adjustObservedState() and introduce signalContentChangeIfNeeded() to
+        let the client know about the state change (or lack of state change).
+
+        Test: fast/events/touch/ios/visibility-change-happens-at-the-second-timer.html
+
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::ContentChangeObserver::didInstallDOMTimer):
+        (WebCore::ContentChangeObserver::didRemoveDOMTimer):
+        (WebCore::ContentChangeObserver::stopObservingDOMTimerExecute):
+        (WebCore::ContentChangeObserver::stopObservingStyleRecalc):
+        (WebCore::ContentChangeObserver::clearTimersAndReportContentChange):
+        (WebCore::ContentChangeObserver::didContentVisibilityChange):
+        (WebCore::ContentChangeObserver::addObservedDOMTimer):
+        (WebCore::ContentChangeObserver::removeObservedDOMTimer):
+        (WebCore::ContentChangeObserver::setShouldObserveStyleRecalc):
+        (WebCore::ContentChangeObserver::adjustObservedState):
+        (WebCore::ContentChangeObserver::signalContentChangeIfNeeded):
+        * page/ios/ContentChangeObserver.h:
+        (WebCore::ContentChangeObserver::isObservingDOMTimerScheduling const):
+        (WebCore::ContentChangeObserver::addObservedDOMTimer): Deleted.
+        (WebCore::ContentChangeObserver::setShouldObserveStyleRecalc): Deleted.
+
+2019-03-02  Zalan Bujtas  <[email protected]>
+
         [ContentChangeObserver] Move away from WKContentChange values
         https://bugs.webkit.org/show_bug.cgi?id=195240
         <rdar://problem/48532358>

Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp (242320 => 242321)


--- trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp	2019-03-03 00:02:34 UTC (rev 242320)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp	2019-03-03 00:15:11 UTC (rev 242321)
@@ -65,8 +65,7 @@
         return;
     LOG_WITH_STREAM(ContentObservation, stream << "didInstallDOMTimer: register this timer: (" << &timer << ") and observe when it fires.");
 
-    setHasIndeterminateState();
-    addObservedDOMTimer(timer);
+    registerDOMTimer(timer);
 }
 
 void ContentChangeObserver::didRemoveDOMTimer(const DOMTimer& timer)
@@ -75,10 +74,8 @@
         return;
     LOG_WITH_STREAM(ContentObservation, stream << "removeDOMTimer: remove registered timer (" << &timer << ")");
 
-    removeObservedDOMTimer(timer);
-    // FIXME: Just because this is the last timer, it does not mean we are in a determined state.
-    if (!hasObservedDOMTimer())
-        m_page.chrome().client().observedContentChange(m_page.mainFrame());
+    unregisterDOMTimer(timer);
+    notifyContentChangeIfNeeded();
 }
 
 void ContentChangeObserver::startObservingDOMTimerExecute(const DOMTimer& timer)
@@ -96,19 +93,10 @@
         return;
     LOG_WITH_STREAM(ContentObservation, stream << "stopObservingDOMTimerExecute: stop observing (" << &timer << ") timer callback.");
 
-    removeObservedDOMTimer(timer);
-    stopObservingContentChanges();
-    // Check if the timer callback triggered either a sync or async style update.
-    if (hasDeterminateState()) {
-        LOG_WITH_STREAM(ContentObservation, stream << "stopObservingDOMTimerExecute: (" << &timer << ") in determined state.");
-        m_page.chrome().client().observedContentChange(m_page.mainFrame());
-        return;
-    }
-    if (WebCore::hasPendingStyleRecalc(m_page)) {
-        // 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);
-    }
+    m_isObservingContentChanges = false;
+    unregisterDOMTimer(timer);
+    setShouldObserveStyleRecalc(WebCore::hasPendingStyleRecalc(m_page));
+    notifyContentChangeIfNeeded();
 }
 
 void ContentChangeObserver::startObservingStyleRecalc()
@@ -129,12 +117,8 @@
     LOG(ContentObservation, "stopObservingStyleRecalc: stop observing style recalc");
 
     setShouldObserveStyleRecalc(false);
-    if (hasDeterminateState()) {
-        LOG(ContentObservation, "stopObservingStyleRecalc: has determinate state, report content change.");
-        m_page.chrome().client().observedContentChange(m_page.mainFrame());
-        return;
-    }
-    LOG(ContentObservation, "stopObservingStyleRecalc: can't decide it yet.");
+    adjustObservedState(Event::StyleRecalcFinished);
+    notifyContentChangeIfNeeded();
 }
 
 void ContentChangeObserver::clearTimersAndReportContentChange()
@@ -141,7 +125,7 @@
 {
     if (!hasObservedDOMTimer())
         return;
-    LOG(ContentObservation, "clearTimersAndReportContentChange: remove registered timers and report content change.");
+    LOG_WITH_STREAM(ContentObservation, stream << "clearTimersAndReportContentChange: remove registered timers and report content change." << observedContentChange());
 
     clearObservedDOMTimers();
     m_page.chrome().client().observedContentChange(m_page.mainFrame());
@@ -157,18 +141,19 @@
     clearTimersAndReportContentChange();
 }
 
-void ContentChangeObserver::didContentVisibilityChange()
+void ContentChangeObserver::contentVisibilityDidChange()
 {
-    setHasVisibleChangeState();
+    LOG(ContentObservation, "contentVisibilityDidChange: visible content change did happen.");
+    adjustObservedState(Event::ContentVisibilityChanged);
 }
 
 void ContentChangeObserver::startObservingContentChanges()
 {
     ASSERT(!hasPendingStyleRecalc(m_page));
+    clearObservedDOMTimers();
     startObservingDOMTimerScheduling();
-    setHasNoChangeState();
-    clearObservedDOMTimers();
     m_isObservingContentChanges = true;
+    adjustObservedState(Event::ContentObservationStarted);
 }
 
 void ContentChangeObserver::stopObservingContentChanges()
@@ -182,14 +167,25 @@
     return WKObservedContentChange();
 }
 
-void ContentChangeObserver::removeObservedDOMTimer(const DOMTimer& timer)
+void ContentChangeObserver::registerDOMTimer(const DOMTimer& timer)
 {
+    m_DOMTimerList.add(&timer);
+    adjustObservedState(Event::InstalledDOMTimer);
+}
+
+void ContentChangeObserver::unregisterDOMTimer(const DOMTimer& timer)
+{
     m_DOMTimerList.remove(&timer);
-    // Force reset the content change flag when the last observed content modifier is removed. We should not be in an indeterminate state anymore.
-    if (!hasObservedDOMTimer() && observedContentChange() == WKContentIndeterminateChange)
-        setHasNoChangeState();
+    adjustObservedState(Event::RemovedDOMTimer);
 }
 
+void ContentChangeObserver::setShouldObserveStyleRecalc(bool shouldObserve)
+{
+    if (shouldObserve)
+        LOG(ContentObservation, "Wait until next style recalc fires.");
+    m_shouldObserveStyleRecalc = shouldObserve;
+}
+
 bool ContentChangeObserver::hasDeterminateState() const
 {
     if (hasVisibleChangeState())
@@ -197,6 +193,40 @@
     return observedContentChange() == WKContentNoChange && !hasObservedDOMTimer() && !hasPendingStyleRecalc(m_page);
 }
 
+void ContentChangeObserver::adjustObservedState(Event event)
+{
+    switch (event) {
+    case Event::ContentObservationStarted:
+        setHasNoChangeState();
+        break;
+    case Event::InstalledDOMTimer:
+        // Expecting a timer fire. Promote to an indeterminate state.
+        ASSERT(!hasVisibleChangeState());
+        setHasIndeterminateState();
+        break;
+    case Event::RemovedDOMTimer:
+    case Event::StyleRecalcFinished:
+        // Demote to "no change" when there's no pending activity anymore.
+        if (observedContentChange() == WKContentIndeterminateChange && !hasObservedDOMTimer() && !hasPendingStyleRecalc(m_page))
+            setHasNoChangeState();
+        break;
+    case Event::ContentVisibilityChanged:
+        setHasVisibleChangeState();
+        break;
+    }
+}
+
+void ContentChangeObserver::notifyContentChangeIfNeeded()
+{
+    if (!hasDeterminateState()) {
+        LOG(ContentObservation, "notifyContentChangeIfNeeded: not in a determined state yet.");
+        return;
+    }
+    LOG_WITH_STREAM(ContentObservation, stream << "notifyContentChangeIfNeeded: sending observedContentChange ->" << observedContentChange());
+    m_page.chrome().client().observedContentChange(m_page.mainFrame());
+}
+
+
 static Visibility elementImplicitVisibility(const Element& element)
 {
     auto* renderer = element.renderer();
@@ -254,7 +284,7 @@
     if ((m_previousDisplay == DisplayType::None && style->display() != DisplayType::None)
         || (m_previousVisibility == Visibility::Hidden && style->visibility() != Visibility::Hidden)
         || (m_previousImplicitVisibility == Visibility::Hidden && elementImplicitVisibility(m_element) == Visibility::Visible))
-        m_contentChangeObserver->didContentVisibilityChange();
+        m_contentChangeObserver->contentVisibilityDidChange();
 }
 
 ContentChangeObserver::StyleRecalcScope::StyleRecalcScope(Page* page)

Modified: trunk/Source/WebCore/page/ios/ContentChangeObserver.h (242320 => 242321)


--- trunk/Source/WebCore/page/ios/ContentChangeObserver.h	2019-03-03 00:02:34 UTC (rev 242320)
+++ trunk/Source/WebCore/page/ios/ContentChangeObserver.h	2019-03-03 00:15:11 UTC (rev 242321)
@@ -44,7 +44,7 @@
 
     void didInstallDOMTimer(const DOMTimer&, Seconds timeout, bool singleShot);
     void didRemoveDOMTimer(const DOMTimer&);
-    void didContentVisibilityChange();
+    void contentVisibilityDidChange();
     void didSuspendActiveDOMObjects();
     void willDetachPage();
 
@@ -89,12 +89,12 @@
     void startObservingStyleRecalc();
     void stopObservingStyleRecalc();
 
-    void addObservedDOMTimer(const DOMTimer& timer) { m_DOMTimerList.add(&timer); }
+    void registerDOMTimer(const DOMTimer&);
+    void unregisterDOMTimer(const DOMTimer&);
     bool isObservingDOMTimerScheduling() const { return m_isObservingDOMTimerScheduling; }
-    void removeObservedDOMTimer(const DOMTimer&);
     bool containsObservedDOMTimer(const DOMTimer& timer) const { return m_DOMTimerList.contains(&timer); }
 
-    void setShouldObserveStyleRecalc(bool shouldObserve) { m_shouldObserveStyleRecalc = shouldObserve; }
+    void setShouldObserveStyleRecalc(bool);
     bool shouldObserveStyleRecalc() const { return m_shouldObserveStyleRecalc; }
 
     bool isObservingContentChanges() const { return m_isObservingContentChanges; }
@@ -110,6 +110,17 @@
     bool hasObservedDOMTimer() const { return !m_DOMTimerList.isEmpty(); }
     bool hasDeterminateState() const;
 
+    void notifyContentChangeIfNeeded();
+
+    enum class Event {
+        ContentObservationStarted,
+        InstalledDOMTimer,
+        RemovedDOMTimer,
+        StyleRecalcFinished,
+        ContentVisibilityChanged
+    };
+    void adjustObservedState(Event);
+
     Page& m_page;
     HashSet<const DOMTimer*> m_DOMTimerList;
     bool m_shouldObserveStyleRecalc { false };
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to