cc: Make LayerTreeHostImpl unittests use a delegating output surface.

cc/test/fake_output_surface.cc

Index: cc/test/fake_output_surface.cc
diff --git a/cc/test/fake_output_surface.cc b/cc/test/fake_output_surface.cc
index 9a10249e1f6178f3ba0f369f3f5d6cdc15c2b643..101bbf03463d6908f3b7bb7d7c9dd246f1288d76 100644
--- a/cc/test/fake_output_surface.cc
+++ b/cc/test/fake_output_surface.cc
@@ -46,24 +46,28 @@ FakeOutputSurface::~FakeOutputSurface() = default;
 void FakeOutputSurface::SwapBuffers(CompositorFrame frame) {
   ReturnResourcesHeldByParent();
 
-  std::unique_ptr<CompositorFrame> frame_copy(new CompositorFrame);
-  *frame_copy = std::move(frame);
-  if (frame_copy->delegated_frame_data || !context_provider()) {
-    last_sent_frame_ = std::move(frame_copy);
+  last_sent_frame_ = base::MakeUnique<CompositorFrame>(std::move(frame));

[vmpstr, 2016/07/19 23:27:01]

nit: maybe just

last_sent_frame_.reset(new CompositorFrame(std::move(frame)));

Also, is this a unique_ptr because it's playing a role of optional?

[danakj, 2016/07/20 01:03:25]

Sure.
Ya.

+  ++num_sent_frames_;
+
+  if (last_sent_frame_->delegated_frame_data) {
+    auto* frame_data = last_sent_frame_->delegated_frame_data.get();
+    last_swap_rect_ = frame_data->render_pass_list.back()->damage_rect;
+  } else if (!context_provider()) {
+    // Unknown for software frames.
+    last_swap_rect_ = gfx::Rect();

[enne (OOO), 2016/07/19 23:00:50]

This is really empty and not the full rect?

[danakj, 2016/07/20 01:03:25]

Software can partial swap, but it does it thru the software device not thru the
output surface. It only swaps metadata. This is kinda a mess but I'd rather set
it empty than leave it set to whatever it used to be. I could DCHECK that you
don't try to query for software, and I've added that.

+  } else {
+    last_swap_rect_ = last_sent_frame_->gl_frame_data->sub_buffer_rect;

[vmpstr, 2016/07/19 23:27:02]

tiny nit: can you swap else and the last else if?

I think this reads better:

if (we have delegated frame data)
  ...
else if (we have a context)
  ...
else
  ...

[danakj, 2016/07/20 01:03:25]

Done.

+  }
 
+  if (last_sent_frame_->delegated_frame_data || !context_provider()) {
     if (last_sent_frame_->delegated_frame_data) {

[vmpstr, 2016/07/19 23:27:02]

isn't 62-63 kind of corresponds to the if on line 52? Or can software frames
also have these resources held by parent?

[danakj, 2016/07/20 01:03:25]

Yes, delegated frames can be hardware or software.

-      resources_held_by_parent_.insert(
-          resources_held_by_parent_.end(),
-          last_sent_frame_->delegated_frame_data->resource_list.begin(),
-          last_sent_frame_->delegated_frame_data->resource_list.end());
+      auto* frame_data = last_sent_frame_->delegated_frame_data.get();
+      resources_held_by_parent_.insert(resources_held_by_parent_.end(),
+                                       frame_data->resource_list.begin(),
+                                       frame_data->resource_list.end());
     }
-
-    ++num_sent_frames_;
-  } else {
-    last_swap_rect_ = frame_copy->gl_frame_data->sub_buffer_rect;
-    last_sent_frame_ = std::move(frame_copy);
-    ++num_sent_frames_;
   }
+
   PostSwapBuffersComplete();
 }