Never lock the Pepper proxy lock on the IO thread

ppapi/shared_impl/tracked_callback.cc

 // 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.
 
 #include "ppapi/shared_impl/tracked_callback.h"
 
 #include "base/bind.h"
 #include "base/compiler_specific.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
@@ skipping 19 matching lines @@
 
 int32_t RunCompletionTask(TrackedCallback::CompletionTask completion_task,
                           int32_t result) {
   ProxyLock::AssertAcquired();
   int32_t task_result = completion_task.Run(result);
   if (result != PP_ERROR_ABORTED)
     result = task_result;
   return result;
 }
 
-void AcquireProxyLockAndRun(const base::Closure& closure) {
-  ProxyAutoLock acquire;
-  closure.Run();
-}
-
-
 }  // namespace
 
 // TrackedCallback -------------------------------------------------------------
 
 // Note: don't keep a Resource* since it may go out of scope before us.
 TrackedCallback::TrackedCallback(Resource* resource,
                                  const PP_CompletionCallback& callback)
     : is_scheduled_(false),
       resource_id_(resource ? resource->pp_resource() : 0),
       completed_(false),
@@ skipping 189 matching lines @@
     aborted_ = true;
   // We might abort when there's already a scheduled callback, but callers
   // should never try to PostRun more than once otherwise.
   DCHECK(result == PP_ERROR_ABORTED || !is_scheduled_);
 
   if (is_blocking()) {
     // We might not have a MessageLoop to post to, so we must Signal
     // directly.
     SignalBlockingCallback(result);
   } else {
-    // Note we can't use "RunWhileLocked" from proxy_lock.h, because it requires
-    // that the ProxyLock is held (to protect Resource and Var ref-counting).
-    // Here, we must not acquire the ProxyLock, because we may be on the IO
-    // thread. But we're not passing any parameters that would require
-    // the ProxyLock (such as Var or Resource).
     base::Closure callback_closure(
-        base::Bind(&ppapi::AcquireProxyLockAndRun,
-            base::Bind(&TrackedCallback::Run, this, result)));
+        RunWhileLocked(base::Bind(&TrackedCallback::Run, this, result)));

[raymes, 2015/03/25 01:43:59]

I don't really understand this change but maybe bbudge would be better to
comment on it since he looked at the TrackedCallback stuff more?

[dmichael (off chromium), 2015/03/25 17:35:04]

That CL hasn't landed yet, and you're right...  this change really belongs
there, so I moved it out of this CL.

     if (target_loop_) {
       target_loop_->PostClosure(FROM_HERE, callback_closure, 0);
     } else {
       // We must be running in-process and on the main thread (the Enter
       // classes protect against having a null target_loop_ otherwise).
       DCHECK(IsMainThread());
       DCHECK(PpapiGlobals::Get()->IsHostGlobals());
       base::MessageLoop::current()->PostTask(FROM_HERE, callback_closure);
     }
   }
@@ skipping 17 matching lines @@
   // tracker. Then MarkAsCompleted before waking up the blocked thread,
   // which could potentially re-enter.
   scoped_refptr<TrackedCallback> thiz(this);
   MarkAsCompletedWithLock();
   // Wake up the blocked thread. See BlockUntilComplete for where the thread
   // Wait()s.
   operation_completed_condvar_->Signal();
 }
 
 }  // namespace ppapi