[ContentChangeObserver] Fix failing test cases
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Mar 2019 20:08:54 +0000 (20:08 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Mar 2019 20:08:54 +0000 (20:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195524
<rdar://problem/48745101>

Reviewed by Simon Fraser.

Source/WebCore:

1. Do not start DOM timer install observation when we already detected change at touchstart.
2. hasPendingActivity() should only care about ContentChangeObserver flags.
3. Do not try to notify the client when we are in the mouseMoved dispatch call (currently it could happen
when a timer gets intalled and removed right away).

* page/ios/ContentChangeObserver.cpp:
(WebCore::ContentChangeObserver::adjustObservedState):
(WebCore::ContentChangeObserver::isNotifyContentChangeAllowed const): Deleted.
* page/ios/ContentChangeObserver.h:
(WebCore::ContentChangeObserver::hasPendingActivity const):
(WebCore::ContentChangeObserver::isObservationTimeWindowActive const):

LayoutTests:

They've been failing ever since the 32ms fixed time window was introduced.

* fast/events/touch/ios/content-observation/click-instead-of-hover-simple.html:
* fast/events/touch/ios/content-observation/stuck-with-hover-state.html:

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

LayoutTests/ChangeLog
LayoutTests/fast/events/touch/ios/content-observation/click-instead-of-hover-simple.html
LayoutTests/fast/events/touch/ios/content-observation/stuck-with-hover-state.html
LayoutTests/fast/events/touch/ios/content-observation/visibility-change-happens-at-the-second-timer.html
LayoutTests/fast/events/touch/ios/content-observation/visibility-change-happens-on-timer-hops.html
Source/WebCore/ChangeLog
Source/WebCore/page/ios/ContentChangeObserver.cpp
Source/WebCore/page/ios/ContentChangeObserver.h

index d54176c..0d826af 100644 (file)
@@ -1,3 +1,16 @@
+2019-03-10  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Fix failing test cases
+        https://bugs.webkit.org/show_bug.cgi?id=195524
+        <rdar://problem/48745101>
+
+        Reviewed by Simon Fraser.
+
+        They've been failing ever since the 32ms fixed time window was introduced. 
+
+        * fast/events/touch/ios/content-observation/click-instead-of-hover-simple.html:
+        * fast/events/touch/ios/content-observation/stuck-with-hover-state.html:
+
 2019-03-10  Simon Fraser  <simon.fraser@apple.com>
 
         Mark two tests as failing after r242624
index 7c59f31..b017eab 100644 (file)
@@ -25,7 +25,7 @@ async function test() {
 
        await tapAtPoint(x, y);
 
-    testRunner.notifyDone();
+    setTimeout("testRunner.notifyDone()", 50);
 }
 </script>
 </head>
index 1640a18..339bb08 100644 (file)
@@ -50,8 +50,7 @@ tapthis.addEventListener("mouseover", function( event ) {
     setTimeout(function() {
         becomesVisible.style.visibility = "visible";
         document.body.offsetHeight;
-        if (window.testRunner)
-            setTimeout(testRunner.notifyDone(), 0);
+        setTimeout("testRunner.notifyDone()", 0);
     }, 20);
 }, false);
 
index 2e5b004..83b48f4 100644 (file)
@@ -50,8 +50,7 @@ tapthis.addEventListener("mouseover", function( event ) {
         setTimeout(function() {
             becomesVisible.style.visibility = "visible";
             document.body.offsetHeight;
-            if (window.testRunner)
-                setTimeout(testRunner.notifyDone(), 0);
+            setTimeout("testRunner.notifyDone()", 0);
         }, 10);
     }, 0);
 }, false);
