Tab Capture: Backing store readbacks to YV12 VideoFrames.

content/public/browser/render_widget_host.h

Index: content/public/browser/render_widget_host.h
diff --git a/content/public/browser/render_widget_host.h b/content/public/browser/render_widget_host.h
index a0e8d8a70d3754cab16206f905e10bd48e5741dd..a6b2bd336fad663610487fe79c7e5b8dc4f3f880 100644
--- a/content/public/browser/render_widget_host.h
+++ b/content/public/browser/render_widget_host.h
@@ -28,6 +28,10 @@ namespace gfx {
 class Rect;
 }
 
+namespace media {
+class VideoFrame;
+}
+
 namespace content {
 
 class RenderProcessHost;
@@ -168,8 +172,8 @@ class CONTENT_EXPORT RenderWidgetHost : public IPC::Sender {
   // contents, but e.g. in the omnibox.
   virtual void SetActive(bool active) = 0;
 
-  // Copies the given subset of the backing store, and passes the result as a
-  // bitmap to a callback.
+  // Copies the given subset of the backing store, and passes the result as an
+  // ARGB bitmap to a callback.
   //
   // If |src_rect| is empty, the whole contents is copied. If non empty
   // |accelerated_dst_size| is given and accelerated compositing is active, the
@@ -195,6 +199,24 @@ class CONTENT_EXPORT RenderWidgetHost : public IPC::Sender {
   virtual bool CopyFromBackingStoreToCGContext(const CGRect& dest_rect,
                                                CGContextRef target) = 0;
 #endif
+  // Returns true if CopyFromBackingStoreToVideoFrame is possible on the current

[scherkus (not reviewing), 2013/02/05 22:40:44]

nit: add () to function names referenced in comments

[ncarter (slow), 2013/02/06 23:54:44]

Done.

+  // backing store. Depends on platform support and compositor state. Callers
+  // should check this before each call to CopyFromBackingStoreToVideoFrame().

[scherkus (not reviewing), 2013/02/05 22:40:44]

can we promote this "should" into a "must" and have CFBSTVF() crash if you
didn't check?

it appears that the behaviour today is to return an empty frame, which can lead
to problems -- refer to my other comment:
https://codereview.chromium.org/12090109/diff/35001/ui/surface/accelerated_su...

[ncarter (slow), 2013/02/06 23:54:44]

Thanks for sharing your experience (which sounds hard-won) about empty frames
possibly leading to trouble. As mentioned in another comment, I am moving to a
"bool success" param with a caller-allocated VideoFrame, which will eliminate
the need to synthesize empty frames.

Callers have to handle failure invocations of the callback, and the case
discussed here is just one (avoidable) case. Your suggestions about crashing the
browser don't mesh with either my instincts, or the recommendations here:
http://dev.chromium.org/developers/coding-style ["Use CHECK() if the consequence
of a failed assertion would be a security vulnerability, where crashing the
browser is preferable.  Because this takes down the whole browser, sometimes
there are better options than CHECK()."] I think NOTREACHED() is appropriate for
cases like this.

[scherkus (not reviewing), 2013/02/07 18:25:03]

SGTM

+  virtual bool CanCopyToVideoFrame() const = 0;
+  // Copies a given subset of the backing store into a YV12 VideoFrame, and pass

[scherkus (not reviewing), 2013/02/05 22:40:44]

nit: blank line before comments to avoid walloftext

[ncarter (slow), 2013/02/06 23:54:44]

Done.

+  // it to the callback. The resulting VideoFrame will always be exactly
+  // |dst_size|. If |src_rect| does not match |dst_size|, the copied content
+  // will be scaled and letterboxed with black borders. Callbacks are permitted
+  // to take a refcount on the resulting VideoFrame for further processing on
+  // other threads.
+  //
+  // CopyFromBackingStoreToVideoFrame is not always possible. Callers should
+  // check the return value of CanCopyToVideoFrame(), and if false, fall back to
+  // a different method such as CopyFromBackingStore.
+  virtual void CopyFromBackingStoreToVideoFrame(
+      const gfx::Rect& src_rect,
+      const gfx::Size& dst_size,
+      const base::Callback<void(media::VideoFrame*)>& callback) = 0;
 
   // Send a command to the renderer to turn on full accessibility.
   virtual void EnableFullAccessibilityMode() = 0;