gtk: Some code cleanup for the message-pump.

gtk: Some code cleanup for the message-pump.

GTK message-pump defines its own observer, but it has the same name as the
message-pump observers used in other platforms. So rename the GTK version
to MessagePumpGdkObserver.

Also, GTK version of message-pump dispatcher is never used, so get rid of
that.

BUG=145600
R=piman@chromium.org, thakis@chromium.org

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

[sadrul, 2013-09-03 23:19:10]

This is what I had in mind with regards to the MessagePumpObserver
disambiguation. I haven't looked enough at the change in
https://codereview.chromium.org/23191007/ to see if this will be useful for
that, though.

[ccameron, 2013-09-03 23:35:48]

I may be able to build the workaround on top of this -- the only required change
is that we allow for two MessageLoop::Observer types (the change in
message_loop.h). Is that okay?

https://codereview.chromium.org/23537016/diff/1/base/message_loop/message_loo...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/23537016/diff/1/base/message_loop/message_loo...
base/message_loop/message_loop.cc:429: if (run_loop_->dispatcher_ && type() ==
TYPE_UI) {
I'll have to make this exist when TOOLKIT_GTK is defined (likely by having a
non-UI-but-still-sort-of-UI type).

https://codereview.chromium.org/23537016/diff/1/base/message_loop/message_loop.h
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/23537016/diff/1/base/message_loop/message_loo...
base/message_loop/message_loop.h:94: typedef void* Dispatcher;
This is the only area where we'd have difficulty -- could we "typedef
MessagePumpGdkObserver GdkObserver" instead?

[sadrul, 2013-09-05 17:15:12]

+piman@

This is the first CL. The second CL is:
https://codereview.chromium.org/23880006/

The final CL is: https://codereview.chromium.org/23450020/. All three changes
combined is https://codereview.chromium.org/23450020/#ps29001

Please review.

[piman, 2013-09-05 17:37:46]

https://codereview.chromium.org/23537016/diff/24001/base/message_loop/message...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/23537016/diff/24001/base/message_loop/message...
base/message_loop/message_loop.cc:428: #if !defined(OS_MACOSX) &&
!defined(OS_ANDROID) && !defined(USE_GTK_MESSAGE_PUMP)
I'm not sure I understand why it's ok not to run with a dispatcher any more. Are
we never using one?

[sadrul, 2013-09-05 17:53:12]

https://codereview.chromium.org/23537016/diff/24001/base/message_loop/message...
File base/message_loop/message_loop.cc (right):

https://codereview.chromium.org/23537016/diff/24001/base/message_loop/message...
base/message_loop/message_loop.cc:428: #if !defined(OS_MACOSX) &&
!defined(OS_ANDROID) && !defined(USE_GTK_MESSAGE_PUMP)
On 2013/09/05 17:37:47, piman wrote:
> I'm not sure I understand why it's ok not to run with a dispatcher any more.
Are
> we never using one?

Chrome currently does not use any Dispatcher in linux-gtk, no (this CL also
removes the MessagePumpDispatcher defined in message_pump_gtk.h).

A different option would be to have a new define, e.g.
MESSAGE_LOOP_USES_DISPATCHER in message_loop.h, and use that in the code to make
this more readable/obvious.

[piman, 2013-09-05 18:12:43]

lgtm

https://codereview.chromium.org/23537016/diff/24001/base/message_loop/message...
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/23537016/diff/24001/base/message_loop/message...
base/message_loop/message_loop.h:97: typedef void* Dispatcher;
Do we need this, then? Can we remove it?

[sadrul, 2013-09-05 18:32:23]

Thanks!

+brettw@ for owners review.

https://codereview.chromium.org/23537016/diff/24001/base/message_loop/message...
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/23537016/diff/24001/base/message_loop/message...
base/message_loop/message_loop.h:97: typedef void* Dispatcher;
On 2013/09/05 18:12:43, piman wrote:
> Do we need this, then? Can we remove it?

Done (required a couple of additional ifdef changes for RunLoop).

[ccameron, 2013-09-06 17:59:42]

Adding thakis@ for base OWNERS review.

[Nico, 2013-09-06 21:42:31]

lgtm

https://codereview.chromium.org/23537016/diff/37001/base/message_loop/message...
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/23537016/diff/37001/base/message_loop/message...
base/message_loop/message_loop.h:22: #include
"base/message_loop/message_pump_observer.h"
Do you need the full includes? Forward declaring should be enough, no?

(as far as I can tell, this is just used for the typedef, and this compiles as
far as I can tell:

$ cat test.cc
class A;

class B {
  typedef A BA;
};

)

[sadrul, 2013-09-07 00:59:34]

https://codereview.chromium.org/23537016/diff/37001/base/message_loop/message...
File base/message_loop/message_loop.h (right):

https://codereview.chromium.org/23537016/diff/37001/base/message_loop/message...
base/message_loop/message_loop.h:22: #include
"base/message_loop/message_pump_observer.h"
On 2013/09/06 21:42:31, Nico wrote:
> Do you need the full includes? Forward declaring should be enough, no?
> 
> (as far as I can tell, this is just used for the typedef, and this compiles as
> far as I can tell:
> 
> $ cat test.cc
> class A;
> 
> class B {
>   typedef A BA;
> };
> 
> )

Yep. Done.

[sadrul, 2013-09-07 01:14:10]

Committed patchset #6 manually as r221846 (presubmit successful).