PPAPI: Make blocking completion callbacks work.

ppapi/thunk/enter.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_THUNK_ENTER_H_
 #define PPAPI_THUNK_ENTER_H_
 
+#include <string>
+
 #include "base/basictypes.h"
+#include "base/memory/ref_counted.h"
+#include "base/synchronization/lock.h"
+#include "ppapi/c/pp_errors.h"
 #include "ppapi/c/pp_resource.h"
 #include "ppapi/shared_impl/api_id.h"
 #include "ppapi/shared_impl/function_group_base.h"
 #include "ppapi/shared_impl/ppapi_globals.h"
 #include "ppapi/shared_impl/proxy_lock.h"
 #include "ppapi/shared_impl/resource.h"
 #include "ppapi/shared_impl/resource_tracker.h"
 #include "ppapi/thunk/ppapi_thunk_export.h"
 #include "ppapi/thunk/ppb_instance_api.h"
 #include "ppapi/thunk/resource_creation_api.h"
 
 namespace ppapi {
+class TrackedCallback;
+
 namespace thunk {
 
 // Enter* helper objects: These objects wrap a call from the C PPAPI into
 // the internal implementation. They make sure the lock is acquired and will
 // automatically set up some stuff for you.
 //
 // You should always check whether the enter succeeded before using the object.
 // If this fails, then the instance or resource ID supplied was invalid.
 //
 // The |report_error| arguments to the constructor should indicate if errors
@@ skipping 15 matching lines @@
 // This helps us define our RAII Enter classes easily. To make an RAII class
 // which locks the proxy lock on construction and unlocks on destruction,
 // inherit from |LockOnEntry<true>|. For cases where you don't want to lock,
 // inherit from |LockOnEntry<false>|. This allows us to share more code between
 // Enter* and Enter*NoLock classes.
 template <bool lock_on_entry>
 struct LockOnEntry;
 
 template <>
 struct LockOnEntry<false> {
-// TODO(dmichael) assert the lock is held.
+#if (!NDEBUG)
+  LockOnEntry() {
+    // You must already hold the lock to use Enter*NoLock.
+    AssertLockHeld();
+  }
+  ~LockOnEntry() {
+    // You must not release the lock before leaving the scope of the
+    // Enter*NoLock.
+    AssertLockHeld();
+  }
+ private:
+  static void AssertLockHeld() {
+    base::Lock* proxy_lock = PpapiGlobals::Get()->GetProxyLock();
+    if (proxy_lock)
+      proxy_lock->AssertAcquired();
+  }
+#endif
 };
 
 template <>
 struct LockOnEntry<true> {
   LockOnEntry() {
     ppapi::ProxyLock::Acquire();
   }
   ~LockOnEntry() {
     ppapi::ProxyLock::Release();
   }
 };
 
 // Keep non-templatized since we need non-inline functions here.
 class PPAPI_THUNK_EXPORT EnterBase {
  public:
   EnterBase();
-  explicit EnterBase(const PP_CompletionCallback& callback);
+  explicit EnterBase(PP_Resource resource);
+  EnterBase(PP_Resource resource, const PP_CompletionCallback& callback);
   virtual ~EnterBase();
 
-  // Sets the result.
+  // Sets the result for calls that use a completion callback. It handles making
+  // sure that "Required" callbacks are scheduled to run asynchronously and
+  // "Blocking" callbacks cause the caller to block. (Interface implementations,
+  // therefore, should not do any special casing based on the type of the
+  // callback.)
   //
   // Returns the "retval()". This is to support the typical usage of
   //   return enter.SetResult(...);
   // without having to write a separate "return enter.retval();" line.
   int32_t SetResult(int32_t result);
 
   // Use this value as the return value for the function.
   int32_t retval() const { return retval_; }
 
+  bool succeeded() const { return retval_ == PP_OK; }
+  bool failed() const { return !succeeded(); }
+
+  const scoped_refptr<TrackedCallback>& callback() { return callback_; }
+
  protected:
   // Helper function to return a function group from a PP_Instance. Having this
   // code be in the non-templatized base keeps us from having to instantiate
   // it in every template.
   FunctionGroupBase* GetFunctions(PP_Instance instance, ApiID id) const;
 
   // Helper function to return a Resource from a PP_Resource. Having this
   // code be in the non-templatized base keeps us from having to instantiate
   // it in every template.
-  Resource* GetResource(PP_Resource resource) const;
+  static Resource* GetResource(PP_Resource resource);
+
+  // Checks whether the callback is valid (i.e., if it is either non-blocking,
+  // or blocking and we're on a background thread). If the callback is invalid,
+  // this will set retval_ = PP_ERROR_BLOCKS_MAIN_THREAD, and if report_error is
+  // set, it will log a message to the programmer.
+  void SetStateForCallbackError(bool report_error);
+  bool CallbackIsValid() const;
 
   // Does error handling associated with entering a resource. The resource_base
   // is the result of looking up the given pp_resource. The object is the
-  // result of converting the base to the desired object (converted back to a
-  // Resource* so this function doesn't have to be templatized). The reason for
-  // passing both resource_base and object is that we can differentiate "bad
-  // resource ID" from "valid resource ID not of the currect type."
+  // result of converting the base to the desired object (converted to a void*
+  // so this function doesn't have to be templatized). The reason for passing
+  // both resource_base and object is that we can differentiate "bad resource
+  // ID" from "valid resource ID not of the correct type."
   //
   // This will set retval_ = PP_ERROR_BADRESOURCE if the object is invalid, and
   // if report_error is set, log a message to the programmer.
   void SetStateForResourceError(PP_Resource pp_resource,
                                 Resource* resource_base,
                                 void* object,
                                 bool report_error);
 
   // Same as SetStateForResourceError except for function API.
   void SetStateForFunctionError(PP_Instance pp_instance,
                                 void* object,
                                 bool report_error);
 
+  // For Enter objects that need a resource, we'll store a pointer to the
+  // Resource object so that we don't need to look it up more than once. For
+  // Enter objects with no resource, this will be NULL.
+  Resource* resource_;
+
  private:
-  // Holds the callback. The function will only be non-NULL when the
-  // callback is requried. Optional callbacks don't require any special
-  // handling from us at this layer.
-  PP_CompletionCallback callback_;
+  // Holds the callback. For Enter objects that aren't given a callback, this
+  // will be NULL.
+  scoped_refptr<TrackedCallback> callback_;
 
   int32_t retval_;
 };
 
 }  // namespace subtle
 
 // EnterFunction --------------------------------------------------------------
 
 template<typename FunctionsT, bool lock_on_entry = true>
 class EnterFunction : public subtle::EnterBase,
                       public subtle::LockOnEntry<lock_on_entry> {
  public:
   EnterFunction(PP_Instance instance, bool report_error)
-      : EnterBase() {
+      : EnterBase(instance) {
     Init(instance, report_error);
   }
   EnterFunction(PP_Instance instance,
                 const PP_CompletionCallback& callback,
                 bool report_error)
-      : EnterBase(callback) {
+      : EnterBase(0 /* resource */, callback) {
+    // TODO(dmichael): This means that the callback_ we get is not associated
+    //                 even with the instance, but we need to handle that for
+    //                 MouseLock (maybe others?).
     Init(instance, report_error);
   }
 
   ~EnterFunction() {}
 
-  bool succeeded() const { return !!functions_; }
-  bool failed() const { return !functions_; }
+/*  bool succeeded() const { return !!functions_ && CallbackIsValid(); }
+  bool failed() const { return !succeeded(); }*/
 
   FunctionsT* functions() { return functions_; }
 
  private:
   void Init(PP_Instance instance, bool report_error) {
     FunctionGroupBase* base = GetFunctions(instance, FunctionsT::kApiID);
     if (base)
       functions_ = base->GetAs<FunctionsT>();
     else
       functions_ = NULL;
+    // Validate the callback.
+    SetStateForCallbackError(report_error);
+    // Validate the resource (note, if both are wrong, we will return
+    // PP_ERROR_BADARGUMENT; last in wins).
     SetStateForFunctionError(instance, functions_, report_error);
   }
 
   FunctionsT* functions_;
 
   DISALLOW_COPY_AND_ASSIGN(EnterFunction);
 };
 
 // Like EnterFunction but assumes the lock is already held.
 template<typename FunctionsT>
 class EnterFunctionNoLock : public EnterFunction<FunctionsT, false> {
  public:
   EnterFunctionNoLock(PP_Instance instance, bool report_error)
       : EnterFunction<FunctionsT, false>(instance, report_error) {
   }
 };
 
 // Used when a caller has a resource, and wants to do EnterFunction for the
 // instance corresponding to that resource.
 template<typename FunctionsT>
 class EnterFunctionGivenResource : public EnterFunction<FunctionsT> {
  public:
   EnterFunctionGivenResource(PP_Resource resource, bool report_error)
       : EnterFunction<FunctionsT>(GetInstanceForResource(resource),
                                   report_error) {
   }
 
  private:
   static PP_Instance GetInstanceForResource(PP_Resource resource) {
-    Resource* object =
-        PpapiGlobals::Get()->GetResourceTracker()->GetResource(resource);
+    Resource* object = subtle::EnterBase::GetResource(resource);
     return object ? object->pp_instance() : 0;
   }
 };
 
 // EnterResource ---------------------------------------------------------------
 
 template<typename ResourceT, bool lock_on_entry = true>
 class EnterResource : public subtle::EnterBase,
                       public subtle::LockOnEntry<lock_on_entry> {
  public:
   EnterResource(PP_Resource resource, bool report_error)
-      : EnterBase() {
+      : EnterBase(resource) {
     Init(resource, report_error);
   }
-  EnterResource(PP_Resource resource,
-                const PP_CompletionCallback& callback,
+  EnterResource(PP_Resource resource, const PP_CompletionCallback& callback,
                 bool report_error)
-      : EnterBase(callback) {
+      : EnterBase(resource, callback) {
     Init(resource, report_error);
   }
   ~EnterResource() {}
 
-  bool succeeded() const { return !!object_; }
-  bool failed() const { return !object_; }
+/*  bool succeeded() const { return !!object_ && CallbackIsValid(); }
+  bool failed() const { return !succeeded(); }*/
 
   ResourceT* object() { return object_; }
   Resource* resource() { return resource_; }
 
  private:
   void Init(PP_Resource resource, bool report_error) {
-    resource_ = GetResource(resource);
     if (resource_)
       object_ = resource_->GetAs<ResourceT>();
     else
       object_ = NULL;
+    // Validate the callback.
+    SetStateForCallbackError(report_error);
+    // Validate the resource (note, if both are wrong, we will return
+    // PP_ERROR_BADRESOURCE; last in wins).
     SetStateForResourceError(resource, resource_, object_, report_error);
   }
 
-  Resource* resource_;
   ResourceT* object_;
 
   DISALLOW_COPY_AND_ASSIGN(EnterResource);
 };
 
 // ----------------------------------------------------------------------------
 
 // Like EnterResource but assumes the lock is already held.
 template<typename ResourceT>
 class EnterResourceNoLock : public EnterResource<ResourceT, false> {
  public:
   EnterResourceNoLock(PP_Resource resource, bool report_error)
       : EnterResource<ResourceT, false>(resource, report_error) {
   }
+  EnterResourceNoLock(PP_Resource resource, PP_CompletionCallback callback,
+                      bool report_error)
+      : EnterResource<ResourceT, false>(resource, callback, report_error) {
+  }
 };
 
 // Simpler wrapper to enter the resource creation API. This is used for every
 // class so we have this helper function to save template instantiations and
 // typing.
 class PPAPI_THUNK_EXPORT EnterResourceCreation
     : public EnterFunction<ResourceCreationAPI> {
  public:
   EnterResourceCreation(PP_Instance instance);
   ~EnterResourceCreation();
 };
 
 // Simpler wrapper to enter the instance API from proxy code. This is used for
 // many interfaces so we have this helper function to save template
 // instantiations and typing.
 class PPAPI_THUNK_EXPORT EnterInstance
     : public EnterFunction<PPB_Instance_FunctionAPI> {
  public:
   EnterInstance(PP_Instance instance);
   EnterInstance(PP_Instance instance, const PP_CompletionCallback& callback);
   ~EnterInstance();
 };

[dmichael (off chromium), 2012/04/19 20:02:00]

I needed EnterInstanceNoLock to make AudioConfigShared work right (it was
double-locking).

 
+class PPAPI_THUNK_EXPORT EnterInstanceNoLock
+    : public EnterFunctionNoLock<PPB_Instance_FunctionAPI> {
+ public:
+  EnterInstanceNoLock(PP_Instance instance);
+  EnterInstanceNoLock(PP_Instance instance,
+                      const PP_CompletionCallback& callback);
+  ~EnterInstanceNoLock();
+};
+
+
 }  // namespace thunk
 }  // namespace ppapi
 
 #endif  // PPAPI_THUNK_ENTER_H_