This testcase hits a rare condition where the abs pos frame is under the first continuation of the containing block frame, but the placeholder frame is under the next continuation of th containing block frame. This means that the walk in FindContainingBlocks which adds the ForceDescendIntoIfVisible bit to all ancestors of modified frames (walking through placeholders) never marks the first continuation of the containing block frame, which is the actual frame that contains the modified frame. That means that we don't walk into the containing block for the modified abs pos frame, and so we don't call MarkAbsoluteFramesForDisplayList, which computes the dirty rect for the abs pos frame, and if it is non-empty it would walk it's ancestor chain through placeholders and add the NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO bit. But we don't do that, so when we come to build the display list for the placeholder frame, the placeholder frame has size 0x0 of course, so we determine it is not visible, and even though the ForceDescendIntoIfVisible bit is set on it, since it's not visible we don't descend into it. We need the NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO bit which would have been set.
So for this to work we need to make sure in addition to the ancestor chain walking through placeholders has the ForceDescendIntoIfVisible bit set, we also need to make sure the normal GetParent ancestor chain has that bit set. So anywhere that we use GetDisplayListParent where this kind of fix seems like it is needed we need to modify. We try to be a bit clever to try to avoid any extra work for this rare case. We do this by checking if we are going to be jumping to the placeholder, and if so we make sure to mark the parent of the current frame, before continuing up the GetDisplayListParent walk. This doesn't visit anymore frames as in the common case we stop the walk when we get to the parent frame because the ForceDescendIntoIfVisible bit is set on it. This does add a function call overhead though.
When we hit this condition we do have to walk more frames. To make sure this condition isn't too common, and thus aren't making ourselves walk a lot more frames, I did a full try run on linux to see how often it was hit. We hit it 3 times. layout/base/crashtests/1464641.html which is a reduced testcase of another retained display list bug. testing/web-platform/tests/svg/layout/svg-with-precent-dimensions-relayout.html. And layout/reftests/display-list/1709452-1.html which is another retained display list bug, which also has to do with continuations and it is almost an identical copy of the reduced testcase of this bug.
Differential Revision: https://phabricator.services.mozilla.com/D174458
After moving FrameChildListID into mozilla namespace, `kPrincipalList` etc. are
also exposed in the mozilla namespace. In the next part, I'll convert
FrameChildListID enum into an enum class, so the naming pollution shouldn't be
an issue.
This patch has a nice side effect that it is now easier to remove all the
aliases of FrameChildListID (`kPrincipalList` etc.) defined in multiple places
since it is confusion to have the same thing written in different ways, e.g.
`nsIFrame::kPrincipalList`, `mozilla::layout::kPrincipalList`,
`FrameChildListID::kPrincipalList`, `kPrincipalList`.
This patch doesn't change behavior.
Differential Revision: https://phabricator.services.mozilla.com/D161863
After moving FrameChildListID into mozilla namespace, `kPrincipalList` etc. are
also exposed in the mozilla namespace. In the next part, I'll convert
FrameChildListID enum into an enum class, so the naming pollution shouldn't be
an issue.
This patch has a nice side effect that it is now easier to remove all the
aliases of FrameChildListID (`kPrincipalList` etc.) defined in multiple places
since it is confusion to have the same thing written in different ways, e.g.
`nsIFrame::kPrincipalList`, `mozilla::layout::kPrincipalList`,
`FrameChildListID::kPrincipalList`, `kPrincipalList`.
This patch doesn't change behavior.
Differential Revision: https://phabricator.services.mozilla.com/D161863
This is basically equivalent to marking the viewport frame modified when the root scroll frame is marked modified.
We wrap the top layer items in a display wrap list that uses the viewport frame as it's mFrame here
https://searchfox.org/mozilla-central/rev/bb37e6fe8bbe383a57a4ad21a201e26416613ca1/layout/generic/ViewportFrame.cpp#236
So when we invalidate the root scroll frame for getting a new display port we do a partial update. The wrap list item for the top layer items has a new display item parent, a display async zoom item (the creation of the display port changed this), so the a new display wrap list item is created. All of it's children are matched with the old children of the old display wrap list so those go away, but the old display wrap list item itself sticks around: it's mFrame is the viewport frame which was not marked modified. So the next paint we have these two identical display wrap list items (one empty), and we hit the assert.
So it seems like we need to mark the viewport frame modified if the root scroll frame is marked modified if we have top layer items (since those top layers items end up being inside the root scroll frames display list). This in turn would disable a partial update for the next paint because ShouldBuildPartial would return false because a viewport frame is modified
https://searchfox.org/mozilla-central/rev/bb37e6fe8bbe383a57a4ad21a201e26416613ca1/layout/painting/RetainedDisplayListBuilder.cpp#1309
I'm not sure this is even a bad thing, since we also disable partial updates if the canvas frame (child of the root scroll frame) is marked modified. Perhaps we would want to disable partial updates if the root scroll frame is marked modified, but we can't detect root scroll frames purely from their layout frame type.
Differential Revision: https://phabricator.services.mozilla.com/D109848
