Assertion failure: !(mState & (nsFrameState(1) << (43))) || !(mState & (nsFrameState(1) << (53))) (Non-display SVG do not maintain visual overflow rects), at /Users/jruderman/trees/mozilla-central/layout/generic/nsFrame.cpp:5150

So I avoid doing much with sticky positioned SVG text frames, but I guess I need to be more careful about what I add to my OverflowChangedTracker.

Presumably I need to avoid updating overflow of things that don't maintain overflow, either in OverflowChangedTracker itself or when adding frames to it. Not sure whether nsIFrame::HasOverflowAreas is right here, or whether I should do the same check as that assertion.

I don't think you want HasOverflowAreas. Probably best to just check frame->GetStateBits() & NS_FRAME_IS_NONDISPLAY, and exempt all nondisplay frames. Probably makes sense to do this check in the spot where we append to StickyScrollContainer::mFrames, and reject nondisplay frames at that point.

(Specifically, I think we should add !(frame->GetStateBits() & NS_FRAME_IS_NONDISPLAY) to the list of things we check when we call AddFrame for the ssc, in nsFrame::Init.)

(We probably want to add the same check in nsFrame::DestroyFrom, for symmetry, but it doesn't matter as much there because calling RemoveFrame() on un-tracked frames will just silently fail.)

That works. (This applies on top of the patch from bug 904197.)

(In reply to Corey Ford [:corey] from comment #7) > (This applies on top of the patch from bug 904197.) I should probably reorder those patches, since this one should be higher priority to land (& perhaps uplift to aurora).

Like so. Mind running this by Try and/or requesting uplift, if you think either of those are warranted?

(In reply to Corey Ford [:corey] from comment #8) > I should probably reorder those patches, since this one should be higher > priority to land (& perhaps uplift to aurora). Agreed. (though fwiw I'm intending to look into fixing the 1px-off issue on the other bug's reftests in the next few days, so that it can land.) (In reply to Corey Ford [:corey] from comment #9) > Like so. Mind running this by Try and/or requesting uplift, if you think > either of those are warranted? Will do both.

Once more, asking for unit tests this time: https://tbpl.mozilla.org/?tree=Try&rev=c1a548a6a87b

Landed on inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/94b97b38878f

(In reply to Daniel Holbert [:dholbert] from comment #10) > (In reply to Corey Ford [:corey] from comment #9) > > Like so. Mind running this by Try and/or requesting uplift, if you think > > either of those are warranted? > > Will do both. In retrospect, I think probably not worth requesting uplift for this, given that (a) the feature is disabled by default (on Aurora at least; won't be disabled on Nightly for much longer) (b) for any people who opt to turn on the pref: no one's going to hit this in real-world content (c) for any people who *both* turn on the pref and actually *do* hit content that triggers this: I don't think it's a particularly dangerous situation / scary assertion. I think it's just a case where we're spinning our wheels and wasting a few CPU cycles.

(In reply to Daniel Holbert [:dholbert] from comment #14) > (c) for any people who *both* turn on the pref and actually *do* hit > content that triggers this: I don't think it's a particularly dangerous > situation / scary assertion. I think it's just a case where we're spinning > our wheels and wasting a few CPU cycles. And crashing. But you're right that it's quite unlikely to be hit.

Oh, but not in opt builds, right.