Fix painting problem with transparent plugins because plugins were ignoring t...

Fix painting problem with transparent plugins because plugins were ignoring the alpha channel sometimes.

This change introduces another buffer which holds the background image for transparent plugins.  Before
painting these plugins, their backing store is overwritten with the background data.

This change also uses an ACK from the renderer to figure out when it can paint, similar to how the
renderer does it, which gives us throttling and also doesn't lead to painting when the tab is hidden.



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

[jam, 2008-09-30 07:54:23]

Note I still can't track the cause of the painting issue when scrolling on WoW. 
I'll debug more, but at least now you can start the review.

[darin (slow to review), 2008-09-30 23:46:02]

LGTM

http://codereview.chromium.org/5040/diff/409/639
File chrome/common/plugin_messages_internal.h (right):

http://codereview.chromium.org/5040/diff/409/639#newcode106
Line 106: IPC_SYNC_MESSAGE_ROUTED1_0(PluginMsg_Paint,
nit: For extra clarity at places where this message is sent, perhaps you could
rename this to PluginMsg_SyncPaint?

http://codereview.chromium.org/5040/diff/409/639#newcode111
Line 111: IPC_MESSAGE_ROUTED0(PluginMsg_PaintAck)
Since this message is not in response to PluginMsg_Paint but rather is in
response to PluginMsg_InvalidateRect, perhaps it would be better to name this
message PluginMsg_InvalidateRect_ACK?  PluginMsg_WasPainted would also be fine,
I think.

http://codereview.chromium.org/5040/diff/409/640
File chrome/plugin/webplugin_proxy.cc (right):

http://codereview.chromium.org/5040/diff/409/640#newcode100
Line 100: Paint(rect);
i'm surprised that we don't clear damaged_rect_ here.  it looks like we will do
an extra Paint once we reach DidPaint.

it seems like we should do damaged_rect_ = damage_rect_.Union(rect) regardless
of the value of waiting_for_paint_, but if we call Paint, then we should clear
damaged_rect_.

http://codereview.chromium.org/5040/diff/409/640#newcode204
Line 204: Paint(damaged_rect);
what is the reason for having both of these methods when they are otherwise
identical?

http://codereview.chromium.org/5040/diff/409/644
File chrome/renderer/webplugin_delegate_proxy.cc (right):

http://codereview.chromium.org/5040/diff/409/644#newcode451
Line 451: backing_hdc, rect.x()-plugin_rect_.x(), rect.y()-plugin_rect_.y(),
SRCCOPY);
nit: 80 chars limit?

[jam, 2008-10-01 00:04:56]

http://codereview.chromium.org/5040/diff/409/639
File chrome/common/plugin_messages_internal.h (right):

http://codereview.chromium.org/5040/diff/409/639#newcode106
Line 106: IPC_SYNC_MESSAGE_ROUTED1_0(PluginMsg_Paint,
On 2008/09/30 23:46:03, Darin Fisher (Google) wrote:
> nit: For extra clarity at places where this message is sent, perhaps you could
> rename this to PluginMsg_SyncPaint?

I'd prefer not adding Sync prefixes to our message names.  i.e. all the other
sync ones don't have it.

http://codereview.chromium.org/5040/diff/409/639#newcode111
Line 111: IPC_MESSAGE_ROUTED0(PluginMsg_PaintAck)
On 2008/09/30 23:46:03, Darin Fisher (Google) wrote:
> Since this message is not in response to PluginMsg_Paint but rather is in
> response to PluginMsg_InvalidateRect, perhaps it would be better to name this
> message PluginMsg_InvalidateRect_ACK?  PluginMsg_WasPainted would also be
fine,
> I think.

Done.

http://codereview.chromium.org/5040/diff/409/640
File chrome/plugin/webplugin_proxy.cc (right):

http://codereview.chromium.org/5040/diff/409/640#newcode100
Line 100: Paint(rect);
On 2008/09/30 23:46:03, Darin Fisher (Google) wrote:
> i'm surprised that we don't clear damaged_rect_ here.  it looks like we will
do
> an extra Paint once we reach DidPaint.
> 
> it seems like we should do damaged_rect_ = damage_rect_.Union(rect) regardless
> of the value of waiting_for_paint_, but if we call Paint, then we should clear
> damaged_rect_.

This code (which I just brought back) took me a while to understand, I believe
this doesn't do an extra paint because if a paint message was due to an
invalidate call, then damage_rect will be empty.  If it wasn't empty, then it
was already accumulated and the paint would have happened in response to a did
paint message.  Anyways for readability I have updated the code per your
suggestion.

http://codereview.chromium.org/5040/diff/409/640#newcode204
Line 204: Paint(damaged_rect);
On 2008/09/30 23:46:03, Darin Fisher (Google) wrote:
> what is the reason for having both of these methods when they are otherwise
> identical?

removed (it had earlier had more code)

http://codereview.chromium.org/5040/diff/409/644
File chrome/renderer/webplugin_delegate_proxy.cc (right):

http://codereview.chromium.org/5040/diff/409/644#newcode451
Line 451: backing_hdc, rect.x()-plugin_rect_.x(), rect.y()-plugin_rect_.y(),
SRCCOPY);
On 2008/09/30 23:46:03, Darin Fisher (Google) wrote:
> nit: 80 chars limit?

Done.

[darin (slow to review), 2008-10-01 00:30:57]

OK, LGTM