Closed
Bug 1254831
Opened 8 years ago
Closed 8 years ago
[e10s] Win8 TEST-UNEXPECTED-PASS in some mathml/mfrac reftests with e10s enabled
Categories
(Core :: MathML, defect)
Core
MathML
Tracking
()
RESOLVED
FIXED
mozilla48
People
(Reporter: RyanVM, Assigned: jkitch)
References
(Blocks 1 open bug)
Details
Attachments
(2 files)
4.04 KB,
patch
|
fredw
:
review+
|
Details | Diff | Splinter Review |
3.33 KB,
patch
|
fredw
:
review+
|
Details | Diff | Splinter Review |
Fred, I don't know these tests well enough to say whether the results are what we would expect or not, but they are permanently passing on Windows 8 when e10s is forced on. Any thoughts? :) https://treeherder.mozilla.org/logviewer.html#?job_id=17687025&repo=try
Flags: needinfo?(fred.wang)
Updated•8 years ago
|
Flags: needinfo?(fred.wang)
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jkitch.bug
Assignee | ||
Comment 1•8 years ago
|
||
I can reproduce this on Windows 10. Those tests have been problematic for a while. Pass in bug 975681 Subsequently marked as random failure on Windows XP Marked as Perma-fail on Windows in bug 947654 Now only failing when e10s is disabled. With e10s off, they fail because the fraction bar is not updated after an attribute change. I'll have a closer look to see if I can identify any underlying issues, but having the tests start working is a good thing.
Updated•8 years ago
|
Reporter | ||
Comment 2•8 years ago
|
||
Is the rendering correct in the screenshots taken by the harness? If so, I'm happy to annotate this away. I just don't want to assume that passing is automatically the right thing here :)
Assignee | ||
Comment 3•8 years ago
|
||
Yes, rendering is correct.
Reporter | ||
Comment 4•8 years ago
|
||
Great, I'll go ahead and annotate this for now. I'm leaving the bug open under the assumption that you're intending to take a closer look at these tests based on comment 1. Feel free to remove the leave-open if that's not the case.
Keywords: leave-open
Comment 6•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/83b5406a8395
Comment 8•8 years ago
|
||
Backed out for https://hg.mozilla.org/integration/mozilla-inbound/rev/83b5406a8395 from comment 5 for failing on Windows 8 x64 opt. Backout: https://hg.mozilla.org/integration/mozilla-inbound/rev/7a6646425f62 Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=24127114&repo=mozilla-inbound REFTEST TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/mathml/mfrac-B-2.html | image comparison (==), max difference: 255, number of differing pixels: 84 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/mathml/mfrac-B-3.html | image comparison (==), max difference: 255, number of differing pixels: 42 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/mathml/mfrac-C-2.html | image comparison (==), max difference: 255, number of differing pixels: 84 REFTEST TEST-UNEXPECTED-FAIL | file:///C:/slave/test/build/tests/reftest/tests/layout/reftests/mathml/mfrac-D-2.html | image comparison (==), max difference: 255, number of differing pixels: 126
Flags: needinfo?(ryanvm)
Reporter | ||
Comment 9•8 years ago
|
||
Argh, pretty sure I had the new condition on the wrong side of a parenthesis. http://images.memes.com/meme/702133
Flags: needinfo?(ryanvm)
Reporter | ||
Comment 11•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/3740e26a27e4
Reporter | ||
Comment 12•8 years ago
|
||
I had a fun insight recently that's probably relevant here - Win8 is our only Windows platform that runs reftests with D2D enabled currently. I found that out when running Win10 reftests (currently not supported, but being worked on) and hitting the same failure. So more realistically, this is likely a D2D+e10s (or APZ maybe) issue.
Assignee | ||
Comment 13•8 years ago
|
||
Stepping through the code, reflow correctly picks up on the modified line thickness, and the modified fraction bar is added to the display list for painting, but the invalidation algorithm does not always detect that the rendering has changed. As a result, the region is not redrawn. Explicitly invalidating the frame when linethickness changes is sufficient to resolve the problem. A cursory search on DXR found other layout/ AttributeChanged overrides performing the task as well.
Attachment #8736661 -
Flags: review?(fred.wang)
Comment 14•8 years ago
|
||
Comment on attachment 8736661 [details] [diff] [review] 1. Patch Review of attachment 8736661 [details] [diff] [review]: ----------------------------------------------------------------- Thanks.
Attachment #8736661 -
Flags: review?(fred.wang) → review+
Assignee | ||
Comment 15•8 years ago
|
||
Rather trivial fix. Some #ifdefed out code was not included in the DisplayList refactoring some time ago. This makes it compile when SHOW_BOUNDING_BOX is defined. No changes occur when it isn't defined.
Attachment #8736662 -
Flags: review?(fred.wang)
Updated•8 years ago
|
Attachment #8736662 -
Flags: review?(fred.wang) → review+
Assignee | ||
Comment 16•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=69bb11995dc8
Keywords: leave-open → checkin-needed
Comment 17•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c5403f965018 https://hg.mozilla.org/integration/mozilla-inbound/rev/31b2154a1927
Keywords: checkin-needed
Comment 18•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/c5403f965018 https://hg.mozilla.org/mozilla-central/rev/31b2154a1927
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•