Add CompletionTask to PPAPI TrackedCallback.

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 854e9dfb1509ebb843cdbc9633b1c6aca5371f5a..1963bc1dd6532da90850026c0a1fb6cf40d5b273 100644
--- a/ppapi/shared_impl/tracked_callback.cc
+++ b/ppapi/shared_impl/tracked_callback.cc
@@ -128,6 +128,9 @@ void TrackedCallback::Run(int32_t result) {
       PostRun(result);
       return;
     }
+
+    result = RunCompletionTask(result);

[yzshen1, 2013/08/07 21:09:10]

One thing that may be tricky is that this completion task may do things such as
running the tracked callback again. I think we should handle that case
carefully.

For example, if the completion task run the tracked callback, we will probably
end up running the PP_CompletionCallback twice. (Because at this point we don't
test whether the callback is in completed state anymore.)

[dmichael (off chromium), 2013/08/08 17:29:58]

I think the answer is "don't do that". The completion task needs to do fairly
simple things like write output parameters. IMHO, a comment would suffice.

That said, we should probably move this RunCompletionTask until after
MarkAsCompleted, so that it's consistent with the blocking case. That would make
doing "Run()" a no-op, if a CompletionTask tries to do Run() and a DCHECK if it
does PostRun.

[bbudge, 2013/08/08 18:34:00]

I moved the task invocation to after MarkAsCompleted. We first have to copy the
task to a local given the comment below. However I'm not sure it's safe to
access object state in BlockUntilComplete after receiving the signal to wake up.

+
     // Copy |callback_| now, since |MarkAsCompleted()| may delete us.
     PP_CompletionCallback callback = callback_;
     // Do this before running the callback in case of reentrancy (which
@@ -164,6 +167,11 @@ void TrackedCallback::PostRun(int32_t result) {
   is_scheduled_ = true;
 }
 
+void TrackedCallback::SetCompletionTask(const CompletionTask& completion_task) {
+  DCHECK(completion_task_.is_null());
+  completion_task_ = completion_task;
+}
+
 // static
 bool TrackedCallback::IsPending(
     const scoped_refptr<TrackedCallback>& callback) {
@@ -195,7 +203,8 @@ int32_t TrackedCallback::BlockUntilComplete() {
 
   while (!completed())
     operation_completed_condvar_->Wait();
-  return result_for_blocked_callback_;
+
+  return RunCompletionTask(result_for_blocked_callback_);
 }
 
 void TrackedCallback::MarkAsCompleted() {
@@ -211,4 +220,10 @@ void TrackedCallback::MarkAsCompleted() {
   tracker_ = NULL;
 }
 
+int32_t TrackedCallback::RunCompletionTask(int32_t result) {
+  if (!completion_task_.is_null())
+    result = completion_task_.Run(result);
+  return result;

[teravest, 2013/08/08 16:49:52]

nit: I think you want to call completion_task_.Reset() here.

While it's strange, there's nothing stopping someone from calling
BlockUntilComplete() multiple times for a given TrackedCallback, but I think you
only want to run the completion_task_ once.

[bbudge, 2013/08/08 18:34:00]

Yes, good catch. Another danger is if the task bundles up a ref to this, we have
a cycle and the callback will never be deleted. Done.

+}
+
 }  // namespace ppapi