Bug 20081 - [Qt] Add support for windowless NPAPI plugins
: [Qt] Add support for windowless NPAPI plugins
Status: RESOLVED FIXED
: WebKit
Plug-ins
: 528+ (Nightly build)
: PC Linux
: P2 Enhancement
Assigned To:
: http://www.communitymx.com/content/so...
: Qt
:
: 24157 29799 30149 30170 30207 30214 30248 30251 30355 30356
  Show dependency treegraph
 
Reported: 2008-07-17 10:04 PST by
Modified: 2009-10-14 20:52 PST (History)


Attachments
Zattoo's modified PluginViewWin.cpp file (26.17 KB, text/x-c++src)
2008-07-18 12:04 PST, Chris Kurecka
no flags Details
Zattoo's modified PluginView.h file (9.25 KB, text/x-chdr)
2008-07-18 12:05 PST, Chris Kurecka
no flags Details
patch, not for review (18.89 KB, patch)
2009-07-28 13:34 PST, Yael
no flags Review Patch | Details | Formatted Diff | Diff
Implement keyboard event forwarding for windowless plugins (4.94 KB, patch)
2009-08-03 14:14 PST, Kenneth Rohde Christiansen
no flags Review Patch | Details | Formatted Diff | Diff
Implement keyboard event forwarding for windowless plugins (4.98 KB, patch)
2009-08-03 14:29 PST, Kenneth Rohde Christiansen
no flags Review Patch | Details | Formatted Diff | Diff
Windowless patch - makes painting work (14.20 KB, patch)
2009-09-25 03:02 PST, Girish Ramakrishnan
no flags Review Patch | Details | Formatted Diff | Diff
Windowless patch - makes painting work (2) (11.21 KB, patch)
2009-09-25 05:58 PST, Girish Ramakrishnan
no flags Review Patch | Details | Formatted Diff | Diff
Windowless patch - makes painting work (3) (11.83 KB, patch)
2009-09-27 22:01 PST, Girish Ramakrishnan
hausmann: review-
Review Patch | Details | Formatted Diff | Diff
Windowless patch - painting and event handling (4) (15.32 KB, patch)
2009-09-29 04:41 PST, Girish Ramakrishnan
no flags Review Patch | Details | Formatted Diff | Diff
Windowless patch - painting and event handling (5) (15.76 KB, patch)
2009-09-29 06:40 PST, Girish Ramakrishnan
no flags Review Patch | Details | Formatted Diff | Diff
Windowless patch - painting and event handling (6) (22.15 KB, patch)
2009-09-29 23:19 PST, Girish Ramakrishnan
no flags Review Patch | Details | Formatted Diff | Diff
Windowless patch - painting and event handling (7) (22.80 KB, patch)
2009-09-30 22:37 PST, Girish Ramakrishnan
no flags Review Patch | Details | Formatted Diff | Diff
Windowless patch - painting and event handling (8) (23.01 KB, patch)
2009-10-02 02:06 PST, Girish Ramakrishnan
no flags Review Patch | Details | Formatted Diff | Diff
Patch 1/4 - [Qt] Add support for windowless NPAPI plugins (23.26 KB, patch)
2009-10-05 04:54 PST, Girish Ramakrishnan
hausmann: review+
Review Patch | Details | Formatted Diff | Diff
Patch 2/4 - Use X Pixmap instead of QPixmap. (6.46 KB, patch)
2009-10-05 04:55 PST, Girish Ramakrishnan
hausmann: review+
Review Patch | Details | Formatted Diff | Diff
Patch 3/4 - Add PluginQuirkUsesScreenDefaultVisual for Flash. (11.69 KB, patch)
2009-10-05 04:56 PST, Girish Ramakrishnan
hausmann: review-
Review Patch | Details | Formatted Diff | Diff
Patch 4/4 - Make painting and events work when page is zoomed. (5.86 KB, patch)
2009-10-05 04:57 PST, Girish Ramakrishnan
no flags Review Patch | Details | Formatted Diff | Diff
Patch 3/4 - Add PluginQuirkUsesScreenDefaultVisual for Flash (Take 2) (11.67 KB, patch)
2009-10-06 04:32 PST, Girish Ramakrishnan
hausmann: review+
Review Patch | Details | Formatted Diff | Diff
Patch 4/4 - Make painting and events work when page is zoomed. (Take 2) (5.91 KB, patch)
2009-10-06 04:33 PST, Girish Ramakrishnan
hausmann: review+
Review Patch | Details | Formatted Diff | Diff
Enable printing (2.31 KB, patch)
2009-10-06 04:40 PST, Girish Ramakrishnan
hausmann: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-07-17 10:04:43 PST
Now that Firefox 3 supports windowless mode in Linux, some plugins are starting to take advantage of it. We should add support for it in QtWebKit.
------- Comment #1 From 2008-07-18 12:04:56 PST -------
Created an attachment (id=22367) [details]
Zattoo's modified PluginViewWin.cpp file
------- Comment #2 From 2008-07-18 12:05:28 PST -------
Created an attachment (id=22368) [details]
Zattoo's modified PluginView.h file
------- Comment #3 From 2008-07-18 12:08:31 PST -------
Sorry marcoil, I meant my attachments to go against https://bugs.webkit.org/show_bug.cgi?id=20095, which was open in another tab.  I'm not sure how to delete them.
------- Comment #4 From 2008-07-18 18:41:20 PST -------
Me neither, but don't worry about it.
------- Comment #5 From 2008-07-31 09:51:19 PST -------
I have two not-fully-working implementations of this:

