Change the cpp wrappers of audio input/video capture to use CompletionCallbackWithOutput.

ppapi/utility/completion_callback_factory.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_UTILITY_COMPLETION_CALLBACK_FACTORY_H_
 #define PPAPI_UTILITY_COMPLETION_CALLBACK_FACTORY_H_
 
 #include "ppapi/cpp/completion_callback.h"
 #include "ppapi/utility/non_thread_safe_ref_count.h"
 
@@ skipping 552 matching lines @@
     ~CallbackData() {
       back_pointer_->Release();
       delete dispatcher_;
     }
 
     Dispatcher* dispatcher() { return dispatcher_; }
 
     static void Thunk(void* user_data, int32_t result) {
       Self* self = static_cast<Self*>(user_data);
       T* object = self->back_pointer_->GetObject();
-      if (object)
-        (*self->dispatcher_)(object, result);
+      (*self->dispatcher_)(object, result);
       delete self;
     }
 
    private:
     typedef CallbackData<Dispatcher> Self;
     BackPointer* back_pointer_;  // We own a ref to this refcounted object.
     Dispatcher* dispatcher_;  // We own this pointer.
 
     // Disallow copying & assignment.
     CallbackData(const CallbackData<Dispatcher>&);
     CallbackData<Dispatcher>& operator=(const CallbackData<Dispatcher>&);
   };
 
   template <typename Method>
   class Dispatcher0 {
    public:
     Dispatcher0() : method_(NULL) {}
     explicit Dispatcher0(Method method) : method_(method) {
     }
     void operator()(T* object, int32_t result) {
-      (object->*method_)(result);
+      if (object)

[brettw, 2012/04/03 21:23:03]

I don't totally get this change, can you clarify why this check is necessary in
a comment?

[yzshen1, 2012/04/04 00:36:34]

Because I change Thunk() to call Dispatcher*::operator() even if |object| is
NULL, so that it can do cleanup if necessary.

I have added comments in Thunk().

On 2012/04/03 21:23:03, brettw wrote:

+        (object->*method_)(result);
     }
    private:
     Method method_;
   };
 
   template <typename Output, typename Method>
   class DispatcherWithOutput0 {
    public:
     typedef Output OutputType;
     typedef internal::CallbackOutputTraits<Output> Traits;
 
-    DispatcherWithOutput0() : method_(NULL) {}
-    DispatcherWithOutput0(Method method) : method_(method) {
+    DispatcherWithOutput0()
+        : method_(NULL),
+          output_() {
+    }
+    DispatcherWithOutput0(Method method)
+        : method_(method),
+          output_() {
     }
     void operator()(T* object, int32_t result) {
-      (object->*method_)(result, Traits::StorageToPluginArg(output_));
+      // We must call Traits::StorageToPluginArg() even if we don't need to call
+      // the callback anymore, otherwise we may leak resource or var references.
+      if (object)
+        (object->*method_)(result, Traits::StorageToPluginArg(output_));
+      else
+        Traits::StorageToPluginArg(output_);
     }
     typename Traits::StorageType* output() {
       return &output_;
     }
    private:
     Method method_;
 
     typename Traits::StorageType output_;
   };
 
   template <typename Method, typename A>
   class Dispatcher1 {
    public:
-    Dispatcher1() : method_(NULL) {}
+    Dispatcher1()
+        : method_(NULL),
+          a_() {

[brettw, 2012/04/03 21:23:03]

Do you initialize these arguments just for completeness? The default constructor
for the dispatchers should only be called for objects that will soon be
replaced.

[yzshen1, 2012/04/04 00:36:34]

Yes. I understand that.

If we want to init |method_| in this case, there is no point to ignore other
parameters. Besides, if A is a class, its constructor will be called even if it
is not put in this list. So I think it makes sense to explicitly list all
members here.

On 2012/04/03 21:23:03, brettw wrote:

+    }
     Dispatcher1(Method method, const A& a)
         : method_(method),
           a_(a) {
     }
     void operator()(T* object, int32_t result) {
-      (object->*method_)(result, a_);
+      if (object)
+        (object->*method_)(result, a_);
     }
    private:
     Method method_;
     A a_;
   };
 
   template <typename Output, typename Method, typename A>
   class DispatcherWithOutput1 {
    public:
     typedef Output OutputType;
     typedef internal::CallbackOutputTraits<Output> Traits;
 
-    DispatcherWithOutput1() : method_(NULL) {}
+    DispatcherWithOutput1()
+        : method_(NULL),
+          a_(),
+          output_() {
+    }
     DispatcherWithOutput1(Method method, const A& a)
         : method_(method),
-          a_(a) {
+          a_(a),
+          output_() {
     }
     void operator()(T* object, int32_t result) {
-      (object->*method_)(result, Traits::StorageToPluginArg(output_), a_);
+      // We must call Traits::StorageToPluginArg() even if we don't need to call
+      // the callback anymore, otherwise we may leak resource or var references.
+      if (object)
+        (object->*method_)(result, Traits::StorageToPluginArg(output_), a_);
+      else
+        Traits::StorageToPluginArg(output_);
     }
     typename Traits::StorageType* output() {
       return &output_;
     }
    private:
     Method method_;
     A a_;
 
     typename Traits::StorageType output_;
   };
 
   template <typename Method, typename A, typename B>
   class Dispatcher2 {
    public:
-    Dispatcher2() : method_(NULL) {}
+    Dispatcher2()
+        : method_(NULL),
+          a_(),
+          b_() {
+    }
     Dispatcher2(Method method, const A& a, const B& b)
         : method_(method),
           a_(a),
           b_(b) {
     }
     void operator()(T* object, int32_t result) {
-      (object->*method_)(result, a_, b_);
+      if (object)
+        (object->*method_)(result, a_, b_);
     }
    private:
     Method method_;
     A a_;
     B b_;
   };
 
   template <typename Output, typename Method, typename A, typename B>
   class DispatcherWithOutput2 {
    public:
     typedef Output OutputType;
     typedef internal::CallbackOutputTraits<Output> Traits;
 
-    DispatcherWithOutput2() : method_(NULL) {}
+    DispatcherWithOutput2()
+        : method_(NULL),
+          a_(),
+          b_(),
+          output_() {
+    }
     DispatcherWithOutput2(Method method, const A& a, const B& b)
         : method_(method),
           a_(a),
-          b_(b) {
+          b_(b),
+          output_() {
     }
     void operator()(T* object, int32_t result) {
-      (object->*method_)(result, Traits::StorageToPluginArg(output_), a_, b_);
+      // We must call Traits::StorageToPluginArg() even if we don't need to call
+      // the callback anymore, otherwise we may leak resource or var references.
+      if (object)
+        (object->*method_)(result, Traits::StorageToPluginArg(output_), a_, b_);
+      else
+        Traits::StorageToPluginArg(output_);
     }
     typename Traits::StorageType* output() {
       return &output_;
     }
    private:
     Method method_;
     A a_;
     B b_;
 
     typename Traits::StorageType output_;
   };
 
   template <typename Method, typename A, typename B, typename C>
   class Dispatcher3 {
    public:
-    Dispatcher3() : method_(NULL) {}
+    Dispatcher3()
+        : method_(NULL),
+          a_(),
+          b_(),
+          c_() {
+    }
     Dispatcher3(Method method, const A& a, const B& b, const C& c)
         : method_(method),
           a_(a),
           b_(b),
           c_(c) {
     }
     void operator()(T* object, int32_t result) {
-      (object->*method_)(result, a_, b_, c_);
+      if (object)
+        (object->*method_)(result, a_, b_, c_);
     }
    private:
     Method method_;
     A a_;
     B b_;
     C c_;
   };
 
   template <typename Output, typename Method, typename A, typename B,
             typename C>
   class DispatcherWithOutput3 {
    public:
     typedef Output OutputType;
     typedef internal::CallbackOutputTraits<Output> Traits;
 
-    DispatcherWithOutput3() : method_(NULL) {}
+    DispatcherWithOutput3()
+        : method_(NULL),
+          a_(),
+          b_(),
+          c_(),
+          output_() {
+    }
     DispatcherWithOutput3(Method method, const A& a, const B& b, const C& c)
         : method_(method),
           a_(a),
           b_(b),
-          c_(c) {
+          c_(c),
+          output_() {
     }
     void operator()(T* object, int32_t result) {
-      (object->*method_)(result, Traits::StorageToPluginArg(output_),
-                         a_, b_, c_);
+      // We must call Traits::StorageToPluginArg() even if we don't need to call
+      // the callback anymore, otherwise we may leak resource or var references.
+      if (object) {
+        (object->*method_)(result, Traits::StorageToPluginArg(output_),
+                           a_, b_, c_);
+      } else {
+        Traits::StorageToPluginArg(output_);
+      }
     }
     typename Traits::StorageType* output() {
       return &output_;
     }
    private:
     Method method_;
     A a_;
     B b_;
     C c_;
 
@@ skipping 38 matching lines @@
   CompletionCallbackFactory(const CompletionCallbackFactory&);
   CompletionCallbackFactory& operator=(const CompletionCallbackFactory&);
 
   T* object_;
   BackPointer* back_pointer_;
 };
 
 }  // namespace pp
 
 #endif  // PPAPI_UTILITY_COMPLETION_CALLBACK_FACTORY_H_