Closed
Bug 1429744
Opened 3 years ago
Closed 3 years ago
Perma failing layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the style rule. - got false, expected true
Categories
(Core :: Layout, defect, P5)
Core
Layout
Tracking
()
RESOLVED
FIXED
mozilla60
People
(Reporter: intermittent-bug-filer, Assigned: bradwerth)
Details
(Keywords: intermittent-failure, Whiteboard: [stockwell fixed:logic])
Attachments
(1 file)
Filed by: apavel [at] mozilla.com https://treeherder.mozilla.org/logviewer.html#?job_id=155588651&repo=autoland https://queue.taskcluster.net/v1/task/U6smevWAQ5ybyA8zcff0sw/runs/0/artifacts/public/logs/live_backing.log [task 2018-01-11T10:54:10.072Z] 10:54:10 INFO - TEST-START | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html [task 2018-01-11T10:54:10.121Z] 10:54:10 INFO - GECKO(1149) | ++DOMWINDOW == 18 (0x7f74a41ec800) [pid = 1149] [serial = 18] [outer = 0x7f74b0985e20] [task 2018-01-11T10:54:10.245Z] 10:54:10 INFO - TEST-INFO | started process screentopng [task 2018-01-11T10:54:10.694Z] 10:54:10 INFO - TEST-INFO | screentopng: exit 0 [task 2018-01-11T10:54:10.696Z] 10:54:10 INFO - Buffered messages logged at 10:54:10 [task 2018-01-11T10:54:10.697Z] 10:54:10 INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 1: Stylesheet now has 2 rules. [task 2018-01-11T10:54:10.698Z] 10:54:10 INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 1: Removed expected number of rules. [task 2018-01-11T10:54:10.700Z] 10:54:10 INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 1: Added expected number of rules. [task 2018-01-11T10:54:10.702Z] 10:54:10 INFO - test2Setup: called [task 2018-01-11T10:54:10.703Z] 10:54:10 INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Stylesheet now has 2 rules. [task 2018-01-11T10:54:10.705Z] 10:54:10 INFO - styleFirstAddProcessor: called with event @import url("imported_no_op.css"); [task 2018-01-11T10:54:10.705Z] 10:54:10 INFO - Buffered messages finished [task 2018-01-11T10:54:10.706Z] 10:54:10 INFO - TEST-UNEXPECTED-FAIL | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the style rule. - got false, expected true [task 2018-01-11T10:54:10.707Z] 10:54:10 INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:312:5 [task 2018-01-11T10:54:10.707Z] 10:54:10 INFO - test2Setup@chrome://mochitests/content/chrome/layout/inspector/tests/chrome/test_parseStyleSheetObservers.html:92:3 [task 2018-01-11T10:54:10.709Z] 10:54:10 INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the import rule. [task 2018-01-11T10:54:10.710Z] 10:54:10 INFO - GECKO(1149) | MEMORY STAT | vsize 2012MB | residentFast 285MB | heapAllocated 92MB [task 2018-01-11T10:54:10.715Z] 10:54:10 INFO - TEST-OK | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | took 227ms [task 2018-01-11T10:54:10.717Z] 10:54:10 INFO - GECKO(1149) | ++DOMWINDOW == 19 (0x7f74a6ab0400) [pid = 1149] [serial = 19] [outer = 0x7f74b0985e20] [task 2018-01-11T10:54:10.718Z] 10:54:10 INFO - TEST-START | Shutdown [task 2018-01-11T10:54:10.719Z] 10:54:10 INFO - Passed: 12 [task 2018-01-11T10:54:10.720Z] 10:54:10 INFO - Failed: 1 [task 2018-01-11T10:54:10.721Z] 10:54:10 INFO - Todo: 0 [task 2018-01-11T10:54:10.722Z] 10:54:10 INFO - Mode: non-e10s [task 2018-01-11T10:54:10.724Z] 10:54:10 INFO - Slowest: 1148ms - chrome://mochitests/content/chrome/layout/inspector/tests/chrome/test_parseStyleSheetObservers.html [task 2018-01-11T10:54:10.725Z] 10:54:10 INFO - TEST-INFO | Ran 2 Loops [task 2018-01-11T10:54:10.726Z] 10:54:10 INFO - SimpleTest FINISHED
Comment hidden (Intermittent Failures Robot) |
Comment 2•3 years ago
|
||
In the last 7 days this bug has failed 30 times. The fails happen on Linux x64, MacOSX 64 nightly, OSX 10.10, Windows 7, windows 10-64 and windows 7-32 nightly Here is an example of the most recent log: https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-central&job_id=155585821&lineNumber=1624 Here is a relevant snippet from aforementioned log: 02:38:16 INFO - TEST-UNEXPECTED-FAIL | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the style rule. - got false, expected true 02:38:16 INFO - SimpleTest.is@chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:312:5 02:38:16 INFO - test2Setup@chrome://mochitests/content/chrome/layout/inspector/tests/chrome/test_parseStyleSheetObservers.html:92:3 02:38:16 INFO - TEST-PASS | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | Test 2: Got the import rule. 02:38:16 INFO - GECKO(753) | MEMORY STAT | vsize 4358MB | residentFast 339MB | heapAllocated 97MB 02:38:16 INFO - TEST-OK | layout/inspector/tests/chrome/test_parseStyleSheetObservers.html | took 131ms 02:38:16 INFO - GECKO(753) | ++DOMWINDOW == 18 (0x1147b2400) [pid = 753] [serial = 18] [outer = 0x1217e8e10] 02:38:16 INFO - TEST-START | Shutdown
Flags: needinfo?(bugs)
Updated•3 years ago
|
Whiteboard: [stockwell needswork]
Comment 3•3 years ago
|
||
Brad: the test failures reported above don't reflect the comment here: // Create a Promise to watch for two StyleRuleAdded events. The first invocation should // be the style rule, and the second should be the import rule. We use the same processor // for both events, but the processor will only return true (completing the Promise) when // the import rule has been processed. from: https://searchfox.org/mozilla-central/source/layout/inspector/tests/chrome/test_parseStyleSheetObservers.html Can you double-check the assertions here? Thx!
Flags: needinfo?(bugs) → needinfo?(bwerth)
Assignee | ||
Comment 4•3 years ago
|
||
This failure is happening because the import rule StyleRuleAdded event is arriving before the style rule StyleRuleAdded event. That's not a test problem; it's a code problem and I'll have to dig a bit. I'll also improve the test so that the failure message in this case states the true cause of the failure.
Assignee | ||
Comment 5•3 years ago
|
||
Huh, well I can't actually find the bit of code that enforces import rules to be deferred after style rules. The deferral is mediated through a check to StyleSheet::RuleHasPendingChildSheet -- and if the rule has the sheet ready, then the processing of the rule isn't deferred at all. This seems correct. So there's an assumption in the test that isn't enforced by the code. I'm updating the test to reflect this new understanding. It makes the test less meaningful, but it won't fail for this reason any more.
Flags: needinfo?(bwerth)
Comment hidden (mozreview-request) |
Assignee | ||
Updated•3 years ago
|
Assignee: nobody → bwerth
Assignee | ||
Updated•3 years ago
|
Attachment #8942279 -
Flags: review?(cam)
Comment hidden (Intermittent Failures Robot) |
Assignee | ||
Updated•3 years ago
|
Attachment #8942279 -
Flags: review?(cam) → review?(emilio)
Comment 8•3 years ago
|
||
mozreview-review |
Comment on attachment 8942279 [details] Bug 1429744: Relax a test of StyleRuleAdded events to allow for the possibility that style rules could arrive before import rules. https://reviewboard.mozilla.org/r/212566/#review220108 Yeah, this test is racy. The reason the order may change is because of the `RuleHasPendingChildSheet` check here: https://searchfox.org/mozilla-central/rev/9f2721b19b10a9a441fa5ca7116720d2fcebcc4a/layout/style/ServoStyleSheet.cpp#323 I think we may want to kill that weird notification order, but maybe the inspector relies on import rules being added already having its associated sheet? We could file a bug to investigate it. Anyway, there's nothing preventing that sheet to be in the cache and be loaded sync. Could you double-check / confirm that we actually have a stylesheet there, and that it's loaded? Also, pointing this out in the commit message would be great. r=me with that, thanks for fixing this!
Attachment #8942279 -
Flags: review?(emilio) → review+
Comment hidden (mozreview-request) |
Comment 10•3 years ago
|
||
Pushed by bwerth@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/922a011fced2 Relax a test of StyleRuleAdded events to allow for the possibility that style rules could arrive before import rules. r=emilio
Comment 11•3 years ago
|
||
mozreview-review |
Comment on attachment 8942279 [details] Bug 1429744: Relax a test of StyleRuleAdded events to allow for the possibility that style rules could arrive before import rules. https://reviewboard.mozilla.org/r/212566/#review220322 Static analysis found 1 defect in this patch. - 1 defect found by mozlint You can run this analysis locally with: - `./mach lint check path/to/file` (Python/Javascript/wpt) If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx ::: layout/inspector/tests/chrome/test_parseStyleSheetObservers.html:68 (Diff revision 2) > let foundStyle = false; > let styleFirstAddProcessor = function(event) { > info("styleFirstAddProcessor: called with event "+ event.rule.cssText); > if (event.rule.type == CSSRule.IMPORT_RULE) { > foundImport = true; > + isnot(event.rule.styleSheet, null, "Test 2: import rule has stylesheet loaded."); Error: 'isnot' is not defined. [eslint: no-undef]
Assignee | ||
Comment 12•3 years ago
|
||
(In reply to Code Review Bot [:reviewbot] from comment #11) > ::: layout/inspector/tests/chrome/test_parseStyleSheetObservers.html:68 > (Diff revision 2) > > let foundStyle = false; > > let styleFirstAddProcessor = function(event) { > > info("styleFirstAddProcessor: called with event "+ event.rule.cssText); > > if (event.rule.type == CSSRule.IMPORT_RULE) { > > foundImport = true; > > + isnot(event.rule.styleSheet, null, "Test 2: import rule has stylesheet loaded."); > > Error: 'isnot' is not defined. [eslint: no-undef] This test should be on the ignore list; not sure why lint triggered on it. Running lint locally shows that all the SimpleTest usages fail. Setting "checkin-needed" to get help on landing it.
Keywords: checkin-needed
Comment 13•3 years ago
|
||
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
Updated•3 years ago
|
Keywords: checkin-needed
Comment 14•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/922a011fced2
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox60:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
Comment 15•3 years ago
|
||
bugherderuplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/0a7b0fd5bf12
status-firefox59:
--- → fixed
Updated•3 years ago
|
Whiteboard: [stockwell needswork] → [stockwell fixed:logic]
You need to log in
before you can comment on or make changes to this bug.
Description
•