Bug 1489671 [wpt PR 12901] - Loosen timing expectation in IntersectionObserver WPTs, a=testonly
authorAli Juma <ajuma@chromium.org>
Thu, 13 Sep 2018 02:51:55 +0000
changeset 492157 075ae925437941495e90ce5d3ebb52376219fb6e
parent 492156 d188605003c838c1692e549759a269c230c513d3
child 492158 d91e5930746ebee939dec7da28246f738206451b
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1489671, 12901, 1214176, 590195
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1489671 [wpt PR 12901] - Loosen timing expectation in IntersectionObserver WPTs, a=testonly Automatic update from web-platform-testsLoosen timing expectation in IntersectionObserver WPTs IntersectionObserver WPTs expect that intersection observations are delivered within two step_timeouts of requestAnimationFrame. This expectation holds in Blink, but doesn't necessarily hold in other engines. In WebKit, requestAnimationFrame and layout are separate tasks, so it's possible for the first step_timeout to fire in between rAF and layout, leading to test flakiness. This CL switches these WPTs to using double-rAF rather than rAF-setTimeout-setTimeout when waiting for observations to be delivered. Change-Id: Iad80943db184bc4519ab323c3bd7cfac1256e175 Reviewed-on: https://chromium-review.googlesource.com/1214176 Commit-Queue: Ali Juma <ajuma@chromium.org> Reviewed-by: Stefan Zager <szager@chromium.org> Cr-Commit-Position: refs/heads/master@{#590195} -- wpt-commits: 2dce278555acc0b91ca98d2ee9ce8219c91f4e98 wpt-pr: 12901
testing/web-platform/meta/MANIFEST.json
testing/web-platform/tests/intersection-observer/resources/intersection-observer-test-utils.js
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -616073,17 +616073,17 @@
    "0cc117cb3a69d0f2ed19505f7d14f824cbad6d71",
    "support"
   ],
   "intersection-observer/resources/iframe-no-root-subframe.html": [
    "ee63a06ca0ff30eb6bc82d28350cf8d85313251b",
    "support"
   ],
   "intersection-observer/resources/intersection-observer-test-utils.js": [
-   "5ad811932f06c34f1bb3bfd70094a0cd5c046c56",
+   "8683c8b570c8bffbe413315c2b36ec1e47a6d650",
    "support"
   ],
   "intersection-observer/resources/observer-in-iframe-subframe.html": [
    "9d0769ae44a1bb4a6195c006999b0959f706330c",
    "support"
   ],
   "intersection-observer/resources/timestamp-subframe.html": [
    "143e4f6e23a7688949420a07ccd20e3c211a6f6b",
--- a/testing/web-platform/tests/intersection-observer/resources/intersection-observer-test-utils.js
+++ b/testing/web-platform/tests/intersection-observer/resources/intersection-observer-test-utils.js
@@ -1,33 +1,83 @@
 // Here's how waitForNotification works:
 //
 // - myTestFunction0()
 //   - waitForNotification(myTestFunction1)
 //     - requestAnimationFrame()
 //   - Modify DOM in a way that should trigger an IntersectionObserver callback.
 // - BeginFrame
 //   - requestAnimationFrame handler runs
-//     - First step_timeout()
+//     - Second requestAnimationFrame()
 //   - Style, layout, paint
 //   - IntersectionObserver generates new notifications
 //     - Posts a task to deliver notifications
-// - First step_timeout handler runs
-//   - Second step_timeout()
 // - Task to deliver IntersectionObserver notifications runs
 //   - IntersectionObserver callbacks run
-// - Second step_timeout handler runs
+// - Second requestAnimationFrameHandler runs
+//     - step_timeout()
+// - step_timeout handler runs
 //   - myTestFunction1()
 //     - [optional] waitForNotification(myTestFunction2)
 //       - requestAnimationFrame()
 //     - Verify newly-arrived IntersectionObserver notifications
 //     - [optional] Modify DOM to trigger new notifications
+//
+// Ideally, it should be sufficient to use requestAnimationFrame followed
+// by two step_timeouts, with the first step_timeout firing in between the
+// requestAnimationFrame handler and the task to deliver notifications.
+// However, the precise timing of requestAnimationFrame, the generation of
+// a new display frame (when IntersectionObserver notifications are
+// generated), and the delivery of these events varies between engines, making
+// this tricky to test in a non-flaky way.
+//
+// In particular, in WebKit, requestAnimationFrame and the generation of
+// a display frame are two separate tasks, so a step_timeout called within
+// requestAnimationFrame can fire before a display frame is generated.
+//
+// In Gecko, on the other hand, requestAnimationFrame and the generation of
+// a display frame are a single task, and IntersectionObserver notifications
+// are generated during this task. However, the task posted to deliver these
+// notifications can fire after the following requestAnimationFrame.
+//
+// This means that in general, by the time the second requestAnimationFrame
+// handler runs, we know that IntersectionObservations have been generated,
+// and that a task to deliver these notifications has been posted (though
+// possibly not yet delivered). Then, by the time the step_timeout() handler
+// runs, these notifications have been delivered.
+//
+// Since waitForNotification uses a double-rAF, it is now possible that
+// IntersectionObservers may have generated more notifications than what is
+// under test, but have not yet scheduled the new batch of notifications for
+// delivery. As a result, observer.takeRecords should NOT be used in tests:
+//
+// - myTestFunction0()
+//   - waitForNotification(myTestFunction1)
+//     - requestAnimationFrame()
+//   - Modify DOM in a way that should trigger an IntersectionObserver callback.
+// - BeginFrame
+//   - requestAnimationFrame handler runs
+//     - Second requestAnimationFrame()
+//   - Style, layout, paint
+//   - IntersectionObserver generates a batch of notifications
+//     - Posts a task to deliver notifications
+// - Task to deliver IntersectionObserver notifications runs
+//   - IntersectionObserver callbacks run
+// - BeginFrame
+//   - Second requestAnimationFrameHandler runs
+//     - step_timeout()
+//   - IntersectionObserver generates another batch of notifications
+//     - Post task to deliver notifications
+// - step_timeout handler runs
+//   - myTestFunction1()
+//     - At this point, observer.takeRecords will get the second batch of
+//       notifications.
 function waitForNotification(t, f) {
   requestAnimationFrame(function() {
-    t.step_timeout(function() { t.step_timeout(f); });
+    requestAnimationFrame(function() { t.step_timeout(f); });
   });
 }
 
 // The timing of when runTestCycle is called is important.  It should be
 // called:
 //
 //   - Before or during the window load event, or
 //   - Inside of a prior runTestCycle callback, *before* any assert_* methods