Skip to content

Commit

Permalink
Bug 1865012 Part 2 - Add nsFrameList::RemoveLastChild, and destroy fr…
Browse files Browse the repository at this point in the history
…ame list in reverse order. r=dholbert

With only the Part 1, `layout/generic/crashtests/1683126.html` will timeout.
It can be reproduced locally via

```
./mach crashtest layout/generic/crashtests/1683126.html
```

It is because if we destroy a frame list from the first child to the last child,
each time we destroy the first child [1], the second child become the new first
child, and we have to update the first-in-flow and first-continuation cache for
later continuations, which is a linear time operation.

[1] See nsSplittableFrame::RemoveFromFlow() with the argument being the first
child in a frame list.

Differential Revision: https://phabricator.services.mozilla.com/D197967
  • Loading branch information
aethanyc committed Jan 23, 2024
1 parent d7ac9e2 commit efe6e54
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 4 deletions.
13 changes: 11 additions & 2 deletions layout/generic/nsFrameList.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,10 @@ void nsFrameList::Delete(mozilla::PresShell* aPresShell) {
}

void nsFrameList::DestroyFrames(FrameDestroyContext& aContext) {
while (nsIFrame* frame = RemoveFirstChild()) {
while (nsIFrame* frame = RemoveLastChild()) {
frame->Destroy(aContext);
}
mLastChild = nullptr;
MOZ_ASSERT(!mFirstChild && !mLastChild, "We should've destroyed all frames!");
}

void nsFrameList::RemoveFrame(nsIFrame* aFrame) {
Expand Down Expand Up @@ -95,6 +95,15 @@ nsIFrame* nsFrameList::RemoveFirstChild() {
return nullptr;
}

nsIFrame* nsFrameList::RemoveLastChild() {
if (mLastChild) {
nsIFrame* lastChild = mLastChild;
RemoveFrame(lastChild);
return lastChild;
}
return nullptr;
}

void nsFrameList::DestroyFrame(FrameDestroyContext& aContext,
nsIFrame* aFrame) {
MOZ_ASSERT(aFrame, "null ptr");
Expand Down
5 changes: 3 additions & 2 deletions layout/generic/nsFrameList.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,11 @@ class nsFrameList {
[[nodiscard]] nsFrameList TakeFramesAfter(nsIFrame* aFrame);

/**
* Take the first frame (if any) out of the frame list.
* @return the first child, or nullptr if the list is empty
* Take the first (or last) child (if any) out of the frame list.
* @return the first (or last) child, or nullptr if the list is empty
*/
nsIFrame* RemoveFirstChild();
nsIFrame* RemoveLastChild();

/**
* The following two functions are intended to be used in concert for
Expand Down

0 comments on commit efe6e54

Please sign in to comment.