Timestamps should be the same for all rendering update steps
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Apr 2020 00:22:49 +0000 (00:22 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 28 Apr 2020 00:22:49 +0000 (00:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207153

Patch by Said Abou-Hallawa <sabouhallawa@apple.com> on 2020-04-27
Reviewed by Simon Fraser.

Source/WebCore:

The HTML 5 event loop sepcs states that timestamps should be the same for
all rendering update steps.

Specs link (step 9):
    https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-processing-model

This patch also fixes some issues in IntersectionObserver.

Test: intersection-observer/intersection-observer-callback-timestamp.html

* dom/Document.cpp:
(WebCore::Document::updateIntersectionObservations):
(WebCore::Document::notifyIntersectionObserversTimerFired): Deleted.
* dom/Document.h:
-- Handle the case when two floats are areEssentiallyEqual().
-- Execute the IntersectionObserver immediately and do not wait until the
   next CFRunLoop event since this does not implement the specs.

* page/DOMWindow.cpp:
(WebCore::DOMWindow::freezeNowTimestamp):
(WebCore::DOMWindow::unfreezeNowTimestamp):
(WebCore::DOMWindow::frozenNowTimestamp const):
* page/DOMWindow.h:
Provide a frozen now() timestamp in seconds to be used internally only.

* page/IntersectionObserver.cpp:
(WebCore::IntersectionObserver::nowTimestamp const):
Use the frozenNowTimestamp().

* page/Page.cpp:
(WebCore::Page::updateRendering):
Freeze the timestamps while serving the rendering update steps.

LayoutTests:

* animations/animation-callback-timestamp-expected.txt:
* animations/animation-callback-timestamp.html:
Ensure the rAF callback timestamp is less than Performance.now().

* animations/animation-multiple-callbacks-timestamp-expected.txt:
* animations/animation-multiple-callbacks-timestamp.html:
Ensure the rAF callbacks receive the same timestamps.

* intersection-observer/intersection-observer-callback-timestamp-expected.txt: Added.
* intersection-observer/intersection-observer-callback-timestamp.html: Added.
A new test to ensure the IntersectionObsever and the rAF callbacks receive the same timestamps.

* platform/ios-wk2/TestExpectations:
* platform/mac-wk1/TestExpectations:

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

16 files changed:
LayoutTests/ChangeLog
LayoutTests/animations/animation-callback-timestamp-expected.txt
LayoutTests/animations/animation-callback-timestamp.html
LayoutTests/animations/animation-multiple-callbacks-timestamp-expected.txt
LayoutTests/animations/animation-multiple-callbacks-timestamp.html
LayoutTests/intersection-observer/intersection-observer-callback-timestamp-expected.txt [new file with mode: 0644]
LayoutTests/intersection-observer/intersection-observer-callback-timestamp.html [new file with mode: 0644]
LayoutTests/platform/ios-wk2/TestExpectations
LayoutTests/platform/mac-wk1/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/page/DOMWindow.cpp
Source/WebCore/page/DOMWindow.h
Source/WebCore/page/IntersectionObserver.cpp
Source/WebCore/page/Page.cpp

index 8a38937..4ee7a9a 100644 (file)
@@ -1,3 +1,25 @@
+2020-04-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Timestamps should be the same for all rendering update steps
+        https://bugs.webkit.org/show_bug.cgi?id=207153
+
+        Reviewed by Simon Fraser.
+
+        * animations/animation-callback-timestamp-expected.txt:
+        * animations/animation-callback-timestamp.html:
+        Ensure the rAF callback timestamp is less than Performance.now().
+
+        * animations/animation-multiple-callbacks-timestamp-expected.txt:
+        * animations/animation-multiple-callbacks-timestamp.html:
+        Ensure the rAF callbacks receive the same timestamps.
+
+        * intersection-observer/intersection-observer-callback-timestamp-expected.txt: Added.
+        * intersection-observer/intersection-observer-callback-timestamp.html: Added.
+        A new test to ensure the IntersectionObsever and the rAF callbacks receive the same timestamps.
+
+        * platform/ios-wk2/TestExpectations:
+        * platform/mac-wk1/TestExpectations:
+
 2020-04-27  Ryan Haddad  <ryanhaddad@apple.com>
 
         [ macOS/iOS wk2 Debug ] webrtc/datachannel/multiple-connections.html is flaky timing out.
index 39c0f42..e9104be 100644 (file)
@@ -1,4 +1,18 @@
-PASS All the differences between requestAnimationFrame() callback timestamps and Performance.now() were within 3ms.
+Test performance.now() is at least as the timestamp the rAF callback.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Math.round(window.performance.now()) is >= Math.round(timestamp1)
+PASS Math.round(window.performance.now()) is >= Math.round(timestamp1)
+PASS Math.round(window.performance.now()) is >= Math.round(timestamp1)
+PASS Math.round(window.performance.now()) is >= Math.round(timestamp1)
+PASS Math.round(window.performance.now()) is >= Math.round(timestamp1)
+PASS Math.round(window.performance.now()) is >= Math.round(timestamp1)
+PASS Math.round(window.performance.now()) is >= Math.round(timestamp1)
+PASS Math.round(window.performance.now()) is >= Math.round(timestamp1)
+PASS Math.round(window.performance.now()) is >= Math.round(timestamp1)
+PASS Math.round(window.performance.now()) is >= Math.round(timestamp1)
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 6d45632..ecf62d6 100644 (file)
@@ -5,37 +5,26 @@
 </head>
 <body>
     <script>
+        var timestamp1 = 0;
         var currentFrame = 0;
-        var failed = false;
+        const MaxFrames = 10;
 
-        function finishTest()
-        {
-            if (failed)
-                testFailed("Some of the requestAnimationFrame() callback timestamps were larger than Performance.now() by more than 3ms.");
-            else
-                testPassed("All the differences between requestAnimationFrame() callback timestamps and Performance.now() were within 3ms.")
-            finishJSTest();
-        }
-        
         function doAnimation(timestamp)
         {
-            const Tolerance = 3;
-            const WarmupFrames = 5;
+            timestamp1 = timestamp;
+            shouldBeGreaterThanOrEqual("Math.round(window.performance.now())", "Math.round(timestamp1)");
 
-            var performanceNow = window.performance.now();
-            if (++currentFrame > WarmupFrames && Math.abs(timestamp - performanceNow) >= Tolerance) {
-                debug("requestAnimationFrame() timestamp = " + timestamp + ", window.performance.now() = " + performanceNow);
-                failed = true;
+            if (++currentFrame == MaxFrames) {
+                finishJSTest();
+                return;
             }
 
-            const MaxFrames = 25;
-            if (currentFrame == MaxFrames)
-                finishTest();
-            else
-                requestAnimationFrame(doAnimation);
+            requestAnimationFrame(doAnimation);
         }
-        
+
         window.jsTestIsAsync = true;
+
+        description("Test performance.now() is at least as the timestamp the rAF callback.");
         requestAnimationFrame(doAnimation);
     </script>
     <script src="../resources/js-test-post.js"></script>
index 1b2df23..9bde9cd 100644 (file)
@@ -1,4 +1,18 @@
-PASS All the multiple requestAnimationFrame() callbacks, which were fired at the same time, received the same timestamp.
+Test the timestamps of all the rAF callbacks are the same.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Math.round(timestamp1) is Math.round(timestamp2)
+PASS Math.round(timestamp1) is Math.round(timestamp2)
+PASS Math.round(timestamp1) is Math.round(timestamp2)
+PASS Math.round(timestamp1) is Math.round(timestamp2)
+PASS Math.round(timestamp1) is Math.round(timestamp2)
+PASS Math.round(timestamp1) is Math.round(timestamp2)
+PASS Math.round(timestamp1) is Math.round(timestamp2)
+PASS Math.round(timestamp1) is Math.round(timestamp2)
+PASS Math.round(timestamp1) is Math.round(timestamp2)
+PASS Math.round(timestamp1) is Math.round(timestamp2)
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 750e2c7..5139764 100644 (file)
@@ -5,43 +5,34 @@
 </head>
 <body>
     <script>
-        var currentFrame = 0;
         var timestamp1 = 0;
-        var failed = false;
-
-        function finishTest()
-        {
-            if (failed)
-                testFailed("Some of the multiple requestAnimationFrame() callbacks, which were fired at the same time, received different timestamps.");
-            else
-                testPassed("All the multiple requestAnimationFrame() callbacks, which were fired at the same time, received the same timestamp.");
-            finishJSTest();
-        }
+        var timestamp2 = 0;
+        var currentFrame = 0;
+        const MaxFrames = 10;
 
         function doAnimation1(timestamp)
         {
-            timestamp1 = timestamp;
-            
-            const MaxFrames = 25;
-            if (currentFrame == MaxFrames)
-                finishTest();
-            else {
-                requestAnimationFrame(doAnimation1);
-                requestAnimationFrame(doAnimation2);
+            if (++currentFrame > MaxFrames) {
+                finishJSTest();
+                return;
             }
+
+            timestamp1 = timestamp;
+            requestAnimationFrame(doAnimation1);
+            requestAnimationFrame(doAnimation2);
         }
 
         function doAnimation2(timestamp)
         {
-            const WarmupFrames = 5;
-            if (++currentFrame > WarmupFrames && timestamp != timestamp1) {
-                testFailed("timestamp = " + timestamp + ", timestamp1 = " + timestamp1  + ", window.performance.now() = " + window.performance.now());
-                failed = true;
-            }
+            timestamp2 = timestamp;
+            shouldBe("Math.round(timestamp1)", "Math.round(timestamp2)");
         }
-        
+
         window.jsTestIsAsync = true;
+
+        description("Test the timestamps of all the rAF callbacks are the same.");
         requestAnimationFrame(doAnimation1);
+        requestAnimationFrame(doAnimation2);
     </script>
     <script src="../resources/js-test-post.js"></script>
 </body>
diff --git a/LayoutTests/intersection-observer/intersection-observer-callback-timestamp-expected.txt b/LayoutTests/intersection-observer/intersection-observer-callback-timestamp-expected.txt
new file mode 100644 (file)
index 0000000..9fc61c4
--- /dev/null
@@ -0,0 +1,11 @@
+Test the IntersectionObserver timestamp is the same as the timestamp of the rAF callback.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Math.round(animateTimestamp) is Math.round(intersectTimestamp)
+PASS Math.round(animateTimestamp) is Math.round(intersectTimestamp)
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/intersection-observer/intersection-observer-callback-timestamp.html b/LayoutTests/intersection-observer/intersection-observer-callback-timestamp.html
new file mode 100644 (file)
index 0000000..e52ed9e
--- /dev/null
@@ -0,0 +1,99 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+    <script src="../resources/js-test-pre.js"></script>
+       <style>
+           #scrollable {
+           width: 100px;
+               height: 100px;
+               border: 1px solid black;
+               overflow-y: auto;
+       }
+       #container {
+               width: 100px;
+               height: 200px;
+       }
+       #box {
+               position: relative;
+               top: 100px;
+               width: 100px;
+               height: 100px;
+               background-color: green;
+       }
+       </style>
+</head>
+<body>
+    <div id="scrollable">
+        <div id="container">
+            <div id="box"></div>
+        </div>
+    </div>
+    <script>
+        var animateTimestamp = 0;
+        var intersectTimestamp = 0;
+        var MaxFrames = 2;
+        var currentFrame = 0;
+
+        function scrollBox()
+        {
+            var scrollable = document.querySelector("#scrollable");
+            var height = scrollable.clientHeight;
+            scrollable.scroll(0, (height / MaxFrames) * currentFrame);
+            document.body.offsetHeight;
+        }
+
+        function handleAnimate(timestamp)
+        {
+            if (++currentFrame > MaxFrames) {
+                finishJSTest();
+                return;
+            }
+
+             animateTimestamp = timestamp;
+            scrollBox();
+            requestAnimationFrame(handleAnimate);
+        }
+
+        function handleIntersect(entries, observer)
+        {
+            entries.forEach((entry) => {
+                intersectTimestamp = entry.time;
+                shouldBe("Math.round(animateTimestamp)", "Math.round(intersectTimestamp)");
+            });
+        }
+
+        function buildThresholdList()
+        {
+            let thresholds = [];
+
+             for (let i = 1.0; i <= MaxFrames; i++) {
+                let ratio = i / MaxFrames;
+                thresholds.push(ratio);
+            }
+
+             return thresholds;
+        }
+
+        function createObserver()
+        {
+            let observer;
+
+             let options = {
+                root: document.querySelector("#scrollable"),
+                rootMargin: "0px",
+                threshold: buildThresholdList()
+            };
+
+             observer = new IntersectionObserver(handleIntersect, options);
+            observer.observe(document.querySelector("#box"));
+        }
+
+        window.jsTestIsAsync = true;
+
+        description("Test the IntersectionObserver timestamp is the same as the timestamp of the rAF callback.");
+        createObserver();
+        requestAnimationFrame(handleAnimate);
+    </script>
+    <script src="../resources/js-test-post.js"></script>
+</body>
+</html>
index fd3c5b1..c29f771 100644 (file)
@@ -1325,8 +1325,6 @@ webkit.org/b/206946 http/tests/security/cookies/third-party-cookie-blocking-main
 
 webkit.org/b/205309 scrollingcoordinator/ios/scroll-position-after-reattach.html [ ImageOnlyFailure ]
 
-webkit.org/b/207153 animations/animation-callback-timestamp.html [ Pass Failure ]
-
 webkit.org/b/207156 [ Release ] http/tests/websocket/tests/hybi/workers/close.html [ Pass Failure ]
 
 webkit.org/b/207161 imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-transfer.html [ Pass Failure ]
index f2e2c52..98779c1 100644 (file)
@@ -901,8 +901,6 @@ webkit.org/b/208165 http/tests/cookies/document-cookie-after-showModalDialog.htm
 
 webkit.org/b/207560 media/airplay-target-availability.html [ Pass Failure ]
 
-webkit.org/b/207153 [ Debug ] animations/animation-callback-timestamp.html [ Pass Failure ]
-
 webkit.org/b/207568 inspector/page/overrideUserAgent.html [ Pass Failure ]
 
 webkit.org/b/207857 fast/canvas/webgl/texImage2D-mse-flipY-false.html [ Pass Timeout ]
index 8fbe22c..cd5f435 100644 (file)
@@ -1,3 +1,43 @@
+2020-04-27  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        Timestamps should be the same for all rendering update steps
+        https://bugs.webkit.org/show_bug.cgi?id=207153
+
+        Reviewed by Simon Fraser.
+
+        The HTML 5 event loop sepcs states that timestamps should be the same for 
+        all rendering update steps.
+
+        Specs link (step 9):
+            https://html.spec.whatwg.org/multipage/webappapis.html#event-loop-processing-model
+
+        This patch also fixes some issues in IntersectionObserver.
+
+        Test: intersection-observer/intersection-observer-callback-timestamp.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::updateIntersectionObservations):
+        (WebCore::Document::notifyIntersectionObserversTimerFired): Deleted.
+        * dom/Document.h:
+        -- Handle the case when two floats are areEssentiallyEqual().
+        -- Execute the IntersectionObserver immediately and do not wait until the 
+           next CFRunLoop event since this does not implement the specs.
+
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::freezeNowTimestamp):
+        (WebCore::DOMWindow::unfreezeNowTimestamp):
+        (WebCore::DOMWindow::frozenNowTimestamp const):
+        * page/DOMWindow.h:
+        Provide a frozen now() timestamp in seconds to be used internally only.
+
+        * page/IntersectionObserver.cpp:
+        (WebCore::IntersectionObserver::nowTimestamp const):
+        Use the frozenNowTimestamp().
+
+        * page/Page.cpp:
+        (WebCore::Page::updateRendering):
+        Freeze the timestamps while serving the rendering update steps.
+
 2020-04-27  Dean Jackson  <dino@apple.com>
 
         getShaderPrecisionFormat returns the wrong values
