Try to force plugins to use software rendering until we can support...

Try to force plugins to use software rendering until we can support
hardware acceleration on the Mac.
BUG=25406, 25092
TEST=Flash should no longer crash immediately after loading on
sites with complex Flash applications.


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

[Amanda Walker, 2009-10-29 16:54:21]

Stuart: review
John: FYI

[jam, 2009-10-29 17:24:45]

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

http://codereview.chromium.org/339068/diff/1/2#newcode286
Line 286: }
Note: wmode is only for Flash, so you should check that this is a Flash plugin
since it doesn't make sense to add it otherwise.

Also, turning it to wmode=transparent is really slow.  I'd be surprised if
YouTube videos play smoothly.  Have you tested (also HQ mode) on mid-range Macs?

Lastly, if you must do this, definitely use wmode=opaque, and not transparent. 
The latter forces us to check the pixels behind the background every frame to
see if they changed, in which case we force a synchronous paint.  This isn't
needed in this scenario since the plugin was windowed so the pixels behind it
never mattered.

[jam, 2009-10-29 17:28:33]

On Thu, Oct 29, 2009 at 10:24 AM, <jam@chromium.org> wrote:

>
> http://codereview.chromium.org/339068/diff/1/2
> File chrome/renderer/webplugin_delegate_proxy.cc (right):
>
> http://codereview.chromium.org/339068/diff/1/2#newcode286
> Line 286: }
> Note: wmode is only for Flash, so you should check that this is a Flash
> plugin since it doesn't make sense to add it otherwise.
>
> Also, turning it to wmode=transparent is really slow.  I'd be surprised
> if YouTube videos play smoothly.  Have you tested (also HQ mode) on
> mid-range Macs?
>
> Lastly, if you must do this, definitely use wmode=opaque, and not
> transparent.  The latter forces us to check the pixels behind the
> background every frame to see if they changed, in which case we force a
> synchronous paint.  This isn't needed in this scenario since the plugin
> was windowed so the pixels behind it never mattered.


oops, the last part isn't 100% right since I notice now that you don't set
transparent_ to true.  However, you probably don't want Flash thinking it's
transparent.


>
>
> http://codereview.chromium.org/339068
>

[Amanda Walker, 2009-10-29 18:18:25]

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

http://codereview.chromium.org/339068/diff/1/2#newcode286
Line 286: }
On 2009/10/29 17:24:46, John Abd-El-Malek wrote:
> Note: wmode is only for Flash, so you should check that this is a Flash plugin
> since it doesn't make sense to add it otherwise.

Good point, will fix.

> Also, turning it to wmode=transparent is really slow.  I'd be surprised if
> YouTube videos play smoothly.  Have you tested (also HQ mode) on mid-range
Macs?

YouTube already plays in this mode on Macs.  This patch gives no perceptible
change in YouTube performance even on low end Macs.  It does mean lower
performance than in other browsers if Flash does try to do accelerated drawing,
but running with reduced performance hit is better than an immediate crash. 
This is the cause of most or all of the top flash crashers on the Mac.

> Lastly, if you must do this, definitely use wmode=opaque, and not transparent.

Ah, good suggestion.  will do.

[Amanda Walker, 2009-10-29 18:44:55]

New patch up.  Checks for Flash, uses wmode=opaque, edited comment.

Just to clarify, all currently working Flash content on the Mac uses
non-accelerated drawing (including YouTube and other video sites).  This does
not change performance for content that worked before this patch.  It does
however, cause a much wider range of Flash content to work without crashing,
even though it does so with a performance hit compared to other platforms that
support windowed plugins.  Opened tracking bug 26235 "Enable accelerated drawing
for Mac plugins".

[stuartmorgan, 2009-10-29 18:53:01]

LGTM

[jam, 2009-10-29 19:25:21]

lgtm

[darin (slow to review), 2009-10-29 19:54:24]

http://codereview.chromium.org/339068/diff/3002/3003
File chrome/renderer/webplugin_delegate_proxy.cc (right):

http://codereview.chromium.org/339068/diff/3002/3003#newcode281
Line 281: if (!transparent_ && mime_type_ == "application/x-shockwave-flash" ) {
nit: extra space there at the end

[Amanda Walker, 2009-10-29 20:08:13]

http://codereview.chromium.org/339068/diff/3002/3003
File chrome/renderer/webplugin_delegate_proxy.cc (right):

http://codereview.chromium.org/339068/diff/3002/3003#newcode281
Line 281: if (!transparent_ && mime_type_ == "application/x-shockwave-flash" ) {
On 2009/10/29 19:54:24, darin wrote:
> nit: extra space there at the end

Oops, missed that before committing.  Will fix.