Don't lay out OOFs inside fragmentation if side-effects are disabled.

The OOF positioning code depends on the LayoutBox data being up-to-date, which doesn't happen if we lay out with side-effects disabled (as that would be side-effect). Just don't lay them out. It's not safe. This also reverts CL:3823699, as it should now be obsolete with this brutal fix. Bug: 1347713 Change-Id: Icb63237618647489159446a50d8eb7984243a7f9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3829547 Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Reviewed-by: Alison Maher <almaher@microsoft.com> Commit-Queue: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/main@{#1034974}
chromium · Aug 15, 2022 · 9d4a4e5 · 9d4a4e5
1 parent 6a1743f
commit 9d4a4e5
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 2 deletions.
diff --git a/third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.cc b/third_party/blink/renderer/core/layout/ng/ng_out_of_flow_layout_part.cc
@@ -20,6 +20,7 @@
 #include "third_party/blink/renderer/core/layout/ng/legacy_layout_tree_walking.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_absolute_utils.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_constraint_space_builder.h"
+#include "third_party/blink/renderer/core/layout/ng/ng_disable_side_effects_scope.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_fragment.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_layout_result.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_out_of_flow_positioned_node.h"
@@ -256,6 +257,12 @@ bool NGOutOfFlowLayoutPart::SweepLegacyCandidates(
 
 void NGOutOfFlowLayoutPart::HandleFragmentation(
     ColumnBalancingInfo* column_balancing_info) {
+  // OOF fragmentation depends on LayoutBox data being up-to-date, which isn't
+  // the case if side-effects are disabled. So we cannot safely do anything
+  // here.
+  if (NGDisableSideEffectsScope::IsDisabled())
+    return;
+
   if (!column_balancing_info &&
       (!container_builder_->IsBlockFragmentationContextRoot() ||
        has_block_fragmentation_))
@@ -1109,8 +1116,7 @@ void NGOutOfFlowLayoutPart::LayoutFragmentainerDescendants(
         // column balancing. However, if the containing block has not finished
         // layout, we should wait to lay out the OOF in case its position is
         // dependent on its containing block's final size.
-        if (containing_block->PhysicalFragments().IsEmpty() ||
-            containing_block->PhysicalFragments().back().BreakToken()) {
+        if (containing_block->PhysicalFragments().back().BreakToken()) {
           delayed_descendants_.push_back(descendant);
           continue;
         }

diff --git a/...xternal/wpt/css/css-multicol/crashtests/table-caption-change-descendant-display-type.html b/...xternal/wpt/css/css-multicol/crashtests/table-caption-change-descendant-display-type.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
+<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1347713">
+<div style="columns:2;">
+  <div style="display:table-caption; columns:2;">
+    <div style="columns:2;">
+      <div style="position:relative;">
+        <div id="foo" style="position:absolute;"></div>
+      </div>
+    </div>
+  </div>
+</div>
+<script>
+  document.body.offsetTop;
+  foo.style.display = "flex";
+</script>