Closed
Bug 1369350
Opened 7 years ago
Closed 7 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)"
Categories
(Core :: CSS Parsing and Computation, defect, P4)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla55
Tracking | Status | |
---|---|---|
firefox55 | --- | fixed |
People
(Reporter: intermittent-bug-filer, Assigned: bkelly)
References
Details
(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:timing])
Attachments
(1 file, 1 obsolete file)
Comment hidden (Intermittent Failures Robot) |
Comment hidden (Intermittent Failures Robot) |
Comment 3•7 years ago
|
||
This doesn't seem stylo-only. Could something have caused pushPrefEnv to not be reliable?
Comment hidden (Intermittent Failures Robot) |
Updated•7 years ago
|
Priority: -- → P4
Comment hidden (Intermittent Failures Robot) |
Comment 6•7 years ago
|
||
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.
Comment 8•7 years ago
|
||
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.
Comment 9•7 years ago
|
||
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)
Comment 10•7 years ago
|
||
In particular, I believe this started with this push:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=7c6116ac71e9
Assignee | ||
Comment 11•7 years ago
|
||
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.
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 13•7 years ago
|
||
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
Flags: needinfo?(bkelly)
Assignee | ||
Comment 14•7 years ago
|
||
Assignee | ||
Comment 15•7 years ago
|
||
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.)
Assignee | ||
Comment 16•7 years ago
|
||
Assignee | ||
Comment 17•7 years ago
|
||
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)
Updated•7 years ago
|
Attachment #8875367 -
Flags: review?(dbaron) → review+
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Comment 19•7 years ago
|
||
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+
Comment 20•7 years ago
|
||
Pushed by bkelly@mozilla.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
Comment hidden (Intermittent Failures Robot) |
Comment 22•7 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•7 years ago
|
Whiteboard: [stockwell needswork] → [stockwell fixed:timing]
Comment hidden (Intermittent Failures Robot) |
You need to log in
before you can comment on or make changes to this bug.
Description
•