Add support for threadsafe completion callback factory.

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"
+#include "ppapi/utility/completion_callback_factory_thread_traits.h"
 
 /// @file
 /// This file defines the API to create CompletionCallback objects that are
 /// bound to member functions.
 namespace pp {
 
 // TypeUnwrapper --------------------------------------------------------------
 
 namespace internal {
 
@@ skipping 16 matching lines @@
 
 /// CompletionCallbackFactory<T> may be used to create CompletionCallback
 /// objects that are bound to member functions.
 ///
 /// If a factory is destroyed, then any pending callbacks will be cancelled
 /// preventing any bound member functions from being called.  The CancelAll()
 /// method allows pending callbacks to be cancelled without destroying the
 /// factory.
 ///
 /// <strong>Note: </strong><code>CompletionCallbackFactory<T></code> isn't
-/// thread safe, but you can make it more thread-friendly by passing a
-/// thread-safe refcounting class as the second template element. However, it
+/// thread safe, but it is somewhat thread-friendly when used with a
+/// thread-safe traits class as the second template element. However, it
 /// only guarantees safety for creating a callback from another thread, the
 /// callback itself needs to execute on the same thread as the thread that
 /// creates/destroys the factory. With this restriction, it is safe to create
 /// the <code>CompletionCallbackFactory</code> on the main thread, create
 /// callbacks from any thread and pass them to CallOnMainThread().
 ///
 /// <strong>Example: </strong>
 ///
 /// @code
 ///   class MyClass {
@@ skipping 120 matching lines @@
 /// As with regular completion callbacks, you can optionally add up to three
 /// bound arguments. These are passed following the output argument.
 ///
 /// Your callback may take the output argument as a copy (common for small
 /// types like integers, a const reference (common for structures and
 /// resources to avoid an extra copy), or as a non-const reference. One
 /// optimization you can do if your callback function may take large arrays
 /// is to accept your output argument as a non-const reference and to swap()
 /// the argument with a vector of your own to store it. This means you don't
 /// have to copy the buffer to consume it.
-template <typename T, typename RefCount = NonThreadSafeRefCount>
+template <typename T, typename ThreadTraits = ThreadSafeThreadTraits>
 class CompletionCallbackFactory {
  public:
 
   /// This constructor creates a <code>CompletionCallbackFactory</code>
   /// bound to an object. If the constructor is called without an argument,
   /// the default value of <code>NULL</code> is used. The user then must call
   /// Initialize() to initialize the object.
   ///
   /// param[in] object Optional parameter. An object whose member functions
   /// are to be bound to CompletionCallbacks created by this
   /// <code>CompletionCallbackFactory</code>. The default value of this
   /// parameter is <code>NULL</code>.
   explicit CompletionCallbackFactory(T* object = NULL)
       : object_(object) {
+    // Assume that we don't need to lock since construction should be complete
+    // before the pointer is used on another thread.
     InitBackPointer();
   }
 
   /// Destructor.
   ~CompletionCallbackFactory() {
+    // Assume that we don't need to lock since this object should not be used
+    // from multiple threads during destruction.
     ResetBackPointer();
   }
 
   /// CancelAll() cancels all <code>CompletionCallbacks</code> allocated from
   /// this factory.
   void CancelAll() {
+    ThreadTraits::AutoLock lock(lock_);
+
     ResetBackPointer();
     InitBackPointer();
   }
+
   /// Initialize() binds the <code>CallbackFactory</code> to a particular
   /// object. Use this when the object is not available at
   /// <code>CallbackFactory</code> creation, and the <code>NULL</code> default
   /// is passed to the constructor. The object may only be initialized once,
   /// either by the constructor, or by a call to Initialize().
   ///
   /// @param[in] object The object whose member functions are to be bound to
   /// the <code>CompletionCallback</code> created by this
   /// <code>CompletionCallbackFactory</code>.
   void Initialize(T* object) {
     PP_DCHECK(object);

[viettrungluu, 2012/07/10 22:47:58]

Maybe you should have a comment about the non-thread-safety of Initialize()
(i.e., that you shouldn't even think about using the factory until it's been
initialized)?

     PP_DCHECK(!object_);  // May only initialize once!
     object_ = object;
   }
 
   /// GetObject() returns the object that was passed at initialization to
   /// Intialize().
   ///
   /// @return the object passed to the constructor or Intialize().
   T* GetObject() {
     return object_;
   }
 
   /// NewCallback allocates a new, single-use <code>CompletionCallback</code>.
   /// The <code>CompletionCallback</code> must be run in order for the memory
   /// allocated by the methods to be freed.
   ///
   /// @param[in] method The method to be invoked upon completion of the
   /// operation.
   ///
   /// @return A <code>CompletionCallback</code>.
   template <typename Method>
   CompletionCallback NewCallback(Method method) {
     PP_DCHECK(object_);

[viettrungluu, 2012/07/10 22:47:58]

While you're here, I believe this dcheck is wholly redundant.

     return NewCallbackHelper(new Dispatcher0<Method>(method));
   }
 
   /// NewOptionalCallback() allocates a new, single-use
   /// <code>CompletionCallback</code> that might not run if the method
   /// taking it can complete synchronously. Thus, if after passing the
   /// CompletionCallback to a Pepper method, the method does not return
   /// PP_OK_COMPLETIONPENDING, then you should manually call the
   /// CompletionCallback's Run method, or memory will be leaked.
   ///
@@ skipping 34 matching lines @@
   /// @param[in] method The method to be invoked upon completion of the
   /// operation. Method should be of type:
   /// <code>void (T::*)(int32_t result, const A& a)</code>
   ///
   /// @param[in] a Passed to <code>method</code> when the completion callback
   /// runs.
   ///
   /// @return A <code>CompletionCallback</code>.
   template <typename Method, typename A>
   CompletionCallback NewCallback(Method method, const A& a) {
     PP_DCHECK(object_);

[viettrungluu, 2012/07/10 22:47:58]

"

     return NewCallbackHelper(new Dispatcher1<Method, A>(method, a));
   }
 
   /// NewOptionalCallback() allocates a new, single-use
   /// <code>CompletionCallback</code> that might not run if the method
   /// taking it can complete synchronously. Thus, if after passing the
   /// CompletionCallback to a Pepper method, the method does not return
   /// PP_OK_COMPLETIONPENDING, then you should manually call the
   /// CompletionCallback's Run method, or memory will be leaked.
   ///
@@ skipping 46 matching lines @@
   ///
   /// @param[in] a Passed to <code>method</code> when the completion callback
   /// runs.
   ///
   /// @param[in] b Passed to <code>method</code> when the completion callback
   /// runs.
   ///
   /// @return A <code>CompletionCallback</code>.
   template <typename Method, typename A, typename B>
   CompletionCallback NewCallback(Method method, const A& a, const B& b) {
     PP_DCHECK(object_);

[viettrungluu, 2012/07/10 22:47:58]

"

     return NewCallbackHelper(new Dispatcher2<Method, A, B>(method, a, b));
   }
 
   /// NewOptionalCallback() allocates a new, single-use
   /// <code>CompletionCallback</code> that might not run if the method
   /// taking it can complete synchronously. Thus, if after passing the
   /// CompletionCallback to a Pepper method, the method does not return
   /// PP_OK_COMPLETIONPENDING, then you should manually call the
   /// CompletionCallback's Run method, or memory will be leaked.
   ///
@@ skipping 61 matching lines @@
   /// @param[in] b Passed to <code>method</code> when the completion callback
   /// runs.
   ///
   /// @param[in] c Passed to <code>method</code> when the completion callback
   /// runs.
   ///
   /// @return A <code>CompletionCallback</code>.
   template <typename Method, typename A, typename B, typename C>
   CompletionCallback NewCallback(Method method, const A& a, const B& b,
                                  const C& c) {
     PP_DCHECK(object_);

[viettrungluu, 2012/07/10 22:47:58]

"

     return NewCallbackHelper(new Dispatcher3<Method, A, B, C>(method, a, b, c));
   }
 
   /// NewOptionalCallback() allocates a new, single-use
   /// <code>CompletionCallback</code> that might not run if the method
   /// taking it can complete synchronously. Thus, if after passing the
   /// CompletionCallback to a Pepper method, the method does not return
   /// PP_OK_COMPLETIONPENDING, then you should manually call the
   /// CompletionCallback's Run method, or memory will be leaked.
   ///
@@ skipping 50 matching lines @@
         typename internal::TypeUnwrapper<Output>::StorageType,
         void (T::*)(int32_t, Output, A, B, C),
         typename internal::TypeUnwrapper<A>::StorageType,
         typename internal::TypeUnwrapper<B>::StorageType,
         typename internal::TypeUnwrapper<C>::StorageType>(method, a, b, c));
   }
 
  private:
   class BackPointer {
    public:
-    typedef CompletionCallbackFactory<T, RefCount> FactoryType;
+    typedef CompletionCallbackFactory<T, ThreadTraits> FactoryType;
 
     explicit BackPointer(FactoryType* factory)
         : factory_(factory) {
     }
 
     void AddRef() {
       ref_.AddRef();
     }
 
     void Release() {
       if (ref_.Release() == 0)
         delete this;
     }
 
     void DropFactory() {
       factory_ = NULL;

[viettrungluu, 2012/07/10 22:47:58]

*** It's not entirely obvious to me why factory_ doesn't need to be protected
... but my head hurts.

I guess this should only be called by the "owning" factory, which protects it
with its lock?

     }
 
     T* GetObject() {
       return factory_ ? factory_->GetObject() : NULL;

[viettrungluu, 2012/07/10 22:47:58]

But then I worry about this....

     }
 
    private:
-    RefCount ref_;
+    typename ThreadTraits::RefCount ref_;
     FactoryType* factory_;
   };
 
   template <typename Dispatcher>
   class CallbackData {
    public:
     // Takes ownership of the given dispatcher pointer.
     CallbackData(BackPointer* back_pointer, Dispatcher* dispatcher)
         : back_pointer_(back_pointer),
           dispatcher_(dispatcher) {
@@ skipping 250 matching lines @@
     }
    private:
     Method method_;
     A a_;
     B b_;
     C c_;
 
     typename Traits::StorageType output_;
   };
 
+  // Creates the back pointer object and takes a reference to it. This assumes
+  // either that the lock is held or that it is not needed.
   void InitBackPointer() {
     back_pointer_ = new BackPointer(this);
     back_pointer_->AddRef();
   }
 
+  // Releases our reference to the back pointer object and clears the pointer.
+  // This assumes either that the lock is held or that it is not needed.
   void ResetBackPointer() {
     back_pointer_->DropFactory();
     back_pointer_->Release();
+    back_pointer_ = NULL;
   }
 
   // Takes ownership of the dispatcher pointer, which should be heap allocated.
   template <typename Dispatcher>
   CompletionCallback NewCallbackHelper(Dispatcher* dispatcher) {
+    ThreadTraits::AutoLock lock(lock_);
+
     PP_DCHECK(object_);  // Expects a non-null object!
     return CompletionCallback(
         &CallbackData<Dispatcher>::Thunk,
         new CallbackData<Dispatcher>(back_pointer_, dispatcher));
   }
 
   // Takes ownership of the dispatcher pointer, which should be heap allocated.
   template <typename Dispatcher> CompletionCallbackWithOutput<
       typename internal::TypeUnwrapper<
           typename Dispatcher::OutputType>::StorageType>
   NewCallbackWithOutputHelper(Dispatcher* dispatcher) {
+    ThreadTraits::AutoLock lock(lock_);
+
     PP_DCHECK(object_);  // Expects a non-null object!
     CallbackData<Dispatcher>* data =
         new CallbackData<Dispatcher>(back_pointer_, dispatcher);
 
     return CompletionCallbackWithOutput<typename Dispatcher::OutputType>(
         &CallbackData<Dispatcher>::Thunk,
         data,
         data->dispatcher()->output());
   }
 
   // Disallowed:
   CompletionCallbackFactory(const CompletionCallbackFactory&);
   CompletionCallbackFactory& operator=(const CompletionCallbackFactory&);
 
   T* object_;

[viettrungluu, 2012/07/10 22:47:58]

Maybe this should have a comment that this is only initialized once (and then
never changed).

+
+  typename ThreadTraits::Lock lock_;

[viettrungluu, 2012/07/10 22:47:58]

Probably should indicate that this protects back_pointer_.

+
   BackPointer* back_pointer_;
 };
 
 }  // namespace pp
 
 #endif  // PPAPI_UTILITY_COMPLETION_CALLBACK_FACTORY_H_