index 7dce179..fa44d20 100644 (file)
@@ -1,3 +1,23 @@
+2019-03-10  Zalan Bujtas  <zalan@apple.com>
+
+        [ContentChangeObserver] Fix failing test cases
+        https://bugs.webkit.org/show_bug.cgi?id=195524
+        <rdar://problem/48745101>
+
+        Reviewed by Simon Fraser.
+
+        1. Do not start DOM timer install observation when we already detected change at touchstart.
+        2. hasPendingActivity() should only care about ContentChangeObserver flags.
+        3. Do not try to notify the client when we are in the mouseMoved dispatch call (currently it could happen
+        when a timer gets intalled and removed right away).
+
+        * page/ios/ContentChangeObserver.cpp:
+        (WebCore::ContentChangeObserver::adjustObservedState):
+        (WebCore::ContentChangeObserver::isNotifyContentChangeAllowed const): Deleted.
+        * page/ios/ContentChangeObserver.h:
+        (WebCore::ContentChangeObserver::hasPendingActivity const):
+        (WebCore::ContentChangeObserver::isObservationTimeWindowActive const):
+
 2019-03-10  Simon Fraser  <simon.fraser@apple.com>
 
         ScrollingTree should have the final say on where layers go
index fdd0f12..002a2d9 100644 (file)
@@ -225,13 +225,6 @@ bool ContentChangeObserver::hasDeterminateState() const
     return observedContentChange() == WKContentNoChange && !hasPendingActivity();
 }
 
-#if !ASSERT_DISABLED
-bool ContentChangeObserver::isNotifyContentChangeAllowed() const
-{
-    return m_document.settings().contentChangeObserverEnabled() && !m_mouseMovedEventIsBeingDispatched;
-}
-#endif
-
 void ContentChangeObserver::adjustObservedState(Event event)
 {
     auto adjustStateAndNotifyContentChangeIfNeeded = [&] {
@@ -239,12 +232,16 @@ void ContentChangeObserver::adjustObservedState(Event event)
         if (observedContentChange() == WKContentIndeterminateChange && !hasPendingActivity())
             setHasNoChangeState();
 
+        // Do not notify the client unless we couldn't make the decision synchronously.
+        if (m_mouseMovedEventIsBeingDispatched) {
+            LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: in mouseMoved call. No need to notify the client.");
+            return;
+        }
         if (!hasDeterminateState()) {
-            LOG(ContentObservation, "notifyContentChangeIfNeeded: not in a determined state yet.");
+            LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: not in a determined state yet.");
             return;
         }
-        LOG_WITH_STREAM(ContentObservation, stream << "notifyContentChangeIfNeeded: sending observedContentChange ->" << observedContentChange());
-        ASSERT(isNotifyContentChangeAllowed());
+        LOG_WITH_STREAM(ContentObservation, stream << "adjustStateAndNotifyContentChangeIfNeeded: sending observedContentChange ->" << observedContentChange());
         ASSERT(m_document.page());
         ASSERT(m_document.frame());
         m_document.page()->chrome().client().observedContentChange(*m_document.frame());
@@ -265,9 +262,10 @@ void ContentChangeObserver::adjustObservedState(Event event)
         if (!isBetweenTouchEndAndMouseMoved()) {
             setHasNoChangeState();
             clearObservedDOMTimers();
-        }
+            setShouldObserveDOMTimerScheduling(true);
+        } else
+            setShouldObserveDOMTimerScheduling(!hasVisibleChangeState());
         setIsBetweenTouchEndAndMouseMoved(false);
-        setShouldObserveDOMTimerScheduling(true);
         break;
     case Event::EndedMouseMovedEventDispatching:
         setShouldObserveDOMTimerScheduling(false);
index 59c6341..f7a465d 100644 (file)
@@ -139,11 +139,8 @@ private:
     void setIsBetweenTouchEndAndMouseMoved(bool isBetween) { m_isBetweenTouchEndAndMouseMoved = isBetween; }
     bool isBetweenTouchEndAndMouseMoved() const { return m_isBetweenTouchEndAndMouseMoved; }
 
-    bool hasPendingActivity() const { return hasObservedDOMTimer() || m_document.hasPendingStyleRecalc() || isObservationTimeWindowActive(); }
+    bool hasPendingActivity() const { return hasObservedDOMTimer() || m_isWaitingForStyleRecalc || isObservationTimeWindowActive(); }
     bool isObservationTimeWindowActive() const { return m_contentObservationTimer.isActive(); }
-#if !ASSERT_DISABLED
-    bool isNotifyContentChangeAllowed() const;
-#endif
 
     void completeDurationBasedContentObservation();