Closed Bug 1254831 Opened 5 years ago Closed 5 years ago
[e10s] Win8 TEST-UNEXPECTED-PASS in some mathml/mfrac reftests with e10s enabled
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
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.
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 :)
Yes, rendering is correct.
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.
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
Argh, pretty sure I had the new condition on the wrong side of a parenthesis. http://images.memes.com/meme/702133
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.
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 on attachment 8736661 [details] [diff] [review] 1. Patch Review of attachment 8736661 [details] [diff] [review]: ----------------------------------------------------------------- Thanks.
Attachment #8736661 - Flags: review?(fred.wang) → review+
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)
Attachment #8736662 - Flags: review?(fred.wang) → review+
You need to log in before you can comment on or make changes to this bug.