1897135 - Cleanup in nsFrameManager, nsCSSFrameConstructor, and PresShell

Status: RESOLVED FIXED

(Core :: Layout, task)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8) (Away, back on Mar 24)]

See the patches for the details.

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8) (Away, back on Mar 24)]

Attachment: Bug 1897135 Part 1 - Sort #includes and forward declarations in nsFrameManager, nsCSSFrameConstructor, and PresShell. r?#layout

Also, delete unused DEBUG_UNDISPLAYED_MAP and DEBUG_DISPLAY_CONTENTS_MAP in
nsFrameManager.cpp.

Delete the comments between the #includes in PresShell.cpp per coding style
guideline: "Don't place comments between non-conditional includes." in
https://firefox-source-docs.mozilla.org/code-quality/coding-style/coding_style_cpp.html#include-directives

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8) (Away, back on Mar 24)]

Attachment: Bug 1897135 Part 2 - Remove redundant mFrameManager member in PresShell. r?#layout

We can just use mFrameConstructor to call GetRootFrame(). In order to do so
while keeping PresShell::GetRootFrame() as a inline method, we need to include
nsCSSFrameConstructor header in PresShell.h, and remove PresShell header
in nsCSSFrameConstructor.h. That means we can no longer inline
RestyleManager() since it needs to access PresShell, but it's OK since
RestyleManager is used only in the frame constructor, and is probably not in
the hot path.

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8) (Away, back on Mar 24)]

Attachment: Bug 1897135 Part 3 - Change two APIs to ensure root frame is a ViewportFrame. r?#layout

Move SetRootFrame() to nsFrameManager.cpp because calling IsViewportFrame()
requires full definition of nsIFrame.

An alternative idea without adding an assertion is that we can change
mRootFrame, GetRootFrame() and SetRootFrame() in nsFrameManager to use
ViewportFrame*. However, given that most of the caller are getting root frame
via PresShell::GetRootFrame(), and I don't see the exisiting usage requires
accessing ViewportFrame. Therefore, this idea doesn't seem beneficial, and
might make other files depending on "ViewportFrame.h" unnecessarily.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5dbc0205b48a
Part 1 - Sort #includes and forward declarations in nsFrameManager, nsCSSFrameConstructor, and PresShell. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/813bb8dcc655
Part 2 - Remove redundant mFrameManager member in PresShell. r=dholbert
https://hg.mozilla.org/integration/autoland/rev/0cd4c114c5e6
Part 3 - Change two APIs to ensure root frame is a ViewportFrame. r=dholbert

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/5dbc0205b48a
https://hg.mozilla.org/mozilla-central/rev/813bb8dcc655
https://hg.mozilla.org/mozilla-central/rev/0cd4c114c5e6