570837 - have 64 bits of frame state

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

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

roc and I are both running into a need for more frame state bits.  In the past I'd proposed moving class-private bits into their own word for blocks so we could squeeze a few more bits out, but that's more work than I have time for right now, and it probably won't get us enough bits to last very long anyway.

So here's a series of patches to have 64 bits of frame state.

Comment 1

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

Attachment: patch 1: define state bits using macros, and other fixes to allow size to be changed

The main change here is to define all state bits using macros.  This makes the definitions of the bits much more readable (a problem that would get worse with 64-bit... imagine distinguishing 0x80000000000000 and 0x800000000000000), and also makes it easy to find all the places that define frame state bits.

However, it's strictly necessary here so that we don't break places that do mState &= ~NS_FRAME_... or similar.


There's also one fix replacing RemoveStateBits(0xffffffff) with RemoveStateBits(~nsFrameState(0)).

Comment 2

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

Attachment: patch 2: add a FIXME

Should really look into this, but add a FIXME for now.

Comment 3

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

Attachment: patch 3: remove unused frame state bits

In order to do patch 1, I needed to find everything that was set as a frame state bit.  Once I had that list, it was easy enough to figure out which ones were unused.  This removes them.

Comment 4

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

Attachment: patch 4: switch to 64 bits of frame state

Comment 5

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

Regarding patch 2:

The current value of NS_STATE_NEED_LAYOUT comes from bug 54710; see bug 54710 comment 7 and bug 54710 comment 8.  That was September 2000.

NS_STATE_IS_HORIZONTAL and NS_STATE_IS_ROOT have had their current values since March 2000, but NS_STATE_BOX_CHILD_RESERVED wasn't explicitly noted until 2007.

Then again, nsTextBoxFrame inherits from nsLeafBoxFrame (not nsBoxFrame), so maybe this is totally irrelevant and I should just remove the comment.  But the comments in bug 54710 suggest that maybe it's not irrelevant.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can we just change it to an unused bit and remove the comment?

Comment 7

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

I can change it to NS_BOX_CHILD_RESERVED.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

You mean NS_STATE_BOX_CHILD_RESERVED? That sounds fine.

Comment 9

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

Attachment: patch 2: fix potentially-overlapping bit

Comment 10

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

Comment on attachment 450028 [details] [diff] [review]
patch 2: fix potentially-overlapping bit

I'll consider comment 8 to be r=roc on this.

Comment 11

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

http://hg.mozilla.org/mozilla-central/rev/ebc6875a1ec4
http://hg.mozilla.org/mozilla-central/rev/04eaa2543795
http://hg.mozilla.org/mozilla-central/rev/5d4130a21bf2
http://hg.mozilla.org/mozilla-central/rev/3680d856d412