- At http://git.collabora.co.uk/?p=user/marcoil/webkit.git;a=shortlog;h=refs/heads/marcoil/plugins-wmode-qt there's a branch that makes the plugin draw directly on the window. This produces flickering artifacts that I think are the result of bad interaction betweeen XRender and X normal drawing.
- The branch at http://git.collabora.co.uk/?p=user/marcoil/webkit.git;a=shortlog;h=refs/heads/marcoil/plugins-wmode-qt-offscreen uses an off-screen pixmap and makes the plugin render into it. The problem is that uninitalized pixmaps have no alpha (thus no transparency). If alpha is added to the pixmap, then Gtk (from the plugins) complains of non-compatible colormaps between the pixmap and the screen.

I have no idea on how to solve these issues, so I'm publishing these in the hope someone else will be able to improve on it.
------- Comment #6 From 2009-03-26 03:13:12 PST -------
I'm no longer working on this, so taking me out.
------- Comment #7 From 2009-07-28 13:34:05 PST -------
Created an attachment (id=33666) [details]
patch, not for review

This patch is not final. It contains 2 ways of passing the X11 drawable to the plugin. At this point, none of them fully work. I will keep working on it after my vacation.
------- Comment #8 From 2009-08-03 14:12:04 PST -------
Hi Yael, I cleaned up some of your event handling code. Please consult my upcoming patch.
------- Comment #9 From 2009-08-03 14:14:27 PST -------
Created an attachment (id=34003) [details]
Implement keyboard event forwarding for windowless plugins

Implement keyboard event forwarding for windowless plugins.

Windowless plugins are not there yet, but this is one small step of the way.
------- Comment #10 From 2009-08-03 14:29:07 PST -------
Created an attachment (id=34004) [details]
Implement keyboard event forwarding for windowless plugins

ChangeLog fixes
------- Comment #11 From 2009-08-03 14:37:33 PST -------
(From update of attachment 34004 [details])
r=me

> 
>         Based on work my Yael Aharon.

I guess that should become a "by" when landing ;-)

> +bool PluginView::dispatchNPEvent(NPEvent& event)

A ### TODO would be nice here to point out that we should unify this function across all ports.
------- Comment #12 From 2009-08-03 14:58:34 PST -------
(From update of attachment 34004 [details])
Landed in 46733
------- Comment #13 From 2009-09-17 01:07:54 PST -------
I am working on this bug. 

Quick status - I am currently working outside webkit and trying to fix the problem faced by Marc. 

