cc: Clean up RecordingSource API

cc/playback/recording_source.cc

Index: cc/playback/recording_source.cc
diff --git a/cc/playback/recording_source.cc b/cc/playback/recording_source.cc
index 9806e9ed380d751bb028abbae86074809da8d615..cdeef725f1092804ae54819fd90049fb686ebf57 100644
--- a/cc/playback/recording_source.cc
+++ b/cc/playback/recording_source.cc
@@ -29,6 +29,10 @@ const bool kDefaultClearCanvasSetting = true;
 
 namespace cc {
 
+RecordingSource::Inputs::Inputs() = default;
+
+RecordingSource::Inputs::~Inputs() = default;
+
 RecordingSource::RecordingSource()
     : slow_down_raster_scale_factor_for_debug_(0),
       generate_discardable_images_metadata_(false),
@@ -36,13 +40,12 @@ RecordingSource::RecordingSource()
       is_solid_color_(false),
       clear_canvas_with_debug_color_(kDefaultClearCanvasSetting),
       solid_color_(SK_ColorTRANSPARENT),
-      background_color_(SK_ColorTRANSPARENT),
-      painter_reported_memory_usage_(0) {}
+      background_color_(SK_ColorTRANSPARENT) {}
 
 RecordingSource::~RecordingSource() {}
 
 void RecordingSource::ToProtobuf(proto::RecordingSource* proto) const {
-  RectToProto(recorded_viewport_, proto->mutable_recorded_viewport());
+  RectToProto(inputs_.recorded_viewport, proto->mutable_recorded_viewport());
   SizeToProto(size_, proto->mutable_size());
   proto->set_slow_down_raster_scale_factor_for_debug(
       slow_down_raster_scale_factor_for_debug_);
@@ -53,8 +56,8 @@ void RecordingSource::ToProtobuf(proto::RecordingSource* proto) const {
   proto->set_clear_canvas_with_debug_color(clear_canvas_with_debug_color_);
   proto->set_solid_color(static_cast<uint64_t>(solid_color_));
   proto->set_background_color(static_cast<uint64_t>(background_color_));
-  if (display_list_)
-    display_list_->ToProtobuf(proto->mutable_display_list());
+  if (inputs_.display_list)
+    inputs_.display_list->ToProtobuf(proto->mutable_display_list());
 }
 
 void RecordingSource::FromProtobuf(
@@ -62,7 +65,7 @@ void RecordingSource::FromProtobuf(
     ClientPictureCache* client_picture_cache,
     std::vector<uint32_t>* used_engine_picture_ids) {
   DCHECK(client_picture_cache);
-  recorded_viewport_ = ProtoToRect(proto.recorded_viewport());
+  inputs_.recorded_viewport = ProtoToRect(proto.recorded_viewport());
   size_ = ProtoToSize(proto.size());
   slow_down_raster_scale_factor_for_debug_ =
       proto.slow_down_raster_scale_factor_for_debug();
@@ -74,15 +77,15 @@ void RecordingSource::FromProtobuf(
   solid_color_ = static_cast<SkColor>(proto.solid_color());
   background_color_ = static_cast<SkColor>(proto.background_color());
 
-  // This might not exist if the |display_list_| of the serialized
+  // This might not exist if the |inputs_.display_list| of the serialized
   // RecordingSource was null, wich can happen if |Clear()| is
   // called.
   if (proto.has_display_list()) {
-    display_list_ = DisplayItemList::CreateFromProto(
+    inputs_.display_list = DisplayItemList::CreateFromProto(
         proto.display_list(), client_picture_cache, used_engine_picture_ids);
     FinishDisplayItemListUpdate();
   } else {
-    display_list_ = nullptr;
+    inputs_.display_list = nullptr;
   }
 }
 
@@ -103,9 +106,9 @@ void RecordingSource::UpdateInvalidationForNewViewport(
 void RecordingSource::FinishDisplayItemListUpdate() {
   TRACE_EVENT0("cc", "RecordingSource::FinishDisplayItemListUpdate");
   DetermineIfSolidColor();
-  display_list_->EmitTraceSnapshot();
+  inputs_.display_list->EmitTraceSnapshot();
   if (generate_discardable_images_metadata_)
-    display_list_->GenerateDiscardableImagesMetadata();
+    inputs_.display_list->GenerateDiscardableImagesMetadata();
 }
 
 void RecordingSource::SetNeedsDisplayRect(const gfx::Rect& layer_rect) {
@@ -130,14 +133,14 @@ bool RecordingSource::UpdateAndExpandInvalidation(
   invalidation_.Clear();
 
   gfx::Rect new_recorded_viewport = painter->PaintableRegion();
-  if (new_recorded_viewport != recorded_viewport_) {
-    UpdateInvalidationForNewViewport(recorded_viewport_, new_recorded_viewport,
-                                     invalidation);
-    recorded_viewport_ = new_recorded_viewport;
+  if (new_recorded_viewport != inputs_.recorded_viewport) {
+    UpdateInvalidationForNewViewport(inputs_.recorded_viewport,
+                                     new_recorded_viewport, invalidation);
+    inputs_.recorded_viewport = new_recorded_viewport;

[danakj, 2016/07/13 20:05:34]

I don't understand the reason behind this CL, can you explain why you're putting
things into a struct in this class?

For eg recorded_viewport is moving to the struct, but size_ isn't? They are both
set in this method.

This class isn't even exposed to blink, it goes through picture layer right?

[Khushal, 2016/07/13 20:16:50]

We want to separate the data PictureLayer gets from ContentLayerClient, which it
pulls by going through the recording source. The size comes from the
PictureLayer itself.

Is there a better way to create that distinction?

[danakj, 2016/07/13 20:19:33]

Why? They both come from blink.

[Khushal, 2016/07/13 20:55:16]

On the client side we will mostly be replicating the interaction that cc had
with blink on the service side. So we will implement a ContentLayerClient and
when the recording source asks it to paint, we will return the same display list
and recorded viewport.

The size of the layer here comes from the layer during the update layers step
after it has been received from blink. And we will set all the layer properties
when LTH tells its client to UpdateLayerTreeHost.

[danakj, 2016/07/14 20:15:17]

I find it really strange that this function is setting things in inputs_ and
things not in inputs_ regardless.
I think we can change this a bit more fundamentally, and discussed with vmpstr
about it. We suggest that the inputs_ here should be members on PictureLayerImpl
instead, and move this UpdateAndExpandInvalidation over there.

Then just create the RecordingSource at the time of commit, instead of having it
as a member of PictureLayer. vmpstr believes we could/should work toward just
eliminate the RecordingSource as a class entirely as followup, and I could
believe that would be best.

     updated = true;
   }
 
-  if (!updated && !invalidation->Intersects(recorded_viewport_))
+  if (!updated && !invalidation->Intersects(inputs_.recorded_viewport))
     return false;
 
   if (invalidation->IsEmpty())
@@ -169,8 +172,9 @@ bool RecordingSource::UpdateAndExpandInvalidation(
 
   // TODO(vmpstr): Add a slow_down_recording_scale_factor_for_debug_ to be able
   // to slow down recording.
-  display_list_ = painter->PaintContentsToDisplayList(painting_control);
-  painter_reported_memory_usage_ = painter->GetApproximateUnsharedMemoryUsage();
+  inputs_.display_list = painter->PaintContentsToDisplayList(painting_control);
+  inputs_.painter_reported_memory_usage =
+      painter->GetApproximateUnsharedMemoryUsage();
 
   FinishDisplayItemListUpdate();
 
@@ -208,11 +212,12 @@ bool RecordingSource::IsSuitableForGpuRasterization() const {
   // before checking for suitability. There are cases where an update will not
   // create a display list (e.g., if the size is empty). We return true in these
   // cases because the gpu suitability bit sticks false.
-  return !display_list_ || display_list_->IsSuitableForGpuRasterization();
+  return !inputs_.display_list ||
+         inputs_.display_list->IsSuitableForGpuRasterization();
 }
 
 const DisplayItemList* RecordingSource::GetDisplayItemList() {
-  return display_list_.get();
+  return inputs_.display_list.get();
 }
 
 scoped_refptr<RasterSource> RecordingSource::CreateRasterSource(
@@ -222,25 +227,25 @@ scoped_refptr<RasterSource> RecordingSource::CreateRasterSource(
 }
 
 void RecordingSource::DetermineIfSolidColor() {
-  DCHECK(display_list_);
+  DCHECK(inputs_.display_list);
   is_solid_color_ = false;
   solid_color_ = SK_ColorTRANSPARENT;
 
-  if (!display_list_->ShouldBeAnalyzedForSolidColor())
+  if (!inputs_.display_list->ShouldBeAnalyzedForSolidColor())
     return;
 
   TRACE_EVENT1("cc", "RecordingSource::DetermineIfSolidColor", "opcount",
-               display_list_->ApproximateOpCount());
+               inputs_.display_list->ApproximateOpCount());
   gfx::Size layer_size = GetSize();
   skia::AnalysisCanvas canvas(layer_size.width(), layer_size.height());
-  display_list_->Raster(&canvas, nullptr, gfx::Rect(), 1.f);
+  inputs_.display_list->Raster(&canvas, nullptr, gfx::Rect(), 1.f);
   is_solid_color_ = canvas.GetColorIfSolid(&solid_color_);
 }
 
 void RecordingSource::Clear() {
-  recorded_viewport_ = gfx::Rect();
-  display_list_ = nullptr;
-  painter_reported_memory_usage_ = 0;
+  inputs_.recorded_viewport = gfx::Rect();
+  inputs_.display_list = nullptr;
+  inputs_.painter_reported_memory_usage = 0;
   is_solid_color_ = false;
 }