index 306584d..0e18cc8 100644 (file)
@@ -561,7 +561,6 @@ Document::Document(Frame* frame, const URL& url, unsigned documentClasses, unsig
     , m_fullscreenManager { makeUniqueRef<FullscreenManager>(*this) }
 #endif
 #if ENABLE(INTERSECTION_OBSERVER)
-    , m_intersectionObserversNotifyTimer(*this, &Document::notifyIntersectionObserversTimerFired)
     , m_intersectionObserversInitialUpdateTimer(*this, &Document::scheduleTimedRenderingUpdate)
 #endif
     , m_loadEventDelayTimer(*this, &Document::loadEventDelayTimerFired)
@@ -7614,12 +7613,12 @@ void Document::updateIntersectionObservations()
     if (!frameView)
         return;
 
-    m_intersectionObserversInitialUpdateTimer.stop();
-
     bool needsLayout = frameView->layoutContext().isLayoutPending() || (renderView() && renderView()->needsLayout());
     if (needsLayout || hasPendingStyleRecalc())
         return;
 
+    Vector<WeakPtr<IntersectionObserver>> intersectionObserversWithPendingNotifications;
+
     for (const auto& observer : m_intersectionObservers) {
         bool needNotify = false;
         auto timestamp = observer->nowTimestamp();
@@ -7646,9 +7645,11 @@ void Document::updateIntersectionObservations()
                     else
                         intersectionRatio = 1;
 
-                    auto& thresholds = observer->thresholds();
-                    while (thresholdIndex < thresholds.size() && thresholds[thresholdIndex] <= intersectionRatio)
+                    for (auto threshold : observer->thresholds()) {
+                        if (!(threshold <= intersectionRatio || WTF::areEssentiallyEqual<float>(threshold, intersectionRatio)))
+                            break;
                         ++thresholdIndex;
+                    }
                 }
             }
 
