[LayoutNG] Fix access to deleted OOF descendant

This patch fixes the reported crash. The reproducible case is https://ajuntament.barcelona.cat/widgets/socials/bcn/index.html?lang=es&numRows=1&header&menu&boxColor=aab0b0&buttonColor=aab0b0&buttonRadius=50&fontColor=ffffff&id=3c58a16711f992fee70edcf13ef72da3 Unfortunately, I have been unable to create a minimal reproducible case that causes a crash. The added test case exercises new code path, just without a crash. The bug observed at the crashing URL was: LayoutResult::OOFPositionedDescendants had an invalid descendant that was already removed from the tree. What made this failure extra annoying is that memory occupied by invalid descendant was reused for a different LayoutObject. Removal of the descendant caused LayoutObject::MarkContainerChainForLayout to be called, but the container chain was not fully traversed because ObjectIsRelayoutBoundary returned true and aborted the traversal. My understanding is as following. In 2015, leviw introduced an invalidation optimization. The optimization happens when child needs layout, but can guarantee that it will not affect layout of its parent. In this case, child stops marking parents for relayout, and is instead put on LayoutSubtreeRootList, which gets traversed for layout directly. Method that does this is LocalFrameView::ScheduleRelayoutOfSubtree The method that decides whether child affects parents layout is: ObjectIsRelayoutBoundary(). I've fixed this method for NG, so that cached layout result with oof descendants causes full container chain traversal. Bug: 933054 Change-Id: Ie3daab53937297f1cea75b0974c3fce353c86200 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1500610 Commit-Queue: Aleks Totic <atotic@chromium.org> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/master@{#637821}
web-platform-tests · Mar 5, 2019 · f3d0066 · f3d0066
1 parent 26beb17
commit f3d0066
Showing 1 changed file with 47 additions and 0 deletions.
diff --git a/css/css-position/position-absolute-container-dynamic-002.html b/css/css-position/position-absolute-container-dynamic-002.html
@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<title>CSS Position Absolute: dynamic changes to containing block height</title>
+<link rel="author" href="mailto:atotic@google.com">
+<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=933054">
+<meta name="assert" content="Chrome regression: abspos descendant responds to containing block size change through line items">
+<style>
+
+#container {
+  position: relative;
+}
+#intermediate {
+  overflow: hidden;
+  width:200px;
+  height:200px;
+  background:red;
+}
+#block {
+  height:200px;
+  background:green;
+}
+#target {
+  position: absolute;
+  width: 200px;
+  height: 100px;
+  background:green;
+}
+</style>
+<!-- Test for crbug.com/933054
+  Relayout optimizations cause OOF descendant not to be
+  repositioned
+-->
+<div id="container">
+  <div id="intermediate">
+    <div id="block"></div>
+    <div id="target"></div>
+  </div>
+</div>
+
+<script>
+document.body.offsetTop;
+test(() => {
+  document.getElementById("block").style.height = "100px";
+  assert_equals(document.querySelector("#target").offsetTop, 100);
+}, '#target static position responded to parent relayout');
+</script>