Closed Bug 1369350 Opened 3 years ago Closed 3 years ago
_dont _use _document _colors .html | color is blocked - got "rgb(255, 255, 0)", expected "rgb(0, 0, 0)"
This doesn't seem stylo-only. Could something have caused pushPrefEnv to not be reliable?
I've been doing some retriggers to isolate this; so far the earliest occurrence I got is on: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=4e8564ce92d8056d6282a499cac1933f4a90a769&filter-searchStr=stylo-seq+mochitest but I suspect I'll get one earlier.
thanks for looking into this :dbaron.
Whiteboard: [stockwell needswork]
I'm focusing primarily on this range: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&filter-searchStr=stylo-seq%20mochitest&group_state=expanded&tochange=e6f613b7ec08ca91240e1c28e687bad0ff38f2e4&tochange=9bf2eafa8def&fromchange=83f3573ea26840208086aa5bd7d41aff37a0ddf1 most likely bug 1363829.
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?
In particular, I believe this started with this push: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=7c6116ac71e9
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: https://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#1549
Assignee: nobody → bkelly
Status: NEW → ASSIGNED
Might as well test a potential test fix as well: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c5dd13e704590ee0a36a228c1b90306560f5c61 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: https://dxr.mozilla.org/mozilla-central/source/layout/base/nsPresContext.cpp#771 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.
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/b8c1a978f8fe Add additional delay to test_dont_use_document_colors.html since nsPresContext delays applying prefs. r=dbaron
Whiteboard: [stockwell needswork] → [stockwell fixed:timing]
You need to log in before you can comment on or make changes to this bug.