Fix videos stopping playing. An invalidate doesn't always lead to a paint (i...

Fix videos stopping playing.  Only send an invalidate to the renderer if it's within the clipping region, otherwise we'll wait forever for a did paint message that doesn't come.
BUG=115
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=3123

[darin (slow to review), 2008-10-05 05:58:36]

I think we need to decouple painting from invalidation more.  The plugin should
not paint each time it issues an invalidate.  It should wait for the renderer to
acknowledge the last paint (any paint) before it paints into the transport
bitmap again.  Then, you don't need the "is hidden" magic to turn off painting
in background tabs.  However, your changes to implement ScrollView::inWindow are
definitely right on for other reasons.

http://codereview.chromium.org/6257/diff/1/3
File chrome/plugin/webplugin_proxy.cc (right):

http://codereview.chromium.org/6257/diff/1/3#newcode98
Line 98: Paint(damaged_rect_);
painting here means we are doing a lot of work.  see my other comment.

http://codereview.chromium.org/6257/diff/1/7
File chrome/renderer/webplugin_delegate_proxy.cc (right):

http://codereview.chromium.org/6257/diff/1/7#newcode584
Line 584: // don't want the plugin process to keep painting unnecessarily.
but now the plugin in a foreground tab may paint itself more times than we can
display it, leading to bad performance (especially on single core machines). 
we'll also be allowing the plugin to paint itself when it is scrolled out of
view.

[jam, 2008-10-06 19:18:21]

On 2008/10/05 05:58:36, darin wrote:
> I think we need to decouple painting from invalidation more.  The plugin
should
> not paint each time it issues an invalidate.  It should wait for the renderer
to
> acknowledge the last paint (any paint) before it paints into the transport
> bitmap again.  Then, you don't need the "is hidden" magic to turn off painting
> in background tabs.  However, your changes to implement ScrollView::inWindow
are
> definitely right on for other reasons.
> 

Yeah I had wanted to fix this later, but when I sent this out I thought the best
thing to do was to first fix the painting issue, then worry about optimizing it.
 However since then I started debugging more to see why the invalidates weren't
leading to paints, and it turned out to be that the plugin was invalidating
regions outside of the clipping rectangle.  Checking for this in the plugin side
before sending an invalidate fixes the issue with videos on blogger & the wii
experience.  I'll split this change into two separate ones.

[jam, 2008-10-06 20:30:47]

Updated snapshot.

[darin (slow to review), 2008-10-06 20:39:44]

http://codereview.chromium.org/6257/diff/205/39
File chrome/plugin/webplugin_proxy.cc (right):

http://codereview.chromium.org/6257/diff/205/39#newcode98
Line 98: delegate_->clip_rect().Intersects(damaged_rect_)) {
above this you modify the damaged_rect_ even if the damage is excluded by the
clip rect.  is that intentional?

should our transport DIB only be the size of the clip rect?

[jam, 2008-10-06 22:11:08]

http://codereview.chromium.org/6257/diff/205/39
File chrome/plugin/webplugin_proxy.cc (right):

http://codereview.chromium.org/6257/diff/205/39#newcode98
Line 98: delegate_->clip_rect().Intersects(damaged_rect_)) {
On 2008/10/06 20:39:44, darin wrote:
> above this you modify the damaged_rect_ even if the damage is excluded by the
> clip rect.  is that intentional?

I figured it doesn't make a difference either way... Updated code though since
it's cleaner as you suggest.

> 
> should our transport DIB only be the size of the clip rect?

Per our discussion I don't think this will work as it'll lead to synchronous
paints when scrolling.

[darin (slow to review), 2008-10-06 22:16:59]

OK