PPAPI: implement GetSupportedPreviewSizes

ppapi/proxy/image_capture_resource.h

+// Copyright 2014 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 PPAPI_PROXY_IMAGE_CAPTURE_RESOURCE_H_
+#define PPAPI_PROXY_IMAGE_CAPTURE_RESOURCE_H_
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"

[dmichael (off chromium), 2015/02/09 22:27:00]

nit: Not using this header

[Justin Chuang, 2015/02/10 16:01:02]

Done.

+#include "ppapi/c/pp_size.h"
+#include "ppapi/proxy/connection.h"
+#include "ppapi/proxy/plugin_resource.h"
+#include "ppapi/proxy/ppapi_proxy_export.h"
+#include "ppapi/shared_impl/resource.h"
+#include "ppapi/thunk/ppb_image_capture_api.h"
+
+namespace ppapi {
+namespace proxy {
+
+class CameraCapabilitiesResource;
+class ImageCaptureConfigResource;
+
+class PPAPI_PROXY_EXPORT ImageCaptureResource
+    : public PluginResource,
+      public thunk::PPB_ImageCapture_API {
+ public:
+  ImageCaptureResource(Connection connection, PP_Instance instance);
+  virtual ~ImageCaptureResource();

[dmichael (off chromium), 2015/02/09 22:27:00]

This should be override (not marked virtual). Same for
CameraCapabilitiesResource

[Justin Chuang, 2015/02/10 16:01:03]

Done. Thank you! I haven't noticed this rule also applies to virtual destructor.

+
+  // Resource overrides:
+  virtual thunk::PPB_ImageCapture_API* AsPPB_ImageCapture_API() override {

[dmichael (off chromium), 2015/02/09 22:27:00]

We should only mark a function virtual OR override; not both. Here you want
override. Same on the PPB_ImageCapture_API overrides.

[Justin Chuang, 2015/02/10 16:01:02]

Done in this file and the other files.

+    return this;
+  }
+
+  // PPB_ImageCapture_API overrides:
+  virtual int32_t Open(PP_Var device_id,
+                       scoped_refptr<TrackedCallback> callback) override;
+  virtual void Close() override;
+  virtual int32_t GetCameraCapabilities(
+      PP_Resource* capabilities,
+      scoped_refptr<TrackedCallback> callback) override;

[dmichael (off chromium), 2015/02/09 22:27:00]

nit: Maybe slightly prefer const scoped_refptr<>&

[Justin Chuang, 2015/02/10 16:01:03]

Done.

+
+ private:
+  enum OpenState { BEFORE_OPEN, OPENED, CLOSED };
+
+  // Host -> plugin messages.
+  void OnPluginMsgGetPreviewSizesReply(
+      const ResourceMessageReplyParams& params,
+      const std::vector<PP_Size>& preview_sizes);
+
+  // reply to open

[dmichael (off chromium), 2015/02/09 22:27:00]

You could leave out this comment and just put this method right under
OnPluginMsgGetPreviewSizesReply.

[Justin Chuang, 2015/02/10 16:01:03]

Removed the comments and blank line.

+  void OnPluginMsgOpenReply(const ResourceMessageReplyParams& params);
+
+  // other

[dmichael (off chromium), 2015/02/09 22:27:00]

unnecessary comment

[Justin Chuang, 2015/02/10 16:01:03]

Done.

+  bool IsOpened() const { return open_state_ == OPENED; }

[dmichael (off chromium), 2015/02/09 22:27:00]

nit: I'm used to seeing simple inlined functions like this using lowercase with
underscores...
is_opened 

[Justin Chuang, 2015/02/10 16:01:02]

It's true. Done.

+
+  // Holds a reference of the callback so that Close() can cancel it.
+  scoped_refptr<TrackedCallback> open_callback_;
+  OpenState open_state_;
+
+  PP_Resource* get_capabilities_output_;
+  scoped_refptr<TrackedCallback> get_capabilities_callback_;
+  scoped_refptr<CameraCapabilitiesResource> camera_capabilities_;
+
+  DISALLOW_COPY_AND_ASSIGN(ImageCaptureResource);
+};
+
+}  // namespace proxy
+}  // namespace ppapi
+
+#endif  // PPAPI_PROXY_IMAGE_CAPTURE_RESOURCE_H_