Replace reentrant check to be more explicit and avoid a data race when comparing

Replace reentrant ASSERT checks in MessageQueueManager with a non-racy version.

ASSERT(!crit_.CurrentThreadIsOwner()) was racy due to use of a rtc::IsThreadRefEqual which cannot compare the thread handlers without a lock unless one is already sure it is the thread owning the crit.

Committed: https://crrev.com/cdf6172b7fcb32bb4363243377e08731cc9a4b5d
Cr-Commit-Position: refs/heads/master@{#13411}

[andresp, 2016-07-07 12:52:10]

Description was changed from

==========
Replace reentrant check to be more explicit and avoid a data race when comparing
if the thread is not owning the lock already.
==========

to

==========
Replace reentrant ASSERT checks in MessageQueueManager with a non-racy version.

ASSERT(!crit_.CurrentThreadIsOwner()) was racy due to use of a
rtc::IsThreadRefEqual which cannot compare the thread handlers without a lock
unless one is already sure it is the thread owning the crit.
==========

[andresp, 2016-07-07 12:52:10]

andresp@webrtc.org changed reviewers:
+ pthatcher@webrtc.org, tommi@webrtc.org

[andresp, 2016-07-07 12:57:01]

Patchset #1 (id:1) has been deleted

[andresp, 2016-07-07 12:57:13]

Patchset #1 (id:20001) has been deleted

[tommi, 2016-07-07 14:35:39]

On 2016/07/07 13:03:41, andresp wrote:

Is there a chance that the assert could have ever incorrectly fire?

[andresp, 2016-07-07 15:01:54]

On 2016/07/07 14:35:39, tommi-webrtc wrote:
> On 2016/07/07 13:03:41, andresp wrote:
> 
> Is there a chance that the assert could have ever incorrectly fire?
I think the answer you are looking for is no. But it depends on your compiler,
all bets are off if your code has races.

[tommi, 2016-07-07 15:45:41]

On 2016/07/07 15:01:54, andresp wrote:
> On 2016/07/07 14:35:39, tommi-webrtc wrote:
> > On 2016/07/07 13:03:41, andresp wrote:
> > 
> > Is there a chance that the assert could have ever incorrectly fire?
> I think the answer you are looking for is no. But it depends on your compiler,
> all bets are off if your code has races.

Assuming only the calling thread can assign the value that's being checked for,
I can't think of how a race could result in the wrong result (regardless of how
badly a compiler might screw things up - it could, but I don't have the
imagination at the moment to imagine how :)).
Regardless, if this functionality isn't being used, I think it's good to get rid
of it. lgtm.

[andresp, 2016-07-08 08:27:58]

The CQ bit was checked by andresp@webrtc.org

[andresp, 2016-07-08 08:27:58]

The patchset sent to the CQ was uploaded after l-g-t-m from tommi@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2131503002/#ps60001 (title:
"rebase")

[commit-bot: I haz the power, 2016-07-08 08:28:02]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-07-08 09:45:44]

Description was changed from

==========
Replace reentrant ASSERT checks in MessageQueueManager with a non-racy version.

ASSERT(!crit_.CurrentThreadIsOwner()) was racy due to use of a
rtc::IsThreadRefEqual which cannot compare the thread handlers without a lock
unless one is already sure it is the thread owning the crit.
==========

to

==========
Replace reentrant ASSERT checks in MessageQueueManager with a non-racy version.

ASSERT(!crit_.CurrentThreadIsOwner()) was racy due to use of a
rtc::IsThreadRefEqual which cannot compare the thread handlers without a lock
unless one is already sure it is the thread owning the crit.
==========

[commit-bot: I haz the power, 2016-07-08 09:45:47]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-07-08 09:45:50]

Description was changed from

==========
Replace reentrant ASSERT checks in MessageQueueManager with a non-racy version.

ASSERT(!crit_.CurrentThreadIsOwner()) was racy due to use of a
rtc::IsThreadRefEqual which cannot compare the thread handlers without a lock
unless one is already sure it is the thread owning the crit.
==========

to

==========
Replace reentrant ASSERT checks in MessageQueueManager with a non-racy version.

ASSERT(!crit_.CurrentThreadIsOwner()) was racy due to use of a
rtc::IsThreadRefEqual which cannot compare the thread handlers without a lock
unless one is already sure it is the thread owning the crit.

Committed: https://crrev.com/cdf6172b7fcb32bb4363243377e08731cc9a4b5d
Cr-Commit-Position: refs/heads/master@{#13411}
==========

[commit-bot: I haz the power, 2016-07-08 09:45:51]

Patchset 2 (id:??) landed as
https://crrev.com/cdf6172b7fcb32bb4363243377e08731cc9a4b5d
Cr-Commit-Position: refs/heads/master@{#13411}