Closed Bug 1369350 Opened 3 years ago Closed 3 years ago

Intermittent layout/style/test/test_dont_use_document_colors.html | color is blocked - got "rgb(255, 255, 0)", expected "rgb(0, 0, 0)"


(Core :: CSS Parsing and Computation, defect, P4)




Tracking Status
firefox55 --- fixed


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



(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:timing])


(1 file, 1 obsolete file)

This doesn't seem stylo-only.  Could something have caused pushPrefEnv to not be reliable?
Priority: -- → P4
I've been doing some retriggers to isolate this; so far the earliest occurrence I got is on:
but I suspect I'll get one earlier.
thanks for looking into this :dbaron.
Whiteboard: [stockwell needswork]
OK, I think this is a regression from bug 1363829, though it took an unusually large number of retriggers on the exact landing changeset to get a failure, relative to later changesets.

bkelly, any thoughts?
Blocks: 1363829
Flags: needinfo?(bkelly)
I'll take a look tomorrow, but my guess is that the issue is PushPrefEnv().  It uses a setTimeout(f, 0) to allow preferences to apply before calling its callback.  Speeding up setTimeout(f, 0) slightly may have caused this test to start losing a race there.
We're probably racing with this nsPresContext timer:
Assignee: nobody → bkelly
Flags: needinfo?(bkelly)
Might as well test a potential test fix as well:

Note, I'm leaning towards special casing this in the test vs trying to change SpecialPowers.pushPrefEnv().  It seems that this is due to nsPresShell's specific additional delay after preferences are changed and not a general preference thing.  (Assuming my theories all hold up.)
Comment on attachment 8875367 [details] [diff] [review]
Add additional delay to test_dont_use_document_colors.html since nsPresShell delays applying prefs. r=dbaron

This test is racing the setTimeout(f, 0) after built in to the pushPrefEnv() call against an nsPresContext specific nsITimer:

The nsPresContext is using a 0ms nsITimer which will round trip through the TimerThread.

The setTimeout(f, 0) can now result in a simple NS_DispatchToCurrentThread(r).

This patch makes the test wait for a setTimeout(f, 1) to account for the nsITimer in nsPresContext.  I think this is appropriate since the extra delay is only needed for nsPresContext preferences and not all preferences in general.
Attachment #8875367 - Flags: review?(dbaron)
Attachment #8875367 - Flags: review?(dbaron) → review+
Updated the comments and commit message to reference nsPresContext correctly.  I had previously written nsPresShell.
Attachment #8875367 - Attachment is obsolete: true
Attachment #8875699 - Flags: review+
Pushed by
Add additional delay to test_dont_use_document_colors.html since nsPresContext delays applying prefs. r=dbaron
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Whiteboard: [stockwell needswork] → [stockwell fixed:timing]
You need to log in before you can comment on or make changes to this bug.