When backing store enabled, I see two approaches:
1. Provide plugin with a Drawable that already has content of Qt's backing store. So, we just grab the backing store contents into a Drawable before the plugin's paint event, give it to the plugin and after the plugin paint we put the Drawable back in the backing store. I think, this will work well even if use ARGB visuals but this involves copying back and forth will make it slow.
2. Make plugin paint directly to Qt's backing store. Will be much faster.

When backing store disabled, we can assume paint on screen. And make the plugin draw directly to the window. This will flicker but they asked for it (as in marc's patch).

I have been testing with the diamondx plugin (with a windowless patch from bug 137189 of mozilla). You can find the git repo here - http://git.forwardbias.in/?p=diamondx.git;a=shortlog;h=refs/heads/windowless.

I have managed to get approach 1 to work outside webkit, remains to be seen if it will work with webkit :) It's flicker free. I wrote a pure qt4 'host' that loads a netscape plugin. http://git.forwardbias.in/?p=npploader.git;a=shortlog;h=refs/heads/grab_bs. It's _very_ hacky. Ideally, we should use QWindowSurface (which is exported but private).

Also, the gdk code and Qt code use different X11 displays. So to sync the painting in the drawable, I have to end up calling XSync :/

I am working on approach 2, but I am getting all sorts of gdk errors atm.
------- Comment #14 From 2009-09-17 06:09:50 PST -------
You should notice that some plugins are using the glib refcounting which can lead to problems (like with the Flash plugin). The problem is that the GtkSocket takes a ref on the GtkPlug, which never happens when using a QX11EmbedContainer, thus we are always left with 1 ref less, which means that the plugin is sometimes destroying parts of itself during resize etc.
------- Comment #15 From 2009-09-17 07:20:30 PST -------
Ok, approach 2 works too. http://git.forwardbias.in/?p=npploader.git;a=shortlog;h=refs/heads/direct_bs. I will try integrating the approach into qt/webkit now.
------- Comment #16 From 2009-09-21 10:17:41 PST -------
Before integrating to WebKit, I thought I should first test with flash instead of diamondx. I had to make a lot of changes to my npploader. After a lot of pain, transparent flash works in npploader!

http://git.forwardbias.in/?p=npploader.git;a=shortlog;h=refs/heads/master

