Skip to content

GitLab

T
tangled
Commit a7d66f97 authored by Philip Rogers's avatar Philip Rogers Committed by Commit Bot
Browse files
Options

[BlinkGenPropertyTrees] Pass all unit tests with BGPT 

This patch prepares for promoting BlinkGenPropertyTrees (BGPT) to
experimental by ensuring all unit tests pass. This patch has three
changes:
1) The foreign layer BGPT code has been disabled when SPV2 is on in
LocalFrameView::PaintTree.
2) Some CompositedLayerMapping tests of non-BGPT code have been changed
to only run without BGPT.
3) ScrollingCoordinatorTest of sticky have been updated to look up
sticky constraints on property trees when BGPT is enabled.

Bug: 836902
Change-Id: If5c32b3e02c0908bf505bddbb2d4b635b8049555
Reviewed-on: https://chromium-review.googlesource.com/c/1287169Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601428}
parent 723b1eed
Hide whitespace changes
Inline Side-by-side
Showing with 67 additions and 57 deletions
third_party/blink/renderer/core/frame/local_frame_view.cc
View file @ a7d66f97
......@@ -2870,7 +2870,8 @@ void LocalFrameView::PaintTree() {
if (auto* web_local_frame_impl = WebLocalFrameImpl::FromFrame(frame_))
web_local_frame_impl->UpdateDevToolsOverlays();
if (RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled()) {
if (RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled() &&
!RuntimeEnabledFeatures::SlimmingPaintV2Enabled()) {
// BlinkGenPropertyTrees just needs a transient PaintController to
// collect the foreign layers which doesn't need caching. It also
// shouldn't affect caching status of DisplayItemClients because it's
......
third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_test.cc
View file @ a7d66f97
......@@ -343,6 +343,23 @@ TEST_P(ScrollingCoordinatorTest, fastScrollingForFixedPosition) {
}
}
// BlinkGenPropertyTrees (BGPT) changes where the sticky constraints are stored.
// Without BGPT, sticky constraints are stored on cc::Layer (via
// GraphicsLayer::SetStickyPositionConstraint). With BGPT, sticky constraints
// are stored on transform property tree nodes.
static cc::LayerStickyPositionConstraint GetStickyConstraint(Element* element) {
if (!RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled()) {
cc::Layer* layer = CcLayerFromElement(element);
DCHECK(layer);
return layer->sticky_position_constraint();
}
const auto* properties =
element->GetLayoutObject()->FirstFragment().PaintProperties();
DCHECK(properties);
return *properties->StickyTranslation()->GetStickyConstraint();
}
TEST_P(ScrollingCoordinatorTest, fastScrollingForStickyPosition) {
RegisterMockedHttpURLLoad("sticky-position.html");
NavigateTo(base_url_ + "sticky-position.html");
......@@ -356,11 +373,7 @@ TEST_P(ScrollingCoordinatorTest, fastScrollingForStickyPosition) {
Document* document = GetFrame()->GetDocument();
{
Element* element = document->getElementById("div-tl");
ASSERT_TRUE(element);
cc::Layer* layer = CcLayerFromElement(element);
ASSERT_TRUE(layer);
cc::LayerStickyPositionConstraint constraint =
layer->sticky_position_constraint();
auto constraint = GetStickyConstraint(element);
ASSERT_TRUE(constraint.is_sticky);
EXPECT_TRUE(constraint.is_anchored_top && constraint.is_anchored_left &&
!constraint.is_anchored_right &&
......@@ -374,44 +387,28 @@ TEST_P(ScrollingCoordinatorTest, fastScrollingForStickyPosition) {
}
{
Element* element = document->getElementById("div-tr");
ASSERT_TRUE(element);
cc::Layer* layer = CcLayerFromElement(element);
ASSERT_TRUE(layer);
cc::LayerStickyPositionConstraint constraint =
layer->sticky_position_constraint();
auto constraint = GetStickyConstraint(element);
ASSERT_TRUE(constraint.is_sticky);
EXPECT_TRUE(constraint.is_anchored_top && !constraint.is_anchored_left &&
constraint.is_anchored_right && !constraint.is_anchored_bottom);
}
{
Element* element = document->getElementById("div-bl");
ASSERT_TRUE(element);
cc::Layer* layer = CcLayerFromElement(element);
ASSERT_TRUE(layer);
cc::LayerStickyPositionConstraint constraint =
layer->sticky_position_constraint();
auto constraint = GetStickyConstraint(element);
ASSERT_TRUE(constraint.is_sticky);
EXPECT_TRUE(!constraint.is_anchored_top && constraint.is_anchored_left &&
!constraint.is_anchored_right && constraint.is_anchored_bottom);
}
{
Element* element = document->getElementById("div-br");
ASSERT_TRUE(element);
cc::Layer* layer = CcLayerFromElement(element);
ASSERT_TRUE(layer);
cc::LayerStickyPositionConstraint constraint =
layer->sticky_position_constraint();
auto constraint = GetStickyConstraint(element);
ASSERT_TRUE(constraint.is_sticky);
EXPECT_TRUE(!constraint.is_anchored_top && !constraint.is_anchored_left &&
constraint.is_anchored_right && constraint.is_anchored_bottom);
}
{
Element* element = document->getElementById("span-tl");
ASSERT_TRUE(element);
cc::Layer* layer = CcLayerFromElement(element);
ASSERT_TRUE(layer);
cc::LayerStickyPositionConstraint constraint =
layer->sticky_position_constraint();
auto constraint = GetStickyConstraint(element);
ASSERT_TRUE(constraint.is_sticky);
EXPECT_TRUE(constraint.is_anchored_top && constraint.is_anchored_left &&
!constraint.is_anchored_right &&
......@@ -419,11 +416,7 @@ TEST_P(ScrollingCoordinatorTest, fastScrollingForStickyPosition) {
}
{
Element* element = document->getElementById("span-tlbr");
ASSERT_TRUE(element);
cc::Layer* layer = CcLayerFromElement(element);
ASSERT_TRUE(layer);
cc::LayerStickyPositionConstraint constraint =
layer->sticky_position_constraint();
auto constraint = GetStickyConstraint(element);
ASSERT_TRUE(constraint.is_sticky);
EXPECT_TRUE(constraint.is_anchored_top && constraint.is_anchored_left &&
constraint.is_anchored_right && constraint.is_anchored_bottom);
......@@ -434,11 +427,7 @@ TEST_P(ScrollingCoordinatorTest, fastScrollingForStickyPosition) {
}
{
Element* element = document->getElementById("composited-top");
ASSERT_TRUE(element);
cc::Layer* layer = CcLayerFromElement(element);
ASSERT_TRUE(layer);
cc::LayerStickyPositionConstraint constraint =
layer->sticky_position_constraint();
auto constraint = GetStickyConstraint(element);
ASSERT_TRUE(constraint.is_sticky);
EXPECT_TRUE(constraint.is_anchored_top);
EXPECT_EQ(gfx::Rect(100, 110, 10, 10),
......
third_party/blink/renderer/core/paint/compositing/composited_layer_mapping_test.cc
View file @ a7d66f97
......@@ -59,6 +59,17 @@ class CompositedLayerMappingTest : public RenderingTest {
void TearDown() override { RenderingTest::TearDown(); }
};
// Tests the pre-BlinkGenPropertyTrees composited layer mapping code. With BGPT,
// some layer updates are skipped (see: CLM::UpdateGraphicsLayerConfiguration
// and CLM::UpdateStickyConstraints).
class CompositedLayerMappingTestWithoutBGPT
: private ScopedBlinkGenPropertyTreesForTest,
public CompositedLayerMappingTest {
public:
CompositedLayerMappingTestWithoutBGPT()
: ScopedBlinkGenPropertyTreesForTest(false) {}
};
TEST_F(CompositedLayerMappingTest, SubpixelAccumulationChange) {
SetBodyInnerHTML(
"<div id='target' style='will-change: transform; background: lightblue; "
......@@ -467,7 +478,7 @@ TEST_F(CompositedLayerMappingTest, ClippedBigLayer) {
RecomputeInterestRect(paint_layer->GraphicsLayerBacking()));
}
TEST_F(CompositedLayerMappingTest, ClippingMaskLayer) {
TEST_F(CompositedLayerMappingTestWithoutBGPT, ClippingMaskLayer) {
if (RuntimeEnabledFeatures::SlimmingPaintV2Enabled())
return;
......@@ -1252,7 +1263,8 @@ TEST_F(CompositedLayerMappingTest,
EXPECT_FLOAT_EQ(10, scrolling_layer2->GetPosition().y());
}
TEST_F(CompositedLayerMappingTest, AncestorClippingMaskLayerUpdates) {
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClippingMaskLayerUpdates) {
SetBodyInnerHTML(R"HTML(
<style>
#ancestor { width: 100px; height: 100px; overflow: hidden; }
......@@ -1332,7 +1344,8 @@ TEST_F(CompositedLayerMappingTest, AncestorClippingMaskLayerUpdates) {
EXPECT_FALSE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest, AncestorClippingMaskLayerSiblingUpdates) {
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClippingMaskLayerSiblingUpdates) {
SetBodyInnerHTML(R"HTML(
<style>
#ancestor { width: 200px; height: 200px; overflow: hidden; }
......@@ -1468,7 +1481,8 @@ TEST_F(CompositedLayerMappingTest, AncestorClippingMaskLayerSiblingUpdates) {
EXPECT_FALSE(child2_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest, AncestorClippingMaskLayerGrandchildUpdates) {
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClippingMaskLayerGrandchildUpdates) {
SetBodyInnerHTML(R"HTML(
<style>
#ancestor { width: 200px; height: 200px; overflow: hidden; }
......@@ -1581,7 +1595,8 @@ TEST_F(CompositedLayerMappingTest, AncestorClippingMaskLayerGrandchildUpdates) {
EXPECT_FALSE(grandchild_mapping->AncestorClippingLayer());
}
TEST_F(CompositedLayerMappingTest, AncestorClipMaskRequiredByBorderRadius) {
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskRequiredByBorderRadius) {
// Verify that we create the mask layer when the child is contained within
// the rectangular clip but not contained within the rounded rect clip.
SetBodyInnerHTML(R"HTML(
......@@ -1623,7 +1638,7 @@ TEST_F(CompositedLayerMappingTest, AncestorClipMaskRequiredByBorderRadius) {
EXPECT_TRUE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest,
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskNotRequiredByNestedBorderRadius) {
// This case has the child within all ancestors and does not require a
// mask.
......@@ -1661,7 +1676,7 @@ TEST_F(CompositedLayerMappingTest,
EXPECT_FALSE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest,
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskRequiredByParentBorderRadius) {
// This case has the child within the grandparent but not the parent, and does
// require a mask so that the parent will clip the corners.
......@@ -1702,7 +1717,7 @@ TEST_F(CompositedLayerMappingTest,
EXPECT_EQ(120, layer_size.height());
}
TEST_F(CompositedLayerMappingTest,
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskNotRequiredByParentBorderRadius) {
// This case has the child within the grandparent but not the parent, and does
// not require a mask because the parent does not have border radius
......@@ -1740,7 +1755,7 @@ TEST_F(CompositedLayerMappingTest,
EXPECT_FALSE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest,
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskRequiredByGrandparentBorderRadius1) {
// This case has the child clipped by the grandparent border radius but not
// the parent, and requires a mask to clip to the grandparent. Although in
......@@ -1783,7 +1798,7 @@ TEST_F(CompositedLayerMappingTest,
EXPECT_EQ(120, layer_size.height());
}
TEST_F(CompositedLayerMappingTest,
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskRequiredByGrandparentBorderRadius2) {
// Similar to the previous case, but here we really do need the mask.
SetBodyInnerHTML(R"HTML(
......@@ -1823,7 +1838,7 @@ TEST_F(CompositedLayerMappingTest,
EXPECT_EQ(160, layer_size.height());
}
TEST_F(CompositedLayerMappingTest,
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskNotRequiredByBorderRadiusInside) {
// Verify that we do not create the mask layer when the child is contained
// within the rounded rect clip.
......@@ -1866,7 +1881,7 @@ TEST_F(CompositedLayerMappingTest,
EXPECT_FALSE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest,
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskNotRequiredByBorderRadiusOutside) {
// Verify that we do not create the mask layer when the child is outside
// the ancestors rectangular clip.
......@@ -1909,7 +1924,8 @@ TEST_F(CompositedLayerMappingTest,
EXPECT_FALSE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest, AncestorClipMaskRequiredDueToScaleUp) {
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskRequiredDueToScaleUp) {
// Verify that we include the mask when the untransformed child does not
// intersect the border radius but the transformed child does. Here the
// child is inside the parent and scaled to expand to be clipped.
......@@ -1943,7 +1959,8 @@ TEST_F(CompositedLayerMappingTest, AncestorClipMaskRequiredDueToScaleUp) {
EXPECT_TRUE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest, AncestorClipMaskNotRequiredDueToScaleDown) {
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskNotRequiredDueToScaleDown) {
// Verify that we exclude the mask when the untransformed child does
// intersect the border radius but the transformed child does not. Here the
// child is bigger than the parent and scaled down such that it does not
......@@ -1978,7 +1995,8 @@ TEST_F(CompositedLayerMappingTest, AncestorClipMaskNotRequiredDueToScaleDown) {
EXPECT_FALSE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest, AncestorClipMaskRequiredDueToTranslateInto) {
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskRequiredDueToTranslateInto) {
// Verify that we include the mask when the untransformed child does not
// intersect the border radius but the transformed child does. Here the
// child is outside the parent and translated to be clipped.
......@@ -2012,7 +2030,7 @@ TEST_F(CompositedLayerMappingTest, AncestorClipMaskRequiredDueToTranslateInto) {
EXPECT_TRUE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest,
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskNotRequiredDueToTranslateOut) {
// Verify that we exclude the mask when the untransformed child does
// intersect the border radius but the transformed child does not. Here the
......@@ -2047,7 +2065,8 @@ TEST_F(CompositedLayerMappingTest,
EXPECT_FALSE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest, AncestorClipMaskRequiredDueToRotation) {
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskRequiredDueToRotation) {
// Verify that we include the mask when the untransformed child does not
// intersect the border radius but the transformed child does. Here the
// child is just within the mask-not-required area but when rotated requires
......@@ -2082,7 +2101,7 @@ TEST_F(CompositedLayerMappingTest, AncestorClipMaskRequiredDueToRotation) {
EXPECT_TRUE(child_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest,
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskRequiredByBorderRadiusWithCompositedDescendant) {
// This case has the child and grandchild within the ancestors and would
// in principle not need a mask, but does because we cannot efficiently
......@@ -2121,7 +2140,7 @@ TEST_F(CompositedLayerMappingTest,
EXPECT_TRUE(parent_mapping->AncestorClippingMaskLayer());
}
TEST_F(CompositedLayerMappingTest,
TEST_F(CompositedLayerMappingTestWithoutBGPT,
AncestorClipMaskGrandparentBorderRadiusCompositedDescendant) {
// This case has the child clipped by the grandparent border radius but not
// the parent, and does not itself require a mask to clip to the grandparent.
......@@ -2396,7 +2415,8 @@ TEST_F(CompositedLayerMappingTest, TransformedRasterizationForInlineTransform) {
// This tests that when the scroller becomes no longer scrollable if a sticky
// element is promoted for another reason we do remove its composited sticky
// constraint as it doesn't need to move on the compositor.
TEST_F(CompositedLayerMappingTest, CompositedStickyConstraintRemovedAndAdded) {
TEST_F(CompositedLayerMappingTestWithoutBGPT,
CompositedStickyConstraintRemovedAndAdded) {
SetBodyInnerHTML(R"HTML(
<style>
.scroller { overflow: auto; height: 200px; }
......@@ -2545,7 +2565,7 @@ TEST_F(CompositedLayerMappingTest, ClipPathNoChildContainmentLayer) {
ASSERT_FALSE(mapping->ClippingLayer());
}
TEST_F(CompositedLayerMappingTest, ForegroundLayerSizing) {
TEST_F(CompositedLayerMappingTestWithoutBGPT, ForegroundLayerSizing) {
// This test verifies the foreground layer is sized to the clip rect.
SetBodyInnerHTML(R"HTML(
<div id='target' style='position:relative; z-index:0; width:100px;
......
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