Make cc::CompositorFrames movable [Part 1 of 2]

content/browser/devtools/protocol/page_handler.cc

Index: content/browser/devtools/protocol/page_handler.cc
diff --git a/content/browser/devtools/protocol/page_handler.cc b/content/browser/devtools/protocol/page_handler.cc
index d472554b9e3ce1f415166559a8743ef171160182..c3d6c98e2e29c8b59bbc54adcb697174a5b4528e 100644
--- a/content/browser/devtools/protocol/page_handler.cc
+++ b/content/browser/devtools/protocol/page_handler.cc
@@ -145,8 +145,8 @@ void PageHandler::Detached() {
 }
 
 void PageHandler::OnSwapCompositorFrame(
-    const cc::CompositorFrameMetadata& frame_metadata) {
-  last_compositor_frame_metadata_ = frame_metadata;
+    cc::CompositorFrameMetadata frame_metadata) {
+  last_compositor_frame_metadata_ = std::move(frame_metadata);
   has_compositor_frame_metadata_ = true;
 
   if (screencast_enabled_)
@@ -155,10 +155,16 @@ void PageHandler::OnSwapCompositorFrame(
 }
 
 void PageHandler::OnSynchronousSwapCompositorFrame(
-    const cc::CompositorFrameMetadata& frame_metadata) {
-  last_compositor_frame_metadata_ = has_compositor_frame_metadata_ ?
-      next_compositor_frame_metadata_ : frame_metadata;
-  next_compositor_frame_metadata_ = frame_metadata;
+    cc::CompositorFrameMetadata frame_metadata) {
+  if (has_compositor_frame_metadata_) {
+    last_compositor_frame_metadata_ =
+        std::move(next_compositor_frame_metadata_);
+    next_compositor_frame_metadata_ = std::move(frame_metadata);
+  } else {
+    last_compositor_frame_metadata_ = std::move(frame_metadata);

[danakj, 2016/06/24 21:45:18]

nit: you could keep the next_cfm part outside the if/else

if (has)
  last = std::move(next);
else
  last = frame_metadata.Clone();
next = std::move(frame_metadata);

[Fady Samuel, 2016/06/24 22:24:37]

Done.

+    next_compositor_frame_metadata_ = last_compositor_frame_metadata_.Clone();
+  }
+
   has_compositor_frame_metadata_ = true;
 
   if (screencast_enabled_)
@@ -425,11 +431,10 @@ void PageHandler::InnerSwapCompositorFrame() {
   if (snapshot_size_dip.width() > 0 && snapshot_size_dip.height() > 0) {
     gfx::Rect viewport_bounds_dip(gfx::ToRoundedSize(viewport_size_dip));
     view->CopyFromCompositingSurface(
-        viewport_bounds_dip,
-        snapshot_size_dip,
+        viewport_bounds_dip, snapshot_size_dip,
         base::Bind(&PageHandler::ScreencastFrameCaptured,
                    weak_factory_.GetWeakPtr(),
-                   last_compositor_frame_metadata_),
+                   last_compositor_frame_metadata_.Clone()),

[danakj, 2016/06/24 21:45:18]

use base::Passed( of Clone()) here

[Fady Samuel, 2016/06/24 22:24:37]

Done.

         kN32_SkColorType);
     frames_in_flight_++;
   }
@@ -451,12 +456,12 @@ void PageHandler::ScreencastFrameCaptured(
     return;
   }
   base::PostTaskAndReplyWithResult(
-      base::WorkerPool::GetTaskRunner(true).get(),
-      FROM_HERE,
-      base::Bind(&EncodeScreencastFrame,
-                 bitmap, screencast_format_, screencast_quality_),
+      base::WorkerPool::GetTaskRunner(true).get(), FROM_HERE,
+      base::Bind(&EncodeScreencastFrame, bitmap, screencast_format_,
+                 screencast_quality_),
       base::Bind(&PageHandler::ScreencastFrameEncoded,
-                 weak_factory_.GetWeakPtr(), metadata, base::Time::Now()));
+                 weak_factory_.GetWeakPtr(), metadata.Clone(),

[danakj, 2016/06/24 21:45:18]

use base::Passed(&metadata) here,

then you avoid this copy, and the copy into the Bind in each place.

[Fady Samuel, 2016/06/24 22:24:37]

Done.

+                 base::Time::Now()));
 }
 
 void PageHandler::ScreencastFrameEncoded(