Revert "Make 3D Rendering Context follow DOM tree for absolute/fixed …

…position."

This reverts commit ed024d7.

Reason for revert: The original rationale that this would not change
any behavior without TransformInterop enabled was incorrect, because
backface-visibility: hidden (which does not create a containing block
for absolute/fixed) causes NeedsTransform() to be true, which thus
causes UpdateTransform to update rendering_context_id and
should_flatten_inherited_transform.

Original change's description:
> Make 3D Rendering Context follow DOM tree for absolute/fixed position.
>
> When TransformInterop is enabled, make the notion of 3D Rendering
> Context follow the DOM tree for absolute and fixed-positioned elements
> like it does for everything else.
>
> When TransformInterop is not enabled, there are no differences between
> following the DOM tree versus the containing block tree, which this
> DCHECK()s temporarily while storing the data in two places.  This is
> because the objects that changes of rendering_context_id and
> should_flatten_inherited_transform are associated with are all either
> containing blocks for absolute and fixed positioned elements or (for
> SVG) are associated closely enough with such containing blocks.
>
> (This is intended to change behavior only when
> RuntimeEnabledFeatures::TransformInteropEnabled(), though it should
> (once the temporary members are removed) unconditionally reduce the
> size of structs used on the stack.)
>
> Bug: 1189985
> Change-Id: I5d3bff009e2fbac00d3d011a6a9adc77ad2f9829
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2776711
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Commit-Queue: David Baron <dbaron@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#878158}

Bug: 1189985
Fixed: 1204790, 1204853
Change-Id: Idb2a635edd9b762454bb23c536cb37bc2ac7330f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2873105
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: David Baron <dbaron@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#879166}

third_party/blink/renderer/core/paint/paint_property_tree_builder.cc

@@ -162,10 +162,8 @@ void PaintPropertyTreeBuilder::SetupContextForFrame(
 
   PaintPropertyTreeBuilderFragmentContext& context = full_context.fragments[0];
   context.current.paint_offset += PhysicalOffset(frame_view.Location());
-  context.rendering_context_id = 0;
-  context.should_flatten_inherited_transform = true;
-  context.current.old_rendering_context_id = 0;
-  context.current.old_should_flatten_inherited_transform = true;
+  context.current.rendering_context_id = 0;
+  context.current.should_flatten_inherited_transform = true;
   context.absolute_position = context.current;
   full_context.container_for_absolute_position = nullptr;
   full_context.container_for_fixed_position = nullptr;
@@ -623,15 +621,9 @@ void FragmentPaintPropertyTreeBuilder::UpdatePaintOffsetTranslation(
   if (paint_offset_translation) {
     FloatSize new_translation(ToIntSize(*paint_offset_translation));
     TransformPaintPropertyNode::State state{new_translation};
-    DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-           context_.should_flatten_inherited_transform ==
-               context_.current.old_should_flatten_inherited_transform);
     state.flags.flattens_inherited_transform =
-        context_.should_flatten_inherited_transform;
-    DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-           context_.rendering_context_id ==
-               context_.current.old_rendering_context_id);
-    state.rendering_context_id = context_.rendering_context_id;
+        context_.current.should_flatten_inherited_transform;
+    state.rendering_context_id = context_.current.rendering_context_id;
     state.direct_compositing_reasons =
         full_context_.direct_compositing_reasons &
         CompositingReason::kDirectReasonsForPaintOffsetTranslationProperty;
@@ -678,15 +670,9 @@ void FragmentPaintPropertyTreeBuilder::UpdateStickyTranslation() {
       state.compositor_element_id = CompositorElementIdFromUniqueObjectId(
           box_model.UniqueId(),
           CompositorElementIdNamespace::kStickyTranslation);
-      DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-             context_.rendering_context_id ==
-                 context_.current.old_rendering_context_id);
-      state.rendering_context_id = context_.rendering_context_id;
-      DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-             context_.should_flatten_inherited_transform ==
-                 context_.current.old_should_flatten_inherited_transform);
+      state.rendering_context_id = context_.current.rendering_context_id;
       state.flags.flattens_inherited_transform =
-          context_.should_flatten_inherited_transform;
+          context_.current.should_flatten_inherited_transform;
 
       auto* layer = box_model.Layer();
       const auto* scroller_properties = layer->AncestorScrollContainerLayer()
@@ -873,15 +859,9 @@ void FragmentPaintPropertyTreeBuilder::UpdateTransformForSVGChild(
         state.direct_compositing_reasons =
             direct_compositing_reasons &
             CompositingReasonsForTransformProperty();
-        DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-               context_.should_flatten_inherited_transform ==
-                   context_.current.old_should_flatten_inherited_transform);
         state.flags.flattens_inherited_transform =
