PPAPI: Make blocking completion callbacks work.

ppapi/shared_impl/tracked_callback.cc

Index: ppapi/shared_impl/tracked_callback.cc
diff --git a/ppapi/shared_impl/tracked_callback.cc b/ppapi/shared_impl/tracked_callback.cc
index f97acfdd29ae453fe16a139952853beda0fb60e9..f3be66beba147aa11fae9339936df6e4a745263c 100644
--- a/ppapi/shared_impl/tracked_callback.cc
+++ b/ppapi/shared_impl/tracked_callback.cc
@@ -8,6 +8,8 @@
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
+#include "base/synchronization/lock.h"
+#include "ppapi/c/dev/ppb_message_loop_dev.h"
 #include "ppapi/c/pp_completion_callback.h"
 #include "ppapi/c/pp_errors.h"
 #include "ppapi/shared_impl/callback_tracker.h"
@@ -23,14 +25,25 @@ namespace ppapi {
 TrackedCallback::TrackedCallback(
     Resource* resource,
     const PP_CompletionCallback& callback)
-    : ALLOW_THIS_IN_INITIALIZER_LIST(abort_impl_factory_(this)),
-      resource_id_(resource->pp_resource()),
+    : ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this)),
+      resource_id_(resource ? resource->pp_resource() : 0),
       completed_(false),
       aborted_(false),