Now to integrate it into Qt/WebKit.
------- Comment #17 From 2009-09-21 11:09:57 PST -------
(In reply to comment #16)
> Before integrating to WebKit, I thought I should first test with flash instead
> of diamondx. I had to make a lot of changes to my npploader. After a lot of
> pain, transparent flash works in npploader!
> 
> http://git.forwardbias.in/?p=npploader.git;a=shortlog;h=refs/heads/master
> 
> Now to integrate it into Qt/WebKit.

Cool news! thanks for the great work Girish!
------- Comment #18 From 2009-09-25 03:02:38 PST -------
Created an attachment (id=40102) [details]
Windowless patch - makes painting work

Painting works with windowless plugins with no flicker. Does not work when the view is scrolled, I need to get the scroll offsets right.

My initial version drew directly to the backing store. But it has a couple of problems - the backing store offset changes when new widgets appear _over_ the QWebView. This means that we need to detect this (which is very hard) and also do a setwindow call. The code says that calling setwindow multiple times crashes plugin (doesn't say which plugin). In general, I thought it was safer to just use a offscreen drawable.

Builds over yael's patch, the main fixes are how the drawable is grabbed, painted by plugin.

I need comment on the below and on the patch in general:
1. The gdkDisplay is a part of the PluginPackage, I am not sure if this is the right place. I was hoping that after a plugin is loaded (but no new plugin instance created), we use RTLD_NOLOAD using dlopen to detect if gdk-x11 is already open. There seems to be no mechanism to achieve this using QLibrary (besides QLibrary will load gdk-x11 even if the plugin does not need it).
------- Comment #19 From 2009-09-25 03:26:17 PST -------
One option is to load gdk-x11 *always* on startup and make Qt use the gdk default display. However, this does not work (Qt bug). Note that, if I do a XOpenDisplay manually and provide that display to Qt, it works. Just not with gdk's display, I haven't debugged this problem.
------- Comment #20 From 2009-09-25 03:45:09 PST -------
(In reply to comment #19)
> One option is to load gdk-x11 *always* on startup and make Qt use the gdk
> default display. However, this does not work (Qt bug). Note that, if I do a
> XOpenDisplay manually and provide that display to Qt, it works. Just not with
> gdk's display, I haven't debugged this problem.

I think always loading gdk-x11 is the best option. Flushing the gdk display shouldn't be too bad. But why do you have to flush the Qt display? Can't you simply use QPainter::drawPixmap to paint the pixmap?

We cannot change the way people construct their QApplication or change the existing Qt connection.
------- Comment #21 From 2009-09-25 03:57:21 PST -------
Ah, of course, We cannot change the way people construct their QApplication or change the existing Qt connection. What was I thinking...

Flushing Qt connection is required so that changes to the m_drawable are reflected to the plugin.
------- Comment #22 From 2009-09-25 05:58:50 PST -------
Created an attachment (id=40107) [details]
Windowless patch - makes painting work (2)

* Moved gdk display detection code to PluginViewQt (Simon's suggestion)
* Fixed coding style
------- Comment #23 From 2009-09-25 06:50:22 PST -------
(In reply to comment #18)

> QWebView. This means that we need to detect this (which is very hard) and also
> do a setwindow call. The code says that calling setwindow multiple times
> crashes plugin (doesn't say which plugin). 

This is due to Flash crashing when setWindow is called with a different size. Flash destroys its internals and reconstructs them, but as it has one ref less (due to us not using GtkSocket) it becomes 0 and some internals die and result in a crash.

As you are dealing with windowless you might not have this problem.
------- Comment #24 From 2009-09-27 22:01:04 PST -------
Created an attachment (id=40216) [details]
Windowless patch - makes painting work (3)

Scrolling works now. The rendering is now ready for review.
------- Comment #25 From 2009-09-27 22:05:11 PST -------
Here's my todo list for this feature to be 'complete':
1. Make it work with QWebGraphicsItem. We most likely cannot support transparent mode but opaque mode should work.
2. Make it work with printing
3. Test grabWidget
4. Test Keyboard and Focus (some code already exists)
5. Implement Mouse handling (Click, Wheel)
6. Check if dialogs and widgets displayed by plugins work (flash shows gtk popup menus and dialogs)
7. Optimize opaque mode. Need not XCopyArea from the backing store to drawable.

Let me know, if I am missed something.
------- Comment #26 From 2009-09-28 06:12:45 PST -------
(From update of attachment 40216 [details])
This is a great start!

After a quick discussion with Girish on IRC we agreed that makes more sense to fix point 4/5 and 6 in the TODO before landing. Otherwise I don't see any issues with the existing patch, except that it would be good to replace the unconditional enabling of the supportswindowless variable with something slightly more conditional depending on the availability of the backing store for example.
------- Comment #27 From 2009-09-29 04:41:47 PST -------
Created an attachment (id=40294) [details]
Windowless patch - painting and event handling (4)

4,5,7 now work. So painting, mouse, focus, keyboard handling should all work.

6, I am not entirely sure.  The diamondx plugin manages to show popups and the gtk file dialog. However, the flash plugin fails to show the context menu. This seems to be the case with chrome/windowless too. So, maybe flash decides not to show the context menu for some reason. No idea how to fix this/proceed. Is this a blocker for landing?

Also fixes the case of not having a backing store, we just use the pixmap and painter (no content propagation).

check-webkit-style reports no errors :)
------- Comment #28 From 2009-09-29 06:40:55 PST -------
Created an attachment (id=40299) [details]
Windowless patch - painting and event handling (5)

New patch after review by kenneth on irc.

Changes:
1. Create static funcs for setting the XEvent (follow the trend for setting keyboard event).
2. Add better comment for why setNPWindowRect is not called multiple times for windowed/full mode.
3. Add comment for -offset
4. s/hasBackingStore/hasValidBackingStore
5. Display detection code moved to getPluginDisplay()
6. Use QX11Info * in platformStart()
------- Comment #29 From 2009-09-29 23:19:43 PST -------
Created an attachment (id=40345) [details]
Windowless patch - painting and event handling (6)

Some updates.

Context menu problem - This is a gtk/qt integration issue and is not a problem that can be solved by this patch. If you use gtk inside Qt application, we are unable to show a gtk context menu. I am not 100% sure why but this appears to be because of gtk failing to grab mouse since Qt has grabbed it. Problem reproducible with diamondx plugin.

GrabWidget - works, with no changes - yay, we can see flash in screenshots :)

Also, I ended up modifying the original patch with the following changes (after reading mozilla code)
1. Implement enter/leave notifications
2. Implement focusin/out.
3. I also made the setX... functions pointer based - I couldn't find any coding policy on this for WebKit but the Qt way atleast is to pass a pointer (instead of a ref) when the object gets modified.
------- Comment #30 From 2009-09-30 01:10:43 PST -------
Here's the bug list, so it can help us decide if we want to land it:
1. Focusing the plugin does not defocus webview. The focus handling is correct and the keyevents are sent correctly. But the lineedit/input cursor keeps blinking. (Side note: There seems to be a general focus problem where the cursor does not blink at all when used with plugins - unrelated to this patch)
2. Flash get confused when the mouse release happens outside the plugin (because we don't generate a release).
3. When a messagebox pops up, right when the flash gets focus (because of the onblur handler of input) it will start selecting text in lineedit/input even when no mouse button is pressed. Strangely, this happens in chrome too.
4. QGraphicsWebView does not work
5. Printing crashes
6. Using ARGB visuals crashes
------- Comment #31 From 2009-09-30 01:55:16 PST -------
(In reply to comment #30)
> Here's the bug list, so it can help us decide if we want to land it:
> 1. Focusing the plugin does not defocus webview. The focus handling is correct
> and the keyevents are sent correctly. But the lineedit/input cursor keeps
> blinking. (Side note: There seems to be a general focus problem where the
> cursor does not blink at all when used with plugins - unrelated to this patch)

With windowed plugins this works because the focus is transferred on X level and QWebView/QWebPage receive a FocusOut event, that will deactivate the focuscontroller. We may have to emulate that behaviour.

> 6. Using ARGB visuals crashes

Oops, given the popularity of composition managers these days that's a problem.

Would a backingstore-less painting help here?
------- Comment #32 From 2009-09-30 02:36:53 PST -------
> With windowed plugins this works because the focus is transferred on X level
> and QWebView/QWebPage receive a FocusOut event, that will deactivate the
> focuscontroller. We may have to emulate that behaviour.
> 
I have tried to do this, but doesn't help. See the initial lines of 
PluginView::handleMouseEvent. Do I need to do anything else?

> > 6. Using ARGB visuals crashes
> 
> Oops, given the popularity of composition managers these days that's a problem.
> 
> Would a backingstore-less painting help here?

To clarify, the existence of a compmgr does not crash webkit. It crashes only
when Qt uses ARGB visuals (which I think is not so common) i.e when
Qt::WA_TranslucentWindow is used.
------- Comment #33 From 2009-09-30 02:57:27 PST -------
(In reply to comment #32)
> > With windowed plugins this works because the focus is transferred on X level
> > and QWebView/QWebPage receive a FocusOut event, that will deactivate the
> > focuscontroller. We may have to emulate that behaviour.
> > 
> I have tried to do this, but doesn't help. See the initial lines of 
> PluginView::handleMouseEvent. Do I need to do anything else?

Ahh, I found the potential solution: Try using PluginView::focusPluginElement().

> > > 6. Using ARGB visuals crashes
> > 
> > Oops, given the popularity of composition managers these days that's a problem.
> > 
> > Would a backingstore-less painting help here?
> 
> To clarify, the existence of a compmgr does not crash webkit. It crashes only
> when Qt uses ARGB visuals (which I think is not so common) i.e when
> Qt::WA_TranslucentWindow is used.

Hmm, I see... you're right, that makes it slightly less vulnerable. Not a showstopper then IMHO. But would be great to at least avoid the crash if it's easy to detect :) (maybe by comparing the default depths of qt and gtk pixmaps?)
------- Comment #34 From 2009-09-30 22:37:03 PST -------
Created an attachment (id=40422) [details]
Windowless patch - painting and event handling (7)

Can we try to land this and get some blackteam testing?

Changes since last patch:
1. Don't crash when printing. We just skip painting for the time being.
2. We provide a 'white' background for transparent plugins when we have no backing store.

At this point, we are feature par with the windowed mode plugin :)
------- Comment #35 From 2009-10-01 05:02:01 PST -------
Some good news and some depressing news.

The good news - I had initially tried to get the plugin to paint to a 32-bit QPixmap. I could not succeed because it was giving me X match errors. I have now figured the problem and solved it. I am able to get DiamondX to paint, even!  The beauty of this approach is that it's very clean and we can get it to work in qwebview, graphicsview, printing all in a single shot.

The depressing news - Flash does not paint. No errors, nothing. :|

Anyone has friends at Adobe, I can get in touch with :) ? Otherwise, it's back to 24-bit QPixmap and all sort of hackery to make it work with different paint devices.
------- Comment #36 From 2009-10-01 12:33:55 PST -------
(From update of attachment 40422 [details])
> +#include <QLabel>

I guess this isn't really needed ;-)

> +            XCopyArea(QX11Info::display(), backingStorePixmap->handle(), m_drawable.handle(), gc, 
> +                offset.x() + m_windowRect.x() + m_clipRect.x(), offset.y() + m_windowRect.y() + m_clipRect.y(), 

There's a trailing whitespace after the above two lines!!!

(OMG, I would've never thought I'd get that anal in a patch review ;-)

>      setCallingPlugin(true);
> -    bool accepted = m_plugin->pluginFuncs();
> +    bool accepted = m_plugin->pluginFuncs()->event(m_instance, &event);

This is my favourite part of the patch.

> +    const QX11Info *x11Info = 0;

Coding style :)


Regardless of my silly style picks this patch is a great piece of work!
------- Comment #37 From 2009-10-02 02:06:12 PST -------
Created an attachment (id=40500) [details]
Windowless patch - painting and event handling (8)

Changes since last patch
   * Simon's last review (coding style, whitespace)
   * Fixes crash reported by Simon in some sites (e.g, facebook/biotronic game)
   * Don't paint anything when the x11 graphics system is not being used i.e check if pixmap handle is valid.
       - This can be fixed by using plain X Pixmap. However, I will do this later.
------- Comment #38 From 2009-10-04 07:49:22 PST -------
So what is left on the TODO now, before this can be considered for inclusion?

Does it work with zoom? does it just zoom it as it was an image or actually set another size on the plugin? What does firefox do here?
------- Comment #39 From 2009-10-04 20:23:24 PST -------
in firefox, the associated layout element for a given plugin (<object> or <embed> tags) will be zoomed and the new size will bubbled until it reaches the embeded xwindow plugin (so flash would be resized accordingly to page zoom)

> Does it work with zoom? does it just zoom it as it was an image or actually set
> another size on the plugin? What does firefox do here?
------- Comment #40 From 2009-10-04 21:40:39 PST -------
Kenneth, thanks for reminding me about zooming :) I forgot to test it with that.

Important history is being lost, so I put up the patch in http://gitorious.org/~girish/qtwebkit/girishs-clone/commits/windowless. Changes include:
1. Use X Pixmaps (93c3aaf)
2. Add PluginQuirkUsesScreenDefaultVisual for Flash (654645). Inspired by Kenneth's suggestion on irc.
3. Make zoom work (06e79ab)

I think it's ready for landing. Here are the pending bugs:
1. Printing does not work (but no crash).
2. -graphicssystem opengl crashes. We cannot paint X11 Pixmap using QPainter. I will disable this in a separate patch (since we need this history)
3. QGVLauncher crashes on exit - WebCore::Chrome::repaint uses the client. The client is alreay gone by then.
4. QGraphicsWebView works, yay! But I need a way to detect painting on qgv and turn off transparency. Any suggestions?
5. When using ARGB visuals on tlw, mouse over crashes. 

In the process of debugging 5, I think I have found a bug. It appears that svn commit 48604 (0292c6) has introduced a bug - it calls the view's winId(). This actually ends up creating a native window. I don't think is what we want. For the windowless case, we want the window's winId (and not the views). Even in the windowed case, only the container needs to have a winId and not the view itself. Simon, Kenneth, is that correct?
------- Comment #41 From 2009-10-05 04:22:38 PST -------
Patches with the change log are at http://gitorious.org/~girish/qtwebkit/girishs-clone/commits/windowless_20081
------- Comment #42 From 2009-10-05 04:54:44 PST -------
Created an attachment (id=40625) [details]
Patch 1/4 - [Qt] Add support for windowless NPAPI plugins
------- Comment #43 From 2009-10-05 04:55:28 PST -------
Created an attachment (id=40626) [details]
Patch 2/4 - Use X Pixmap instead of QPixmap.
------- Comment #44 From 2009-10-05 04:56:33 PST -------
Created an attachment (id=40627) [details]
Patch 3/4 - Add PluginQuirkUsesScreenDefaultVisual for Flash.
------- Comment #45 From 2009-10-05 04:57:09 PST -------
Created an attachment (id=40628) [details]
Patch 4/4 -  Make painting and events work when page is zoomed.
------- Comment #46 From 2009-10-05 09:15:41 PST -------
(From update of attachment 40626 [details])
tools like git-send-bugzilla or bugzilla-tool post-diff/post-commits can take care of setting the correct mime type and patch checkbox for you.
------- Comment #47 From 2009-10-06 00:22:45 PST -------
(From update of attachment 40626 [details])
The ChangeLog needs a little tweak though to mention that this is in the Qt windowless plugin support. I'll fix that during landing.
------- Comment #48 From 2009-10-06 03:29:17 PST -------
(From update of attachment 40627 [details])
There are a few coding style issues left and one detail:

> +
> +        No new tests. (OOPS!)

Please remove this line :)



> +                                   ((NPSetWindowCallbackStruct *)m_npWindow.ws_info)->depth);

Coding style :)


