Clean up Pepper ImageData resource class.

ppapi/proxy/ppb_image_data_proxy.h

Index: ppapi/proxy/ppb_image_data_proxy.h
diff --git a/ppapi/proxy/ppb_image_data_proxy.h b/ppapi/proxy/ppb_image_data_proxy.h
index 2f2cd60a6831dc596e448454509b3a64e80b41cb..d7a6bd28e293957945c51e987b7f12b80bd7c0a2 100644
--- a/ppapi/proxy/ppb_image_data_proxy.h
+++ b/ppapi/proxy/ppb_image_data_proxy.h
@@ -31,26 +31,14 @@ namespace proxy {
 
 class SerializedHandle;
 
-// The proxied image data resource. Unlike most resources, this needs to be
-// public in the header since a number of other resources need to access it.
+// ImageData is an abstract base class for image data resources. Unlike most
+// resources, ImageData must be public in the header since a number of other
+// resources need to access it.
 class PPAPI_PROXY_EXPORT ImageData
     : public ppapi::Resource,
       public NON_EXPORTED_BASE(ppapi::thunk::PPB_ImageData_API),
       public ppapi::PPB_ImageData_Shared {
  public:
-#if !defined(OS_NACL)
-  ImageData(const ppapi::HostResource& resource,
-            const PP_ImageDataDesc& desc,
-            ImageHandle handle);
-#else
-  // In NaCl, we only allow creating an ImageData using a SharedMemoryHandle.
-  // ImageHandle can differ by host platform. We need something that is
-  // more consistent across platforms for NaCl, so that we can communicate to
-  // the host OS in a consistent way.
-  ImageData(const ppapi::HostResource& resource,
-            const PP_ImageDataDesc& desc,
-            const base::SharedMemoryHandle& handle);
-#endif
   virtual ~ImageData();
 
   // Resource overrides.
@@ -60,42 +48,79 @@ class PPAPI_PROXY_EXPORT ImageData
 
   // PPB_ImageData API.
   virtual PP_Bool Describe(PP_ImageDataDesc* desc) OVERRIDE;
-  virtual void* Map() OVERRIDE;
-  virtual void Unmap() OVERRIDE;
   virtual int32_t GetSharedMemory(int* handle, uint32_t* byte_count) OVERRIDE;
-  virtual SkCanvas* GetPlatformCanvas() OVERRIDE;
-  virtual SkCanvas* GetCanvas() OVERRIDE;
   virtual void SetIsCandidateForReuse() OVERRIDE;
 
   const PP_ImageDataDesc& desc() const { return desc_; }
 
-  // Prepares this image data to be recycled to the plugin. The contents will be
-  // cleared if zero_contents is set.
+  // Prepares this image data to be recycled to the plugin. Clears the contents
+  // if zero_contents is true.
   void RecycleToPlugin(bool zero_contents);
 
+ protected:
+  ImageData(const ppapi::HostResource& resource,
+            const PP_ImageDataDesc& desc);
+
+  PP_ImageDataDesc desc_;
+
+  // Set to true when this ImageData is a good candidate for reuse.
+  bool is_candidate_for_reuse_;
+
+  DISALLOW_COPY_AND_ASSIGN(ImageData);
+};
+
+// PlatformImageData is a full featured image data resource which can access
+// the underlying platform-specific canvas and ImageHandle. This can't be used
+// by NaCl apps.
 #if !defined(OS_NACL)
+class PPAPI_PROXY_EXPORT PlatformImageData : public ImageData {
+ public:
+  PlatformImageData(const ppapi::HostResource& resource,
+                    const PP_ImageDataDesc& desc,
+                    ImageHandle handle);
+  virtual ~PlatformImageData();
+
+  // PPB_ImageData API.
+  virtual void* Map() OVERRIDE;
+  virtual void Unmap() OVERRIDE;
+  virtual SkCanvas* GetPlatformCanvas() OVERRIDE;
+  virtual SkCanvas* GetCanvas() OVERRIDE;
+
   static ImageHandle NullHandle();
   static ImageHandle HandleFromInt(int32_t i);
-#endif
 
  private:
-  PP_ImageDataDesc desc_;
-
-#if defined(OS_NACL)
-  base::SharedMemory shm_;
-  uint32 size_;
-  int map_count_;
-#else
   scoped_ptr<TransportDIB> transport_dib_;
 
   // Null when the image isn't mapped.
   scoped_ptr<SkCanvas> mapped_canvas_;
-#endif
 
-  // Set to true when this ImageData is a good candidate for reuse.
-  bool is_candidate_for_reuse_;
+  DISALLOW_COPY_AND_ASSIGN(PlatformImageData);
+};
+#endif  // !defined(OS_NACL)
 
-  DISALLOW_COPY_AND_ASSIGN(ImageData);
+// SimpleImageData is a simple, platform independent image data resource which

[dmichael (off chromium), 2013/06/10 18:07:29]

teeny nit: I would hyphenate platform-independent.

[bbudge, 2013/06/10 23:28:37]

Done.

+// can be used by NaCl. It can also be used by trusted apps when access to the
+// platform canvas isn't needed.
+class PPAPI_PROXY_EXPORT SimpleImageData : public ImageData {
+ public:
+  SimpleImageData(const ppapi::HostResource& resource,
+                  const PP_ImageDataDesc& desc,
+                  const base::SharedMemoryHandle& handle);
+  virtual ~SimpleImageData();
+
+  // PPB_ImageData API.
+  virtual void* Map() OVERRIDE;
+  virtual void Unmap() OVERRIDE;
+  virtual SkCanvas* GetPlatformCanvas() OVERRIDE;
+  virtual SkCanvas* GetCanvas() OVERRIDE;
+
+ private:
+  base::SharedMemory shm_;
+  uint32 size_;
+  int map_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(SimpleImageData);
 };
 
 class PPB_ImageData_Proxy : public InterfaceProxy {
@@ -106,7 +131,8 @@ class PPB_ImageData_Proxy : public InterfaceProxy {
   static PP_Resource CreateProxyResource(PP_Instance instance,
                                          PP_ImageDataFormat format,
                                          const PP_Size& size,
-                                         PP_Bool init_to_zero);
+                                         PP_Bool init_to_zero,
+                                         PP_Bool platform);
 
   // InterfaceProxy implementation.
   virtual bool OnMessageReceived(const IPC::Message& msg);
@@ -115,6 +141,8 @@ class PPB_ImageData_Proxy : public InterfaceProxy {
   // This can only be called on the host side of the proxy.
   // On failure, will return invalid resource (0). On success it will return a
   // valid resource and the out params will be written.
+  // |platform| determines whether a PlatformImageData or SimpleImageData is
+  // created.
   // |desc| contains the result of Describe.
   // |image_handle| and |byte_count| contain the result of GetSharedMemory.
   // NOTE: if |init_to_zero| is false, you should write over the entire image
@@ -124,7 +152,7 @@ class PPB_ImageData_Proxy : public InterfaceProxy {
       PP_ImageDataFormat format,
       const PP_Size& size,
       bool init_to_zero,
-      bool is_nacl_plugin,
+      bool platform,

[dmichael (off chromium), 2013/06/10 18:07:29]

suggestion: you could use an enum for this, to make call sites more
self-documenting.

[bbudge, 2013/06/10 23:28:37]

Done. Moved the one in PPB_ImageData_Impl to PPB_ImageData_Shared.

       PP_ImageDataDesc* desc,
       IPC::PlatformFileForTransit* image_handle,
       uint32_t* byte_count);
@@ -133,20 +161,22 @@ class PPB_ImageData_Proxy : public InterfaceProxy {
 
  private:
   // Plugin->Host message handlers.
-  void OnHostMsgCreate(PP_Instance instance,
-                       int32_t format,
-                       const PP_Size& size,
-                       PP_Bool init_to_zero,
-                       HostResource* result,
-                       std::string* image_data_desc,
-                       ImageHandle* result_image_handle);
-  void OnHostMsgCreateNaCl(PP_Instance instance,
-                           int32_t format,
-                           const PP_Size& size,
-                           PP_Bool init_to_zero,
-                           HostResource* result,
-                           std::string* image_data_desc,
-                           ppapi::proxy::SerializedHandle* result_image_handle);
+  void OnHostMsgCreatePlatform(
+      PP_Instance instance,
+      int32_t format,
+      const PP_Size& size,
+      PP_Bool init_to_zero,
+      HostResource* result,
+      PP_ImageDataDesc* desc,
+      ImageHandle* result_image_handle);
+  void OnHostMsgCreateSimple(
+      PP_Instance instance,
+      int32_t format,
+      const PP_Size& size,
+      PP_Bool init_to_zero,
+      HostResource* result,
+      PP_ImageDataDesc* desc,
+      ppapi::proxy::SerializedHandle* result_image_handle);
 
   // Host->Plugin message handlers.
   void OnPluginMsgNotifyUnusedImageData(const HostResource& old_image_data);