Enqueuing new frames in a Surface should cause Displays to reaggregate it

Keep track of which Surfaces were included in a Display last
frame, so if a new frame is enqueued in a Surface the Displays containing it will tell their clients to redraw. This way the browser compositor
doesn't have to commit if only the renderer contents changed.

BUG=

Review URL: https://codereview.chromium.org/432093003

Cr-Commit-Position: refs/heads/master@{#290103}
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@290103 0039d316-1c4b-4281-b951-d872f2087c98

11 files changed, 122 insertions, 24 deletions

diff --git a/cc/surfaces/display.cc b/cc/surfaces/display.cc
index b9929ba..df64a6c 100644
--- a/cc/surfaces/display.cc
+++ b/cc/surfaces/display.cc
@@ -14,6 +14,7 @@
 #include "cc/surfaces/display_client.h"
 #include "cc/surfaces/surface.h"
 #include "cc/surfaces/surface_aggregator.h"
+#include "cc/surfaces/surface_manager.h"
 
 namespace cc {
 
@@ -21,14 +22,17 @@ Display::Display(DisplayClient* client,
                  SurfaceManager* manager,
                  SharedBitmapManager* bitmap_manager)
     : client_(client), manager_(manager), bitmap_manager_(bitmap_manager) {
+  manager_->AddObserver(this);
 }
 
 Display::~Display() {
+  manager_->RemoveObserver(this);
 }
 
 void Display::Resize(SurfaceId id, const gfx::Size& size) {
   current_surface_id_ = id;
   current_surface_size_ = size;
+  client_->DisplayDamaged();
 }
 
 void Display::InitializeOutputSurface() {
@@ -85,8 +89,10 @@ bool Display::Draw() {
   if (!output_surface_)
     return false;
 
+  contained_surfaces_.clear();
+
   scoped_ptr<CompositorFrame> frame =
-      aggregator_->Aggregate(current_surface_id_);
+      aggregator_->Aggregate(current_surface_id_, &contained_surfaces_);
   if (!frame)
     return false;
 
@@ -113,6 +119,11 @@ bool Display::Draw() {
   return true;
 }
 
+void Display::OnSurfaceDamaged(SurfaceId surface) {
+  if (contained_surfaces_.find(surface) != contained_surfaces_.end())
+    client_->DisplayDamaged();
+}
+
 SurfaceId Display::CurrentSurfaceId() {
   return current_surface_id_;
 }
diff --git a/cc/surfaces/display.h b/cc/surfaces/display.h
index 93606ba..8af18b57 100644
--- a/cc/surfaces/display.h
+++ b/cc/surfaces/display.h
@@ -11,6 +11,7 @@
 #include "cc/resources/returned_resource.h"
 #include "cc/surfaces/surface_aggregator.h"
 #include "cc/surfaces/surface_id.h"
+#include "cc/surfaces/surface_manager.h"
 #include "cc/surfaces/surfaces_export.h"
 
 namespace gfx {
@@ -28,13 +29,13 @@ class Surface;
 class SurfaceAggregator;
 class SurfaceIdAllocator;
 class SurfaceFactory;
-class SurfaceManager;
 
 // A Display produces a surface that can be used to draw to a physical display
 // (OutputSurface). The client is responsible for creating and sizing the
 // surface IDs used to draw into the display and deciding when to draw.
 class CC_SURFACES_EXPORT Display : public OutputSurfaceClient,
-                                   public RendererClient {
+                                   public RendererClient,
+                                   public SurfaceDamageObserver {
  public:
   Display(DisplayClient* client,
           SurfaceManager* manager,
@@ -72,6 +73,9 @@ class CC_SURFACES_EXPORT Display : public OutputSurfaceClient,
   virtual void SetFullRootLayerDamage() OVERRIDE {}
   virtual void RunOnDemandRasterTask(Task* on_demand_raster_task) OVERRIDE {}
 
+  // SurfaceDamageObserver implementation.
+  virtual void OnSurfaceDamaged(SurfaceId surface) OVERRIDE;
+
  private:
   void InitializeOutputSurface();
 
@@ -86,6 +90,8 @@ class CC_SURFACES_EXPORT Display : public OutputSurfaceClient,
   scoped_ptr<SurfaceAggregator> aggregator_;
   scoped_ptr<DirectRenderer> renderer_;
 
+  std::set<SurfaceId> contained_surfaces_;
+
   DISALLOW_COPY_AND_ASSIGN(Display);
 };
 
diff --git a/cc/surfaces/display_client.h b/cc/surfaces/display_client.h
index e5f0b31..6134bf5 100644
--- a/cc/surfaces/display_client.h
+++ b/cc/surfaces/display_client.h
@@ -14,6 +14,7 @@ class OutputSurface;
 class DisplayClient {
  public:
   virtual scoped_ptr<OutputSurface> CreateOutputSurface() = 0;
+  virtual void DisplayDamaged() = 0;
 
  protected:
   virtual ~DisplayClient() {}
diff --git a/cc/surfaces/surface_aggregator.cc b/cc/surfaces/surface_aggregator.cc
index 7ffa965..e4e77a1 100644
--- a/cc/surfaces/surface_aggregator.cc
+++ b/cc/surfaces/surface_aggregator.cc
@@ -135,6 +135,7 @@ bool SurfaceAggregator::TakeResources(Surface* surface,
 void SurfaceAggregator::HandleSurfaceQuad(const SurfaceDrawQuad* surface_quad,
                                           RenderPass* dest_pass) {
   SurfaceId surface_id = surface_quad->surface_id;
+  contained_surfaces_->insert(surface_id);
   // If this surface's id is already in our referenced set then it creates
   // a cycle in the graph and should be dropped.
   if (referenced_surfaces_.count(surface_id))
@@ -287,7 +288,11 @@ void SurfaceAggregator::CopyPasses(const RenderPassList& source_pass_list,
   }
 }
 
-scoped_ptr<CompositorFrame> SurfaceAggregator::Aggregate(SurfaceId surface_id) {
+scoped_ptr<CompositorFrame> SurfaceAggregator::Aggregate(
+    SurfaceId surface_id,
+    std::set<SurfaceId>* contained_surfaces) {
+  contained_surfaces_ = contained_surfaces;
+  contained_surfaces_->insert(surface_id);
   Surface* surface = manager_->GetSurfaceForId(surface_id);
   DCHECK(surface);
   const CompositorFrame* root_surface_frame = surface->GetEligibleFrame();
diff --git a/cc/surfaces/surface_aggregator.h b/cc/surfaces/surface_aggregator.h
index ec36b4b..de9a6d6 100644
--- a/cc/surfaces/surface_aggregator.h
+++ b/cc/surfaces/surface_aggregator.h
@@ -29,7 +29,9 @@ class CC_SURFACES_EXPORT SurfaceAggregator {
   SurfaceAggregator(SurfaceManager* manager, ResourceProvider* provider);
   ~SurfaceAggregator();
 
-  scoped_ptr<CompositorFrame> Aggregate(SurfaceId surface_id);
+  scoped_ptr<CompositorFrame> Aggregate(
+      SurfaceId surface_id,
+      std::set<SurfaceId>* contained_surfaces);
 
  private:
   RenderPass::Id RemapPassId(RenderPass::Id surface_local_pass_id,
@@ -72,6 +74,9 @@ class CC_SURFACES_EXPORT SurfaceAggregator {
   typedef std::set<SurfaceId> SurfaceSet;
   SurfaceSet referenced_surfaces_;
 
+  // This is the set of surfaces that were used in the last draw.
+  SurfaceSet* contained_surfaces_;
+
   // This is the pass list for the aggregated frame.
   RenderPassList* dest_pass_list_;
 
diff --git a/cc/surfaces/surface_aggregator_unittest.cc b/cc/surfaces/surface_aggregator_unittest.cc
index 6525157..07d8185 100644
--- a/cc/surfaces/surface_aggregator_unittest.cc
+++ b/cc/surfaces/surface_aggregator_unittest.cc
@@ -61,7 +61,9 @@ class SurfaceAggregatorTest : public testing::Test {
 TEST_F(SurfaceAggregatorTest, ValidSurfaceNoFrame) {
   SurfaceId one_id(7);
   factory_.Create(one_id, SurfaceSize());
-  scoped_ptr<CompositorFrame> frame = aggregator_.Aggregate(one_id);
+  std::set<SurfaceId> surface_set;
+  scoped_ptr<CompositorFrame> frame =
+      aggregator_.Aggregate(one_id, &surface_set);
   EXPECT_FALSE(frame);
   factory_.Destroy(one_id);
 }
@@ -82,9 +84,12 @@ class SurfaceAggregatorValidSurfaceTest : public SurfaceAggregatorTest {
   }
 
   void AggregateAndVerify(test::Pass* expected_passes,
-                          size_t expected_pass_count) {
+                          size_t expected_pass_count,
+                          SurfaceId* surface_ids,
+                          size_t expected_surface_count) {
+    std::set<SurfaceId> surface_set;
     scoped_ptr<CompositorFrame> aggregated_frame =
-        aggregator_.Aggregate(root_surface_id_);
+        aggregator_.Aggregate(root_surface_id_, &surface_set);
 
     ASSERT_TRUE(aggregated_frame);
     ASSERT_TRUE(aggregated_frame->delegated_frame_data);
@@ -94,6 +99,11 @@ class SurfaceAggregatorValidSurfaceTest : public SurfaceAggregatorTest {
 
     TestPassesMatchExpectations(
         expected_passes, expected_pass_count, &frame_data->render_pass_list);
+
+    EXPECT_EQ(expected_surface_count, surface_set.size());
+    for (size_t i = 0; i < expected_surface_count; i++) {
+      EXPECT_TRUE(surface_set.find(surface_ids[i]) != surface_set.end());
+    }
   }
 
   void SubmitFrame(test::Pass* passes,
@@ -135,7 +145,8 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, SimpleFrame) {
 
   SubmitFrame(passes, arraysize(passes), root_surface_id_);
 
-  AggregateAndVerify(passes, arraysize(passes));
+  SurfaceId ids[] = {root_surface_id_};
+  AggregateAndVerify(passes, arraysize(passes), ids, arraysize(ids));
 }
 
 TEST_F(SurfaceAggregatorValidSurfaceTest, MultiPassSimpleFrame) {
@@ -148,7 +159,8 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, MultiPassSimpleFrame) {
 
   SubmitFrame(passes, arraysize(passes), root_surface_id_);
 
-  AggregateAndVerify(passes, arraysize(passes));
+  SurfaceId ids[] = {root_surface_id_};
+  AggregateAndVerify(passes, arraysize(passes), ids, arraysize(ids));
 }
 
 // This tests very simple embedding. root_surface has a frame containing a few
@@ -177,7 +189,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, SimpleSurfaceReference) {
                                  test::Quad::SolidColorQuad(SK_ColorBLACK)};
   test::Pass expected_passes[] = {
       test::Pass(expected_quads, arraysize(expected_quads))};
-  AggregateAndVerify(expected_passes, arraysize(expected_passes));
+  SurfaceId ids[] = {root_surface_id_, embedded_surface_id};
+  AggregateAndVerify(
+      expected_passes, arraysize(expected_passes), ids, arraysize(ids));
 
   factory_.Destroy(embedded_surface_id);
 }
@@ -213,8 +227,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, MultiPassSurfaceReference) {
 
   SubmitFrame(root_passes, arraysize(root_passes), root_surface_id_);
 
+  std::set<SurfaceId> surface_set;
   scoped_ptr<CompositorFrame> aggregated_frame =
-      aggregator_.Aggregate(root_surface_id_);
+      aggregator_.Aggregate(root_surface_id_, &surface_set);
 
   ASSERT_TRUE(aggregated_frame);
   ASSERT_TRUE(aggregated_frame->delegated_frame_data);
@@ -329,7 +344,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, InvalidSurfaceReference) {
                                  test::Quad::SolidColorQuad(SK_ColorBLUE)};
   test::Pass expected_passes[] = {
       test::Pass(expected_quads, arraysize(expected_quads))};
-  AggregateAndVerify(expected_passes, arraysize(expected_passes));
+  SurfaceId ids[] = {root_surface_id_, InvalidSurfaceId()};
+  AggregateAndVerify(
+      expected_passes, arraysize(expected_passes), ids, arraysize(ids));
 }
 
 // Tests a reference to a valid surface with no submitted frame. This quad
@@ -348,7 +365,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, ValidSurfaceReferenceWithNoFrame) {
                                  test::Quad::SolidColorQuad(SK_ColorBLUE)};
   test::Pass expected_passes[] = {
       test::Pass(expected_quads, arraysize(expected_quads))};
-  AggregateAndVerify(expected_passes, arraysize(expected_passes));
+  SurfaceId ids[] = {root_surface_id_, surface_with_no_frame_id};
+  AggregateAndVerify(
+      expected_passes, arraysize(expected_passes), ids, arraysize(ids));
   factory_.Destroy(surface_with_no_frame_id);
 }
 
@@ -364,7 +383,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, SimpleCyclicalReference) {
   test::Quad expected_quads[] = {test::Quad::SolidColorQuad(SK_ColorYELLOW)};
   test::Pass expected_passes[] = {
       test::Pass(expected_quads, arraysize(expected_quads))};
-  AggregateAndVerify(expected_passes, arraysize(expected_passes));
+  SurfaceId ids[] = {root_surface_id_};
+  AggregateAndVerify(
+      expected_passes, arraysize(expected_passes), ids, arraysize(ids));
 }
 
 // Tests a more complex cycle with one intermediate surface.
@@ -399,7 +420,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, TwoSurfaceCyclicalReference) {
                                  test::Quad::SolidColorQuad(SK_ColorCYAN)};
   test::Pass expected_passes[] = {
       test::Pass(expected_quads, arraysize(expected_quads))};
-  AggregateAndVerify(expected_passes, arraysize(expected_passes));
+  SurfaceId ids[] = {root_surface_id_, child_surface_id};
+  AggregateAndVerify(
+      expected_passes, arraysize(expected_passes), ids, arraysize(ids));
   factory_.Destroy(child_surface_id);
 }
 
@@ -429,8 +452,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, RenderPassIdMapping) {
       test::Pass(parent_quad[1], arraysize(parent_quad[1]), parent_pass_id[1])};
 
   SubmitFrame(parent_passes, arraysize(parent_passes), root_surface_id_);
+  std::set<SurfaceId> surface_set;
   scoped_ptr<CompositorFrame> aggregated_frame =
-      aggregator_.Aggregate(root_surface_id_);
+      aggregator_.Aggregate(root_surface_id_, &surface_set);
 
   ASSERT_TRUE(aggregated_frame);
   ASSERT_TRUE(aggregated_frame->delegated_frame_data);
@@ -600,8 +624,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateSharedQuadStateProperties) {
 
   QueuePassAsFrame(root_pass.Pass(), root_surface_id_);
 
+  std::set<SurfaceId> surface_set;
   scoped_ptr<CompositorFrame> aggregated_frame =
-      aggregator_.Aggregate(root_surface_id_);
+      aggregator_.Aggregate(root_surface_id_, &surface_set);
 
   ASSERT_TRUE(aggregated_frame);
   ASSERT_TRUE(aggregated_frame->delegated_frame_data);
@@ -701,8 +726,9 @@ TEST_F(SurfaceAggregatorValidSurfaceTest, AggregateMultiplePassWithTransform) {
 
   factory_.SubmitFrame(root_surface_id_, root_frame.Pass());
 
+  std::set<SurfaceId> surface_set;
   scoped_ptr<CompositorFrame> aggregated_frame =
-      aggregator_.Aggregate(root_surface_id_);
+      aggregator_.Aggregate(root_surface_id_, &surface_set);
 
   ASSERT_TRUE(aggregated_frame);
   ASSERT_TRUE(aggregated_frame->delegated_frame_data);
@@ -862,14 +888,17 @@ TEST_F(SurfaceAggregatorWithResourcesTest, TakeResourcesOneSurface) {
   ResourceProvider::ResourceId ids[] = {11, 12, 13};
   SubmitFrameWithResources(ids, arraysize(ids), &factory, surface_id);
 
-  scoped_ptr<CompositorFrame> frame = aggregator_->Aggregate(surface_id);
+  std::set<SurfaceId> surface_set;
+  scoped_ptr<CompositorFrame> frame =
+      aggregator_->Aggregate(surface_id, &surface_set);
 
   // Nothing should be available to be returned yet.
   EXPECT_TRUE(client.returned_resources().empty());
 
   SubmitFrameWithResources(NULL, 0u, &factory, surface_id);
 
-  frame = aggregator_->Aggregate(surface_id);
+  surface_set.clear();
+  frame = aggregator_->Aggregate(surface_id, &surface_set);
 
   ASSERT_EQ(3u, client.returned_resources().size());
   ResourceProvider::ResourceId returned_ids[3];
diff --git a/cc/surfaces/surface_damage_observer.h b/cc/surfaces/surface_damage_observer.h
new file mode 100644
index 0000000..379c9f7
--- /dev/null
+++ b/cc/surfaces/surface_damage_observer.h
@@ -0,0 +1,19 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CC_SURFACES_SURFACE_DAMAGE_OBSERVER_H_
+#define CC_SURFACES_SURFACE_DAMAGE_OBSERVER_H_
+
+#include "cc/surfaces/surface_id.h"
+
+namespace cc {
+
+class SurfaceDamageObserver {
+ public:
+  virtual void OnSurfaceDamaged(SurfaceId surface_id) = 0;
+};
+
+}  // namespace cc
+
+#endif  // CC_SURFACES_SURFACE_DAMAGE_OBSERVER_H_
diff --git a/cc/surfaces/surface_factory.cc b/cc/surfaces/surface_factory.cc
index 69ee02a..d17e98e 100644
--- a/cc/surfaces/surface_factory.cc
+++ b/cc/surfaces/surface_factory.cc
@@ -40,6 +40,7 @@ void SurfaceFactory::SubmitFrame(SurfaceId surface_id,
   DCHECK(it != surface_map_.end());
   DCHECK(it->second->factory() == this);
   it->second->QueueFrame(frame.Pass());
+  manager_->SurfaceModified(surface_id);
 }
 
 void SurfaceFactory::ReceiveFromChild(
diff --git a/cc/surfaces/surface_manager.cc b/cc/surfaces/surface_manager.cc
index 7027abd..1c3de16 100644
--- a/cc/surfaces/surface_manager.cc
+++ b/cc/surfaces/surface_manager.cc
@@ -35,4 +35,9 @@ Surface* SurfaceManager::GetSurfaceForId(SurfaceId surface_id) {
   return it->second;
 }
 
+void SurfaceManager::SurfaceModified(SurfaceId surface_id) {
+  FOR_EACH_OBSERVER(
+      SurfaceDamageObserver, observer_list_, OnSurfaceDamaged(surface_id));
+}
+
 }  // namespace cc
diff --git a/cc/surfaces/surface_manager.h b/cc/surfaces/surface_manager.h
index 40e55f1..202dfe3 100644
--- a/cc/surfaces/surface_manager.h
+++ b/cc/surfaces/surface_manager.h
@@ -7,6 +7,8 @@
 
 #include "base/containers/hash_tables.h"
 #include "base/macros.h"
+#include "base/observer_list.h"
+#include "cc/surfaces/surface_damage_observer.h"
 #include "cc/surfaces/surface_id.h"
 #include "cc/surfaces/surfaces_export.h"
 
@@ -24,9 +26,20 @@ class CC_SURFACES_EXPORT SurfaceManager {
 
   Surface* GetSurfaceForId(SurfaceId surface_id);
 
+  void AddObserver(SurfaceDamageObserver* obs) {
+    observer_list_.AddObserver(obs);
+  }
+
+  void RemoveObserver(SurfaceDamageObserver* obs) {
+    observer_list_.RemoveObserver(obs);
+  }
+
+  void SurfaceModified(SurfaceId surface_id);
+
  private:
   typedef base::hash_map<SurfaceId, Surface*> SurfaceMap;
   SurfaceMap surface_map_;
+  ObserverList<SurfaceDamageObserver> observer_list_;
 
   DISALLOW_COPY_AND_ASSIGN(SurfaceManager);
 };
diff --git a/cc/surfaces/surfaces_pixeltest.cc b/cc/surfaces/surfaces_pixeltest.cc
index c9c7a46..bd9f56c 100644
--- a/cc/surfaces/surfaces_pixeltest.cc
+++ b/cc/surfaces/surfaces_pixeltest.cc
@@ -90,8 +90,9 @@ TEST_F(SurfacesPixelTest, DrawSimpleFrame) {
   factory_.SubmitFrame(root_surface_id, root_frame.Pass());
 
   SurfaceAggregator aggregator(&manager_, resource_provider_.get());
+  std::set<SurfaceId> surface_set;
   scoped_ptr<CompositorFrame> aggregated_frame =
-      aggregator.Aggregate(root_surface_id);
+      aggregator.Aggregate(root_surface_id, &surface_set);
   factory_.Destroy(root_surface_id);
 
   bool discard_alpha = false;
@@ -172,8 +173,9 @@ TEST_F(SurfacesPixelTest, DrawSimpleAggregatedFrame) {
   }
 
   SurfaceAggregator aggregator(&manager_, resource_provider_.get());
+  std::set<SurfaceId> surface_set;
   scoped_ptr<CompositorFrame> aggregated_frame =
-      aggregator.Aggregate(root_surface_id);
+      aggregator.Aggregate(root_surface_id, &surface_set);
 
   bool discard_alpha = false;
   ExactPixelComparator pixel_comparator(discard_alpha);
@@ -312,8 +314,9 @@ TEST_F(SurfacesPixelTest, DrawAggregatedFrameWithSurfaceTransforms) {
   }
 
   SurfaceAggregator aggregator(&manager_, resource_provider_.get());
+  std::set<SurfaceId> surface_set;
   scoped_ptr<CompositorFrame> aggregated_frame =
-      aggregator.Aggregate(root_surface_id);
+      aggregator.Aggregate(root_surface_id, &surface_set);
 
   bool discard_alpha = false;
   ExactPixelComparator pixel_comparator(discard_alpha);