> +    const int drawableDepth = ((NPSetWindowCallbackStruct *)m_npWindow.ws_info)->depth;

Same here.


> +#ifndef QT_NO_XRENDER
> +    static const bool useXrender = qgetenv("QT_X11_NO_XRENDER").isNull(); // Should also check for XRender >= 0.5
> +#else
> +    static const bool useXrender = false;
> +#endif

Kenneth suggested on chat that this should perhaps be useXRender?
------- Comment #49 From 2009-10-06 04:32:07 PST -------
Created an attachment (id=40709) [details]
Patch 3/4 - Add PluginQuirkUsesScreenDefaultVisual for Flash (Take 2)

Renamed quirk as discussed on irc.
Fixed coding style.
Renamed to useXRender
Added "[Qt] Windowless plugins: " prefix
------- Comment #50 From 2009-10-06 04:33:37 PST -------
Created an attachment (id=40710) [details]
Patch 4/4 -  Make painting and events work when page is zoomed. (Take 2)

Added "[Qt] Windowless plugins: " prefix
------- Comment #51 From 2009-10-06 04:40:44 PST -------
Created an attachment (id=40711) [details]
Enable printing

Bug in Qt has been fixed by Samuel. It is already part of 4.6 branch.
------- Comment #52 From 2009-10-06 04:43:23 PST -------
(From update of attachment 40709 [details])

