Changeset 242689 in webkit

Timestamp:

Mar 10, 2019 1:08:54 PM (5 years ago)

Author:

Alan Bujtas

Message:

[ContentChangeObserver] Fix failing test cases
​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:

Location:

trunk

Files:

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

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/fast/events/touch/ios/content-observation/click-instead-of-hover-simple.html

@@ -26 +26 @@
         await tapAtPoint(x, y);
 
-    testRunner.notifyDone();
+    setTimeout("testRunner.notifyDone()", 50);
 }
 </script>

trunk/LayoutTests/fast/events/touch/ios/content-observation/stuck-with-hover-state.html

@@ -26 +26 @@
         await tapAtPoint(x, y);
 
-    testRunner.notifyDone();
+    setTimeout("testRunner.notifyDone()", 50);
 }
 </script>

trunk/LayoutTests/fast/events/touch/ios/content-observation/visibility-change-happens-at-the-second-timer.html

@@ -51 +51 @@
         becomesVisible.style.visibility = "visible";
         document.body.offsetHeight;
-        if (window.testRunner)
-            setTimeout(testRunner.notifyDone(), 0);
+        setTimeout("testRunner.notifyDone()", 0);
     }, 20);
 }, false);

trunk/LayoutTests/fast/events/touch/ios/content-observation/visibility-change-happens-on-timer-hops.html

@@ -51 +51 @@
             becomesVisible.style.visibility = "visible";
             document.body.offsetHeight;
-            if (window.testRunner)
-                setTimeout(testRunner.notifyDone(), 0);
+            setTimeout("testRunner.notifyDone()", 0);
         }, 10);
     }, 0);

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/page/ios/ContentChangeObserver.cpp

@@ -226 +226 @@
 }
 
-#if !ASSERT_DISABLED
-bool ContentChangeObserver::isNotifyContentChangeAllowed() const
-{
-    return m_document.settings().contentChangeObserverEnabled() && !m_mouseMovedEventIsBeingDispatched;
-}
-#endif
-
 void ContentChangeObserver::adjustObservedState(Event event)
 {
@@ -240 +233 @@
             setHasNoChangeState();
 
-        if (!hasDeterminateState()) {
-            LOG(ContentObservation, "notifyContentChangeIfNeeded: not in a determined state yet.");
+        // 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;
         }
-        LOG_WITH_STREAM(ContentObservation, stream << "notifyContentChangeIfNeeded: sending observedContentChange ->" << observedContentChange());
-        ASSERT(isNotifyContentChangeAllowed());
+        if (!hasDeterminateState()) {
+            LOG(ContentObservation, "adjustStateAndNotifyContentChangeIfNeeded: not in a determined state yet.");
+            return;
+        }
+        LOG_WITH_STREAM(ContentObservation, stream << "adjustStateAndNotifyContentChangeIfNeeded: sending observedContentChange ->" << observedContentChange());
         ASSERT(m_document.page());
         ASSERT(m_document.frame());
@@ -266 +263 @@
             setHasNoChangeState();
             clearObservedDOMTimers();
-        }
+            setShouldObserveDOMTimerScheduling(true);
+        } else
+            setShouldObserveDOMTimerScheduling(!hasVisibleChangeState());
         setIsBetweenTouchEndAndMouseMoved(false);
-        setShouldObserveDOMTimerScheduling(true);
         break;
     case Event::EndedMouseMovedEventDispatching:

trunk/Source/WebCore/page/ios/ContentChangeObserver.h

@@ -140 +140 @@
     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();