Custom layout needs to force NG layout for the subtree. #25575
Merged
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
Sep 16, 2020
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-2414289
branch
3 times, most recently
from
62a3f1f
to
31c1b63
Compare
Just like for MathML, custom layout (display:layout()) has no legacy layout engine implementation, so we need to force NG layout, even when contained by something that requires legacy fallback. Rewrote the mechanism somewhat; calling ComputedStyle() on an element isn't the right thing to do within Element::ShouldForceLegacyLayout() (it called ShouldForceNGLayout(), which did that), since ShouldForceLegacyLayout() is involved when style is about to change, or when it has just been changed. ShouldForceLegacyLayout() will now return true even if the display type requires LayoutNG. Cope with this by just not setting ForceLegacyLayout on any NG LayoutObjects at all. And remove a couple of DCHECKs in NG LayoutObject classes that expected legacy fallback not to be in use on the Element side. This is rather convoluted. :( Also update HTMLSlotElement::AttachLayoutTree(), which recently got modified to support legacy fallback properly. This fixes one of three DCHECK failures that occur in the fuzzer test in bug 1127112. Bug: 1127112 Change-Id: I16b5f5297cb04192056f6c5137c2cefa1bc33298 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414289 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Frédéric Wang <fwang@igalia.com> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/master@{#808311}
chromium-wpt-export-bot
force-pushed
the
chromium-export-cl-2414289
branch
from
31c1b63
to
246027a
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.
Just like for MathML, custom layout (display:layout()) has no legacy
layout engine implementation, so we need to force NG layout, even when
contained by something that requires legacy fallback.
Rewrote the mechanism somewhat; calling ComputedStyle() on an element
isn't the right thing to do within Element::ShouldForceLegacyLayout()
(it called ShouldForceNGLayout(), which did that), since
ShouldForceLegacyLayout() is involved when style is about to change, or
when it has just been changed. ShouldForceLegacyLayout() will now
return true even if the display type requires LayoutNG. Cope with this
by just not setting ForceLegacyLayout on any NG LayoutObjects at all.
And remove a couple of DCHECKs in NG LayoutObject classes that expected
legacy fallback not to be in use on the Element side.
This is rather convoluted. :(
Also update HTMLSlotElement::AttachLayoutTree(), which recently got
modified to support legacy fallback properly.
This fixes one of three DCHECK failures that occur in the fuzzer test
in bug 1127112.
Bug: 1127112
Change-Id: I16b5f5297cb04192056f6c5137c2cefa1bc33298
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2414289
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Frédéric Wang <fwang@igalia.com>
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808311}