Minor coding style issue left, but I'll fix that when landing.

> +        QPixmap* backingStorePixmap = static_cast<QPixmap *>(backingStoreDevice);
------- Comment #53 From 2009-10-06 04:54:31 PST -------
All patches landed in 49158, 49159, 49169, 49170 and 49171
------- Comment #54 From 2009-10-06 05:50:38 PST -------
Here's the TODO list:
1. -graphicssystem opengl crashes. UPDATE: It turns out that it crashes only on
my machine. This is supported by Qt.
2. QGVLauncher crashes on exit - WebCore::Chrome::repaint uses the client. The
client is alreay gone by then.
3. QGraphicsWebView works, yay! But I need a way to detect painting on qgv and
turn off transparency. Any suggestions? UPDATE: Will use pluginParent() and do
a ugly qobject_cast
4. When using ARGB visuals on tlw, mouse over crashes.  UPDATE: Should be
solved with the introduction of pluginParent() and removing winId().
5. Remove the non-transparent sync for opaque mode.
6. Context menu. UPDATE: This requires integration of context menus of gtk
within Qt. The toolkits have to sync up on mouse/keyboard grabs. Qt has to be
fixed, the Qt developers are aware of this problem (but have no solution). No
fix is required in WebKit.

Simon and I agreed that this bug is exhausted. So, I will create separate bugs
for the above TODO list items.
------- Comment #55 From 2009-10-06 09:39:44 PST -------
The QGVLauncher has support for flipping the view upside down.

