SET: Add an explicit renderer mode.

This patch adds an explicit renderer driven animation mode to the
directives. Without it, we rely on the presence of painted pseudo
elements to determine this. However, we sometimes may not paint the
pseudo elements (e.g. visibility: hidden) and still want a renderer
drive animation, which differs in its save path.

Without this patch, the mismatch causes a DCHECK in mac web tests.

R=chrishtr@chromium.org, vasilyt@chromium.org

Bug: 1276999
Change-Id: I1e5fba44f502f4d6d4416fa4fab485b8c117e9cc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3427619
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Vasiliy Telezhnikov <vasilyt@chromium.org>
Reviewed-by: danakj chromium <danakj@chromium.org>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#966278}

cc/document_transition/document_transition_request.cc

@@ -77,10 +77,11 @@ DocumentTransitionRequest::CreatePrepare(
     uint32_t document_tag,
     TransitionConfig root_config,
     std::vector<TransitionConfig> shared_element_config,
-    base::OnceClosure commit_callback) {
+    base::OnceClosure commit_callback,
+    bool is_renderer_driven_animation) {
   return base::WrapUnique(new DocumentTransitionRequest(
       effect, document_tag, root_config, shared_element_config,
-      std::move(commit_callback)));
+      std::move(commit_callback), is_renderer_driven_animation));
 }
 
 // static
@@ -111,14 +112,16 @@ DocumentTransitionRequest::DocumentTransitionRequest(
     uint32_t document_tag,
     TransitionConfig root_config,
     std::vector<TransitionConfig> shared_element_config,
-    base::OnceClosure commit_callback)
+    base::OnceClosure commit_callback,
+    bool is_renderer_driven_animation)
     : type_(Type::kSave),
       effect_(effect),
       root_config_(root_config),
       document_tag_(document_tag),
       shared_element_count_(shared_element_config.size()),
       shared_element_config_(std::move(shared_element_config)),
       commit_callback_(std::move(commit_callback)),
