1641085 - Improve APIs that manipulate overflow list, overflow containers list, and excess overflow containers list

Status: RESOLVED FIXED

(Core :: Layout, task, P2)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

We have a nice set of APIs GetOverflowFrames, StealOverflowFrames, SetOverflowFrames, and DestroyOverflowList() [1] to manipulate overflow list. They also have good documents about their usage and the ownership transfer that the caller should be aware of.

I feel we should add an equivalent set of APIs for overflow containers list (OverflowContainersProperty) and excess overflow containers list (ExcessOverflowContainersProperty).

In addition to this, I'd like to experiment with adding move-semantics to nsFrameList. Not because of performance, but to clearly indicate the ownership transfer of the frames held in the frame list. We have a lot of usages that construct a temporary nsFrameList and then append / insert to some child list in a frame. The temporary list usually becomes empty because the frames are transferred. It would be nice to use move semantics for this.

For example, SetOverflowFrames [2] can take a r-value reference, the caller will need to use SetOverflowFrames(std::move(tempList)) so that the ownership transfer of the frames becomes very clear at the call site.

[1] https://searchfox.org/mozilla-central/rev/7dafc35406b9c945189c617d427f5458933fd3fb/layout/generic/nsContainerFrame.h#535-561
[2] https://searchfox.org/mozilla-central/rev/7dafc35406b9c945189c617d427f5458933fd3fb/layout/generic/nsContainerFrame.cpp#1420

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1641085 Part 1 - Add move semantic to nsFrameList, and use it on SetOverflowFrames().

It's useful to use std::move() to indicate the frames' ownership in one list
is transferred to the another list.

For a frame list managed by AutoFrameListPtr, after moving its frames to
another list, it can be automatically deleted when it is going out of
scope.

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1641085 Part 2 - Add APIs to manipulate overflow containers list.

Not all APIs added in this patch are used immediately, but for the sake of
completeness, they are all added.

Their document will be updated in Part 3 after adding the relevant APIs for
ExcessOverflowContainersProperty().

nsContainerFrame::DrainExcessOverflowContainersList() has several calls to set
OverflowContainersProperty() whose life cycle need special attention. We will
deal with them later in Part 4.

Depends on D88455

Comment 3

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1641085 Part 3 - Add APIs to manipulate excess overflow containers list.

This patch is similar to Part 2, but for adding APIs for
ExcessOverflowContainersProperty().

Depends on D88456

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1641085 Part 4 - Revise DrainAndMergeSelfOverflowList() so that it's free of explicit allocation and deletion of nsFrameList.

Depends on D88457

Comment 5

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1641085 Part 5 - Remove SetPropTableFrames() in nsContainerFrame.

We already use SetProperty() extensively for nsFrameList properties like
OutsideMarkerProperty(), BackdropProperty(), etc, so we can simplify the
interface by removing SetPropTableFrames().

Depends on D88458

Comment 6

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1641085 Part 6 - Remove GetPropTableFrames() and RemovePropTableFrames() in nsContainerFrame.

Depends on D88459

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1641085 Part 7 - Simplify the definition of FrameListPropertyDescriptor.

It's easier to jump to the definition of FrameListPropertyDescriptor in editors
without too many indirections.

Depends on D88460

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2e007acb711c
Part 1 - Add move semantic to nsFrameList, and use it on SetOverflowFrames(). r=mats
https://hg.mozilla.org/integration/autoland/rev/08e7ad0c6072
Part 2 - Add APIs to manipulate overflow containers list. r=mats
https://hg.mozilla.org/integration/autoland/rev/f2e88db56acc
Part 3 - Add APIs to manipulate excess overflow containers list. r=mats
https://hg.mozilla.org/integration/autoland/rev/c40583cb4197
Part 4 - Revise DrainAndMergeSelfOverflowList() so that it's free of explicit allocation and deletion of nsFrameList. r=mats,emilio
https://hg.mozilla.org/integration/autoland/rev/70bfa693b6e7
Part 5 - Remove SetPropTableFrames() in nsContainerFrame. r=mats
https://hg.mozilla.org/integration/autoland/rev/b3abd9cd4d8c
Part 6 - Remove GetPropTableFrames() and RemovePropTableFrames() in nsContainerFrame. r=mats
https://hg.mozilla.org/integration/autoland/rev/81897edbbb9f
Part 7 - Simplify the definition of FrameListPropertyDescriptor. r=mats

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/2e007acb711c
https://hg.mozilla.org/mozilla-central/rev/08e7ad0c6072
https://hg.mozilla.org/mozilla-central/rev/f2e88db56acc
https://hg.mozilla.org/mozilla-central/rev/c40583cb4197
https://hg.mozilla.org/mozilla-central/rev/70bfa693b6e7
https://hg.mozilla.org/mozilla-central/rev/b3abd9cd4d8c
https://hg.mozilla.org/mozilla-central/rev/81897edbbb9f