Doing this with a windowless plugin results in the page being painted all wrong :-)

Can be tested with http://www.popup-killer-review.com/windowless-swf.htm
------- Comment #56 From 2009-10-06 21:14:47 PST -------
(In reply to comment #55)
> The QGVLauncher has support for flipping the view upside down.
> 
> Doing this with a windowless plugin results in the page being painted all wrong
> :-)
> 
> Can be tested with http://www.popup-killer-review.com/windowless-swf.htm

Indeed, it's TODO item 3 :)

I have a patch at https://bugs.webkit.org/show_bug.cgi?id=30149
------- Comment #57 From 2009-10-07 21:00:23 PST -------
A note on the quirk mode added to flash. The problem is that flash appears to be using gdk_visual_get_system() to draw to the Drawable instead of the visual provided by us. As a result, when provided with transparent pixmap (which requires a 32-bit visual) flash fails because it uses the system default visual (for example, 24-bit).

To verify that is indeed the case, I shadowed the above gdk symbol (gdk_visual_get_system) to return a 32-bit visual. And it works with transparent pixmaps. This 'hack' was found unsuitable for Qt/WebKit since many libraries have started using -Bsymbolic-functions and probably not possible with all platforms/compilers.
------- Comment #58 From 2009-10-08 10:44:25 PST -------
Comparing the calls to plugins in PluginView.cpp and in PluginViewQt.cpp I noticed 2 differences and I was wondering if they are intentional

