Abstract PPAPI's ImageData behind webrtc::DesktopFrame interface

remoting/client/plugin/pepper_desktop_frame.h

Index: remoting/client/plugin/pepper_desktop_frame.h
diff --git a/remoting/client/plugin/pepper_desktop_frame.h b/remoting/client/plugin/pepper_desktop_frame.h
new file mode 100644
index 0000000000000000000000000000000000000000..7859abf8d53598ad893a750a6b13162f90cf57bc
--- /dev/null
+++ b/remoting/client/plugin/pepper_desktop_frame.h
@@ -0,0 +1,31 @@
+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef REMOTING_CLIENT_PLUGIN_PEPPER_IMAGE_BUFFER_H_

[Lambros (do not use), 2013/07/03 21:53:26]

nit: ..._DESKTOP_FRAME_H_

+#define REMOTING_CLIENT_PLUGIN_PEPPER_IMAGE_BUFFER_H_
+
+#include "third_party/webrtc/modules/desktop_capture/desktop_frame.h"
+
+namespace pp {
+class ImageData;
+}  // namespace pp
+
+namespace remoting {
+
+// ImageBuffer that uses PPAPI to allocate space for a raw image.
+class PepperDesktopFrame : public webrtc::DesktopFrame {

[Wez, 2013/07/03 21:46:39]

This class is simple enough that you may as well define it in an anonymous
namespace within pepper_view.cc.

+ public:
+  // Constructor.

[Wez, 2013/07/03 21:46:39]

nit: Please provide a more descriptive comment, e.g. "Wraps the supplied
pp::ImageData."

[Lambros (do not use), 2013/07/03 21:53:26]

nit: I think it's better to remove this comment. It is clearly a ctor, and also
the use of a raw pointer already documents non-ownership of the pointer, so I'm
not sure there's anything left to usefully say in a comment here. :)

+  PepperDesktopFrame(pp::ImageData* buffer);

[Lambros (do not use), 2013/07/03 21:53:26]

nit: single-arg ctor should be marked explicit.

+
+  // Access to underlying pepper representation.
+  pp::ImageData& data_object();

[Lambros (do not use), 2013/07/03 21:53:26]

nit: Plain getter for member |foo_| should be named get_foo(), and should be
marked const (which is OK here, the pointer itself is not mutated). Optionally,
you can inline the definition here if you like.

Also, we tend to prefer pointers over non-const references in Chrome code.

+
+ private:
+  pp::ImageData* buffer_;

[Wez, 2013/07/03 21:56:52]

This should be pp::ImageData, not pp::ImageData* since you want the underlying
resource to stay alive while this DesktopFrame exists; pp::ImageData is itself a
reference to a ref-counted resource.

+};
+
+}  // namespace remoting
+
+#endif