@@ -7689,20 +7690,13 @@ void Document::updateIntersectionObservations()
             }
         }
         if (needNotify)
-            m_intersectionObserversWithPendingNotifications.append(makeWeakPtr(observer.get()));
+            intersectionObserversWithPendingNotifications.append(makeWeakPtr(observer.get()));
     }
 
-    if (m_intersectionObserversWithPendingNotifications.size())
-        m_intersectionObserversNotifyTimer.startOneShot(0_s);
-}
-
-void Document::notifyIntersectionObserversTimerFired()
-{
-    for (const auto& observer : m_intersectionObserversWithPendingNotifications) {
+    for (const auto& observer : intersectionObserversWithPendingNotifications) {
         if (observer)
             observer->notify();
     }
-    m_intersectionObserversWithPendingNotifications.clear();
 }
 
 void Document::scheduleInitialIntersectionObservationUpdate()
index 2814b63..2182c43 100644 (file)
@@ -1668,10 +1668,6 @@ private:
     bool canNavigateInternal(Frame& targetFrame);
     bool isNavigationBlockedByThirdPartyIFrameRedirectBlocking(Frame& targetFrame, const URL& destinationURL);
 
-#if ENABLE(INTERSECTION_OBSERVER)
-    void notifyIntersectionObserversTimerFired();
-#endif
-
 #if USE(QUICK_LOOK)
     bool shouldEnforceQuickLookSandbox() const;
     void applyQuickLookSandbox();
@@ -1844,7 +1840,6 @@ private:
 #if ENABLE(INTERSECTION_OBSERVER)
     Vector<WeakPtr<IntersectionObserver>> m_intersectionObservers;
     Vector<WeakPtr<IntersectionObserver>> m_intersectionObserversWithPendingNotifications;
-    Timer m_intersectionObserversNotifyTimer;
     Timer m_intersectionObserversInitialUpdateTimer;
     // This is only non-null when this document is an explicit root.
     std::unique_ptr<IntersectionObserverData> m_intersectionObserverData;
index 27dc45f..18b3809 100644 (file)
@@ -739,6 +739,21 @@ ReducedResolutionSeconds DOMWindow::nowTimestamp() const
     return performance().nowInReducedResolutionSeconds();
 }
 
