Right now, if you add a CSS outline (and you change the outline-width and/or offset, which you probably will be doing), we trigger a reflow (via "AllReflowHints"): 642 nsChangeHint nsStyleOutline::CalcDifference(const nsStyleOutline& aOther) const 643 { 644 bool outlineWasVisible = 645 mCachedOutlineWidth > 0 && mOutlineStyle != NS_STYLE_BORDER_STYLE_NONE; 646 bool outlineIsVisible = 647 aOther.mCachedOutlineWidth > 0 && aOther.mOutlineStyle != NS_STYLE_BORDER_STYLE_NONE; 648 if (outlineWasVisible != outlineIsVisible || 649 (outlineIsVisible && (mOutlineOffset != aOther.mOutlineOffset || 650 mOutlineWidth != aOther.mOutlineWidth || 651 mTwipsPerPixel != aOther.mTwipsPerPixel))) { 652 return NS_CombineHint(nsChangeHint_AllReflowHints, 653 nsChangeHint_RepaintFrame); http://mxr.mozilla.org/mozilla-central/source/layout/style/nsStyleStruct.cpp?#642 This code basically dates back to bug 151375, which pushed "outline" outside of the border-box of frames, and made it influence its overflow region. It looks like (and roc confirms via IRC) we added nsChangeHint_AllReflowHints simply so that we could update the overflow region, because reflow was the only way to do this at that time. However, now we have nsChangeHint_UpdateOverflow which we should be able to use, to directly target the overflow region without needing a reflow. Filing this bug on making that change. (billm says he's noticed this causing slowness when switching to a treeherder tab that has something focused deep down in the frame tree. That focused thing becomes active when we switch tabs, which gives it a focus-outline, which triggers an avoidable reflow.)

Thanks!

I think this should do it. This adds UpdateOverflow and SchedulePaint (to kick off DLBI & be sure we don't just get a recompositing paint -- added in similar bug 1038781). I'll give this a Try run, and request review if all is well. (I'll also do a Try run without these new changehints, just to make sure we are actually testing this, and that our tests prove that UpdateOverflow is indeed required.)

tn points out that mozilla-central changeset 73eaf1199ff0 was intended to fix this -- returning NS_CombineHint(nsChangeHint_UpdateOverflow, nsChangeHint_RepaintFrame) for this case. But mats backed that out in mozilla-central changeset 14e550268f98 for some reason, a few weeks later. mats, do you remember why we needed that backout? (I'm curious if there's anything I should test to find out if this new patch causes the same issue that triggered your eventual backout.)

Ah, looks like the backout is somewhat documented in bug 724432 comment 0, and (for a group of related patches) in bug 719177 comment 24. As dbaron noted later on bug 719177, it sounds like bug 984226 covered the work that was needed to be done before we could pick up the ball on using UpdateOverflow hints again. Since bug 984226 is fixed, I suspect that means this should be OK now... (though I haven't looked at bug 984226 in detail yet).

Try run with this bug's patch: https://treeherder.mozilla.org/#/jobs?repo=try&revision=07d80f5799dd And some probably-flawed exploratory Try runs... - just removing AllReflowHints: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7769783afba4 - Adding nsChangeHint_UpdateOverflow, but without SchedulePaint: https://treeherder.mozilla.org/#/jobs?repo=try&revision=18ae5a547061

D'oh, the Try runs of course has assertions because I forgot to update MaxDifference(). New patch / Try runs coming soon (maybe tomorrow) with that fixed.

Try run with MaxDifference / MaxDifferenceNeverInherited tweaked, to account for the updated changehints that we send now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ffa7fddbea4a Tagging heycam to review, since he created MaxDifferenceNeverInherited recently, and I want to be sure I'm tweaking it correctly.

(In reply to Daniel Holbert [:dholbert] from comment #5) > And some probably-flawed exploratory Try runs... > - just removing AllReflowHints: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=7769783afba4 > - Adding nsChangeHint_UpdateOverflow, but without SchedulePaint: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=18ae5a547061 For the record, both of these intentionally-flawed Try runs had reftest failures with missing outlines (and some ignorable assertions due to comment 6). Those reftest failures confirm that both "nsChangeHint_UpdateOverflow" and "nsChangeHint_SchedulePaint" are definitely needed here.

Added a mochitest, which supports making arbitrary style changes & asserting that no reflow/frame-construction cocurred. Right now it just focuses on "outline", but I gave it a generic name so we can extend it. Verified locally that it passes w/ fix, fails w/out. Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=c5ebbb08d35f

Looks like you found the information you asked for. Please make sure you don't regress the test in bug 725664 (I'm not sure if that's in the tree or not).

Thanks! I checked, & it does not regress that test; the test looks the same with & without the patch.

Comment on attachment 8563175 [details] [diff] [review] fix v3 (now with test) Review of attachment 8563175 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. ::: layout/style/test/test_dynamic_change_causing_reflow.html @@ +21,5 @@ > + > +/** Test for Bug 1131371 **/ > + > +/** > + Remove blank line.

Thanks -- fixed that & landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/4c0ccf99cb71