-            context_.should_flatten_inherited_transform;
-        DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-               context_.rendering_context_id ==
-                   context_.current.old_rendering_context_id);
-        state.rendering_context_id = context_.rendering_context_id;
+            context_.current.should_flatten_inherited_transform;
+        state.rendering_context_id = context_.current.rendering_context_id;
         state.flags.is_for_svg_child = true;
         state.compositor_element_id = GetCompositorElementId(
             CompositorElementIdNamespace::kPrimaryTransform);
@@ -913,13 +893,9 @@ void FragmentPaintPropertyTreeBuilder::UpdateTransformForSVGChild(
   }
 
   if (properties_->Transform()) {
-    // TODO(pdr): SVG does not support 3D transforms so this should be
-    // should_flatten_inherited_transform = true.
     context_.current.transform = properties_->Transform();
-    context_.should_flatten_inherited_transform = false;
-    context_.rendering_context_id = 0;
-    context_.current.old_should_flatten_inherited_transform = false;
-    context_.current.old_rendering_context_id = 0;
+    context_.current.should_flatten_inherited_transform = false;
+    context_.current.rendering_context_id = 0;
   }
 }
 
@@ -1024,10 +1000,7 @@ void FragmentPaintPropertyTreeBuilder::UpdateTransform() {
         // !matrix.Is2dTransform() or !matrix.IsFlat(); we're interested
         // *only* in things that cause this element to have a nonzero z
         // position within the 3-D scene.
-        DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-               context_.rendering_context_id ==
-                   context_.current.old_rendering_context_id);
-        if (context_.rendering_context_id &&
+        if (context_.current.rendering_context_id &&
             (matrix.M13() != 0.0 || matrix.M23() != 0.0 ||
              matrix.M43() != 0.0)) {
           UseCounter::Count(object_.GetDocument(),
@@ -1038,7 +1011,7 @@ void FragmentPaintPropertyTreeBuilder::UpdateTransform() {
         // PaintLayer is created. If a node with transform-style: preserve-3d
         // does not exist in an existing rendering context, it establishes a
         // new one.
-        state.rendering_context_id = context_.rendering_context_id;
+        state.rendering_context_id = context_.current.rendering_context_id;
         if (style.Preserves3D() && !state.rendering_context_id) {
           state.rendering_context_id =
               PtrHash<const LayoutObject>::GetHash(&object_);
@@ -1055,11 +1028,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateTransform() {
       state.direct_compositing_reasons =
           full_context_.direct_compositing_reasons &
           CompositingReasonsForTransformProperty();
-      DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-             context_.should_flatten_inherited_transform ==
-                 context_.current.old_should_flatten_inherited_transform);
       state.flags.flattens_inherited_transform =
-          context_.should_flatten_inherited_transform;
+          context_.current.should_flatten_inherited_transform;
       state.backface_visibility =
           object_.HasHiddenBackface()
               ? TransformPaintPropertyNode::BackfaceVisibility::kHidden
@@ -1098,18 +1068,12 @@ void FragmentPaintPropertyTreeBuilder::UpdateTransform() {
   if (const auto* transform = properties_->Transform()) {
     context_.current.transform = transform;
     if (object_.StyleRef().Preserves3D()) {
-      context_.rendering_context_id = transform->RenderingContextId();
-      context_.should_flatten_inherited_transform = false;
-      context_.current.old_rendering_context_id =
-          transform->RenderingContextId();
-      context_.current.old_should_flatten_inherited_transform = false;
+      context_.current.rendering_context_id = transform->RenderingContextId();
+      context_.current.should_flatten_inherited_transform = false;
     } else {
-      context_.rendering_context_id = 0;
-      context_.should_flatten_inherited_transform = true;
-      context_.current.old_rendering_context_id = 0;
-      context_.current.old_should_flatten_inherited_transform = true;
+      context_.current.rendering_context_id = 0;
+      context_.current.should_flatten_inherited_transform = true;
     }
-
     if (transform->IsIdentityOr2DTranslation()) {
       context_.translation_2d_to_layout_shift_root_delta +=
           transform->Translation2D();
@@ -1119,10 +1083,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateTransform() {
     // With kTransformInterop enabled, 3D rendering contexts follow the
     // DOM ancestor chain, so flattening should apply regardless of
     // presence of transform.
-    context_.rendering_context_id = 0;
-    context_.should_flatten_inherited_transform = true;
-    context_.current.old_rendering_context_id = 0;
-    context_.current.old_should_flatten_inherited_transform = true;
+    context_.current.rendering_context_id = 0;
+    context_.current.should_flatten_inherited_transform = true;
   }
 }
 
@@ -1951,12 +1913,6 @@ void FragmentPaintPropertyTreeBuilder::UpdatePerspective() {
   DCHECK(properties_);
 
   if (NeedsPaintPropertyUpdate()) {
-    DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-           context_.should_flatten_inherited_transform ==
-               context_.current.old_should_flatten_inherited_transform);
-    DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-           context_.rendering_context_id ==
-               context_.current.old_rendering_context_id);
     if (NeedsPerspective(object_)) {
       const ComputedStyle& style = object_.StyleRef();
       // The perspective node must not flatten (else nothing will get
@@ -1968,8 +1924,8 @@ void FragmentPaintPropertyTreeBuilder::UpdatePerspective() {
               PerspectiveOrigin(To<LayoutBox>(object_)) +
                   FloatSize(context_.current.paint_offset))};
       state.flags.flattens_inherited_transform =
-          context_.should_flatten_inherited_transform;
-      state.rendering_context_id = context_.rendering_context_id;
+          context_.current.should_flatten_inherited_transform;
+      state.rendering_context_id = context_.current.rendering_context_id;
       OnUpdate(properties_->UpdatePerspective(*context_.current.transform,
                                               std::move(state)));
     } else {
@@ -1979,8 +1935,7 @@ void FragmentPaintPropertyTreeBuilder::UpdatePerspective() {
 
   if (properties_->Perspective()) {
     context_.current.transform = properties_->Perspective();
-    context_.should_flatten_inherited_transform = false;
-    context_.current.old_should_flatten_inherited_transform = false;
+    context_.current.should_flatten_inherited_transform = false;
   }
 }
 
@@ -1998,18 +1953,12 @@ void FragmentPaintPropertyTreeBuilder::UpdateReplacedContentTransform() {
     } else {
       NOTREACHED();
     }
-    DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-           context_.should_flatten_inherited_transform ==
-               context_.current.old_should_flatten_inherited_transform);
-    DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-           context_.rendering_context_id ==
-               context_.current.old_rendering_context_id);
     if (!content_to_parent_space.IsIdentity()) {
       TransformPaintPropertyNode::State state;
       SetTransformNodeStateFromAffineTransform(state, content_to_parent_space);
       state.flags.flattens_inherited_transform =
-          context_.should_flatten_inherited_transform;
-      state.rendering_context_id = context_.rendering_context_id;
+          context_.current.should_flatten_inherited_transform;
+      state.rendering_context_id = context_.current.rendering_context_id;
       OnUpdate(properties_->UpdateReplacedContentTransform(
           *context_.current.transform, std::move(state)));
     } else {
@@ -2031,10 +1980,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateReplacedContentTransform() {
       context_.current.transform = properties_->ReplacedContentTransform();
       // TODO(pdr): SVG does not support 3D transforms so this should be
       // should_flatten_inherited_transform = true.
-      context_.should_flatten_inherited_transform = false;
-      context_.rendering_context_id = 0;
-      context_.current.old_should_flatten_inherited_transform = false;
-      context_.current.old_rendering_context_id = 0;
+      context_.current.should_flatten_inherited_transform = false;
+      context_.current.rendering_context_id = 0;
     }
   }
 }
@@ -2181,15 +2128,9 @@ void FragmentPaintPropertyTreeBuilder::UpdateScrollAndScrollTranslation() {
       FloatPoint scroll_position = FloatPoint(box.ScrollOrigin()) +
                                    box.GetScrollableArea()->GetScrollOffset();
       TransformPaintPropertyNode::State state{-ToFloatSize(scroll_position)};
-      DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-             context_.should_flatten_inherited_transform ==
-                 context_.current.old_should_flatten_inherited_transform);
       state.flags.flattens_inherited_transform =
-          context_.should_flatten_inherited_transform;
-      DCHECK(RuntimeEnabledFeatures::TransformInteropEnabled() ||
-             context_.rendering_context_id ==
-                 context_.current.old_rendering_context_id);
-      state.rendering_context_id = context_.rendering_context_id;
+          context_.current.should_flatten_inherited_transform;
+      state.rendering_context_id = context_.current.rendering_context_id;
       state.direct_compositing_reasons =
           full_context_.direct_compositing_reasons &
           CompositingReason::kDirectReasonsForScrollTranslationProperty;
@@ -2816,10 +2757,8 @@ void FragmentPaintPropertyTreeBuilder::UpdateForSelf() {
     // With kTransformInterop enabled, 3D rendering contexts follow the
     // DOM ancestor chain, so flattening should apply regardless of
     // presence of transform.
-    context_.rendering_context_id = 0;
-    context_.should_flatten_inherited_transform = true;
-    context_.current.old_rendering_context_id = 0;
-    context_.current.old_should_flatten_inherited_transform = true;
+    context_.current.rendering_context_id = 0;
+    context_.current.should_flatten_inherited_transform = true;
   }
   UpdateLocalBorderBoxContext();
   UpdateLayoutShiftRootChanged(IsLayoutShiftRoot(object_, fragment_data_));

third_party/blink/renderer/core/paint/paint_property_tree_builder.h

@@ -86,10 +86,11 @@ struct PaintPropertyTreeBuilderFragmentContext {
 
     // The PaintLayer corresponding to the origin of |paint_offset|.
     Member<const LayoutObject> paint_offset_root;
-
-    // TODO(dbaron): Remove this in a few weeks once we know that the
-    // assertions it supports aren't hit by fuzzers.
-    bool old_should_flatten_inherited_transform = false;
+    // Whether newly created children should flatten their inherited transform
+    // (equivalently, draw into the plane of their parent). Should generally
+    // be updated whenever |transform| is; flattening only needs to happen
+    // to immediate children.
+    bool should_flatten_inherited_transform = false;
 
     // True if any fixed-position children within this context are fixed to the
     // root of the FrameView (and hence above its scroll).
@@ -100,9 +101,9 @@ struct PaintPropertyTreeBuilderFragmentContext {
     // object has changed.
     bool layout_shift_root_changed = false;
 
-    // TODO(dbaron): Remove this in a few weeks once we know that the
-    // assertions it supports aren't hit by fuzzers.
-    unsigned old_rendering_context_id = 0;
+    // Rendering context for 3D sorting. See
+    // TransformPaintPropertyNode::renderingContextId.
+    unsigned rendering_context_id = 0;
     // The clip node describes the accumulated raster clip for the current
     // subtree.  Note that the computed raster region in canvas space for a clip
     // node is independent from the transform and paint offset above. Also the
@@ -143,16 +144,6 @@ struct PaintPropertyTreeBuilderFragmentContext {
   const EffectPaintPropertyNodeOrAlias* current_effect;
   bool this_or_ancestor_opacity_is_zero = false;
 
-  // Whether newly created children should flatten their inherited transform
-  // (equivalently, draw into the plane of their parent). Should generally
-  // be updated whenever |transform| is; flattening only needs to happen
-  // to immediate children.
-  bool should_flatten_inherited_transform = false;
-
-  // Rendering context for 3D sorting. See
-  // TransformPaintPropertyNode::RenderingContextId().
-  unsigned rendering_context_id = 0;
-
   // If the object is a flow thread, this records the clip rect for this
   // fragment.
   base::Optional<PhysicalRect> fragment_clip;

third_party/blink/web_tests/TestExpectations

@@ -6029,16 +6029,12 @@ crbug.com/1008483 external/wpt/css/css-transforms/backface-visibility-hidden-004
 crbug.com/1008483 external/wpt/css/css-transforms/backface-visibility-hidden-005.tentative.html [ Failure ]
 crbug.com/1008483 external/wpt/css/css-transforms/backface-visibility-hidden-animated-002.html [ Failure ]
 crbug.com/1008483 external/wpt/css/css-transforms/preserve-3d-flat-grouping-properties-containing-block.tentative.html [ Failure ]
-crbug.com/1008483 external/wpt/css/css-transforms/3d-rendering-context-and-abspos.html [ Failure ]
-crbug.com/1008483 external/wpt/css/css-transforms/3d-rendering-context-and-fixpos.html [ Failure ]
 
 crbug.com/1008483 virtual/transform-interop/external/wpt/css/css-transforms/3d-rendering-context-behavior.tentative.html [ Pass ]
 crbug.com/1008483 virtual/transform-interop/external/wpt/css/css-transforms/backface-visibility-hidden-004.tentative.html [ Pass ]
 crbug.com/1008483 virtual/transform-interop/external/wpt/css/css-transforms/backface-visibility-hidden-005.tentative.html [ Pass ]
 crbug.com/1008483 virtual/transform-interop/external/wpt/css/css-transforms/backface-visibility-hidden-animated-002.html [ Pass ]
 crbug.com/1008483 virtual/transform-interop/external/wpt/css/css-transforms/preserve-3d-flat-grouping-properties-containing-block.tentative.html [ Pass ]
-crbug.com/1008483 virtual/transform-interop/external/wpt/css/css-transforms/3d-rendering-context-and-abspos.html [ Pass ]
-crbug.com/1008483 virtual/transform-interop/external/wpt/css/css-transforms/3d-rendering-context-and-fixpos.html [ Pass ]
 crbug.com/1008483 virtual/transform-interop/compositing/geometry/require-own-backing-recalc-order.html [ Failure ]
 crbug.com/1008483 virtual/transform-interop/compositing/overflow/scroll-parent-with-non-stacking-context-composited-ancestor.html [ Failure ]
 crbug.com/1008483 virtual/transform-interop/paint/invalidation/stacking-context-lost.html [ Failure ]