335405 - Consider removing the NS_FRAME_OUTSIDE_CHILDREN bit

Status: NEW

(Core :: Layout: Block and Inline, defect)

Description

[rbs]

Removing NS_FRAME_OUTSIDE_CHILDREN will free yet another bit.

Currently, it has the same _meaning_ as 
nsRect* nsIFrame::GetOverflowAreaProperty(PR_FALSE/*aCreateIfNecessary*/) 

In bug 315210, an assertion was added to enforce the invariant that when NS_FRAME_OUTSIDE_CHILDREN is set, the OverflowArea property should exist. In a sense, this only gows to confirm that checking for _existence_ of the property is a flag in itself. It does not have to be constructive.

Note that before roc's rework to paint with display lists, the bit was slightly overloaded in an undocumented way... As the painting code was recursing down the frame tree, it would not constrain the damaged area when it encountered a frame with the NS_FRAME_OUTSIDE_CHILDREN bit set.

It was this (undocumented or abusive?) artefact that MathML was using to work-around some overflow problems (where glyphs such as integrals would bend and stick out of their normal rect -- c.f. attachment 219725 [details]).

But now, this artefact has become irrelevant. The new painting code only relies on nsIFrame::GetOverflowRect(). In fact, the new code even asserts if the invariant NS_FRAME_OUTSIDE_CHILDREN<>OverflowAreaProperty is not met (bug 323733)

From a logical point however, I think the NS_FRAME_OUTSIDE_CHILDREN bit is not needed (anymore). Removing it will free yet another bit. Testing for existence of the OverflowArea property is sufficient in itself and should make everybody's code easier to read/maintain.

LXR for NS_FRAME_OUTSIDE_CHILDREN:
http://lxr.mozilla.org/seamonkey/search?string=NS_FRAME_OUTSIDE_CHILDREN

Comment 1

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Is testing for a property cheap enough that this won't hurt performance?

Comment 2

[rbs]

In general, it would be hard to tell. But in this particular case my gut feeling is that it won't affect performance because there are not many frame properties and all these are in a small hash table

http://lxr.mozilla.org/mozilla/source/content/base/src/nsGkAtomList.h#1350

And this is compounded with the fact the writer (reflow) and reader (paint) of this particular OverflowArea property are quite heavy, so that testing the property would be noise there. It would be a concern if GetProperty() was already showing up on the radars due to the other frame properties that don't have a bit. I am now aware that it shows up.

Comment 3

[rbs]

s/I am now aware that it shows up/I am not aware that it shows up/