Open Bug 762332 Opened 8 years ago Updated 4 years ago
Text Run should have set font size inflation" with -moz-column, rel&abs pos
With user_pref("font.size.inflation.emPerLine", 15); The testcase triggers: ###!!! ABORT: EnsureTextRun should have set font size inflation: 'GetFontSizeInflation() == fontSizeInflation', file layout/generic/nsTextFrameThebes.cpp, line 7389
The assertion still occurs. Is the bug that we forgot to remove the FontSizeInflationProperty somewhere? (gdb) p GetFontSizeInflation() $3 = 1.0333333 (gdb) p fontSizeInflation $4 = 1 (gdb) p this->IsCurrentFontInflation(fontSizeInflation) $5 = false (gdb) p nsLayoutUtils::FontSizeInflationFor(this) $6 = 1 (gdb) p this->HasFontSizeInflation() $9 = true
OS: Mac OS X → All
Hardware: x86_64 → All
If we can't come up with a proper fix for this soon, then I suggest we change it to a non-fatal assertion (or warning) so it doesn't block fuzz-testing font-inflation and/or moz-columns.
Let's make the assertion non-fatal for now, and add the testcase so we can detect if it starts crashing or goes away for some reason. https://tbpl.mozilla.org/?tree=Try&rev=d4fb7ee1827a
Attachment #736063 - Flags: review?(sjohnson)
Comment on attachment 736063 [details] [diff] [review] wallpaper Review of attachment 736063 [details] [diff] [review]: ----------------------------------------------------------------- LGTM
Attachment #736063 - Flags: review?(sjohnson) → review+
Severity: critical → normal
Summary: "ABORT: EnsureTextRun should have set font size inflation" with -moz-column, rel&abs pos → "ASSERTION: EnsureTextRun should have set font size inflation" with -moz-column, rel&abs pos
Whiteboard: [font-inflation] → [font-inflation][leave open]
It's going to go away for crashtest-e10s when 45 merges to aurora tomorrow, making the test permaorange.
I don't understand what the "it" is that's going to go away, nor why RyanVM landed the above changeset or whether it's related to philor's comment.
"It" is (In reply to Mats Palmgren (:mats) from comment #4) > Let's make the assertion non-fatal for now, and add the testcase so we > can detect if it starts crashing or goes away for some reason. and given the context of https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=ac8c124250b5 I would say that while adding crashtests Ryan added one which added an assertion to this test on e10s which it didn't have before, but it only adds it on nightly_build, not on aurora.
And just to add to the fun, on current-aurora the tests appear to run in crashtest.list order, with font-inflation-762332.html running immediately after 750066.html, while on trunk and thus on try for my trunk-as-aurora simulaton they are sorted, so font-inflation-762332.html runs immediately after first-letter-638937-2.html and the first-letter tests run after 1225592.html. So depending on whether that change in ordering is in-tree mozharness or test harness behavior and will merge or is out-of-tree and won't change, I may or may not be right about font-inflation-762332.html having an unexpected-pass from having 0 assertions in crashtest-e10s on aurora tomorrow, depending also on whether it depends on 740199-1.xhtml having set font.size.inflation.emPerLine shortly before it runs, or only on it having set it at all anytime before. Should be exciting for the sheriff on duty.
The lack of an assertion did carry over to actual-aurora, we backed out https://hg.mozilla.org/integration/mozilla-inbound/rev/ac8c124250b5 to fix the orange, according to RyanVM's memory this isn't the first time we've had to, and it would be pretty sweet if someone would either figure out why it now asserts on e10s but only on trunk, or just give up on this whole exercise and either mark it as 0-10000 unconditionally or get rid of the test.
I've debugged this a little bit. What we have is a frame tree like this: block text text block text text The text frames are all for the same content and are next/prev-in-flow with each other. ditto for the block frames. Also, the text frames share the same TextRun. The problem is that we use the font inflation for the first text frame when creating the textrun: http://hg.mozilla.org/mozilla-central/annotate/cb66ffeb6725/layout/generic/nsTextFrame.cpp#l2124 Font size inflation depends on a frame's ancestors though: http://hg.mozilla.org/mozilla-central/annotate/cb66ffeb6725/layout/base/nsLayoutUtils.cpp#l7594 and in this case the latter two text frames have a different nsLayoutUtils::FontSizeInflationFor result from the former two. (because nsFontInflationData::mInflationEnabled was false in the latter block and true in the former) So this assertion can't possibly be correct in the current setup.
You need to log in before you can comment on or make changes to this bug.