Refactor video capture to new design

ppapi/shared_impl/ppb_video_capture_shared.h

 // Copyright (c) 2012 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_SHARED_IMPL_PPB_VIDEO_CAPTURE_SHARED_H_
 #define PPAPI_SHARED_IMPL_PPB_VIDEO_CAPTURE_SHARED_H_
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "base/memory/ref_counted.h"
+#include "ppapi/c/dev/pp_video_capture_dev.h"
 #include "ppapi/shared_impl/ppapi_shared_export.h"
-#include "ppapi/shared_impl/resource.h"
-#include "ppapi/shared_impl/tracked_callback.h"
-#include "ppapi/thunk/ppb_video_capture_api.h"
 
 namespace ppapi {
 
-class PPAPI_SHARED_EXPORT PPB_VideoCapture_Shared
-    : public Resource,
-      NON_EXPORTED_BASE(public thunk::PPB_VideoCapture_API) {
+class PPAPI_SHARED_EXPORT PPB_VideoCapture_Shared {

[yzshen1, 2012/10/25 20:41:05]

I think this class is not very useful now.
All it does is checking the open_state_. (And SetStatus() is only used by the
host side.)
Does it make sense to remove this class and verify the open state at only one
side?

[victorhsieh, 2012/10/26 02:34:54]

The resource still uses open_state_ indirectly.  For example, in shared
StartCapture, it checks the status for both sides, and resource can return
failure immediately when happens.  We might still need the state machine on
resource side, since many of the API definitions say it will return error in
some state?

If we can't change this behavior, I can try to refactor this class.

[yzshen1, 2012/10/29 18:34:23]

The reason why we needed the state machine on both sides was to maintain the
same behavior for in-process and out-of-process mode.
Now that we only have out-of-process mode (in-process mode fakes an IPC channel
and runs the same code), we can have this state machine only at the resource
side, right?

I think that would make the code cleaner.

(Of course, we need to make sure the host side should still work safely without
this state machine.)


On 2012/10/26 02:34:54, Victor Hsieh wrote:

[victorhsieh, 2012/10/30 09:43:28]

Done.  Removed everything of 0.1 from IDL, api, thunk, etc.

On 2012/10/29 18:34:23, yzshen1 wrote:

[victorhsieh, 2012/11/02 04:28:47]

Oops, I guess I missed this comment.  The code is done.

So in the original code, there are 2 states: open_states_ and status_. 
open_states_ are moved to resource for opening state tracking.  But the host
should of course handle mis-state request properly.

status_ is moved to the host, since it's updated passively by SetStatus on
certain media events from browser, and the report to plugin resource.  Chance is
that we might event remove status_ for state transition check? But that's
another story.

Please see if it make sense.

On 2012/10/29 18:34:23, yzshen1 wrote:

  public:
-  explicit PPB_VideoCapture_Shared(PP_Instance instance);
-  explicit PPB_VideoCapture_Shared(const HostResource& host_resource);
+  PPB_VideoCapture_Shared();
   virtual ~PPB_VideoCapture_Shared();
 
-  // Resource implementation.
-  virtual thunk::PPB_VideoCapture_API* AsPPB_VideoCapture_API() OVERRIDE;
-
-  // PPB_VideoCapture_API implementation.
-  virtual int32_t EnumerateDevices(
-      PP_Resource* devices,
-      scoped_refptr<TrackedCallback> callback) OVERRIDE;
-  virtual int32_t Open(const std::string& device_id,
-                       const PP_VideoCaptureDeviceInfo_Dev& requested_info,
-                       uint32_t buffer_count,
-                       scoped_refptr<TrackedCallback> callback) OVERRIDE;
   virtual int32_t StartCapture() OVERRIDE;
-  virtual int32_t ReuseBuffer(uint32_t buffer) OVERRIDE;
   virtual int32_t StopCapture() OVERRIDE;
-  virtual void Close() OVERRIDE;
   virtual int32_t StartCapture0_1(
       const PP_VideoCaptureDeviceInfo_Dev& requested_info,
       uint32_t buffer_count) OVERRIDE;
-  virtual const std::vector<DeviceRefData>& GetDeviceRefData() const OVERRIDE;
-
-  void OnEnumerateDevicesComplete(int32_t result,
-                                  const std::vector<DeviceRefData>& devices);
-  void OnOpenComplete(int32_t result);
 
  protected:
   enum OpenState {
     BEFORE_OPEN,
     OPENED,
     CLOSED
   };
 
   // Subclasses should implement these methods to do impl- and proxy-specific
   // work.
-  virtual int32_t InternalEnumerateDevices(
-      PP_Resource* devices,
-      scoped_refptr<TrackedCallback> callback) = 0;
-  virtual int32_t InternalOpen(
-      const std::string& device_id,
-      const PP_VideoCaptureDeviceInfo_Dev& requested_info,
-      uint32_t buffer_count,
-      scoped_refptr<TrackedCallback> callback) = 0;
   virtual int32_t InternalStartCapture() = 0;
-  virtual int32_t InternalReuseBuffer(uint32_t buffer) = 0;
   virtual int32_t InternalStopCapture() = 0;
-  virtual void InternalClose() = 0;
   virtual int32_t InternalStartCapture0_1(
       const PP_VideoCaptureDeviceInfo_Dev& requested_info,
       uint32_t buffer_count) = 0;
-  virtual const std::vector<DeviceRefData>& InternalGetDeviceRefData(
-      ) const = 0;
 
   // Checks whether |status| is expected and sets |status_| if yes. If |forced|
   // is set to true, this method will bypass sanity check and always set
   // |status_|.
   bool SetStatus(PP_VideoCaptureStatus_Dev status, bool forced);
 
   OpenState open_state_;
   PP_VideoCaptureStatus_Dev status_;
 
-  scoped_refptr<TrackedCallback> enumerate_devices_callback_;
-  scoped_refptr<TrackedCallback> open_callback_;
-
-  // Output parameter of EnumerateDevices(). It should not be accessed after
-  // |enumerate_devices_callback_| is run.
-  PP_Resource* devices_;
-
-  ResourceObjectType resource_object_type_;
-
  private:
   DISALLOW_COPY_AND_ASSIGN(PPB_VideoCapture_Shared);
 };
 
 }  // namespace ppapi
 
 #endif  // PPAPI_SHARED_IMPL_PPB_VIDEO_CAPTURE_SHARED_H_