+void DOMWindow::freezeNowTimestamp()
+{
+    m_frozenNowTimestamp = nowTimestamp();
+}
+
+void DOMWindow::unfreezeNowTimestamp()
+{
+    m_frozenNowTimestamp = WTF::nullopt;
+}
+
+ReducedResolutionSeconds DOMWindow::frozenNowTimestamp() const
+{
+    return m_frozenNowTimestamp.valueOr(nowTimestamp());
+}
+
 Location& DOMWindow::location()
 {
     if (!m_location)
index b8728e4..a739f83 100644 (file)
@@ -351,6 +351,9 @@ public:
 
     Performance& performance() const;
     WEBCORE_EXPORT ReducedResolutionSeconds nowTimestamp() const;
+    void freezeNowTimestamp();
+    void unfreezeNowTimestamp();
+    ReducedResolutionSeconds frozenNowTimestamp() const;
 
 #if PLATFORM(IOS_FAMILY)
     void incrementScrollEventListenersCount();
@@ -470,6 +473,8 @@ private:
 
     mutable RefPtr<Performance> m_performance;
 
+    Optional<ReducedResolutionSeconds> m_frozenNowTimestamp;
+
     // For the purpose of tracking user activation, each Window W has a last activation timestamp. This is a number indicating the last time W got
     // an activation notification. It corresponds to a DOMHighResTimeStamp value except for two cases: positive infinity indicates that W has never
     // been activated, while negative infinity indicates that a user activation-gated API has consumed the last user activation of W. The initial
index 6b36dd7..7137a30 100644 (file)
@@ -256,7 +256,7 @@ Optional<ReducedResolutionSeconds> IntersectionObserver::nowTimestamp() const
     ASSERT(context->isDocument());
     auto& document = downcast<Document>(*context);
     if (auto* window = document.domWindow())
-        return window->nowTimestamp();
+        return window->frozenNowTimestamp();
     
     return WTF::nullopt;
 }
