1993353 - Free some frame bits for anchor positioning.

Status: RESOLVED FIXED

(Core :: Layout, defect, P3)

Description

[Emilio Cobos Álvarez [:emilio]]

This has come up a couple time as part of making anchor position code paths not impose perf overhead.

• It seems that NS_FRAME_HAS_VIEW could be removed fairly trivially as it only applies to a very small number of frames nowadays: https://searchfox.org/firefox-main/rev/dc1c78e9c37aba6ed05a4ec47c4bfcb16f57b51d/layout/generic/nsIFrame.cpp#7798-7802,7809. We could make it a frame class flag (MayHaveView or somesuch)
• NS_FRAME_INDEPENDENT_SELECTION too unless I'm missing something (it's basically nsTextControlFrame | nsListControlFrame right?)

Comment 1

[David Shin[:dshin]]

(In reply to Emilio Cobos Álvarez [:emilio] from comment #0)

• NS_FRAME_INDEPENDENT_SELECTION too unless I'm missing something (it's basically nsTextControlFrame | nsListControlFrame right?)

FWIW, this seems like a strong indication.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Ah so the trick is that it gets propagated to children in nsIFrame::Init. But that is fixable, will dig more.

Comment 3

[Timothy Nikkel (:tnikkel)]

NS_FRAME_SIMPLE_EVENT_REGIONS seems unused.

Comment 4

[Timothy Nikkel (:tnikkel)]

NS_FRAME_PAINTED_THEBES is only used by tests. Could make it debug only bool on nsIFrame and make the tests debug only?

NS_FRAME_UPDATE_LAYER_TREE is only used on displayroots (viewport and menupopup). Could move it to the frame type specific section of frame state bits?

NS_FRAME_DRAWING_AS_PAINTSERVER does not seem perf critical at all. Could be a frame prop, and we could further reduce the overhead of this since we only need to check it if we are already drawing something as paint server, which could (or might already) store that on the nsDisplayListBuilder.

Comment 5

[Timothy Nikkel (:tnikkel)]

Attachment: Bug 1993353. Remove unused frame bit NS_FRAME_SIMPLE_EVENT_REGIONS. r?#layout-reviewers

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/60e99619c472
https://hg.mozilla.org/integration/autoland/rev/f25e7b47a4e2
Remove unused frame bit NS_FRAME_SIMPLE_EVENT_REGIONS. r=layout-reviewers,dshin

Comment 7

[Timothy Nikkel (:tnikkel)]

(In reply to Timothy Nikkel (:tnikkel) from comment #4)

NS_FRAME_UPDATE_LAYER_TREE is only used on displayroots (viewport and menupopup). Could move it to the frame type specific section of frame state bits?

This one seems pretty easy too, I have a patch for this.

Comment 8

[Timothy Nikkel (:tnikkel)]

Oh wait, menupopup frame is a block frame, which only has one frame specific bit available, not sure we want to use it for this.

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/f25e7b47a4e2

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1993353 - Remove NS_FRAME_HAS_VIEW. r=tnikkel

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1993353 - Remove view reparenting code that has been disabled forever. r=tnikkel

The subdocument frame caller is a no-op because nsSubdocumentFrame is a
leaf frame.

Comment 12

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1993353 - Remove NS_FRAME_INDEPENDENT_SELECTION from nsListControlFrame. r=#layout

It doesn't have an independent selection. Only nsTextControlFrame has a
different nsISelectionController.

Comment 13

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1993353 - Remove NS_FRAME_INDEPENDENT_SELECTION. r=#layout

It's only used to check the closest text control. We can abuse the fact
that nsTextControlFrame only has anonymous kids instead to make checking
for a containing text control fast in the common case.

Comment 14

[Emilio Cobos Álvarez [:emilio]]

(In reply to Timothy Nikkel (:tnikkel) from comment #8)

Oh wait, menupopup frame is a block frame, which only has one frame specific bit available, not sure we want to use it for this.

It's a bit ugly but why can't we just store the state separately in nsMenuPopupFrame and ViewportFrame? in two separate booleans seems fine...

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/764938320efc
https://hg.mozilla.org/integration/autoland/rev/64b8cb06af39
Remove NS_FRAME_HAS_VIEW. r=tnikkel,layout-reviewers

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f786affdc7af
https://hg.mozilla.org/integration/autoland/rev/44b5ed772ed5
Remove view reparenting code that has been disabled forever. r=tnikkel,layout-reviewers

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/194051e8a7f9
https://hg.mozilla.org/integration/autoland/rev/6f62ff18e12e
Remove NS_FRAME_INDEPENDENT_SELECTION from nsListControlFrame. r=layout-reviewers,dshin
https://github.com/mozilla-firefox/firefox/commit/1a9bdcf11bfa
https://hg.mozilla.org/integration/autoland/rev/c836d1ea5c4f
Remove NS_FRAME_INDEPENDENT_SELECTION. r=layout-reviewers,dshin

Comment 18

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/64b8cb06af39
https://hg.mozilla.org/mozilla-central/rev/44b5ed772ed5
https://hg.mozilla.org/mozilla-central/rev/6f62ff18e12e
https://hg.mozilla.org/mozilla-central/rev/c836d1ea5c4f