1798373 - Convert more frame manipulation APIs to take rvalue reference of nsFrameList

Status: RESOLVED FIXED

(Core :: Layout, task)

Description

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

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

Comment 1

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1798373 Part 1 - Change nsFrameList::InsertFrames to take rvalue reference of nsFrameList. r?#layout

Comment 2

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1798373 Part 2 - Change nsFrameList::AppendFrames to take rvalue reference of nsFrameList. r?#layout

Depends on D160837

Comment 3

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1798373 Part 3 - Remove nsFrameList::SetFrames() that takes nsIFrame*. r?#layout

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

Comment 4

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1798373 Part 4 - Remove nsFrameList::SetFrames() that takes nsFrameList. r?#layout

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

Comment 5

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1798373 Part 5 - Change nsContainerFrame::SetInitialChildList() to take rvalue reference of nsFrameList. r?#layout

Depends on D160840

Comment 6

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1798373 Part 6 - Change nsContainerFrame::AppendFrames() to take rvalue reference of nsFrameList. r?#layout

Change nsBlockFrame::AppendFrames() helper, too.

Depends on D160841

Comment 7

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1798373 Part 7 - Change nsContainerFrame::InsertFrames() to take rvalue reference of nsFrameList. r?#layout

Depends on D160842

Comment 8

[Ting-Yu Lin [:TYLin] (PDT, UTC-7)]

Attachment: Bug 1798373 Part 8 - Convert more frame list manipulation methods to take rvalue references of nsFrameList. r?#layout

Depends on D160843

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/7928d6c39dbb
https://hg.mozilla.org/mozilla-central/rev/9ab4a5dbb198
https://hg.mozilla.org/mozilla-central/rev/eb03bad6dd43
https://hg.mozilla.org/mozilla-central/rev/60bc4d2bee78
https://hg.mozilla.org/mozilla-central/rev/3fcd5080f02b
https://hg.mozilla.org/mozilla-central/rev/aacc637af7ac
https://hg.mozilla.org/mozilla-central/rev/9e554c8105fa
https://hg.mozilla.org/mozilla-central/rev/121a88263004