Intermittent dom/base/test/test_intersectionobservers.html | supports getting records before the callback is invoked [takeRecords] - got +0, expected 1

RESOLVED FIXED in Firefox 54

Status

()

defect
RESOLVED FIXED
2 years ago
4 months ago

People

(Reporter: intermittent-bug-filer, Assigned: tschneider)

Tracking

({intermittent-failure})

unspecified
mozilla55
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox53 unaffected, firefox54 fixed, firefox55 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

See Also: → 1324135
I don't understand how this test is supposed to be testing that IntersectionObserver "supports getting records before the callback is invoked". My questions as comments below:

    it('supports getting records before the callback is invoked',

      function(done) {

        var lastestRecords = [];
        io = new IntersectionObserver(function(records) {
          lastestRecords = lastestRecords.concat(records);
        }, {root: rootEl});
        
        io.observe(targetEl1);  // doesn't callback gets called immediately when an intersection occurs?

        window.requestAnimationFrame && requestAnimationFrame(function() {
          // why does this test assume that takeRecords() will happen before the callback above?
          lastestRecords = lastestRecords.concat(io.takeRecords());
        });

        callDelayed(function() {
          
          // when this fails, lastestRecords.length == 0
          // why would lastestRecords.length == 1 indicate that takeRecords did happen before the callback?
          expect(lastestRecords.length).to.be(1);
          expect(lastestRecords[0].intersectionRatio).to.be(1);
          done();
        }, ASYNC_TIMEOUT);

      }
    );
Flags: needinfo?(tschneider)
(In reply to Jet Villegas (:jet) from comment #1)
>         
>         io.observe(targetEl1);  // doesn't callback gets called immediately
> when an intersection occurs?
> 

No, not immediately. Intersection check happens at next painting iteration, scheduling a task to be executed in a following event loop turn.

>         window.requestAnimationFrame && requestAnimationFrame(function() {
>           // why does this test assume that takeRecords() will happen before
> the callback above?
>           lastestRecords = lastestRecords.concat(io.takeRecords());
>         });

RAF's are supposed to happen before the the intersection check/observer notification.

>         callDelayed(function() {
>           
>           // when this fails, lastestRecords.length == 0
>           // why would lastestRecords.length == 1 indicate that takeRecords
> did happen before the callback?

It's not testing the order of which the callbacks above fire, only making sure that the current list of records is cleared after receiving them either via the observer callback or takeRecords. If that isn't the case, lastestRecords.length would be 2.
Flags: needinfo?(tschneider)
After second thought. I'm pretty sure takeRecords will become obsolete. There are justified concerns about its usefulness, especially after https://bugzilla.mozilla.org/show_bug.cgi?id=1319134. Also see https://github.com/WICG/IntersectionObserver/issues/133. According to the spec, the 100ms delay mentioned in that thread is not used anymore. intersection Observer notifications are now fired as early as possible, event before rAF's. I confirmed that this is even already the case in Chrome. Anyhow, we should keep that test as long as takeRecords is officially part of the spec. Will add a patch to fix the intermittent failure for now.
Attachment #8858444 - Flags: review?(bugs)
Comment on attachment 8858444 [details] [diff] [review]
Fix intermittent test failure

r+ with a better commit message, maybe:

Wait until there are intersection records to observe. Prevents an intermittent test failure.
Attachment #8858444 - Flags: review?(bugs) → review+
Keywords: checkin-needed
Pushed by philringnalda@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/65cf5283b925
Wait until there are intersection records to observe. Prevents an intermittent test failure, r=jet
Keywords: checkin-needed
Assignee: nobody → tschneider
https://hg.mozilla.org/mozilla-central/rev/65cf5283b925
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.