In PluginView.cpp:
    JSC::JSLock::DropAllLocks dropAllLocks(<b>JSC::SilenceAssertionsOnly</b>);
    ...
    PluginView::setCurrentPluginView(0);

In PluginViewQt.cpp:
    JSC::JSLock::DropAllLocks dropAllLocks(<b>false</b>);
and we never call 
    PluginView::setCurrentPluginView(0);
------- Comment #59 From 2009-10-09 00:42:07 PST -------
(In reply to comment #58)
> Comparing the calls to plugins in PluginView.cpp and in PluginViewQt.cpp I
> noticed 2 differences and I was wondering if they are intentional
> 
> In PluginView.cpp:
>     JSC::JSLock::DropAllLocks dropAllLocks(<b>JSC::SilenceAssertionsOnly</b>);
>     ...
>     PluginView::setCurrentPluginView(0);
> 
> In PluginViewQt.cpp:
>     JSC::JSLock::DropAllLocks dropAllLocks(<b>false</b>);
> and we never call 
>     PluginView::setCurrentPluginView(0);

It's not intentional. 

I don't know anything about the above code other than it locks js code for mulithread access. There is no bool argument to dropAllLocks, so it appears that it is wrong. And setCurrentPluginView(0) is also appears to be needed (I can't see what it is). I will create patch.