[LayoutNGFragmentPaint] Fix containing-block of OOF-positioned objects. #20185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wpt-pr-bot
approved these changes
Nov 8, 2019
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-1906708
branch
3 times, most recently
from
279bcdb
to
34bb526
Compare
We had an issue in the existing invalidation code when an object could contain an OOF-positioned node, but wasn't a LayoutBlock. This already happens when we have a LayoutInline being a containing-block, but there are other cases where this is true. Clusterfuzz found that LayoutTableSection falls into this category. E.g. The OOF-positioned node would be inserted into the nearest containing-block (the anonymous LayoutTable in this case). When it stopped being a containing-block, the OOF-positioned node was never removed from the LayoutTable. This caused a crash when the OOF's layout was invalidated. The OOF marked itself, and the LayoutView (its new containing block) for layout. But the LayoutView didn't know it had this as an OOF-positioned child. This patch moves the current logic within LayoutBlock into LayoutBoxModelObject. Bug: 1021491, 1021676, 1022545 Change-Id: I0f0b4c8aa655fc7edca5d79379205a8d445713d5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906708 Reviewed-by: Aleks Totic <atotic@chromium.org> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/master@{#714487}
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-1906708
branch
from
34bb526
to
5c7b999
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We had an issue in the existing invalidation code when an object could
contain an OOF-positioned node, but wasn't a LayoutBlock.
This already happens when we have a LayoutInline being a
containing-block, but there are other cases where this is true.
Clusterfuzz found that LayoutTableSection falls into this category.
E.g.
The OOF-positioned node would be inserted into the nearest
containing-block (the anonymous LayoutTable in this case).
When it stopped being a containing-block, the OOF-positioned node was
never removed from the LayoutTable.
This caused a crash when the OOF's layout was invalidated.
The OOF marked itself, and the LayoutView (its new containing block)
for layout.
But the LayoutView didn't know it had this as an OOF-positioned child.
This patch moves the current logic within LayoutBlock into
LayoutBoxModelObject.
Bug: 1021491, 1021676, 1022545
Change-Id: I0f0b4c8aa655fc7edca5d79379205a8d445713d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906708
Reviewed-by: Aleks Totic <atotic@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714487}