Skip to content

Commit

Permalink
Don't ClearNeedsLayout on atomic inline when side-effects are disabled.
Browse files Browse the repository at this point in the history
Otherwise we risk getting an incorrect cache hit, so that we fail to lay
out stuff during the subsequent actual layout.

In the attached test, side-effects are disabled in
ComputeCaptionFragments().

Bug: 1347868
Change-Id: Ib035d0a8e0b4e9ea16f3253a3272d8d3257312b1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3828230
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1034975}
  • Loading branch information
mstensho authored and Chromium LUCI CQ committed Aug 15, 2022
1 parent 9d4a4e5 commit ca758f1
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
8 changes: 5 additions & 3 deletions third_party/blink/renderer/core/layout/ng/ng_block_node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1844,9 +1844,11 @@ const NGLayoutResult* NGBlockNode::LayoutAtomicInline(
parent_constraint_space.ReplacedPercentageResolutionSize());
NGConstraintSpace constraint_space = builder.ToConstraintSpace();
const NGLayoutResult* result = Layout(constraint_space);
// TODO(kojii): Investigate why ClearNeedsLayout() isn't called automatically
// when it's being laid out.
GetLayoutBox()->ClearNeedsLayout();
if (!NGDisableSideEffectsScope::IsDisabled()) {
// TODO(kojii): Investigate why ClearNeedsLayout() isn't called
// automatically when it's being laid out.
GetLayoutBox()->ClearNeedsLayout();
}
return result;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<!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=1347868">
<div style="columns:2; column-fill:auto; height:500px;">
<table>
<caption>
<div style="display:inline-block;">
<div style="writing-mode:vertical-lr;">
<div id="elm" style="display:inline-block;"></div>
<div style="display:inline-block; overflow:auto;"></div>
</div>
</div>
</caption>
</table>
</div>
<script>
document.body.offsetTop;
elm.style.display = "none";
</script>

0 comments on commit ca758f1

Please sign in to comment.