1050183 - waitForCondition in browser/base/content/test/general/head.js sometimes bails out very quickly

Status: RESOLVED INVALID

(Firefox :: General, defect)

Description

[:Gijs (he/him)]

See the discussion in bug 1049545. Splitting this off from that bug, but trying not to make this info get lost... in particular:

(In reply to Marco Bonardo [:mak] (Away 15-31 Aug) from bug 1049545 comment #19)
> (In reply to :Gijs Kruitbosch from bug 1049545 comment #15)
> > but it's still relying on
> > the timeout firing reliably, which isn't happening if we look at the logs
> > where the timeout follows the yielding of the previous promise within less
> > than a second (in all of these cases, interestingly...).
> 
> ah yeah that sounds like a bug, looks like it's timing out after the first
> 100ms... Sounds like the waitForCondition code, or the condition we pass to
> it, is bogus?

(In reply to Marco Bonardo [:mak] (Away 15-31 Aug) from bug 1049545 comment #20)
> I wonder if the problem are the 100ms, so if moving to, let's say, 10 *
> 500ms intervals, would work better and help with other intermittent
> failures.  Fwiw the code works properly in Scratchpad.

Comment 1

[:Gijs (he/him)]

So the current source is:

63 function waitForCondition(condition, nextTest, errorMsg) {
64   var tries = 0;
65   var interval = setInterval(function() {
66     if (tries >= 30) {
67       ok(false, errorMsg);
68       moveOn();
69     }
70     var conditionPassed;
71     try {
72       conditionPassed = condition();
73     } catch (e) {
74       ok(false, e + "\n" + e.stack);
75       conditionPassed = false;
76     }
77     if (conditionPassed) {
78       moveOn();
79     }
80     tries++;
81   }, 100);
82   var moveOn = function() { clearInterval(interval); nextTest(); };
83 }


and we invoked with:

    waitForCondition(() => notificationBox.getNotificationWithValue(value) !== null,
                     deferred.resolve, "Were expecting to get a notification");

for which yes, my best guess is that the 30 times 100ms somehow all fire very early - which is odd, because I would have thought that the purpose of setInterval rather than setTimeout would have been to make sure that doesn't happen. :-\

Comment 2

[:Gijs (he/him)]

Here's a list of consumers:

http://mxr.mozilla.org/mozilla-central/search?find=%2Fbrowser%2Fbase%2Fcontent%2Ftest%2Fgeneral%2F&string=waitForCondition

There are open intermittents filed related to waitForCondition issues in:

browser_bug822367.js - bug 1038133
browser_(devices_)get_user_media - bug 976544

which isn't really that much, if it's more generally broken, although I suppose it depends on how long the condition normally takes to be fulfilled... :-\

Comment 3

[:Gijs (he/him)]

This was happening because the log timestamps are a big fat lie.