From 0076add36568902d207ae332f2b5e7ea9241a989 Mon Sep 17 00:00:00 2001 From: Ting-Yu Lin Date: Sat, 29 Aug 2020 00:22:16 +0000 Subject: [PATCH] Bug 1641085 Part 1 - Add move semantic to nsFrameList, and use it on SetOverflowFrames(). r=mats 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. Differential Revision: https://phabricator.services.mozilla.com/D88455 --- layout/forms/nsFieldSetFrame.cpp | 2 +- layout/generic/nsColumnSetFrame.cpp | 4 ++-- layout/generic/nsContainerFrame.cpp | 4 ++-- layout/generic/nsContainerFrame.h | 4 ++-- layout/generic/nsFirstLetterFrame.cpp | 4 ++-- layout/generic/nsFrameList.h | 20 ++++++++++++++++++++ layout/generic/nsIFrame.h | 1 + layout/tables/nsTableFrame.cpp | 2 +- 8 files changed, 31 insertions(+), 10 deletions(-) diff --git a/layout/forms/nsFieldSetFrame.cpp b/layout/forms/nsFieldSetFrame.cpp index 9ba5dc6b611e..e23921eca05f 100644 --- a/layout/forms/nsFieldSetFrame.cpp +++ b/layout/forms/nsFieldSetFrame.cpp @@ -925,7 +925,7 @@ void nsFieldSetFrame::EnsureChildContinuation(nsIFrame* aChild, if (nsFrameList* oc = GetOverflowFrames()) { oc->AppendFrames(nullptr, nifs); } else { - SetOverflowFrames(nifs); + SetOverflowFrames(std::move(nifs)); } } } diff --git a/layout/generic/nsColumnSetFrame.cpp b/layout/generic/nsColumnSetFrame.cpp index 527294f5baee..d5503e5fcc3d 100644 --- a/layout/generic/nsColumnSetFrame.cpp +++ b/layout/generic/nsColumnSetFrame.cpp @@ -847,9 +847,9 @@ nsColumnSetFrame::ColumnBalanceData nsColumnSetFrame::ReflowChildren( kidNextInFlow->MarkSubtreeDirty(); // Move any of our leftover columns to our overflow list. Our // next-in-flow will eventually pick them up. - const nsFrameList& continuationColumns = mFrames.RemoveFramesAfter(child); + nsFrameList continuationColumns = mFrames.RemoveFramesAfter(child); if (continuationColumns.NotEmpty()) { - SetOverflowFrames(continuationColumns); + SetOverflowFrames(std::move(continuationColumns)); } child = nullptr; diff --git a/layout/generic/nsContainerFrame.cpp b/layout/generic/nsContainerFrame.cpp index 4fb591fc12e7..8dbd51ec779f 100644 --- a/layout/generic/nsContainerFrame.cpp +++ b/layout/generic/nsContainerFrame.cpp @@ -1636,7 +1636,7 @@ void nsContainerFrame::PushChildren(nsIFrame* aFromChild, nextInFlow->mFrames.InsertFrames(nextInFlow, nullptr, tail); } else { // Add the frames to our overflow list - SetOverflowFrames(tail); + SetOverflowFrames(std::move(tail)); } } @@ -2043,7 +2043,7 @@ void nsContainerFrame::MergeSortedOverflow(nsFrameList& aList) { if (overflow) { MergeSortedFrameLists(*overflow, aList, GetContent()); } else { - SetOverflowFrames(aList); + SetOverflowFrames(std::move(aList)); } } diff --git a/layout/generic/nsContainerFrame.h b/layout/generic/nsContainerFrame.h index 8170647493d2..16f4d0e0b199 100644 --- a/layout/generic/nsContainerFrame.h +++ b/layout/generic/nsContainerFrame.h @@ -564,10 +564,10 @@ class nsContainerFrame : public nsSplittableFrame { /** * Set the overflow list. aOverflowFrames must not be an empty list. */ - void SetOverflowFrames(const nsFrameList& aOverflowFrames) { + void SetOverflowFrames(nsFrameList&& aOverflowFrames) { MOZ_ASSERT(aOverflowFrames.NotEmpty(), "Shouldn't be called"); SetProperty(OverflowProperty(), - new (PresShell()) nsFrameList(aOverflowFrames)); + new (PresShell()) nsFrameList(std::move(aOverflowFrames))); } /** diff --git a/layout/generic/nsFirstLetterFrame.cpp b/layout/generic/nsFirstLetterFrame.cpp index 4ecf4dfe225a..3ffdb6a3d960 100644 --- a/layout/generic/nsFirstLetterFrame.cpp +++ b/layout/generic/nsFirstLetterFrame.cpp @@ -259,9 +259,9 @@ void nsFirstLetterFrame::Reflow(nsPresContext* aPresContext, if (!IsFloating()) { CreateNextInFlow(kid); // And then push it to our overflow list - const nsFrameList& overflow = mFrames.RemoveFramesAfter(kid); + nsFrameList overflow = mFrames.RemoveFramesAfter(kid); if (overflow.NotEmpty()) { - SetOverflowFrames(overflow); + SetOverflowFrames(std::move(overflow)); } } else if (!kid->GetNextInFlow()) { // For floating first letter frames (if a continuation wasn't already diff --git a/layout/generic/nsFrameList.h b/layout/generic/nsFrameList.h index 613240974d74..553ac64accb2 100644 --- a/layout/generic/nsFrameList.h +++ b/layout/generic/nsFrameList.h @@ -80,8 +80,28 @@ class nsFrameList { VerifyList(); } + // XXX: Ideally, copy constructor should be removed because a frame should be + // owned by one list. nsFrameList(const nsFrameList& aOther) = default; + // XXX: ideally, copy assignment should be removed because we should use move + // assignment to transfer the ownership. + nsFrameList& operator=(const nsFrameList& aOther) = default; + + /** + * Move the frames in aOther to this list. aOther becomes empty after this + * operation. + */ + nsFrameList(nsFrameList&& aOther) + : mFirstChild(aOther.mFirstChild), mLastChild(aOther.mLastChild) { + aOther.Clear(); + VerifyList(); + } + nsFrameList& operator=(nsFrameList&& aOther) { + SetFrames(aOther); + return *this; + } + /** * Infallibly allocate a nsFrameList from the shell arena. */ diff --git a/layout/generic/nsIFrame.h b/layout/generic/nsIFrame.h index 48ed73f989b5..45ab9ff6277d 100644 --- a/layout/generic/nsIFrame.h +++ b/layout/generic/nsIFrame.h @@ -5734,6 +5734,7 @@ template template /* static */ void nsIFrame::SortFrameList(nsFrameList& aFrameList) { nsIFrame* head = MergeSort(aFrameList.FirstChild()); + aFrameList.Clear(); aFrameList = nsFrameList(head, nsLayoutUtils::GetLastSibling(head)); MOZ_ASSERT(IsFrameListSorted(aFrameList), "After we sort a frame list, it should be in sorted order..."); diff --git a/layout/tables/nsTableFrame.cpp b/layout/tables/nsTableFrame.cpp index 73467adb98f4..7c5ee5b2d307 100644 --- a/layout/tables/nsTableFrame.cpp +++ b/layout/tables/nsTableFrame.cpp @@ -2107,7 +2107,7 @@ void nsTableFrame::PushChildren(const RowGroupArray& aRowGroups, nextInFlow->mFrames.InsertFrames(nextInFlow, prevSibling, frames); } else { // Add the frames to our overflow list. - SetOverflowFrames(frames); + SetOverflowFrames(std::move(frames)); } }