Commit ab40da96 authored by Vladimir Levin's avatar Vladimir Levin Committed by Commit Bot

Isolation: Remove isolation if we have non-svg replaced content.

This patch ensures that we don't put isolation nodes in place if we have
replaced content transform in an non-svg. The reason for this is twofold:

1. Typically, isolation wouldn't buy us anything, since we're likely
   looking at an image, which has no subtree to isolate.
2. When we get the chain of transforms, we need to decide whether we're
   returning a replaced content or an isolation transform since they are
   both leafs of a chain, but not a part of the same chain. Currently we
   return the isolation node first. We could fix this by returning the
   replaced content first, but then we specifically bypass transform
   isolation which may cause other bugs down the line.

R=pdr@chromium.org

Bug: 937571
Change-Id: I39f51a2e7810f058bfdf49c28c8f2e3857565ebe
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1506572Reviewed-by: default avatarPhilip Rogers <pdr@chromium.org>
Commit-Queue: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638271}
parent 13ad69c9
......@@ -335,6 +335,18 @@ static bool NeedsIsolationNodes(const LayoutObject& object) {
if (!object.HasLayer())
return false;
// Non-SVG replaced content should not have isolation nodes. Specifically if
// these nodes generate a replaced content transform, they don't update the
// current transform (See UpdateReplacedContentTransform()). This means that
// if we put isolation nodes, which isolate the current transform, then while
// getting FragmentData::PostScrollTranslation(), we return a wrong transform
// chain (isolation -> "current" transform, instead of replaced transform ->
// "current" transform). Note that using ReplacedContentTransform() and
// isolating that would violate the condition that the replaced content
// transform should not update the current transform (in the non-svg case).
if (NeedsReplacedContentTransform(object) && !object.IsSVGRoot())
return false;
// Paint containment establishes isolation.
// Style & Layout containment also establish isolation.
if (object.ShouldApplyPaintContainment() ||
......
......@@ -3421,6 +3421,28 @@ TEST_P(PaintPropertyTreeBuilderTest, OverflowClipContentsTreeState) {
CHECK_EXACT_VISUAL_RECT(LayoutRect(0, 0, 500, 600), child, clipper);
}
TEST_P(PaintPropertyTreeBuilderTest, ReplacedSvgContentWithIsolation) {
SetBodyInnerHTML(R"HTML(
<style>
body { margin 0px; }
</style>
<svg id='replacedsvg'
style='contain:paint; will-change:transform;' width="100px" height="200px"
viewBox='50 50 100 100'>
</svg>
)HTML");
LayoutBoxModelObject* svg =
ToLayoutBoxModelObject(GetLayoutObjectByElementId("replacedsvg"));
const ObjectPaintProperties* svg_properties =
svg->FirstFragment().PaintProperties();
EXPECT_TRUE(svg_properties->TransformIsolationNode());
EXPECT_TRUE(svg_properties->ReplacedContentTransform());
EXPECT_EQ(svg_properties->TransformIsolationNode()->Parent(),
svg_properties->ReplacedContentTransform());
}
TEST_P(PaintPropertyTreeBuilderTest, ContainPaintOrStyleLayoutTreeState) {
for (const char* containment : {"paint", "style layout"}) {
SCOPED_TRACE(containment);
......
<!doctype html>
<style>
img {
width: 200px;
height: 100px;
}
</style>
<img src="image.png">
<!doctype html>
<html class="reftest-wait">
<title>Ensure images with containment and size are rendered properly</title>
<meta charset="utf-8">
<link rel="match" href="img-with-containment-and-size-ref.html">
<link rel="help" href="https://html.spec.whatwg.org/multipage/#the-img-element">
<style>
img {
contain: paint;
width: 200px;
height: 100px;
will-change: transform;
}
</style>
<script>
var i = new Image();
i.onload = function() {
document.body.appendChild(i);
document.documentElement.classList.remove("reftest-wait");
};
i.src = "image.png";
</script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment