Store recording invalidations in DisplayListRecordingSource, save them via Update.

This fixes the referenced bug, as well as cleaning up the interaction between
PictureLayer and DisplayListRecordingSource a little bit. This way both the
invalidation and picture come from DisplayListRecordingSource, rather than one
coming from PictureLayer and the other from DisplayListRecordingSource, with the
latter modifying the invalidation during Update.

Also renamed LayerImpl::GetInvalidationRegion() to
LayerImpl::GetInvalidationRegionForDebugging() to make clear it's only used for
that purpose.

BUG=591561
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

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

Cr-Commit-Position: refs/heads/master@{#381977}

13 files changed, 127 insertions, 30 deletions

diff --git a/cc/base/invalidation_region.h b/cc/base/invalidation_region.h
index 3cf69f2..7aa4a8b 100644
--- a/cc/base/invalidation_region.h
+++ b/cc/base/invalidation_region.h
@@ -23,7 +23,6 @@ class CC_EXPORT InvalidationRegion {
   void Clear();
   void Union(const gfx::Rect& rect);
   bool IsEmpty() const { return region_.IsEmpty(); }
-  Region* region() { return &region_; }
 
  private:
   void SimplifyIfNeeded();
diff --git a/cc/debug/debug_rect_history.cc b/cc/debug/debug_rect_history.cc
index ebe6936..80c58383 100644
--- a/cc/debug/debug_rect_history.cc
+++ b/cc/debug/debug_rect_history.cc
@@ -72,7 +72,7 @@ void DebugRectHistory::SavePaintRects(LayerImpl* root_layer) {
   // not. Therefore we traverse over all layers, not just the render surface
   // list.
   for (auto* layer : *root_layer->layer_tree_impl()) {
-    Region invalidation_region = layer->GetInvalidationRegion();
+    Region invalidation_region = layer->GetInvalidationRegionForDebugging();
     if (invalidation_region.IsEmpty() || !layer->DrawsContent())
       continue;
 
diff --git a/cc/layers/layer_impl.cc b/cc/layers/layer_impl.cc
index dbfd5fc..3d4294d 100644
--- a/cc/layers/layer_impl.cc
+++ b/cc/layers/layer_impl.cc
@@ -1545,7 +1545,7 @@ void LayerImpl::SetForceRenderSurface(bool force_render_surface) {
   NoteLayerPropertyChanged();
 }
 
-Region LayerImpl::GetInvalidationRegion() {
+Region LayerImpl::GetInvalidationRegionForDebugging() {
   return Region(update_rect_);
 }
 
diff --git a/cc/layers/layer_impl.h b/cc/layers/layer_impl.h
index a8de2bb..4564544 100644
--- a/cc/layers/layer_impl.h
+++ b/cc/layers/layer_impl.h
@@ -565,7 +565,7 @@ class CC_EXPORT LayerImpl {
 
   // Get the correct invalidation region instead of conservative Rect
   // for layers that provide it.
-  virtual Region GetInvalidationRegion();
+  virtual Region GetInvalidationRegionForDebugging();
 
   virtual gfx::Rect GetEnclosingRectInTargetSpace() const;
 
diff --git a/cc/layers/picture_layer.cc b/cc/layers/picture_layer.cc
index 9d7ec69..af5cfb4 100644
--- a/cc/layers/picture_layer.cc
+++ b/cc/layers/picture_layer.cc
@@ -59,9 +59,9 @@ void PictureLayer::PushPropertiesTo(LayerImpl* base_layer) {
       recording_source_->CreateRasterSource(can_use_lcd_text);
   layer_impl->set_gpu_raster_max_texture_size(
       layer_tree_host()->device_viewport_size());
-  layer_impl->UpdateRasterSource(raster_source, invalidation_.region(),
+  layer_impl->UpdateRasterSource(raster_source, &last_updated_invalidation_,
                                  nullptr);
-  DCHECK(invalidation_.IsEmpty());
+  DCHECK(last_updated_invalidation_.IsEmpty());
 }
 
 void PictureLayer::SetLayerTreeHost(LayerTreeHost* host) {
@@ -82,10 +82,8 @@ void PictureLayer::SetLayerTreeHost(LayerTreeHost* host) {
 
 void PictureLayer::SetNeedsDisplayRect(const gfx::Rect& layer_rect) {
   DCHECK(!layer_tree_host() || !layer_tree_host()->in_paint_layer_contents());
-  if (!layer_rect.IsEmpty()) {
-    // Clamp invalidation to the layer bounds.
-    invalidation_.Union(gfx::IntersectRects(layer_rect, gfx::Rect(bounds())));
-  }
+  if (recording_source_)
+    recording_source_->SetNeedsDisplayRect(layer_rect);
   Layer::SetNeedsDisplayRect(layer_rect);
 }
 
@@ -96,12 +94,6 @@ bool PictureLayer::Update() {
   gfx::Rect update_rect = visible_layer_rect();
   gfx::Size layer_size = paint_properties().bounds;
 
-  if (last_updated_visible_layer_rect_ == update_rect &&
-      recording_source_->GetSize() == layer_size && invalidation_.IsEmpty()) {
-    // Only early out if the visible content rect of this layer hasn't changed.
-    return updated;
-  }
-
   recording_source_->SetBackgroundColor(SafeOpaqueBackgroundColor());
   recording_source_->SetRequiresClear(!contents_opaque() &&
                                       !client_->FillsBoundsCompletely());
@@ -118,7 +110,7 @@ bool PictureLayer::Update() {
   // for them.
   DCHECK(client_);
   updated |= recording_source_->UpdateAndExpandInvalidation(
-      client_, invalidation_.region(), layer_size, update_rect,
+      client_, &last_updated_invalidation_, layer_size, update_rect,
       update_source_frame_number_, DisplayListRecordingSource::RECORD_NORMALLY);
   last_updated_visible_layer_rect_ = visible_layer_rect();
 
@@ -127,7 +119,7 @@ bool PictureLayer::Update() {
   } else {
     // If this invalidation did not affect the recording source, then it can be
     // cleared as an optimization.
-    invalidation_.Clear();
+    last_updated_invalidation_.Clear();
   }
 
   return updated;
@@ -192,7 +184,7 @@ void PictureLayer::LayerSpecificPropertiesToProto(
   recording_source_->ToProtobuf(
       picture->mutable_recording_source(),
       layer_tree_host()->image_serialization_processor());
-  RegionToProto(*invalidation_.region(), picture->mutable_invalidation());
+  RegionToProto(last_updated_invalidation_, picture->mutable_invalidation());
   RectToProto(last_updated_visible_layer_rect_,
               picture->mutable_last_updated_visible_layer_rect());
   picture->set_is_mask(is_mask_);
@@ -200,7 +192,7 @@ void PictureLayer::LayerSpecificPropertiesToProto(
 
   picture->set_update_source_frame_number(update_source_frame_number_);
 
-  invalidation_.Clear();
+  last_updated_invalidation_.Clear();
 }
 
 void PictureLayer::FromLayerSpecificPropertiesProto(
@@ -218,7 +210,7 @@ void PictureLayer::FromLayerSpecificPropertiesProto(
       layer_tree_host()->image_serialization_processor());
 
   Region new_invalidation = RegionFromProto(picture.invalidation());
-  invalidation_.Swap(&new_invalidation);
+  last_updated_invalidation_.Swap(&new_invalidation);
   last_updated_visible_layer_rect_ =
       ProtoToRect(picture.last_updated_visible_layer_rect());
   is_mask_ = picture.is_mask();
diff --git a/cc/layers/picture_layer.h b/cc/layers/picture_layer.h
index 248eee8..2d2a03b 100644
--- a/cc/layers/picture_layer.h
+++ b/cc/layers/picture_layer.h
@@ -68,10 +68,10 @@ class CC_EXPORT PictureLayer : public Layer {
   devtools_instrumentation::
       ScopedLayerObjectTracker instrumentation_object_tracker_;
 
-  // Invalidation to use the next time update is called.
-  InvalidationRegion invalidation_;
-
+  // These store the last visible layer and invalidation computed via a call
+  // to Update().
   gfx::Rect last_updated_visible_layer_rect_;
+  Region last_updated_invalidation_;
 
   int update_source_frame_number_;
   bool is_mask_;
diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc
index a12fb282..4f5ce15 100644
--- a/cc/layers/picture_layer_impl.cc
+++ b/cc/layers/picture_layer_impl.cc
@@ -653,7 +653,7 @@ skia::RefPtr<SkPicture> PictureLayerImpl::GetPicture() {
   return raster_source_->GetFlattenedPicture();
 }
 
-Region PictureLayerImpl::GetInvalidationRegion() {
+Region PictureLayerImpl::GetInvalidationRegionForDebugging() {
   // |invalidation_| gives the invalidation contained in the source frame, but
   // is not cleared after drawing from the layer. However, update_rect() is
   // cleared once the invalidation is drawn, which is useful for debugging
diff --git a/cc/layers/picture_layer_impl.h b/cc/layers/picture_layer_impl.h
index 50abaa8..df9bdcf 100644
--- a/cc/layers/picture_layer_impl.h
+++ b/cc/layers/picture_layer_impl.h
@@ -50,7 +50,7 @@ class CC_EXPORT PictureLayerImpl
   void ReleaseResources() override;
   void RecreateResources() override;
   skia::RefPtr<SkPicture> GetPicture() override;
-  Region GetInvalidationRegion() override;
+  Region GetInvalidationRegionForDebugging() override;
 
   // PictureLayerTilingClient overrides.
   ScopedTilePtr CreateTile(const Tile::CreateInfo& info) override;
diff --git a/cc/layers/picture_layer_impl_unittest.cc b/cc/layers/picture_layer_impl_unittest.cc
index fd505af..2c9780e 100644
--- a/cc/layers/picture_layer_impl_unittest.cc
+++ b/cc/layers/picture_layer_impl_unittest.cc
@@ -4459,7 +4459,8 @@ TEST_F(PictureLayerImplTest, NonSolidToSolidNoTilings) {
 
   client.set_fill_with_nonsolid_color(true);
 
-  Region invalidation1(layer_rect);
+  recording_source->SetNeedsDisplayRect(layer_rect);
+  Region invalidation1;
   recording_source->UpdateAndExpandInvalidation(
       &client, &invalidation1, layer_bounds, layer_rect, frame_number++,
       DisplayListRecordingSource::RECORD_NORMALLY);
@@ -4478,7 +4479,8 @@ TEST_F(PictureLayerImplTest, NonSolidToSolidNoTilings) {
 
   client.set_fill_with_nonsolid_color(false);
 
-  Region invalidation2(layer_rect);
+  recording_source->SetNeedsDisplayRect(layer_rect);
+  Region invalidation2;
   recording_source->UpdateAndExpandInvalidation(
       &client, &invalidation2, layer_bounds, layer_rect, frame_number++,
       DisplayListRecordingSource::RECORD_NORMALLY);
diff --git a/cc/layers/picture_layer_unittest.cc b/cc/layers/picture_layer_unittest.cc
index 45a0610..94838eb 100644
--- a/cc/layers/picture_layer_unittest.cc
+++ b/cc/layers/picture_layer_unittest.cc
@@ -45,7 +45,7 @@ class TestSerializationPictureLayer : public PictureLayer {
   }
 
   void set_invalidation(const Region& invalidation) {
-    *invalidation_.region() = invalidation;
+    last_updated_invalidation_ = invalidation;
   }
 
   void set_last_updated_visible_layer_rect(const gfx::Rect& rect) {
@@ -220,6 +220,90 @@ TEST(PictureLayerTest, NoTilesIfEmptyBounds) {
   EXPECT_FALSE(layer_impl->raster_source()->HasRecordings());
 }
 
+TEST(PictureLayerTest, InvalidateRasterAfterUpdate) {
+  gfx::Size layer_size(50, 50);
+  FakeContentLayerClient client;
+  client.set_bounds(layer_size);
+  scoped_refptr<PictureLayer> layer = PictureLayer::Create(&client);
+  layer->SetBounds(gfx::Size(50, 50));
+
+  FakeLayerTreeHostClient host_client(FakeLayerTreeHostClient::DIRECT_3D);
+  TestTaskGraphRunner task_graph_runner;
+  scoped_ptr<FakeLayerTreeHost> host =
+      FakeLayerTreeHost::Create(&host_client, &task_graph_runner);
+  host->SetRootLayer(layer);
+  layer->SetIsDrawable(true);
+  layer->SavePaintProperties();
+
+  gfx::Rect invalidation_bounds(layer_size);
+
+  // The important two lines are the following:
+  layer->SetNeedsDisplayRect(invalidation_bounds);
+  layer->Update();
+
+  host->CommitComplete();
+  FakeImplTaskRunnerProvider impl_task_runner_provider;
+  TestSharedBitmapManager shared_bitmap_manager;
+  scoped_ptr<OutputSurface> output_surface(FakeOutputSurface::Create3d());
+  LayerTreeSettings layer_tree_settings = LayerTreeSettings();
+  layer_tree_settings.image_decode_tasks_enabled = true;
+  FakeLayerTreeHostImpl host_impl(layer_tree_settings,
+                                  &impl_task_runner_provider,
+                                  &shared_bitmap_manager, &task_graph_runner);
+  host_impl.SetVisible(true);
+  host_impl.InitializeRenderer(output_surface.get());
+  host_impl.CreatePendingTree();
+  host_impl.pending_tree()->SetRootLayer(
+      FakePictureLayerImpl::Create(host_impl.pending_tree(), 1));
+  FakePictureLayerImpl* layer_impl = static_cast<FakePictureLayerImpl*>(
+      host_impl.pending_tree()->root_layer());
+  layer->PushPropertiesTo(layer_impl);
+
+  EXPECT_EQ(invalidation_bounds,
+            layer_impl->GetPendingInvalidation()->bounds());
+}
+
+TEST(PictureLayerTest, InvalidateRasterWithoutUpdate) {
+  gfx::Size layer_size(50, 50);
+  FakeContentLayerClient client;
+  client.set_bounds(layer_size);
+  scoped_refptr<PictureLayer> layer = PictureLayer::Create(&client);
+  layer->SetBounds(gfx::Size(50, 50));
+
+  FakeLayerTreeHostClient host_client(FakeLayerTreeHostClient::DIRECT_3D);
+  TestTaskGraphRunner task_graph_runner;
+  scoped_ptr<FakeLayerTreeHost> host =
+      FakeLayerTreeHost::Create(&host_client, &task_graph_runner);
+  host->SetRootLayer(layer);
+  layer->SetIsDrawable(true);
+  layer->SavePaintProperties();
+
+  gfx::Rect invalidation_bounds(layer_size);
+
+  // The important line is the following (note that we do not call Update):
+  layer->SetNeedsDisplayRect(invalidation_bounds);
+
+  host->CommitComplete();
+  FakeImplTaskRunnerProvider impl_task_runner_provider;
+  TestSharedBitmapManager shared_bitmap_manager;
+  scoped_ptr<OutputSurface> output_surface(FakeOutputSurface::Create3d());
+  LayerTreeSettings layer_tree_settings = LayerTreeSettings();
+  layer_tree_settings.image_decode_tasks_enabled = true;
+  FakeLayerTreeHostImpl host_impl(layer_tree_settings,
+                                  &impl_task_runner_provider,
+                                  &shared_bitmap_manager, &task_graph_runner);
+  host_impl.SetVisible(true);
+  host_impl.InitializeRenderer(output_surface.get());
+  host_impl.CreatePendingTree();
+  host_impl.pending_tree()->SetRootLayer(
+      FakePictureLayerImpl::Create(host_impl.pending_tree(), 1));
+  FakePictureLayerImpl* layer_impl = static_cast<FakePictureLayerImpl*>(
+      host_impl.pending_tree()->root_layer());
+  layer->PushPropertiesTo(layer_impl);
+
+  EXPECT_EQ(gfx::Rect(), layer_impl->GetPendingInvalidation()->bounds());
+}
+
 TEST(PictureLayerTest, ClearVisibleRectWhenNoTiling) {
   gfx::Size layer_size(50, 50);
   FakeContentLayerClient client;
diff --git a/cc/playback/display_list_recording_source.cc b/cc/playback/display_list_recording_source.cc
index 52c8277..a6f5584 100644
--- a/cc/playback/display_list_recording_source.cc
+++ b/cc/playback/display_list_recording_source.cc
@@ -117,6 +117,14 @@ void DisplayListRecordingSource::FinishDisplayItemListUpdate() {
     display_list_->GenerateDiscardableImagesMetadata();
 }
 
+void DisplayListRecordingSource::SetNeedsDisplayRect(
+    const gfx::Rect& layer_rect) {
+  if (!layer_rect.IsEmpty()) {
+    // Clamp invalidation to the layer bounds.
+    invalidation_.Union(gfx::IntersectRects(layer_rect, gfx::Rect(size_)));
+  }
+}
+
 bool DisplayListRecordingSource::UpdateAndExpandInvalidation(
     ContentLayerClient* painter,
     Region* invalidation,
@@ -133,6 +141,9 @@ bool DisplayListRecordingSource::UpdateAndExpandInvalidation(
     updated = true;
   }
 
+  invalidation_.Swap(invalidation);
+  invalidation_.Clear();
+
   gfx::Rect new_recorded_viewport = painter->PaintableRegion();
   if (new_recorded_viewport != recorded_viewport_) {
     UpdateInvalidationForNewViewport(recorded_viewport_, new_recorded_viewport,
@@ -150,6 +161,9 @@ bool DisplayListRecordingSource::UpdateAndExpandInvalidation(
   if (!updated && !invalidation->Intersects(recorded_viewport_))
     return false;
 
+  if (invalidation->IsEmpty())
+    return false;
+
   ContentLayerClient::PaintingControlSetting painting_control =
       ContentLayerClient::PAINTING_BEHAVIOR_NORMAL;
 
diff --git a/cc/playback/display_list_recording_source.h b/cc/playback/display_list_recording_source.h
index c69c114..754cec9 100644
--- a/cc/playback/display_list_recording_source.h
+++ b/cc/playback/display_list_recording_source.h
@@ -11,6 +11,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "cc/base/cc_export.h"
+#include "cc/base/invalidation_region.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
@@ -63,6 +64,8 @@ class CC_EXPORT DisplayListRecordingSource {
   void SetBackgroundColor(SkColor background_color);
   void SetRequiresClear(bool requires_clear);
 
+  void SetNeedsDisplayRect(const gfx::Rect& layer_rect);
+
   // These functions are virtual for testing.
   virtual scoped_refptr<DisplayListRasterSource> CreateRasterSource(
       bool can_use_lcd_text) const;
@@ -96,6 +99,8 @@ class CC_EXPORT DisplayListRecordingSource {
 
   void DetermineIfSolidColor();
 
+  InvalidationRegion invalidation_;
+
   DISALLOW_COPY_AND_ASSIGN(DisplayListRecordingSource);
 };
 
diff --git a/cc/test/fake_display_list_recording_source.h b/cc/test/fake_display_list_recording_source.h
index 158c053..28ea1ab 100644
--- a/cc/test/fake_display_list_recording_source.h
+++ b/cc/test/fake_display_list_recording_source.h
@@ -68,7 +68,8 @@ class FakeDisplayListRecordingSource : public DisplayListRecordingSource {
   }
 
   void Rerecord() {
-    Region invalidation = recorded_viewport_;
+    SetNeedsDisplayRect(recorded_viewport_);
+    Region invalidation;
     UpdateAndExpandInvalidation(&client_, &invalidation, size_,
                                 recorded_viewport_, 0, RECORD_NORMALLY);
   }