-      callback_(callback) {
-  tracker_ = PpapiGlobals::Get()->GetCallbackTrackerForInstance(
-      resource->pp_instance()),
-  tracker_->Add(make_scoped_refptr(this));
+      callback_(callback),
+      result_for_blocked_callback_(PP_OK) {
+  // We can only track this callback if the resource is valid. It can be NULL
+  // in error conditions in the Enter* classes, and callbacks which aren't in
+  // the tracker will generally be short-lived.
+  // TODO(dmichael): Add tracking at the instance level, for callbacks that only
+  // have an instance.
+  if (resource) {
+    tracker_ = PpapiGlobals::Get()->GetCallbackTrackerForInstance(
+        resource->pp_instance());
+    tracker_->Add(make_scoped_refptr(this));
+  }
+
+  if (base::Lock* proxy_lock = PpapiGlobals::Get()->GetProxyLock())
+    operation_completed_condvar_.reset(new base::ConditionVariable(proxy_lock));
 }
 
 TrackedCallback::~TrackedCallback() {
@@ -44,22 +57,25 @@ void TrackedCallback::Abort() {
 }
 
 void TrackedCallback::PostAbort() {
-  if (!completed()) {
+  // It doesn't make sense to abort a callback that's not associated with a
+  // resource.
+  // TODO(dmichael): If we allow associating with an instance instead, we must
+  // allow for aborts in the case of the instance being destroyed.
+  DCHECK(resource_id_);
+
+  if (!completed() && !aborted_) {
     aborted_ = true;
-    // Post a task for the abort (only if necessary).
-    if (!abort_impl_factory_.HasWeakPtrs()) {
-      MessageLoop::current()->PostTask(
-          FROM_HERE,
-          RunWhileLocked(base::Bind(&TrackedCallback::Abort,
-                                    abort_impl_factory_.GetWeakPtr())));
-    }
+    MessageLoop::current()->PostTask(
+        FROM_HERE,
+        RunWhileLocked(base::Bind(&TrackedCallback::Abort,
+                                  weak_ptr_factory_.GetWeakPtr())));
   }
 }
 
 void TrackedCallback::Run(int32_t result) {
   if (!completed()) {
     // Cancel any pending calls.
-    abort_impl_factory_.InvalidateWeakPtrs();
+    weak_ptr_factory_.InvalidateWeakPtrs();
 
     // Copy |callback_| and look at |aborted()| now, since |MarkAsCompleted()|
     // may delete us.
@@ -67,10 +83,58 @@ void TrackedCallback::Run(int32_t result) {
     if (aborted())
       result = PP_ERROR_ABORTED;
 
-    // Do this before running the callback in case of reentrancy (which
-    // shouldn't happen, but avoid strange failures).
-    MarkAsCompleted();
-    CallWhileUnlocked(PP_RunCompletionCallback, &callback, result);
+    if (is_blocking()) {
+      // If the condition variable is invalid, we're probably running
+      // in-process, and we should have returned PP_ERROR_BLOCKS_MAIN_THREAD
+      // well before this.
+      DCHECK(operation_completed_condvar_.get());
+      result_for_blocked_callback_ = result;
+      // Retain ourselves, since MarkAsCompleted will remove us from the
+      // tracker. Then MarkAsCompleted before waking up the blocked thread,
+      // which could potentially re-enter.
+      scoped_refptr<TrackedCallback> thiz(this);
+      MarkAsCompleted();
+      // Wake up the blocked thread. See BlockUntilComplete for where the thread
+      // Wait()s.
+      operation_completed_condvar_->Signal();
+    } else {
+      // Do this before running the callback in case of reentrancy (which
+      // shouldn't happen, but avoid strange failures).
+      MarkAsCompleted();
+      // TODO(dmichael): Associate a message loop with the callback; if it's not
+      // the same as the current thread's loop, then post it to the right loop.
+      CallWhileUnlocked(PP_RunCompletionCallback, &callback, result);
+    }
+  }
+}
+
+void TrackedCallback::PostRun(int32_t result) {
+  DCHECK(!completed());
+  if (!completed()) {
+    // There should be no pending calls.
+    DCHECK(!weak_ptr_factory_.HasWeakPtrs());
+    weak_ptr_factory_.InvalidateWeakPtrs();
+
+    if (resource_id_) {
+      // If it has a resource_id_, it's in the tracker, and may be aborted if
+      // the resource is destroyed.
+      MessageLoop::current()->PostTask(
+          FROM_HERE,
+          RunWhileLocked(base::Bind(&TrackedCallback::Run,
+                                    weak_ptr_factory_.GetWeakPtr(),
+                                    result)));
+    } else {
+      // There is no resource_id_ associated with this callback, so it can't be
+      // aborted. We have the message loop retain the callback to make sure it
+      // gets run. This can happen when EnterBase is given an invalid resource,
+      // and in that case no resource or instance will retain this
+      // TrackedCallback.
+      MessageLoop::current()->PostTask(
+          FROM_HERE,
+          RunWhileLocked(base::Bind(&TrackedCallback::Run,
+                                    this,
+                                    result)));
+    }
   }
 }
 
@@ -97,6 +161,21 @@ void TrackedCallback::ClearAndAbort(scoped_refptr<TrackedCallback>* callback) {
   temp->Abort();
 }
 
+int32_t TrackedCallback::BlockUntilComplete() {
+  // Note, we are already holding the proxy lock in all these methods, including
+  // this one (see ppapi/thunk/enter.cc for where it gets acquired).
+
+  // It doesn't make sense to wait on a non-blocking callback.
+  DCHECK(is_blocking());
+  // BlockUntilComplete should never be called for in-process plugins, where
+  // blocking callbacks are not supported.
+  DCHECK(operation_completed_condvar_.get());
+
+  while (!completed())
+    operation_completed_condvar_->Wait();
+  return result_for_blocked_callback_;
+}
+
 void TrackedCallback::MarkAsCompleted() {
   DCHECK(!completed());
 
@@ -104,7 +183,9 @@ void TrackedCallback::MarkAsCompleted() {
   // until we're done.
   scoped_refptr<TrackedCallback> thiz = this;
   completed_ = true;
-  tracker_->Remove(thiz);
+  // We may not have a valid resource, in which case we're not in the tracker.
+  if (resource_id_)
+    tracker_->Remove(thiz);
   tracker_ = NULL;
 }