Never lock the Pepper proxy lock on the IO thread

ppapi/shared_impl/proxy_lock.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_SHARED_IMPL_PROXY_LOCK_H_
 #define PPAPI_SHARED_IMPL_PROXY_LOCK_H_
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 157 matching lines @@
 
 template <typename RunType>
 class RunWhileLockedHelper;
 
 template <>
 class RunWhileLockedHelper<void()> {
  public:
   typedef base::Callback<void()> CallbackType;
   explicit RunWhileLockedHelper(const CallbackType& callback)
       : callback_(new CallbackType(callback)) {
-    // Copying |callback| may adjust reference counts for bound Vars or
-    // Resources; we should have the lock already.
-    ProxyLock::AssertAcquired();

[dmichael (off chromium), 2015/03/24 20:10:26]

I needed to be able to use RunWhileLocked on the IO thread without the
ProxyLock, and I realized that for a while now, VarTracker and ResourceTracker
do their own checking, so we should be protected from mistakes like binding a
ScopedPPVar without the ProxyLock...
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/shared_impl/...
https://code.google.com/p/chromium/codesearch#chromium/src/ppapi/shared_impl/...

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

Is the name a bit misleading now then? Should we rename the function?

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

The name never really was meant to relate to whether the lock is held at the
time "RunWhileLocked" is invoked. The goal is to return a bound callback that
will run with the lock, so call sites look fairly self-explanatory like:
PostTask(FROM_HERE, RunWhileLocked(base::Bind(Foo::DoStuff)));

Arguably the name has always been misleading (maybe should be something like
BindWithProxyLock), but it looks pretty good at call-sites.

Anywho...  You're right, it doesn't belong in this CL so I moved to the other
one for TrackedCallback. If you still want a name change, you can comment there:
https://codereview.chromium.org/923263003/
...and I'd probably do that as a separate CL:

[raymes, 2015/03/26 06:30:51]

Ahh that makes complete sense then. No please keep the name as-is especially if
it makes sense from the call sites.

     // CallWhileLocked and destruction might happen on a different thread from
     // creation.
     thread_checker_.DetachFromThread();
   }
   void CallWhileLocked() {
     // Bind thread_checker_ to this thread so we can check in the destructor.
     DCHECK(thread_checker_.CalledOnValidThread());
     ProxyAutoLock lock;
     {
       // Use a scope and local Callback to ensure that the callback is cleared
@@ skipping 70 matching lines @@
   scoped_ptr<CallbackType> callback_;
   base::ThreadChecker thread_checker_;
 };
 
 template <typename P1, typename P2>
 class RunWhileLockedHelper<void(P1, P2)> {
  public:
   typedef base::Callback<void(P1, P2)> CallbackType;
   explicit RunWhileLockedHelper(const CallbackType& callback)
       : callback_(new CallbackType(callback)) {
-    ProxyLock::AssertAcquired();
     thread_checker_.DetachFromThread();
   }
   void CallWhileLocked(P1 p1, P2 p2) {
     DCHECK(thread_checker_.CalledOnValidThread());
     ProxyAutoLock lock;
     {
       scoped_ptr<CallbackType> temp_callback(callback_.Pass());
       temp_callback->Run(p1, p2);
     }
   }
   ~RunWhileLockedHelper() {
     DCHECK(thread_checker_.CalledOnValidThread());
     if (callback_) {
       ProxyAutoLock lock;
       callback_.reset();
     }
   }
 
  private:
   scoped_ptr<CallbackType> callback_;
   base::ThreadChecker thread_checker_;
 };
 
 template <typename P1, typename P2, typename P3>
 class RunWhileLockedHelper<void(P1, P2, P3)> {
  public:
   typedef base::Callback<void(P1, P2, P3)> CallbackType;
   explicit RunWhileLockedHelper(const CallbackType& callback)
       : callback_(new CallbackType(callback)) {
-    ProxyLock::AssertAcquired();
     thread_checker_.DetachFromThread();
   }
   void CallWhileLocked(P1 p1, P2 p2, P3 p3) {
     DCHECK(thread_checker_.CalledOnValidThread());
     ProxyAutoLock lock;
     {
       scoped_ptr<CallbackType> temp_callback(callback_.Pass());
       temp_callback->Run(p1, p2, p3);
     }
   }
@@ skipping 46 matching lines @@
   internal::RunWhileLockedHelper<FunctionType>* helper =
       new internal::RunWhileLockedHelper<FunctionType>(callback);
   return base::Bind(
       &internal::RunWhileLockedHelper<FunctionType>::CallWhileLocked,
       base::Owned(helper));
 }
 
 }  // namespace ppapi
 
 #endif  // PPAPI_SHARED_IMPL_PROXY_LOCK_H_