Added a debug method AssertAcquired() that compiles to nothing in non-debug b...

Added a debug method AssertAcquired() that compiles to nothing in non-debug builds.  In debug builds, the method will DCHECK if the
current thread does not hold the lock.

I also removed a class that was not being used (AutoLockImpl) which seems like someone just copied the Lock.h file and blindly added
an Impl for the AutoLock class.

Thre are places in my code where I want to use the AutoUnlock class.  I can't see why it was restricted to use by the condition variable
class only, so I just made it into a regular class with no restrictions.

I noticed that JamesR posted a CL about a month ago that made AutoUnlock a public class, but it was never committed, so I added him to this CL for review too.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=12037

[ralphl, 2009-03-17 23:26:51]

This is the change that kills the unit tests on mac and linux

[ralphl, 2009-03-18 18:08:44]

JAR: I noticed you were the last person to change the lock_*. files, so I'm
sending this CL to you.  It's a minor change that works in debug builds only.

I have two needs:
1. Assert that a lock is held when a method is called
2. Release a lock from within a method (using AutoUnlock)

So I added AssertAcquired() for windows builds only (null implementation for
linux and mac) and I made the AutoUnlock class available to everyone, not just
condition variables.

Once this change gets approved and checked in, I'll use it in the multi-threaded
media pipeline work I'm doing.

[jar (doing other things), 2009-03-18 20:40:13]

Please address the items below before landing.

After those changes are made, LGTM.

Thanks,

Jim

http://codereview.chromium.org/48109/diff/1001/12
File base/lock.h (right):

http://codereview.chromium.org/48109/diff/1001/12#newcode22
Line 22: void AssertAcquired() { return lock_.AssertAcquired(); }
Please add a comment describing the semantics of this method.

http://codereview.chromium.org/48109/diff/1001/12#newcode52
Line 52: // A helper class which releases an acquired lock for a given scope.
Consider using the original comment, or adding text that shows that the class
does more than release (it re-acquires).

http://codereview.chromium.org/48109/diff/1001/11
File base/lock_impl_win.cc (right):

http://codereview.chromium.org/48109/diff/1001/11#newcode16
Line 16: owning_thread_id_ = 0;
Is it true that CurrentId() never returns 0?

If so, you could assert it when you get the value "for real."

[jamesr1, 2009-03-18 20:49:35]

Thanks for merging in the AutoUnlock changes - I haven't had a chance to follow
up on that.

[ralphl, 2009-03-18 21:21:49]

OK, fixed it up.  Now letting the try servers have one more shot at it before
committing.

Next I'll check in the change that actually uses the AutoUnlock and
AssertAcquired() methods.

http://codereview.chromium.org/48109/diff/1001/12
File base/lock.h (right):

http://codereview.chromium.org/48109/diff/1001/12#newcode22
Line 22: void AssertAcquired() { return lock_.AssertAcquired(); }
On 2009/03/18 20:40:13, jar wrote:
> Please add a comment describing the semantics of this method.

Done.

http://codereview.chromium.org/48109/diff/1001/12#newcode52
Line 52: // A helper class which releases an acquired lock for a given scope.
On 2009/03/18 20:40:13, jar wrote:
> Consider using the original comment, or adding text that shows that the class
> does more than release (it re-acquires).

Done.

http://codereview.chromium.org/48109/diff/1001/11
File base/lock_impl_win.cc (right):

http://codereview.chromium.org/48109/diff/1001/11#newcode16
Line 16: owning_thread_id_ = 0;
On 2009/03/18 20:40:13, jar wrote:
> Is it true that CurrentId() never returns 0?
> 
> If so, you could assert it when you get the value "for real."

Done.