Fixed race in blocking TrackedCallback

This CL fixes race condition in TrackedCallback. We must pass the
same lock to conditional variable that we use to guard code when
we signal it. Otherwise there is chance that we signal it before
we start waiting and will wait forever.

In previous code we call lock_.release() and then Wait() on
conditional variable. In between the SignalBlockingCallback could
be called and signal before wait will happen.

Bug: 997329
Change-Id: Ifc16f1f8337846a1c253d91f59558ee1731256f6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1771383Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Vasiliy Telezhnikov <vasilyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#692811}

ppapi/shared_impl/tracked_callback.cc

@@ -73,8 +73,7 @@ TrackedCallback::TrackedCallback(Resource* resource,
     if (is_blocking()) {
       // This is a blocking completion callback, so we will need a condition
       // variable for blocking & signalling the calling thread.
-      operation_completed_condvar_.reset(
-          new base::ConditionVariable(proxy_lock));
+      operation_completed_condvar_.reset(new base::ConditionVariable(&lock_));
     } else {
       // It's a non-blocking callback, so we should have a MessageLoopResource
       // to dispatch to. Note that we don't error check here, though. Later,
@@ -191,16 +190,20 @@ int32_t TrackedCallback::BlockUntilComplete() {
   // Protect us from being deleted to ensure operation_completed_condvar_ is
   // available to wait on when we drop our lock.
   scoped_refptr<TrackedCallback> thiz(this);
+
+  // Unlock proxy lock temporarily; We don't want to block whole proxy while
+  // we're waiting for the callback
+  ProxyLock::Release();
+
   while (!completed_) {
-    // Unlock our lock temporarily; any thread that tries to signal us will need
-    // the lock.
-    lock_.Release();
     operation_completed_condvar_->Wait();
-    // Note that the condvar releases the ProxyLock during Wait and re-acquires
-    // the ProxyLock when it's signaled. We reacquire lock_ immediately after,
-    // preserving lock order.
-    ProxyLock::AssertAcquired();
-    lock_.Acquire();
+  }
+
+  // Now we need to get ProxyLock back, but because it's used in outer code to
+  // maintain lock order we need to release our lock first
+  {
+    base::AutoUnlock unlock(lock_);
+    ProxyLock::Acquire();
   }
 
   if (!completion_task_.is_null()) {