Bug 32059 - [Qt] Plugins: Force windowless mode when there is no native window handle
Summary: [Qt] Plugins: Force windowless mode when there is no native window handle
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Girish Ramakrishnan
URL:
Keywords: Qt
Depends on:
Blocks:
 
Reported: 2009-12-02 01:38 PST by Girish Ramakrishnan
Modified: 2010-12-07 12:04 PST (History)
8 users (show)

See Also:


Attachments
Inject wmode=opaque (2.94 KB, patch)
2009-12-02 01:58 PST, Girish Ramakrishnan
hausmann: review-
Details | Formatted Diff | Diff
Inject wmode=opaque (2) (2.65 KB, patch)
2009-12-07 03:19 PST, Girish Ramakrishnan
hausmann: review+
Details | Formatted Diff | Diff
Inject wmode=opaque (3) (2.83 KB, patch)
2009-12-07 04:31 PST, Girish Ramakrishnan
hausmann: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Girish Ramakrishnan 2009-12-02 01:38:00 PST
When flash is instantiated in windowed mode, it requires a native window handle and will not paint to an offscreen pixmap on Windows and X11.

A solution on X11 is to implement composited plugins (https://bugs.webkit.org/show_bug.cgi?id=31232). On Windows, presumably this can be done by creating a fake window and grabbing it's contents.

All that seems to be a lot of work compared :-) For flash atleast, we can force it into windowless mode by injecting wmode transparent/opaque.
Comment 1 Girish Ramakrishnan 2009-12-02 01:58:26 PST
Created attachment 44136 [details]
Inject wmode=opaque

* opaque is injected for comatibility with windowed mode (as opposed to transparent)
Comment 2 WebKit Review Bot 2009-12-02 02:02:50 PST
Attachment 44136 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/plugins/PluginView.cpp:73:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 1
Comment 3 Kenneth Rohde Christiansen 2009-12-02 04:49:26 PST
This was my original idea :-) I think Antonio said that Fennec did that, but I know it is not doing that anymore. Would be nice to find out why
Comment 4 Girish Ramakrishnan 2009-12-02 06:39:15 PST
(In reply to comment #3)
> This was my original idea :-) I think Antonio said that Fennec did that, but I
> know it is not doing that anymore. Would be nice to find out why

I left a comment in #31232 about that (pasted below)

Per my understanding, the approach of using xcomposite was chosen by fennec,
only because they didn't have flash 10 on ARM (flash 10 added support for
windowless mode). 

See https://bugzilla.mozilla.org/show_bug.cgi?id=442109#c10 and
https://bugzilla.mozilla.org/show_bug.cgi?id=442109#c11
Comment 5 Girish Ramakrishnan 2009-12-03 08:14:39 PST
Simon, comments? zecke and kenneth seem to agree.
Comment 6 Simon Hausmann 2009-12-04 15:12:40 PST
Comment on attachment 44136 [details]
Inject wmode=opaque

I agree with the approach, but I have two comments about the implementation:

1) The proposed patch appends the wmode parameter. Are you sure that flash will always use the last wmode parameter if it's specified? Wouldn't it be better to _replace_ the value of an existing wmode if specified and only otherwise append?

2) I think this kind of quirk doesn't belong into PluginView.cpp but rather into FrameLoaderClientQt::createPlugin. IMHO it's much more specific to the needs of Qt (with its QGraphicsView) than to the implementation of the spec or the needs of the other ports.
Comment 7 Girish Ramakrishnan 2009-12-07 03:16:30 PST
(In reply to comment #6)
> (From update of attachment 44136 [details])
> I agree with the approach, but I have two comments about the implementation:
> 
> 1) The proposed patch appends the wmode parameter. Are you sure that flash will
> always use the last wmode parameter if it's specified? Wouldn't it be better to
> _replace_ the value of an existing wmode if specified and only otherwise
> append?
> 

I have tested this on Windows and Linux. It does seem to respect the value that is provided at the end. 

> 2) I think this kind of quirk doesn't belong into PluginView.cpp but rather
> into FrameLoaderClientQt::createPlugin. IMHO it's much more specific to the
> needs of Qt (with its QGraphicsView) than to the implementation of the spec or
> the needs of the other ports.

Great point. Totally agree.
Comment 8 Girish Ramakrishnan 2009-12-07 03:19:16 PST
Created attachment 44397 [details]
Inject wmode=opaque (2)

Simon, I will commit only after testing on the Mac but if you can review in meantime that will be good :-)
Comment 9 WebKit Review Bot 2009-12-07 03:21:21 PST
style-queue ran check-webkit-style on attachment 44397 [details] without any errors.
Comment 10 Simon Hausmann 2009-12-07 03:34:44 PST
Comment on attachment 44397 [details]
Inject wmode=opaque (2)

r=me, although I would still prefer code that would replace the value of an existing wmode parameter and only append if there isn't any present. Just to make it more robust :)

Thanks Girish for working on this!
Comment 11 Girish Ramakrishnan 2009-12-07 04:31:16 PST
Created attachment 44400 [details]
Inject wmode=opaque (3)

Make simon happy :-)
Comment 12 WebKit Review Bot 2009-12-07 04:32:58 PST
style-queue ran check-webkit-style on attachment 44400 [details] without any errors.
Comment 13 Simon Hausmann 2009-12-07 04:34:17 PST
Comment on attachment 44400 [details]
Inject wmode=opaque (3)


r=me
Comment 14 Girish Ramakrishnan 2009-12-07 05:48:11 PST
Landed in r51759
Comment 15 Hui Huang 2010-08-02 12:14:48 PDT
Hi Girish and Simon, I'm working on Symbian Flash player integration with QT Webkit and QGraphicsWebView. Your patch 51759 forces Symbian Flash player to run in Windowless mode by default. Since Symbian Flash player works in both window and windowless modes in QGraphicsWebView. We would like to run Flash Plugin in window mode on Symbian by default for performance and specification compliance reasons. Is it possible to disable this implementation for Symbian? Thanks, Hui.
Comment 16 Viatcheslav Ostapenko 2010-12-02 14:40:27 PST
As I understand, your commit completely disables transparent mode for QGraphicsWebView (which is also windowless). 
Do you have any reason for this?
Comment 17 Viatcheslav Ostapenko 2010-12-07 12:04:21 PST
(In reply to comment #14)
> Landed in r51759

Bug related to this change: https://bugs.webkit.org/show_bug.cgi?id=50495