move quit of paint message_loop into OnStop() in dervied classes

move quit of paint message_loop into callback of pipeline stop to guarantee message loop will not quit till paint is deinit'ed.
BUG=none
TEST=dev platform and desktop

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

[wjia(left Chromium), 2010-06-26 02:16:54]

Now the racing condition during stop is fixed.

[jiesun, 2010-06-28 15:44:06]

I did not get it. why change the message_loop::quit() from glx thread to render
thread will solve the problem? could you describe the racing in more detail.
I think this in pipeline's perspective. there are two sources   of paint task.
1. OnFrameAvailable : we should really call pause() and flush() ( just like
before seeks ) before stop to let buffer return to appropriate filters.
therefore during stop() we are sure there are no buffer exchange, therefore no
OnFrameAvailble() in renderers.
2. glx thread's own "expose" event, we should stop that paint message delivery
upon stop request received. (use flags?)
with these constraints , we are sure the last message in loop is quit message.
how do you think?

[wjia(left Chromium), 2010-06-28 17:23:35]

This is the racing between posting DeInit task and calling Quit(). Splitting
"stop" task doesn't help. It's required to execute DeInit before quitting the
message loop. If Quit() is called in player_x11, it's possible that message_loop
doesn't have any task when Quit() is called. Therefore the message_loop is
ended, but DeInit has not been called yet.

There are many ways to fix this problem. This is the simple one.

On 2010/06/28 15:44:06, jiesun wrote:
> I did not get it. why change the message_loop::quit() from glx thread to
render
> thread will solve the problem? could you describe the racing in more detail.
> I think this in pipeline's perspective. there are two sources   of paint task.
> 1. OnFrameAvailable : we should really call pause() and flush() ( just like
> before seeks ) before stop to let buffer return to appropriate filters.
> therefore during stop() we are sure there are no buffer exchange, therefore no
> OnFrameAvailble() in renderers.
> 2. glx thread's own "expose" event, we should stop that paint message delivery
> upon stop request received. (use flags?)
> with these constraints , we are sure the last message in loop is quit message.
> how do you think?

[scherkus (not reviewing), 2010-06-28 21:00:49]

http://codereview.chromium.org/2870029/diff/1/5
File media/tools/player_x11/player_x11.cc (left):

http://codereview.chromium.org/2870029/diff/1/5#oldcode180
media/tools/player_x11/player_x11.cc:180: message_loop->Quit();
couldn't we move message_loop->Quit() into the pipeline stop callback?

[wjia(left Chromium), 2010-06-28 22:54:43]

http://codereview.chromium.org/2870029/diff/1/5
File media/tools/player_x11/player_x11.cc (left):

http://codereview.chromium.org/2870029/diff/1/5#oldcode180
media/tools/player_x11/player_x11.cc:180: message_loop->Quit();
On 2010/06/28 21:00:49, scherkus wrote:
> couldn't we move message_loop->Quit() into the pipeline stop callback?

Done. That's another way to do it.

[scherkus (not reviewing), 2010-06-28 23:01:59]

nice!

LGTM