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}
(cherry picked from commit c41aca7b408e13fdee88a3b39db1a93813bd9fc1)

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

Cr-Commit-Position: refs/branch-heads/2623@{#637}
Cr-Branched-From: 92d77538a86529ca35f9220bd3cd512cbea1f086-refs/heads/master@{#369907}

13 files changed, 128 insertions, 31 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 5ac2b5d..bbeb1b9 100644
--- a/cc/debug/debug_rect_history.cc
+++ b/cc/debug/debug_rect_history.cc
@@ -67,8 +67,8 @@ void DebugRectHistory::SaveDebugRectsForCurrentFrame(
 void DebugRectHistory::SavePaintRects(LayerImpl* layer) {
   // We would like to visualize where any layer's paint rect (update rect) has
   // changed, regardless of whether this layer is skipped for actual drawing or
-  // not. Therefore we traverse recursively over all layers, not just the render
-  // surface list.
+  // not. Therefore we traverse over all layers, not just the render surface
+  // list.
 
   Region invalidation_region = layer->GetInvalidationRegion();
   if (!invalidation_region.IsEmpty() && layer->DrawsContent()) {
diff --git a/cc/layers/layer_impl.cc b/cc/layers/layer_impl.cc
index b845968..40bc9d5 100644
--- a/cc/layers/layer_impl.cc
+++ b/cc/layers/layer_impl.cc
@@ -1879,7 +1879,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 4dca350..face8a6 100644
--- a/cc/layers/layer_impl.h
+++ b/cc/layers/layer_impl.h
@@ -651,7 +651,7 @@ class CC_EXPORT LayerImpl : public LayerAnimationValueObserver,
 
   // 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 f9df22cd..fb89080 100644
--- a/cc/layers/picture_layer.cc
+++ b/cc/layers/picture_layer.cc
@@ -65,9 +65,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) {
@@ -88,10 +88,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);
 }
 
@@ -102,12 +100,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());
@@ -124,7 +116,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();
 
@@ -133,7 +125,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;
@@ -196,7 +188,7 @@ void PictureLayer::LayerSpecificPropertiesToProto(
 
   proto::PictureLayerProperties* picture = proto->mutable_picture();
   recording_source_->ToProtobuf(picture->mutable_recording_source());
-  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_);
@@ -204,7 +196,7 @@ void PictureLayer::LayerSpecificPropertiesToProto(
 
   picture->set_update_source_frame_number(update_source_frame_number_);
 
-  invalidation_.Clear();
+  last_updated_invalidation_.Clear();
 }
 
 void PictureLayer::FromLayerSpecificPropertiesProto(
@@ -214,7 +206,7 @@ void PictureLayer::FromLayerSpecificPropertiesProto(
   recording_source_->FromProtobuf(picture.recording_source());
 
   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 b7e7152..6f71735 100644
--- a/cc/layers/picture_layer.h
+++ b/cc/layers/picture_layer.h
@@ -70,10 +70,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 0ec6ed7..124e5e7 100644
--- a/cc/layers/picture_layer_impl.cc
+++ b/cc/layers/picture_layer_impl.cc
@@ -648,7 +648,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 03316dcd..4cd9a98 100644
--- a/cc/layers/picture_layer_impl.h
+++ b/cc/layers/picture_layer_impl.h
@@ -53,7 +53,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 9d1bbc9..5622cc3 100644
--- a/cc/layers/picture_layer_impl_unittest.cc
+++ b/cc/layers/picture_layer_impl_unittest.cc
@@ -4530,7 +4530,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);
@@ -4549,7 +4550,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 1ce7567..2cdd8a4 100644
--- a/cc/layers/picture_layer_unittest.cc
+++ b/cc/layers/picture_layer_unittest.cc
@@ -46,7 +46,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) {
@@ -203,6 +203,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 d4e6a3e..bfd8fdd 100644
--- a/cc/playback/display_list_recording_source.cc
+++ b/cc/playback/display_list_recording_source.cc
@@ -111,6 +111,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,
@@ -127,6 +135,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,
@@ -144,6 +155,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 4fb2164..543a040 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"
@@ -59,6 +60,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;
@@ -92,6 +95,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);
   }