477409 - make reftest check its own timeout invariants

Status: ASSIGNED

(Testing :: Reftest, defect)

Description

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8]

Attachment: patch

There have been a lot of correlated load failures on tinderbox lately, so I'm beginning to wonder if there's something wrong with the reftest harness's handling of its load failure timeouts.

Here's a patch that makes it issue an unexpected failure report if it detects that something has gone wrong with its timeouts.

Comment 1

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8]

(Note that there could also be interaction with the slow script dialog involved.  sayrer says mochitest disables it through automation.py.  Maybe we'll want the same once ted's patch in bug 468913 lands.)

Comment 2

[Jesse Ruderman]

Other ideas:

* OnDocumentLoad could complain if there is not an outstanding timer.

* The "if (gClearingForAssertionCheck)" bit might need to be moved down before the "Ignore load events for previous documents" bit.

* The "Ignore load events for previous documents" bit could output some "NOISE:" explaining what it thinks is going on.

* When contentRootElement is null, it should probably do something sane.

Comment 3

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8]

I pushed the existing patch as http://hg.mozilla.org/mozilla-central/rev/29bc88a2d670 .  I'll try to write a patch for some of those other ideas soon...

Comment 4

[Jesse Ruderman]

Here's an example of a pair failure from today:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1234205896.1234211217.21332.gz#err0

Comment 5

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8]

Attachment: patch 2

This is equivalent to:

(In reply to comment #2)
> * The "if (gClearingForAssertionCheck)" bit might need to be moved down before
> the "Ignore load events for previous documents" bit.

but simpler, and I think it could be the fix for the cause of our double-timeouts lately.

Comment 6

[Jesse Ruderman]

"but about:blank is unlikely to take a really long time to load"

I'm not sure how to convince myself that this is the only way to get here with the URL being about:blank.  And we're skeptical enough that about:blank *could* be taking a long time to load.

How about using a unique URL for the clearing, like 
  data:text/html,<!-- CLEAR -->

Comment 7

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8]

Attachment: patch 2

Good idea.

Comment 8

[David Baron :dbaron: 🏴󠁵󠁳󠁣󠁡󠁿 ⌚UTC-8]

http://hg.mozilla.org/mozilla-central/rev/a82e37fed714

Comment 9

[Serge Gautherie (:sgautherie)]

Is there any reason not to land this on m-1.9.1?