Fix painting glitch with text and NPAPI plugins. This was a regression from r167042. The problem wa…

Fix painting glitch with text and NPAPI plugins. This was a regression from r167042. The problem was that for non-transparent windowless plugins we were combining the data below the plugin with the data that the plugin creates. We could just copy directly and not do any alpha-blending.

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

[piman, 2013-03-06 00:58:57]

https://codereview.chromium.org/12487003/diff/1/content/renderer/webplugin_de...
File content/renderer/webplugin_delegate_proxy.cc (right):

https://codereview.chromium.org/12487003/diff/1/content/renderer/webplugin_de...
content/renderer/webplugin_delegate_proxy.cc:413: }
Do we really need all that logic? The plugin should already tell us if it's
transparent or opaque, by calling NPN_SetValue(_, NPPVpluginTransparentBool, _)
- see webkit/plugins/npapi/plugin_host.cc . Could we pipe that to the renderer?

[jam, 2013-03-06 01:58:36]

https://codereview.chromium.org/12487003/diff/1/content/renderer/webplugin_de...
File content/renderer/webplugin_delegate_proxy.cc (right):

https://codereview.chromium.org/12487003/diff/1/content/renderer/webplugin_de...
content/renderer/webplugin_delegate_proxy.cc:413: }
On 2013/03/06 00:58:57, piman wrote:
> Do we really need all that logic? The plugin should already tell us if it's
> transparent or opaque, by calling NPN_SetValue(_, NPPVpluginTransparentBool,
_)
> - see webkit/plugins/npapi/plugin_host.cc . Could we pipe that to the
renderer?

good point, switched to it.

we used to default to windowless being transparent, and now I've changed that. 
seems low risk since we didn't use that flag elsewhere, and the current change
is basically a no-op if NPN_SetValue isn't called.

[piman, 2013-03-06 02:01:14]

On 2013/03/06 01:58:36, jam wrote:
>
https://codereview.chromium.org/12487003/diff/1/content/renderer/webplugin_de...
> File content/renderer/webplugin_delegate_proxy.cc (right):
> 
>
https://codereview.chromium.org/12487003/diff/1/content/renderer/webplugin_de...
> content/renderer/webplugin_delegate_proxy.cc:413: }
> On 2013/03/06 00:58:57, piman wrote:
> > Do we really need all that logic? The plugin should already tell us if it's
> > transparent or opaque, by calling NPN_SetValue(_, NPPVpluginTransparentBool,
> _)
> > - see webkit/plugins/npapi/plugin_host.cc . Could we pipe that to the
> renderer?
> 
> good point, switched to it.
> 
> we used to default to windowless being transparent, and now I've changed that.

> seems low risk since we didn't use that flag elsewhere, and the current change
> is basically a no-op if NPN_SetValue isn't called.

LGTM, thanks!
This assumes that NPN_SetValue is called from Initialize, which makes sense,
though I don't know if all plugins do that. Then again, I don't care much about
all plugins.... if NPAPI Flash (why???) and Silverlight work, then LGTM.

[jam, 2013-03-06 02:47:30]

> LGTM, thanks!
> This assumes that NPN_SetValue is called from Initialize, which makes sense,
> though I don't know if all plugins do that. Then again, I don't care much
about
> all plugins.... if NPAPI Flash (why???) and Silverlight work, then LGTM.

turns out Silverlight doesn't call us if it's transparent, so defaulting to
false is wrong. I've left the default as is for all plugins other than Flash,
since we know that one for sure calls us during Initialize.

[piman, 2013-03-06 07:15:25]

lgtm