Closed Bug 1641085 Opened 4 years ago Closed 4 years ago

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

Categories

(Core :: Layout, task, P2)

task

Tracking

()

RESOLVED FIXED
82 Branch
Tracking Status
firefox82 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

Details

Attachments

(7 files)

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

Depends on: 1642038

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.

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

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

Depends on D88456

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

Depends on D88458

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

Depends on D88460

Assignee: nobody → aethanyc
Status: NEW → ASSIGNED
Pushed by aethanyc@gmail.com:
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
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: