First pass at implementing real Flush callbacks. This does not currently...

First pass at implementing real Flush callbacks. This does not currently
include a test since that needs some work on threading and the testing
interface. I plan on doing that in a separate pass.

TEST=none
BUG=none

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

[darin (slow to review), 2010-06-08 19:13:02]

http://codereview.chromium.org/2712002/diff/1/4
File webkit/glue/plugins/pepper_device_context_2d.cc (right):

http://codereview.chromium.org/2712002/diff/1/4#newcode479
webkit/glue/plugins/pepper_device_context_2d.cc:479:
painted_flush_callbacks_[i].Execute(GetResource());
do we need to worry about re-entrancy here?  can execution of a callback result
in pepper APIs that would modify painted_flush_callbacks_ or otherwise cause
problems?  what if the device is unbound?

also, how is it that we can have more than one flush callback pending?

http://codereview.chromium.org/2712002/diff/1/4#newcode534
webkit/glue/plugins/pepper_device_context_2d.cc:534:
offscreen_callback_scheduled_ = true;
batching these up might not be so good.  it might be better to post a task for
each so that they get interleaved with other work on the renderer's main thread.

[brettw, 2010-06-09 16:58:26]

New snap up

http://codereview.chromium.org/2712002/diff/1/4
File webkit/glue/plugins/pepper_device_context_2d.cc (right):

http://codereview.chromium.org/2712002/diff/1/4#newcode479
webkit/glue/plugins/pepper_device_context_2d.cc:479:
painted_flush_callbacks_[i].Execute(GetResource());
Things will get added to the painted flush callbacks only when the renderer
paints. When the plugin requests a paint, things will go on the unpainted flush
callback queue. So we should be OK here.

I'll file a bug on not allowing multiple flush calls without a callback.
Currently the test depends on this (otherwise, I need to wait for the callback
before continuing the test, and unfortunately a "run pending messages" function
won't fix the problem.

http://codereview.chromium.org/2712002/diff/1/4#newcode534
webkit/glue/plugins/pepper_device_context_2d.cc:534:
offscreen_callback_scheduled_ = true;
Okay, I changed this and the code got a bit better.

[darin (slow to review), 2010-06-09 19:56:17]

On 2010/06/09 16:58:26, brettw wrote:
> New snap up
> 
> http://codereview.chromium.org/2712002/diff/1/4
> File webkit/glue/plugins/pepper_device_context_2d.cc (right):
> 
> http://codereview.chromium.org/2712002/diff/1/4#newcode479
> webkit/glue/plugins/pepper_device_context_2d.cc:479:
> painted_flush_callbacks_[i].Execute(GetResource());
> Things will get added to the painted flush callbacks only when the renderer
> paints. When the plugin requests a paint, things will go on the unpainted
flush
> callback queue. So we should be OK here.

What if BindToInstance(NULL) is called during a flush callback?


> I'll file a bug on not allowing multiple flush calls without a callback.
> Currently the test depends on this (otherwise, I need to wait for the callback
> before continuing the test, and unfortunately a "run pending messages"
function
> won't fix the problem.

Hmm... I'm not sure I follow, but I like disallowing multiple flushes.

[darin (slow to review), 2010-06-09 19:57:44]

On 2010/06/09 19:56:17, darin wrote:
> On 2010/06/09 16:58:26, brettw wrote:
> > New snap up
> > 
> > http://codereview.chromium.org/2712002/diff/1/4
> > File webkit/glue/plugins/pepper_device_context_2d.cc (right):
> > 
> > http://codereview.chromium.org/2712002/diff/1/4#newcode479
> > webkit/glue/plugins/pepper_device_context_2d.cc:479:
> > painted_flush_callbacks_[i].Execute(GetResource());
> > Things will get added to the painted flush callbacks only when the renderer
> > paints. When the plugin requests a paint, things will go on the unpainted
> flush
> > callback queue. So we should be OK here.
> 
> What if BindToInstance(NULL) is called during a flush callback?

Nevermind, it seems harmless since painted_flush_callbacks_.size() is checked at
each iteration.


> 
> 
> > I'll file a bug on not allowing multiple flush calls without a callback.
> > Currently the test depends on this (otherwise, I need to wait for the
callback
> > before continuing the test, and unfortunately a "run pending messages"
> function
> > won't fix the problem.
> 
> Hmm... I'm not sure I follow, but I like disallowing multiple flushes.

[darin (slow to review), 2010-06-09 19:58:18]

LGTM

On Wed, Jun 9, 2010 at 12:56 PM, <darin@chromium.org> wrote:

> On 2010/06/09 16:58:26, brettw wrote:
>
>> New snap up
>>
>
>  http://codereview.chromium.org/2712002/diff/1/4
>> File webkit/glue/plugins/pepper_device_context_2d.cc (right):
>>
>
>  http://codereview.chromium.org/2712002/diff/1/4#newcode479
>> webkit/glue/plugins/pepper_device_context_2d.cc:479:
>> painted_flush_callbacks_[i].Execute(GetResource());
>> Things will get added to the painted flush callbacks only when the
>> renderer
>> paints. When the plugin requests a paint, things will go on the unpainted
>>
> flush
>
>> callback queue. So we should be OK here.
>>
>
> What if BindToInstance(NULL) is called during a flush callback?
>
>
>
>  I'll file a bug on not allowing multiple flush calls without a callback.
>> Currently the test depends on this (otherwise, I need to wait for the
>> callback
>> before continuing the test, and unfortunately a "run pending messages"
>>
> function
>
>> won't fix the problem.
>>
>
> Hmm... I'm not sure I follow, but I like disallowing multiple flushes.
>
>
> http://codereview.chromium.org/2712002/show
>