+      is_renderer_driven_animation_(is_renderer_driven_animation),
       sequence_id_(s_next_sequence_id_++) {}
 
 DocumentTransitionRequest::DocumentTransitionRequest(
@@ -129,6 +132,7 @@ DocumentTransitionRequest::DocumentTransitionRequest(
       document_tag_(document_tag),
       shared_element_count_(shared_element_count),
       commit_callback_(std::move(commit_callback)),
+      is_renderer_driven_animation_(false),
       sequence_id_(s_next_sequence_id_++) {}
 
 DocumentTransitionRequest::DocumentTransitionRequest(Type type,
@@ -137,7 +141,10 @@ DocumentTransitionRequest::DocumentTransitionRequest(Type type,
       document_tag_(document_tag),
       shared_element_count_(0u),
       commit_callback_(base::DoNothing()),
-      sequence_id_(s_next_sequence_id_++) {}
+      is_renderer_driven_animation_(true),
+      sequence_id_(s_next_sequence_id_++) {
+  DCHECK(type_ == Type::kAnimateRenderer || type_ == Type::kRelease);
+}
 
 DocumentTransitionRequest::~DocumentTransitionRequest() = default;
 
@@ -169,7 +176,8 @@ DocumentTransitionRequest::ConstructDirective(
     shared_elements[i].shared_element_resource_id = it->second.resource_id;
   }
   return viz::CompositorFrameTransitionDirective(
-      sequence_id_, type_, effect_, root_config_, std::move(shared_elements));
+      sequence_id_, type_, is_renderer_driven_animation_, effect_, root_config_,
+      std::move(shared_elements));
 }
 
 std::string DocumentTransitionRequest::ToString() const {

cc/document_transition/document_transition_request.h

@@ -34,7 +34,8 @@ class CC_EXPORT DocumentTransitionRequest {
       uint32_t document_tag,
       TransitionConfig root_config,
       std::vector<TransitionConfig> shared_element_config,
-      base::OnceClosure commit_callback);
+      base::OnceClosure commit_callback,
+      bool is_renderer_driven_animation);
 
   // Creates a Type::kSave type of request.
   static std::unique_ptr<DocumentTransitionRequest> CreateStart(
@@ -90,7 +91,8 @@ class CC_EXPORT DocumentTransitionRequest {
                             uint32_t document_tag,
                             TransitionConfig root_config,
                             std::vector<TransitionConfig> shared_element_config,
-                            base::OnceClosure commit_callback);
+                            base::OnceClosure commit_callback,
+                            bool is_renderer_driven_animation);
   DocumentTransitionRequest(uint32_t document_tag,
                             uint32_t shared_element_count,
                             base::OnceClosure commit_callback);
@@ -103,6 +105,7 @@ class CC_EXPORT DocumentTransitionRequest {
   const uint32_t shared_element_count_;
   const std::vector<TransitionConfig> shared_element_config_;
   base::OnceClosure commit_callback_;
+  const bool is_renderer_driven_animation_;
   const uint32_t sequence_id_;
 
   static uint32_t s_next_sequence_id_;

cc/document_transition/document_transition_request_unittest.cc

@@ -18,7 +18,7 @@ TEST(DocumentTransitionRequestTest, PrepareRequest) {
   auto request = DocumentTransitionRequest::CreatePrepare(
       DocumentTransitionRequest::Effect::kRevealLeft,
       /*document_tag=*/0, DocumentTransitionRequest::TransitionConfig(),
-      /*shared_element_config=*/{}, std::move(callback));
+      /*shared_element_config=*/{}, std::move(callback), false);
 
   EXPECT_FALSE(called);
   request->TakeFinishedCallback().Run();
@@ -30,11 +30,14 @@ TEST(DocumentTransitionRequestTest, PrepareRequest) {
   EXPECT_EQ(DocumentTransitionRequest::Effect::kRevealLeft, directive.effect());
   EXPECT_EQ(viz::CompositorFrameTransitionDirective::Type::kSave,
             directive.type());
+  EXPECT_FALSE(directive.is_renderer_driven_animation());
 
   auto duplicate = request->ConstructDirective({});
   EXPECT_EQ(duplicate.sequence_id(), directive.sequence_id());
   EXPECT_EQ(duplicate.effect(), directive.effect());
   EXPECT_EQ(duplicate.type(), directive.type());
+  EXPECT_EQ(duplicate.is_renderer_driven_animation(),
+            directive.is_renderer_driven_animation());
 }
 
 TEST(DocumentTransitionRequestTest, StartRequest) {
@@ -53,6 +56,7 @@ TEST(DocumentTransitionRequestTest, StartRequest) {
   EXPECT_GT(directive.sequence_id(), 0u);
   EXPECT_EQ(viz::CompositorFrameTransitionDirective::Type::kAnimate,
             directive.type());
+  EXPECT_FALSE(directive.is_renderer_driven_animation());
 }
 
 }  // namespace cc

cc/trees/layer_tree_host_unittest.cc

@@ -10087,8 +10087,9 @@ class LayerTreeHostTestDocumentTransitionsPropagatedToMetadata
         DocumentTransitionRequest::CreatePrepare(
             DocumentTransitionRequest::Effect::kExplode,
             /*document_tag=*/0, DocumentTransitionRequest::TransitionConfig(),
-            /*shared_elements=*/{},
-            base::BindLambdaForTesting([this]() { CommitLambdaCalled(); })));
+            /*shared_element_config=*/{},
+            base::BindLambdaForTesting([this]() { CommitLambdaCalled(); }),
+            /*is_renderer_driven_animation=*/false));
     layer_tree_host()->AddDocumentTransitionRequest(
         DocumentTransitionRequest::CreateStart(
             /*document_tag=*/0, /*shared_element_count=*/0,

components/viz/common/quads/compositor_frame_metadata_unittest.cc

@@ -48,7 +48,8 @@ bool AreDelegatedInkMetadataEqual(const gfx::DelegatedInkMetadata& a,
 bool AreTransitionDirectivesEqual(const CompositorFrameTransitionDirective& a,
                                   const CompositorFrameTransitionDirective& b) {
   return a.sequence_id() == b.sequence_id() && a.type() == b.type() &&
-         a.effect() == b.effect();
+         a.effect() == b.effect() &&
+         a.is_renderer_driven_animation() == b.is_renderer_driven_animation();
 }
 
 TEST(CompositorFrameMetadata, Clone) {
@@ -83,7 +84,7 @@ TEST(CompositorFrameMetadata, Clone) {
       gfx::PointF(88.8, 44.4), 1.f, SK_ColorRED,
       base::TimeTicks() + base::Seconds(125), gfx::RectF(1, 2, 3, 4), true);
   metadata.transition_directives.emplace_back(
-      4u, CompositorFrameTransitionDirective::Type::kSave,
+      4u, CompositorFrameTransitionDirective::Type::kSave, true,
       CompositorFrameTransitionDirective::Effect::kCoverUp);
 
   CompositorFrameMetadata clone = metadata.Clone();

components/viz/common/quads/compositor_frame_transition_directive.cc

@@ -4,6 +4,7 @@
 
 #include "components/viz/common/quads/compositor_frame_transition_directive.h"
 
+#include <string>
 #include <utility>
 
 #include "base/time/time.h"
@@ -22,11 +23,13 @@ CompositorFrameTransitionDirective::CompositorFrameTransitionDirective() =
 CompositorFrameTransitionDirective::CompositorFrameTransitionDirective(
     uint32_t sequence_id,
     Type type,
+    bool is_renderer_driven_animation,
     Effect effect,
     const TransitionConfig& root_config,
     std::vector<SharedElement> shared_elements)
     : sequence_id_(sequence_id),
       type_(type),
+      is_renderer_driven_animation_(is_renderer_driven_animation),
       effect_(effect),
       root_config_(root_config),
       shared_elements_(std::move(shared_elements)) {}

components/viz/common/quads/compositor_frame_transition_directive.h

@@ -5,6 +5,7 @@
 #ifndef COMPONENTS_VIZ_COMMON_QUADS_COMPOSITOR_FRAME_TRANSITION_DIRECTIVE_H_
 #define COMPONENTS_VIZ_COMMON_QUADS_COMPOSITOR_FRAME_TRANSITION_DIRECTIVE_H_
 
+#include <string>
 #include <vector>
 
 #include "base/time/time.h"
@@ -105,6 +106,7 @@ class VIZ_COMMON_EXPORT CompositorFrameTransitionDirective {
   CompositorFrameTransitionDirective(
       uint32_t sequence_id,
       Type type,
+      bool is_renderer_driven_animation = false,
       Effect effect = Effect::kNone,
       const TransitionConfig& root_config = TransitionConfig(),
       std::vector<SharedElement> shared_elements = {});
@@ -133,11 +135,18 @@ class VIZ_COMMON_EXPORT CompositorFrameTransitionDirective {
     return shared_elements_;
   }
 
+  // Returns true if this is a directive for a renderer driven animation.
+  bool is_renderer_driven_animation() const {
+    return is_renderer_driven_animation_;
+  }
+
  private:
   uint32_t sequence_id_ = 0;
 
   Type type_ = Type::kSave;
 
+  bool is_renderer_driven_animation_ = false;
+
   Effect effect_ = Effect::kNone;
 
   TransitionConfig root_config_;

components/viz/common/quads/compositor_frame_transition_directive_unittest.cc

@@ -13,17 +13,19 @@ using Effect = CompositorFrameTransitionDirective::Effect;
 using Type = CompositorFrameTransitionDirective::Type;
 
 TEST(CompositorFrameTransitionDirective, GettersReflectParameters) {
-  CompositorFrameTransitionDirective save_directive(1u, Type::kSave,
+  CompositorFrameTransitionDirective save_directive(1u, Type::kSave, true,
                                                     Effect::kCoverLeft);
 
   EXPECT_EQ(1u, save_directive.sequence_id());
   EXPECT_EQ(Type::kSave, save_directive.type());
   EXPECT_EQ(Effect::kCoverLeft, save_directive.effect());
+  EXPECT_TRUE(save_directive.is_renderer_driven_animation());
 
   CompositorFrameTransitionDirective animate_directive(2, Type::kAnimate);
 
   EXPECT_EQ(2u, animate_directive.sequence_id());
   EXPECT_EQ(Type::kAnimate, animate_directive.type());
+  EXPECT_FALSE(animate_directive.is_renderer_driven_animation());
 }
 
 }  // namespace

components/viz/service/display/surface_aggregator_unittest.cc

@@ -9123,7 +9123,7 @@ TEST_F(SurfaceAggregatorWithResourcesTest, TransitionDirectiveFrameBehind) {
   {
     auto frame = BuildCompositorFrameWithResources({}, true, SurfaceId());
     frame.metadata.transition_directives.emplace_back(
-        1, CompositorFrameTransitionDirective::Type::kSave,
+        1, CompositorFrameTransitionDirective::Type::kSave, false,
         CompositorFrameTransitionDirective::Effect::kCoverLeft);
 
     root_sink_->SubmitCompositorFrame(local_surface_id, std::move(frame));
@@ -9138,7 +9138,7 @@ TEST_F(SurfaceAggregatorWithResourcesTest, TransitionDirectiveFrameBehind) {
   {
     auto frame = BuildCompositorFrameWithResources({}, true, SurfaceId());
     frame.metadata.transition_directives.emplace_back(
-        2, CompositorFrameTransitionDirective::Type::kAnimate);
+        2, CompositorFrameTransitionDirective::Type::kAnimate, false);
     root_sink_->SubmitCompositorFrame(local_surface_id, std::move(frame));
   }
   AggregateFrame(surface_id);

components/viz/service/surfaces/surface_saved_frame.cc

@@ -82,7 +82,7 @@ bool SurfaceSavedFrame::IsValid() const {
 void SurfaceSavedFrame::RequestCopyOfOutput(Surface* surface) {
   DCHECK(surface->HasActiveFrame());
 
-  if (surface->GetActiveFrame().metadata.has_shared_element_resources) {
+  if (directive_.is_renderer_driven_animation()) {
     // TODO(khushalsagar) : This should be the only mode once renderer based SET
     // lands.
     copy_root_render_pass_ = false;

components/viz/service/surfaces/surface_saved_frame_unittest.cc

@@ -130,7 +130,7 @@ TEST_F(SurfaceSavedFrameTest, OnlyRootSnapshotNoSharedPass) {
 
   const uint32_t sequence_id = 2u;
   CompositorFrameTransitionDirective directive(
-      sequence_id, CompositorFrameTransitionDirective::Type::kSave,
+      sequence_id, CompositorFrameTransitionDirective::Type::kSave, false,
       CompositorFrameTransitionDirective::Effect::kCoverDown);
   auto saved_frame = CreateSavedFrame(directive);
   saved_frame->RequestCopyOfOutput(GetSurface());
@@ -152,7 +152,7 @@ TEST_F(SurfaceSavedFrameTest, OnlyRootSnapshotNullSharedPass) {
 
   const uint32_t sequence_id = 2u;
   CompositorFrameTransitionDirective directive(
-      sequence_id, CompositorFrameTransitionDirective::Type::kSave,
+      sequence_id, CompositorFrameTransitionDirective::Type::kSave, false,
       CompositorFrameTransitionDirective::Effect::kCoverDown,
       CompositorFrameTransitionDirective::TransitionConfig(),
       CreateSharedElements({CompositorRenderPassId(0u)}));
@@ -192,7 +192,7 @@ TEST_F(SurfaceSavedFrameTest, RemoveSharedElementQuadOnly) {
 
   const uint32_t sequence_id = 2u;
   CompositorFrameTransitionDirective directive(
-      sequence_id, CompositorFrameTransitionDirective::Type::kSave,
+      sequence_id, CompositorFrameTransitionDirective::Type::kSave, false,
       CompositorFrameTransitionDirective::Effect::kCoverDown,
       CompositorFrameTransitionDirective::TransitionConfig(),
       CreateSharedElements({shared_pass_id}));
@@ -265,7 +265,7 @@ TEST_F(SurfaceSavedFrameTest, SharedElementNestedInNonSharedElementPass) {
 
   const uint32_t sequence_id = 2u;
   CompositorFrameTransitionDirective directive(
-      sequence_id, CompositorFrameTransitionDirective::Type::kSave,
+      sequence_id, CompositorFrameTransitionDirective::Type::kSave, false,
       CompositorFrameTransitionDirective::Effect::kCoverDown,
       CompositorFrameTransitionDirective::TransitionConfig(),
       CreateSharedElements({shared_pass_id}));
@@ -345,7 +345,7 @@ TEST_F(SurfaceSavedFrameTest, SharedElementNestedInSharedElementPass) {
 
   const uint32_t sequence_id = 2u;
   CompositorFrameTransitionDirective directive(
-      sequence_id, CompositorFrameTransitionDirective::Type::kSave,
+      sequence_id, CompositorFrameTransitionDirective::Type::kSave, false,
       CompositorFrameTransitionDirective::Effect::kCoverDown,
       CompositorFrameTransitionDirective::TransitionConfig(),
       CreateSharedElements({child_shared_pass_id, parent_shared_pass->id}));

components/viz/service/transitions/surface_animation_manager_unittest.cc

@@ -41,7 +41,8 @@ std::vector<CompositorFrameTransitionDirective> CreateSaveDirectiveAsVector(
         CompositorFrameTransitionDirective::Effect::kCoverDown) {
   std::vector<CompositorFrameTransitionDirective> result;
   result.emplace_back(sequence_id,
-                      CompositorFrameTransitionDirective::Type::kSave, effect);
+                      CompositorFrameTransitionDirective::Type::kSave, false,
+                      effect);
   return result;
 }
 
@@ -434,14 +435,14 @@ TEST_F(SurfaceAnimationManagerTest, ConfigWithAllZeroDurations) {
   CompositorFrameTransitionDirective::TransitionConfig zero_config;
   zero_config.duration = zero_config.delay = base::TimeDelta();
   CompositorFrameTransitionDirective save(
-      1, CompositorFrameTransitionDirective::Type::kSave,
+      1, CompositorFrameTransitionDirective::Type::kSave, false,
       CompositorFrameTransitionDirective::Effect::kCoverDown,
       /*root_config=*/zero_config,
       /*shared_elements=*/
       CreateSharedElements(frame.render_pass_list, {zero_config}));
 
   CompositorFrameTransitionDirective animate(
-      2, CompositorFrameTransitionDirective::Type::kAnimate,
+      2, CompositorFrameTransitionDirective::Type::kAnimate, false,
       CompositorFrameTransitionDirective::Effect::kNone,
       /*root_config=*/zero_config,
       /*shared_elements=*/
@@ -475,13 +476,13 @@ TEST_F(SurfaceAnimationManagerTest, CustomRootConfig) {
   root_config.delay = base::Seconds(1);
 
   CompositorFrameTransitionDirective save(
-      1, CompositorFrameTransitionDirective::Type::kSave,
+      1, CompositorFrameTransitionDirective::Type::kSave, false,
       CompositorFrameTransitionDirective::Effect::kExplode,
       /*root_config=*/root_config,
       /*shared_elements=*/{});
 
   CompositorFrameTransitionDirective animate(
-      2, CompositorFrameTransitionDirective::Type::kAnimate,
+      2, CompositorFrameTransitionDirective::Type::kAnimate, false,
       CompositorFrameTransitionDirective::Effect::kNone,
       /*root_config=*/root_config,
       /*shared_elements=*/{});
@@ -573,7 +574,7 @@ TEST_F(SurfaceAnimationManagerTest, CustomSharedConfig) {
   shared_config.delay = base::Seconds(1);
 
   CompositorFrameTransitionDirective save(
-      1, CompositorFrameTransitionDirective::Type::kSave,
+      1, CompositorFrameTransitionDirective::Type::kSave, false,
       CompositorFrameTransitionDirective::Effect::kNone,
       /*root_config=*/zero_config,
       /*shared_elements=*/
@@ -604,7 +605,7 @@ TEST_F(SurfaceAnimationManagerTest, CustomSharedConfig) {
                                                 new_transform, new_opacity);
 
   CompositorFrameTransitionDirective animate(
-      2, CompositorFrameTransitionDirective::Type::kAnimate,
+      2, CompositorFrameTransitionDirective::Type::kAnimate, false,
       CompositorFrameTransitionDirective::Effect::kNone,
       /*root_config=*/zero_config,
       /*shared_elements=*/

services/viz/public/cpp/compositing/compositor_frame_transition_directive_mojom_traits.cc

@@ -178,9 +178,11 @@ bool StructTraits<viz::mojom::CompositorFrameTransitionDirectiveDataView,
       !data.ReadSharedElements(&shared_elements)) {
     return false;
   }
+  bool is_renderer_driven_animation = data.is_renderer_driven_animation();
 
   *out = viz::CompositorFrameTransitionDirective(
-      sequence_id, type, effect, root_config, std::move(shared_elements));
+      sequence_id, type, is_renderer_driven_animation, effect, root_config,
+      std::move(shared_elements));
   return true;
 }
 

services/viz/public/cpp/compositing/compositor_frame_transition_directive_mojom_traits.h

@@ -92,6 +92,11 @@ struct StructTraits<viz::mojom::CompositorFrameTransitionDirectiveDataView,
     return directive.type();
   }
 
+  static bool is_renderer_driven_animation(
+      const viz::CompositorFrameTransitionDirective& directive) {
+    return directive.is_renderer_driven_animation();
+  }
+
   static viz::CompositorFrameTransitionDirective::Effect effect(
       const viz::CompositorFrameTransitionDirective& directive) {
     return directive.effect();