Closed Bug 1743537 Opened 2 years ago Closed 2 years ago

Clean up related to scroll frame implementation

Categories

(Core :: Layout: Scrolling and Overflow, task)

task

Tracking

()

RESOLVED FIXED
96 Branch
Tracking Status
firefox96 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

Details

Attachments

(6 files)

These patches are written when I investigate the scrollbar implementation for bug 1715112. All the patches are refactors, and shouldn't change the behavior.

They shouldn't be scattered in nsHTMLScrollFrame::Reflow(). Also, reorder these
assignment operations according to the member variables' appearance order in
ScrollReflowInput.

I also moved the constructor out of the class definition because it becomes a
large method, and I'm going to add private: section after the methods section
in a later patch.

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED

We've had some APIs passing ScrollReflowInput by reference while some others
using pointer. Let's unify them by using reference everywhere.

Depends on D132440

It should be sufficient to call SetScrollbarMediatorContent once in
ScrollReflowInput's constructor, which is created in the beginning of
nsHTMLScrollFrame::Reflow(), instead of calling it repeatedly in multiple
TryLayout() calls.

Depends on D132441

Scrollbar's min and pref sizes won't change during reflow, so we can cache them
in ScrollReflowInput to save some repetitive computation in multiple
ReflowScrolledFrame() and TryLayout() calls.

This is also a preparation for Bug 1715112 because we can use the pref sizes to
compute the scrollbar-gutter size in ScrollReflowInput.

Depends on D132442

Make these variables more distinguishable from oldScrolledAreaBounds and
newScrolledAreaBounds.

Depends on D132443

Technically, aContentArea is not 100% wrong; its the content area of the outer
scroll frame, which contains the content area of the inner scrolled frame, the
padding, and the scrollbars. However, it should really be named
aInsideBorderArea as the caller names it. Otherwise, it is easy to cause
confusion with the content area of the inner scrolled frame.

Also, rename aOldScrollArea as well so that we use the term "scroll port"
consistently.

Depends on D132444

Pushed by tlin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fb82680677e1
Part 1 - Move ScrollReflowInput members' initialization into its constructor. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/6046902bfc19
Part 2 - Pass ScrollReflowInput argument by reference. r=emilio
https://hg.mozilla.org/integration/autoland/rev/3e3076fbab8d
Part 3 - Call SetScrollbarMediatorContent in ScrollReflowInput's constructor. r=emilio
https://hg.mozilla.org/integration/autoland/rev/1e32e1c2b2db
Part 4 - Cache scrollbar sizes in ScrollReflowInput. r=layout-reviewers,emilio
https://hg.mozilla.org/integration/autoland/rev/df87dee15817
Part 5 - Rename variables names storing mScrollPort. r=emilio
https://hg.mozilla.org/integration/autoland/rev/3e8f817592dc
Part 6 - Rename arguments of ScrollFrameHelper::LayoutScrollbars(). r=emilio
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: