viz: Fix tracking undrawn surfaces.

Each Surface is currently already explicitly notified by the
SurfaceAggregator when it is drawn, or when its copy-requests start to
be processed. It is not necessary to separately track the undrawn
surfaces from SurfaceAggregator. So use that information to decide
whether a Surface is drawn or not.

BUG=921474

Change-Id: I45aa1d1ccce1d5c5394d3f07bd887cf5426f04fc
Reviewed-on: https://chromium-review.googlesource.com/c/1443545
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#627545}(cherry picked from commit 4fdfe88c693f52a717efa94ca0e721fb2e9ffaf3)
Reviewed-on: https://chromium-review.googlesource.com/c/1451420
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#151}
Cr-Branched-From: e51029943e0a38dd794b73caaf6373d5496ae783-refs/heads/master@{#625896}
diff --git a/components/viz/service/display/display.cc b/components/viz/service/display/display.cc
index 236611f..08a8c58c 100644
--- a/components/viz/service/display/display.cc
+++ b/components/viz/service/display/display.cc
@@ -379,16 +379,6 @@
   // Run callbacks early to allow pipelining and collect presented callbacks.
   RunDrawCallbacks();
 
-  // Mark all the drawn surfaces, so that they can start receiving begin-frames.
-  const auto& undrawn_surfaces = aggregator_->undrawn_surfaces();
-  for (const auto& surface_id : aggregator_->previous_contained_surfaces()) {
-    if (undrawn_surfaces.count(surface_id.first))
-      continue;
-    Surface* surface = surface_manager_->GetSurfaceForId(surface_id.first);
-    if (surface)
-      surface->MarkAsDrawn();
-  }
-
   frame.metadata.latency_info.insert(frame.metadata.latency_info.end(),
                                      stored_latency_info_.begin(),
                                      stored_latency_info_.end());
diff --git a/components/viz/service/display/surface_aggregator.cc b/components/viz/service/display/surface_aggregator.cc
index c4c88f8..c43c243 100644
--- a/components/viz/service/display/surface_aggregator.cc
+++ b/components/viz/service/display/surface_aggregator.cc
@@ -1112,6 +1112,7 @@
 
   Surface* surface = manager_->GetSurfaceForId(surface_id);
   DCHECK(surface);
+  DCHECK(contained_surfaces_.empty());
   contained_surfaces_[surface_id] = surface->GetActiveFrameIndex();
 
   LocalSurfaceId& local_surface_id =
@@ -1139,6 +1140,7 @@
   valid_surfaces_.clear();
   has_cached_render_passes_ = false;
   damage_ranges_.clear();
+  DCHECK(referenced_surfaces_.empty());
   PrewalkResult prewalk_result;
   root_damage_rect_ =
       PrewalkTree(surface, false, 0, true /* will_draw */, &prewalk_result);
@@ -1147,7 +1149,6 @@
   frame.metadata.may_contain_video = prewalk_result.may_contain_video;
 
   CopyUndrawnSurfaces(&prewalk_result);
-  undrawn_surfaces_ = std::move(prewalk_result.undrawn_surfaces);
   referenced_surfaces_.insert(surface_id);
   CopyPasses(root_surface_frame, surface);
   // CopyPasses may have mutated container, need to re-query to erase.
diff --git a/components/viz/service/display/surface_aggregator.h b/components/viz/service/display/surface_aggregator.h
index a7c22b3..335d6d2 100644
--- a/components/viz/service/display/surface_aggregator.h
+++ b/components/viz/service/display/surface_aggregator.h
@@ -51,12 +51,6 @@
   void SetFullDamageForSurface(const SurfaceId& surface_id);
   void set_output_is_secure(bool secure) { output_is_secure_ = secure; }
 
-  // The set of surfaces that are referenced, but do not contribute to the
-  // aggregated CompositorFrame.
-  const base::flat_set<SurfaceId>& undrawn_surfaces() const {
-    return undrawn_surfaces_;
-  }
-
   // Set the color spaces for the created RenderPasses, which is propagated
   // to the output surface.
   void SetOutputColorSpace(const gfx::ColorSpace& blending_color_space,
diff --git a/components/viz/service/display/surface_aggregator_unittest.cc b/components/viz/service/display/surface_aggregator_unittest.cc
index 1233bb5..7d5a4d4 100644
--- a/components/viz/service/display/surface_aggregator_unittest.cc
+++ b/components/viz/service/display/surface_aggregator_unittest.cc
@@ -763,17 +763,23 @@
                 const LocalSurfaceId& local_surface_id,
                 const gfx::Rect& bounds)
       : test_(test),
+        manager_(manager),
         frame_sink_id_(frame_sink_id),
         local_surface_id_(local_surface_id),
         bounds_(bounds) {
     constexpr bool is_root = false;
     constexpr bool needs_sync_points = false;
     support_ = std::make_unique<CompositorFrameSinkSupport>(
-        nullptr, manager, frame_sink_id, is_root, needs_sync_points);
+        nullptr, manager_, frame_sink_id, is_root, needs_sync_points);
   }
 
   ~TestVizClient() = default;
 
+  Surface* GetSurface() const {
+    return manager_->surface_manager()->GetSurfaceForId(
+        SurfaceId(frame_sink_id_, local_surface_id_));
+  }
+
   void SubmitCompositorFrame(SkColor bgcolor) {
     using Quad = SurfaceAggregatorValidSurfaceTest::Quad;
     using Pass = SurfaceAggregatorValidSurfaceTest::Pass;
@@ -817,6 +823,7 @@
 
  private:
   SurfaceAggregatorValidSurfaceTest* const test_;
+  FrameSinkManagerImpl* const manager_;
   std::unique_ptr<CompositorFrameSinkSupport> support_;
   const FrameSinkId frame_sink_id_;
   const LocalSurfaceId local_surface_id_;
@@ -867,8 +874,8 @@
   AggregateAndVerify(expected_passes, {root_surface_id, parent.surface_id(),
                                        child.surface_id()});
   // |child| should not be drawn.
-  EXPECT_THAT(aggregator_.undrawn_surfaces(),
-              ::testing::ElementsAre(child.surface_id()));
+  EXPECT_TRUE(child.GetSurface()->HasUndrawnActiveFrame());
+  EXPECT_FALSE(parent.GetSurface()->HasUndrawnActiveFrame());
 
   // Submit another CompositorFrame from |parent|, this time with a DrawQuad for
   // |child|.
@@ -882,7 +889,7 @@
   AggregateAndVerify(
       {Pass(expected_quads, SurfaceSize())},
       {root_surface_id, parent.surface_id(), child.surface_id()});
-  EXPECT_THAT(aggregator_.undrawn_surfaces(), ::testing::IsEmpty());
+  EXPECT_FALSE(child.GetSurface()->HasUndrawnActiveFrame());
 }
 
 TEST_F(SurfaceAggregatorValidSurfaceTest, UndrawnSurfacesWithCopyRequests) {
@@ -928,7 +935,76 @@
   SurfaceId root_surface_id(support_->frame_sink_id(), root_local_surface_id_);
   AggregateAndVerify(expected_passes, {root_surface_id, parent.surface_id(),
                                        child.surface_id()});
-  EXPECT_THAT(aggregator_.undrawn_surfaces(), ::testing::IsEmpty());
+  EXPECT_FALSE(child.GetSurface()->HasUndrawnActiveFrame());
+  EXPECT_FALSE(parent.GetSurface()->HasUndrawnActiveFrame());
+}
+
+TEST_F(SurfaceAggregatorValidSurfaceTest,
+       SurfacesWithMultipleEmbeddersBothVisibleAndInvisible) {
+  allocator_.GenerateId();
+  TestVizClient child(
+      this, &manager_, kArbitraryFrameSinkId1,
+      allocator_.GetCurrentLocalSurfaceIdAllocation().local_surface_id(),
+      gfx::Rect(10, 10));
+  child.SubmitCompositorFrame(SK_ColorBLUE);
+
+  // First parent submits a CompositorFrame that renfereces |child|, but does
+  // not provide a DrawQuad that embeds it.
+  allocator_.GenerateId();
+  TestVizClient first_parent(
+      this, &manager_, kArbitraryFrameSinkId2,
+      allocator_.GetCurrentLocalSurfaceIdAllocation().local_surface_id(),
+      gfx::Rect(15, 15));
+  first_parent.SetEmbeddedClient(&child, false);
+  first_parent.SubmitCompositorFrame(SK_ColorGREEN);
+
+  // Second parent submits a CompositorFrame referencing |child|, and also
+  // includes a draw-quad for it.
+  allocator_.GenerateId();
+  TestVizClient second_parent(
+      this, &manager_, kArbitraryMiddleFrameSinkId,
+      allocator_.GetCurrentLocalSurfaceIdAllocation().local_surface_id(),
+      gfx::Rect(25, 25));
+  second_parent.SetEmbeddedClient(&child, true);
+  second_parent.SubmitCompositorFrame(SK_ColorYELLOW);
+
+  // Submit a root CompositorFrame that embeds both parents.
+  std::vector<Quad> root_quads = {
+      Quad::SolidColorQuad(SK_ColorWHITE, gfx::Rect(5, 5)),
+      Quad::SurfaceQuad(SurfaceRange(base::nullopt, first_parent.surface_id()),
+                        SK_ColorCYAN, first_parent.bounds(),
+                        /*stretch_content_to_fill_bounds=*/false,
+                        /*ignores_input_event=*/false),
+      Quad::SurfaceQuad(SurfaceRange(base::nullopt, second_parent.surface_id()),
+                        SK_ColorMAGENTA, second_parent.bounds(),
+                        /*stretch_content_to_fill_bounds=*/false,
+                        /*ignores_input_event=*/false),
+      Quad::SolidColorQuad(SK_ColorBLACK, gfx::Rect(5, 5))};
+  std::vector<Pass> root_passes = {Pass(root_quads, SurfaceSize())};
+
+  constexpr float device_scale_factor = 1.0f;
+  SubmitCompositorFrame(support_.get(), root_passes, root_local_surface_id_,
+                        device_scale_factor);
+
+  EXPECT_TRUE(child.GetSurface()->HasUndrawnActiveFrame());
+  EXPECT_TRUE(first_parent.GetSurface()->HasUndrawnActiveFrame());
+  EXPECT_TRUE(second_parent.GetSurface()->HasUndrawnActiveFrame());
+
+  std::vector<Quad> expected_quads = {
+      Quad::SolidColorQuad(SK_ColorWHITE, gfx::Rect(5, 5)),
+      Quad::SolidColorQuad(SK_ColorGREEN, first_parent.bounds()),
+      Quad::SolidColorQuad(SK_ColorYELLOW, second_parent.bounds()),
+      Quad::SolidColorQuad(SK_ColorBLUE, gfx::Rect(10, 10)),
+      Quad::SolidColorQuad(SK_ColorBLACK, gfx::Rect(5, 5))};
+  std::vector<Quad> expected_copy_quads = {};
+  std::vector<Pass> expected_passes = {Pass(expected_quads, SurfaceSize())};
+  SurfaceId root_surface_id(support_->frame_sink_id(), root_local_surface_id_);
+  AggregateAndVerify(expected_passes,
+                     {root_surface_id, first_parent.surface_id(),
+                      second_parent.surface_id(), child.surface_id()});
+  EXPECT_FALSE(child.GetSurface()->HasUndrawnActiveFrame());
+  EXPECT_FALSE(first_parent.GetSurface()->HasUndrawnActiveFrame());
+  EXPECT_FALSE(second_parent.GetSurface()->HasUndrawnActiveFrame());
 }
 
 // This test verifies that in the absence of a primary Surface,
diff --git a/components/viz/service/surfaces/surface.cc b/components/viz/service/surfaces/surface.cc
index b8b940f..fb8c3ea 100644
--- a/components/viz/service/surfaces/surface.cc
+++ b/components/viz/service/surfaces/surface.cc
@@ -600,6 +600,7 @@
     }
     render_pass->copy_requests.clear();
   }
+  MarkAsDrawn();
 }
 
 void Surface::TakeCopyOutputRequestsFromClient() {
@@ -757,6 +758,7 @@
         surface_info_.id().ToString());
   }
   surface_manager_->SurfaceWillBeDrawn(this);
+  MarkAsDrawn();
 }
 
 }  // namespace viz