Add support for threadsafe completion callback factory.

ppapi/utility/completion_callback_factory_thread_traits.h

Index: ppapi/utility/completion_callback_factory_thread_traits.h
diff --git a/ppapi/utility/completion_callback_factory_thread_traits.h b/ppapi/utility/completion_callback_factory_thread_traits.h
new file mode 100644
index 0000000000000000000000000000000000000000..e3e9ae660d1a78f86304e2406071022fbef275ba
--- /dev/null
+++ b/ppapi/utility/completion_callback_factory_thread_traits.h
@@ -0,0 +1,177 @@
+// 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_THREAD_SAFE_THREAD_TRAITS_H_
+#define PPAPI_UTILITY_THREAD_SAFE_THREAD_TRAITS_H_
+
+#include "ppapi/cpp/module.h"
+#include "ppapi/utility/threading/lock.h"
+

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

You should probably include ppapi/cpp/logging.h.

+/// @file
+/// Defines the traits structures for threadsafety of a completion callback
+/// factory. We provide threadsafe and non-threadsafe version. The threadsafe
+/// version is always correct (if you follow the thread usage rules of the
+/// callback factory), but if you know your object will only be used on one
+/// thread, you can uses the non-threadsafe version.
+///
+/// The traits defines three nested classes to perform reference counting,
+/// locks, and scoped locking.
+
+namespace pp {
+
+/// The threadsafe version of thread traits. Using this class as the "traits"

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

s/threadsafe/thread-safe/

+/// template argument to a completion callback factory will make it "somewhat
+/// thread-friendly." It will allow you to create completion callbacks from
+/// background threads and post them to another thread to run.
+///
+/// Care still must be taken to ensure that the completion callbacks are
+/// executed on the same thread that the factory is destroyed on to avoid a
+/// race on destruction.
+///
+/// Implementation note: this uses a lock instead of atomic add instructions.
+/// The number of platforms we need to support right now makes atomic
+/// operations unweildy for this case that we don't actually use that often.

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

s/unweildy/unwieldy/

+/// As a further optimization, we can add support for this.

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

s/\.$/ later./ ?

+class ThreadSafeThreadTraits {
+ public:
+  class RefCount {
+   public:
+    /// Default constructor. In debug mode, this checks that the object is being
+    /// created on the main thread.
+    RefCount() : ref_(0) {
+    }
+
+    /// AddRef() increments the reference counter.
+    ///
+    /// @return An int32_t with the incremented reference counter.
+    int32_t AddRef() {
+      AutoLock lock(lock_);
+      return ++ref_;
+    }
+
+    /// Release() decrements the reference counter.
+    ///
+    /// @return An int32_t with the decremeneted reference counter.
+    int32_t Release() {
+      AutoLock lock(lock_);
+      return --ref_;

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

Maybe it's worth having a PP_DCHECK(ref > 0)?

+    }
+
+   private:
+    Lock lock_;
+    int32_t ref_;
+  };

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

Maybe you should disallow copy/assign?

+
+  typedef pp::Lock Lock;
+  typedef pp::AutoLock AutoLock;
+};
+
+/// The non-threadsafe version of thread traits. Using this class as the
+/// "traits" template argument to a completion callback factory will make it
+/// not threadsafe but with potential extra performance.
+class NonThreadSafeThreadTraits {
+ public:
+  /// A simple reference counter that is not thread-safe.
+  ///
+  /// <strong>Note:</strong> in Debug mode, it checks that it is either called
+  /// on the main thread, or always called on another thread.
+  class RefCount {
+   public:
+    /// Default constructor. In debug mode, this checks that the object is being
+    /// created on the main thread.
+    RefCount() : ref_(0) {
+#ifndef NDEBUG
+      is_main_thread_ = Module::Get()->core()->IsMainThread();
+#endif
+    }
+
+    /// Destructor.
+    ~RefCount() {
+      PP_DCHECK(is_main_thread_ == Module::Get()->core()->IsMainThread());
+    }
+
+    /// AddRef() increments the reference counter.
+    ///
+    /// @return An int32_t with the incremented reference counter.
+    int32_t AddRef() {
+      PP_DCHECK(is_main_thread_ == Module::Get()->core()->IsMainThread());
+      return ++ref_;
+    }
+
+    /// Release() decrements the reference counter.
+    ///
+    /// @return An int32_t with the decremeneted reference counter.
+    int32_t Release() {
+      PP_DCHECK(is_main_thread_ == Module::Get()->core()->IsMainThread());
+      return --ref_;
+    }
+
+   private:
+    int32_t ref_;
+#ifndef NDEBUG
+    bool is_main_thread_;

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

I'm not entirely convinced that we should even let this be used off the main
thread at all.

+#endif
+  };

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

"

+
+  /// A simple object that acts like a lock but does nothing.
+  ///
+  /// MStrong>Note:</strong> in Debug mode, it checks that it is either
+  /// called on the main thread, or always called on another thread. It also
+  /// asserts that the called does not recursively lock.

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

s/called/caller/ ?

+  class Lock {
+   public:
+    Lock() {
+#ifndef NDEBUG
+      is_main_thread_ = Module::Get()->core()->IsMainThread();
+      lock_held_ = false;
+#endif
+    }
+
+    ~Lock() {
+      PP_DCHECK(is_main_thread_ == Module::Get()->core()->IsMainThread());
+      PP_DCHECK(!lock_held_);

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

I don't think it's documented anywhere that locks mustn't be held on destruction
(and I don't think pp::Lock cares).

+    }
+
+    /// Acquires the fake "lock". This does nothing except perform checks in
+    /// debug mode.
+    void Acquire() {
+#ifndef NDEBUG
+      PP_DCHECK(!lock_held_);
+      lock_held_ = true;
+#endif
+    }
+
+    /// Releases the fake "lock". This does nothing except perform checks in
+    /// debug mode.
+    void Release() {
+#ifndef NDEBUG
+      PP_DCHECK(lock_held_);
+      lock_held_ = false;
+#endif
+    }
+
+   private:
+#ifndef NDEBUG
+    bool is_main_thread_;
+    bool lock_held_;
+#endif
+  };
+
+  class AutoLock {

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

It feels kind of dumb that you have to repeat the definition of AutoLock here.
Oh well.

+   public:
+    explicit AutoLock(Lock& lock) : lock_(lock) {
+      lock_.Acquire();
+    }
+    ~AutoLock() {
+      lock_.Release();
+    }
+
+   private:
+    Lock& lock_;
+  };
+};
+
+}  // namespace pp
+
+#endif  // PPAPI_UTILITY_THREAD_SAFE_THREAD_TRAITS_H_