Skip to content

Commit

Permalink
[LayoutNG] Fix for DCHECK(NeedsLayout) for fixed
Browse files Browse the repository at this point in the history
All tests pass, and crashes no longer happen. I believe
that code will not longer crash, but there might be
futher instances of incorrect positioning.

Fix #1
LayoutDescendantCandidates did not sweep newly discovered
candidates. This was done
manually once inside NGOutOfFlowLayoutPart::Run, and
sweep was not performed for LayoutDescendantCandidates
found in Legacy. Fix is to make LayoutDescendantCandidates
perform sweep instead.

Fix #2
fix #1 exposed a bug where duplicate fragments were generated
for a single layout object. This happened when NG was generating
fragments not inside ContainingBlock. Fix one instance of this
inside NGOutOfFlowLayoutPart::IsContainingBlockForDescendant
by making sure that OOF with inline containers are only positioned
inside its ContainingBlock()

Fix #3
NGOutOfFlowLayoutPart::LayoutDescendant offset adjustment.

Bug: 935805
Change-Id: I9f7ebbc7223f40fbbf6ba3739d9385bfd59e3641
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1517093
Commit-Queue: Aleks Totic <atotic@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#641628}
  • Loading branch information
Aleks Totic authored and chromium-wpt-export-bot committed Mar 18, 2019
1 parent dce324a commit c748846
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 1 deletion.
2 changes: 1 addition & 1 deletion css/css-position/position-absolute-crash-chrome-002.html
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<!DOCTYPE html>
<title>CSS Position Absolute: Chrome chrash</title>
<title>CSS Position Absolute: Chrome crash</title>
<link rel="author" href="mailto:atotic@google.com">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
Expand Down
29 changes: 29 additions & 0 deletions css/css-position/position-absolute-crash-chrome-003.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<!DOCTYPE html>
<title>CSS Position Absolute: Chrome crash</title>
<link rel="author" href="mailto:atotic@chromium.org">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=935805">
<meta name="assert" content="Nested abs/fixed/flex do not crash">
<style>
#flex {
display: flex;
}
.abs {
position: absolute;
}
#fixed {
position: fixed;
}
</style>
<div class="abs">
<div id="flex">
<div class="abs">
<div id="fixed"></div>
</div>
</div>
</div>
<script>
test(() => {
}, 'test passes if it does not crash');
</script>
25 changes: 25 additions & 0 deletions css/css-position/position-absolute-in-inline-001.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<!doctype html>
<html>
<head>
<title>Absolute inside inline container location should be correct</title>
<link rel="author" href="mailto:atotic@chromium.org">
<link rel="help" href="https://www.w3.org/TR/css-position-3/#def-cb">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
</head>
<!-- There should be a green square below -->
<body style="margin:0">
<span id="container" style="position:relative;">
<div style="width:100px; height:100px; background:red;"></div>
<div id="target" style="position:absolute; left:0; top:0; width:100px; height:100px; background:green;"></div>
</span>
<script>
test(_ => {
let bounds = document.querySelector("#target").getBoundingClientRect();
let container_bounds = document.querySelector("#container").getBoundingClientRect();
assert_equals(bounds.x, container_bounds.x);
assert_equals(bounds.y, container_bounds.y);
}, "absolute inside inline container location should be correct.");
</script>
</body>
</html>

0 comments on commit c748846

Please sign in to comment.