index 06fc022..7b4a522 100644 (file)
@@ -1341,6 +1341,13 @@ void Page::updateRendering()
 
     layoutIfNeeded();
 
+    // Timestamps should not change while serving the rendering update steps.
+    Vector<WeakPtr<Document>> initialDocuments;
+    forEachDocument([&initialDocuments] (Document& document) {
+        document.domWindow()->freezeNowTimestamp();
+        initialDocuments.append(makeWeakPtr(&document));
+    });
+
     // Flush autofocus candidates
 
     forEachDocument([] (Document& document) {
@@ -1358,7 +1365,7 @@ void Page::updateRendering()
     forEachDocument([] (Document& document) {
         if (!document.domWindow())
             return;
-        auto timestamp = document.domWindow()->nowTimestamp();
+        auto timestamp = document.domWindow()->frozenNowTimestamp();
         if (auto* timelinesController = document.timelinesController())
             timelinesController->updateAnimationsAndSendEvents(timestamp);
         // FIXME: Run the fullscreen steps.
@@ -1386,6 +1393,11 @@ void Page::updateRendering()
         }
     });
 
+    for (auto& document : initialDocuments) {
+        if (document)
+            document->domWindow()->unfreezeNowTimestamp();
+    }
+
     layoutIfNeeded();
     doAfterUpdateRendering();
 }