glx: forward Expose event from child window to parent

glx: forward Expose event from child window to parent

r272290 added a child window to GLSurfaceGLX, but it needs to forward
Expose events to the parent.

BUG=326995
R=ccameron@chromium.org, sadrul@chromium.org

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

[piman, 2014-05-23 03:13:46]

ccameron: please review
sadrul: DEPS on ui/events

[ccameron, 2014-05-23 09:11:59]

lgtm w style nits.

Darn, should have remembered this part of the child window scheme on the
previous patch.

https://codereview.chromium.org/299143002/diff/20001/content/gpu/gpu_main.cc
File content/gpu/gpu_main.cc (right):

https://codereview.chromium.org/299143002/diff/20001/content/gpu/gpu_main.cc#...
content/gpu/gpu_main.cc:148: #elif defined(OS_LINUX)
Maybe avoid nested #if and use ignore_result for the event_source instead of an
unused local.

Something to the effect of:

#elif defined(OS_LINUX) && defined(USE_X11)
  base::MessageLoop main_message_loop(base::MessageLoop::TYPE_UI);
  ignore_result(ui::PlatformEventSource::CreateDefault());
#elif defined(OS_LINUX)
  base::MessageLoop main_message_loop(base::MessageLoop::TYPE_DEFAULT);
#elif ...

https://codereview.chromium.org/299143002/diff/20001/ui/gl/gl_surface_glx.cc
File ui/gl/gl_surface_glx.cc (right):

https://codereview.chromium.org/299143002/diff/20001/ui/gl/gl_surface_glx.cc#...
ui/gl/gl_surface_glx.cc:436: XFlush(g_display);
I was wondering if this would be needed, looks like it was.

This is all much cleaner than https://chromiumcodereview.appspot.com/23452026.

https://codereview.chromium.org/299143002/diff/20001/ui/gl/gl_surface_glx.cc#...
ui/gl/gl_surface_glx.cc:469: return ui::POST_DISPATCH_STOP_PROPAGATION;
In the previous incarnation of this, we did a CONTINUE here -- any good reason
to stop the even there? Might there be side effects?

[piman, 2014-05-23 10:09:48]

https://codereview.chromium.org/299143002/diff/20001/content/gpu/gpu_main.cc
File content/gpu/gpu_main.cc (right):

https://codereview.chromium.org/299143002/diff/20001/content/gpu/gpu_main.cc#...
content/gpu/gpu_main.cc:148: #elif defined(OS_LINUX)
On 2014/05/23 09:12:00, ccameron1 wrote:
> Maybe avoid nested #if and use ignore_result for the event_source instead of
an
> unused local.
> 
> Something to the effect of:
> 
> #elif defined(OS_LINUX) && defined(USE_X11)
>   base::MessageLoop main_message_loop(base::MessageLoop::TYPE_UI);
>   ignore_result(ui::PlatformEventSource::CreateDefault());
> #elif defined(OS_LINUX)
>   base::MessageLoop main_message_loop(base::MessageLoop::TYPE_DEFAULT);
> #elif ...
> 

I don't want to ignore_result, we need to keep the object alive while the
message loop runs (it sets itself as global in the constructor, and unsets
itself in the destructor).

Noted for the #ifdefs, I'll do that in the next rev.

https://codereview.chromium.org/299143002/diff/20001/ui/gl/gl_surface_glx.cc
File ui/gl/gl_surface_glx.cc (right):

https://codereview.chromium.org/299143002/diff/20001/ui/gl/gl_surface_glx.cc#...
ui/gl/gl_surface_glx.cc:436: XFlush(g_display);
On 2014/05/23 09:12:00, ccameron1 wrote:
> I was wondering if this would be needed, looks like it was.

I suppose it'll be kinda implicit at the next resize or swap, it just feels
safer.

Since we're not running a traditional XPending/XNextEvent loop, almost nothing
forces a Flush anywhere...

> 
> This is all much cleaner than https://chromiumcodereview.appspot.com/23452026.

https://codereview.chromium.org/299143002/diff/20001/ui/gl/gl_surface_glx.cc#...
ui/gl/gl_surface_glx.cc:469: return ui::POST_DISPATCH_STOP_PROPAGATION;
On 2014/05/23 09:12:00, ccameron1 wrote:
> In the previous incarnation of this, we did a CONTINUE here -- any good reason
> to stop the even there? Might there be side effects?

At this point this is the only class that cares about the events. Stopping
prevents useless dispatches in other instances, since they're the wrong window
anyway.

[sadrul, 2014-05-23 14:48:12]

lgtm

[piman, 2014-05-23 20:47:53]

https://codereview.chromium.org/299143002/diff/20001/content/gpu/gpu_main.cc
File content/gpu/gpu_main.cc (right):

https://codereview.chromium.org/299143002/diff/20001/content/gpu/gpu_main.cc#...
content/gpu/gpu_main.cc:148: #elif defined(OS_LINUX)
On 2014/05/23 09:12:00, ccameron1 wrote:
> Maybe avoid nested #if

Done.

[piman, 2014-05-23 20:47:59]

The CQ bit was checked by piman@chromium.org

[commit-bot: I haz the power, 2014-05-23 20:49:01]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/piman@chromium.org/299143002/60001

[piman, 2014-05-24 00:09:25]

Committed patchset #4 manually as r272648 (presubmit successful).