Closed Bug 729018 Opened 11 years ago Closed 11 years ago

Intermittent test_aboutcompartments.xul | Test timed out.

Categories

(Toolkit :: about:memory, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: philor, Assigned: n.nethercote)

References

Details

(Keywords: intermittent-failure, Whiteboard: [please leave open after merging])

Attachments

(1 file)

https://tbpl.mozilla.org/php/getParsedLog.php?id=9486893&tree=Mozilla-Inbound
Rev3 WINNT 5.1 mozilla-inbound pgo test mochitest-other on 2012-02-20 21:33:22 PST for push 027f56e65a84

13437 INFO TEST-START | chrome://mochitests/content/chrome/toolkit/components/aboutmemory/tests/test_aboutcompartments.xul
13438 INFO TEST-INFO | chrome://mochitests/content/chrome/toolkit/components/aboutmemory/tests/test_aboutcompartments.xul | must wait for load
13439 ERROR TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/toolkit/components/aboutmemory/tests/test_aboutcompartments.xul | Test timed out.
If you fixed bug 589668, you'd get a screenshot here
13440 INFO TEST-END | chrome://mochitests/content/chrome/toolkit/components/aboutmemory/tests/test_aboutcompartments.xul | finished in 320710ms
Sigh, that didn't take long.
Attached patch patchSplinter Review
Currently, we run a "minimize memory usage" (MMU) immediately upon loading the page, then generate the content, and then trigger an event.  The test waits for that event before doing its cut and paste.  Something must be going wrong the the MMU or the event, because test_aboutmemory.xul is very reliable and it doesn't do those two steps.

This patch changes things so that we generate the content, then run MMU, then regenerate the content (which may or may not have changed).  No events.  It's heavy-handed, but it will *probably* make this test more reliable because it'll remove the delay before content is generated (like test_aboutmemory.xul).

I'll wait before getting review for this and landing it, because I want to see how frequently the current version fails.
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Comment on attachment 599127 [details] [diff] [review]
patch

13 failures in 5 days, so about ~2.5 per day.  Time to try a new strategy.  jlebar, comment 2 explains what's going on.
Attachment #599127 - Flags: review?(justin.lebar+bug)
I did a try run with the attached patch, test_aboutcompartments.xul passed on all platforms.  Hardly conclusive, but better than failing.
(In reply to Nicholas Nethercote [:njn] from comment #17)
> I did a try run with the attached patch, test_aboutcompartments.xul passed
> on all platforms.  Hardly conclusive, but better than failing.

I triggered 3 additional runs of Mochitest-o on all platforms and got no failures.  Beyond that, I reckon we'll just have to land this and see how it goes.
> This patch changes things so that we generate the content, then run MMU, then regenerate the 
> content (which may or may not have changed).

Does this have visible effects (flash of content, then flash of changed content)?

I kind of think that, if the solution is to get rid of the event, we should just go to about:compartments?nogc during the test, and make that do the obvious thing.  This way, you always generate the about:compartments data exactly once.
> Does this have visible effects (flash of content, then flash of changed
> content)?

Currently, we already have visible effects -- a flash of nothing, and then content is added.  With the patch, we have content, and then possibly a change of content, but the change is usually less noticeable.

> I kind of think that, if the solution is to get rid of the event, we should
> just go to about:compartments?nogc during the test, and make that do the
> obvious thing.  This way, you always generate the about:compartments data
> exactly once.

That's another possibility.
> > about:compartments?nogc
>
> That's another possibility.

What would the nogc+verbose URL look like?  |about:compartments?verbose&nogc| ?
> Currently, we already have visible effects -- a flash of nothing, and then content is added.

That's true...  That flash is easily visible on my machine, too.

We could get rid of this (and make it a flash of white or gray, instead of a flash of empty box) by dynamically inserting the stylesheet.

Anyway, I feel dirty computing data we're immediately going to throw away, and only for the purposes of passing a test.  But it's an improvement over where we currently are in terms of the UX, so r=me.
Attachment #599127 - Flags: review?(justin.lebar+bug) → review+
> We could get rid of this (and make it a flash of white or gray, instead of a
> flash of empty box) by dynamically inserting the stylesheet.

With lots of tabs open (e.g. during MemBuster) the current momentary blankness -- whether it's an empty box or a completely white page -- is a bit disconcerting.  I find the "show some compartments then modify the list slightly" behaviour nicer.
Let's cross our fingers:

https://hg.mozilla.org/integration/mozilla-inbound/rev/32397978a342

Mergers, please leave this bug open after merging to mozilla-central.
Whiteboard: [orange] → [orange][please leave open after merging]
The patch landed on m-i 48 hours ago and the test hasn't failed since.  I'm declaring victory.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [orange][please leave open after merging] → [please leave open after merging]
You need to log in before you can comment on or make changes to this bug.