Closed Bug 1798373 Opened 2 years ago Closed 2 years ago

Convert more frame manipulation APIs to take rvalue reference of nsFrameList

Categories

(Core :: Layout, task)

task

Tracking

()

RESOLVED FIXED
108 Branch
Tracking Status
firefox108 --- fixed

People

(Reporter: TYLin, Assigned: TYLin)

References

Details

Attachments

(8 files)

48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

Bug 1797011 enables us to convert more frame manipulation APIs to take rvalue reference of nsFrameList. This conveys the ownership transfer of frames more clearly, but at the expense of adding std::move when passing an existing nsFrameList.

However, when passing a temporary frame list, we don't have to create extra temp variable. For example, if SetInitialChildList takes nsFrameList&&, we can simplify the two lines at [1].

Before:

nsFrameList textList(textFrame, textFrame);
mDisplayFrame->SetInitialChildList(kPrincipalList, textList);

After:

mDisplayFrame->SetInitialChildList(kPrincipalList, nsFrameList(textFrame, textFrame));

[1] https://searchfox.org/mozilla-central/rev/73ff5f1ac45202c4621a1bcee4b35d20027e71ad/layout/forms/nsComboboxControlFrame.cpp#839-840

The removed SetFrames() is slow (it traverses the next sibling chain to find the
last sibling) and has only one caller. Let's remove it.

The rewrite in nsInlineFrame::PullOneFrame() doesn't change behavior because
frame is just removed from the overflow list, so it won't have any next
sibling. We are creating a frame list containing only frame.

Depends on D160838

SetFrame() is equivalent to operator=, so external callers can use operator=
instead. For the two callers wanting to set nsFrameList to AbsoluteFrameList
in nsCSSFrameConstructor, removing SetFrame() disallows it. However, we can
easily change the declaration from nsFrameList to a AbsoluteFrameList to
resolve the problem.

Depends on D160839

Change nsBlockFrame::AppendFrames() helper, too.

Depends on D160841

Pushed by aethanyc@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7928d6c39dbb
Part 1 - Change nsFrameList::InsertFrames to take rvalue reference of nsFrameList. r=emilio
https://hg.mozilla.org/integration/autoland/rev/9ab4a5dbb198
Part 2 - Change nsFrameList::AppendFrames to take rvalue reference of nsFrameList. r=emilio
https://hg.mozilla.org/integration/autoland/rev/eb03bad6dd43
Part 3 - Remove nsFrameList::SetFrames() that takes nsIFrame*. r=emilio
https://hg.mozilla.org/integration/autoland/rev/60bc4d2bee78
Part 4 - Remove nsFrameList::SetFrames() that takes nsFrameList. r=emilio
https://hg.mozilla.org/integration/autoland/rev/3fcd5080f02b
Part 5 - Change nsContainerFrame::SetInitialChildList() to take rvalue reference of nsFrameList. r=emilio
https://hg.mozilla.org/integration/autoland/rev/aacc637af7ac
Part 6 - Change nsContainerFrame::AppendFrames() to take rvalue reference of nsFrameList. r=emilio
https://hg.mozilla.org/integration/autoland/rev/9e554c8105fa
Part 7 - Change nsContainerFrame::InsertFrames() to take rvalue reference of nsFrameList. r=emilio
https://hg.mozilla.org/integration/autoland/rev/121a88263004
Part 8 - Convert more frame list manipulation methods to take rvalue references of nsFrameList. r=emilio
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: