Makes MessagePumpGlib thread DCHECK work when pump is destroyed

Makes MessagePumpGlib thread DCHECK work when pump is destroyed

For tests I'm creating a MessagePumpGlib on a thread, destroying it,
then creating another MessagePumpGlib on another thread. The DCHECK
fires in this case because it only remembers the first thread. I made
it so the thread id is cleared when the MessagePumpGlib is destroyed.

BUG=none
TEST=none
R=sadrul@chromium.org
TBR=darin@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272727

[sadrul, 2014-05-23 21:29:57]

LGTM

+piman@, in case https://codereview.chromium.org/299143002/ may conflict with
this.

https://codereview.chromium.org/289043010/diff/30001/base/message_loop/messag...
File base/message_loop/message_pump_glib.cc (right):

https://codereview.chromium.org/289043010/diff/30001/base/message_loop/messag...
base/message_loop/message_pump_glib.cc:157: if (thread_info && thread_info->pump
== pump) {
Shouldn't thread_info->pump be always be == pump?

[piman, 2014-05-23 21:45:12]

On 2014/05/23 21:29:57, sadrul wrote:
> LGTM
> 
> +piman@, in case https://codereview.chromium.org/299143002/ may conflict with
> this.

It should be OK.
That CL only creates a UI loop in the GPU process (i.e. even not with
--single-process), and not for unit tests, so there should be only 1 per
process.

> 
>
https://codereview.chromium.org/289043010/diff/30001/base/message_loop/messag...
> File base/message_loop/message_pump_glib.cc (right):
> 
>
https://codereview.chromium.org/289043010/diff/30001/base/message_loop/messag...
> base/message_loop/message_pump_glib.cc:157: if (thread_info &&
thread_info->pump
> == pump) {
> Shouldn't thread_info->pump be always be == pump?

[sky, 2014-05-23 21:54:37]

https://codereview.chromium.org/289043010/diff/30001/base/message_loop/messag...
File base/message_loop/message_pump_glib.cc (right):

https://codereview.chromium.org/289043010/diff/30001/base/message_loop/messag...
base/message_loop/message_pump_glib.cc:157: if (thread_info && thread_info->pump
== pump) {
On 2014/05/23 21:29:58, sadrul wrote:
> Shouldn't thread_info->pump be always be == pump?

I believe so, but I wanted to make sure if that isn't the case things don't go
bad.

[sky, 2014-05-23 21:54:40]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2014-05-23 21:56:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/sky@chromium.org/289043010/30001

[sky, 2014-05-23 22:03:06]

Darin, I'm TBRing you as this is a focused change just for debugging.

[darin (slow to review), 2014-05-23 23:25:13]

https://codereview.chromium.org/289043010/diff/30001/base/message_loop/messag...
File base/message_loop/message_pump_glib.cc (right):

https://codereview.chromium.org/289043010/diff/30001/base/message_loop/messag...
base/message_loop/message_pump_glib.cc:157: if (thread_info && thread_info->pump
== pump) {
On 2014/05/23 21:54:37, sky wrote:
> On 2014/05/23 21:29:58, sadrul wrote:
> > Shouldn't thread_info->pump be always be == pump?
> 
> I believe so, but I wanted to make sure if that isn't the case things don't go
> bad.

You might consider a CHECK here.

Did you consider just going through MessageLoop::current() to find the current
thread's MessagePump?

[commit-bot: I haz the power, 2014-05-24 01:42:32]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  win_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-24 12:55:42]

Change committed as 272727

[sky, 2014-05-27 15:57:27]

https://codereview.chromium.org/289043010/diff/30001/base/message_loop/messag...
File base/message_loop/message_pump_glib.cc (right):

https://codereview.chromium.org/289043010/diff/30001/base/message_loop/messag...
base/message_loop/message_pump_glib.cc:157: if (thread_info && thread_info->pump
== pump) {
On 2014/05/23 23:25:13, darin wrote:
> On 2014/05/23 21:54:37, sky wrote:
> > On 2014/05/23 21:29:58, sadrul wrote:
> > > Shouldn't thread_info->pump be always be == pump?
> > 
> > I believe so, but I wanted to make sure if that isn't the case things don't
go
> > bad.
> 
> You might consider a CHECK here.
> 
> Did you consider just going through MessageLoop::current() to find the current
> thread's MessagePump?

This is all to verify we don't start MessagePumpGlib twice. So, we shouldn't
really need a thread lookup.