Bug 20081 - [Qt] Add support for windowless NPAPI plugins
: [Qt] Add support for windowless NPAPI plugins
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: Plug-ins
: 528+ (Nightly build)
: PC Linux
: P2 Enhancement
Assigned To: Nobody
http://www.communitymx.com/content/so...
: Qt
Depends on:
Blocks: 24157 Qt46 30149 30170 30207 30214 30248 30251 30355 30356
  Show dependency treegraph
 
Reported: 2008-07-17 10:04 PDT by marcoil
Modified: 2014-04-18 16:15 PDT (History)
11 users (show)

See Also:


Attachments
Zattoo's modified PluginViewWin.cpp file (26.17 KB, text/x-c++src)
2008-07-18 12:04 PDT, Chris Kurecka
no flags Details
Zattoo's modified PluginView.h file (9.25 KB, text/x-chdr)
2008-07-18 12:05 PDT, Chris Kurecka
no flags Details
patch, not for review (18.89 KB, patch)
2009-07-28 13:34 PDT, Yael
no flags Details | Formatted Diff | Diff
Implement keyboard event forwarding for windowless plugins (4.94 KB, patch)
2009-08-03 14:14 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Implement keyboard event forwarding for windowless plugins (4.98 KB, patch)
2009-08-03 14:29 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Windowless patch - makes painting work (14.20 KB, patch)
2009-09-25 03:02 PDT, Girish Ramakrishnan
no flags Details | Formatted Diff | Diff
Windowless patch - makes painting work (2) (11.21 KB, patch)
2009-09-25 05:58 PDT, Girish Ramakrishnan
no flags Details | Formatted Diff | Diff
Windowless patch - makes painting work (3) (11.83 KB, patch)
2009-09-27 22:01 PDT, Girish Ramakrishnan
hausmann: review-
Details | Formatted Diff | Diff
Windowless patch - painting and event handling (4) (15.32 KB, patch)
2009-09-29 04:41 PDT, Girish Ramakrishnan
no flags Details | Formatted Diff | Diff
Windowless patch - painting and event handling (5) (15.76 KB, patch)
2009-09-29 06:40 PDT, Girish Ramakrishnan
no flags Details | Formatted Diff | Diff
Windowless patch - painting and event handling (6) (22.15 KB, patch)
2009-09-29 23:19 PDT, Girish Ramakrishnan
no flags Details | Formatted Diff | Diff
Windowless patch - painting and event handling (7) (22.80 KB, patch)
2009-09-30 22:37 PDT, Girish Ramakrishnan
no flags Details | Formatted Diff | Diff
Windowless patch - painting and event handling (8) (23.01 KB, patch)
2009-10-02 02:06 PDT, Girish Ramakrishnan
no flags Details | Formatted Diff | Diff
Patch 1/4 - [Qt] Add support for windowless NPAPI plugins (23.26 KB, patch)
2009-10-05 04:54 PDT, Girish Ramakrishnan
hausmann: review+
Details | Formatted Diff | Diff
Patch 2/4 - Use X Pixmap instead of QPixmap. (6.46 KB, patch)
2009-10-05 04:55 PDT, Girish Ramakrishnan
hausmann: review+
Details | Formatted Diff | Diff
Patch 3/4 - Add PluginQuirkUsesScreenDefaultVisual for Flash. (11.69 KB, patch)
2009-10-05 04:56 PDT, Girish Ramakrishnan
hausmann: review-
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 PDT, Girish Ramakrishnan
no flags Details | Formatted Diff | Diff
Patch 3/4 - Add PluginQuirkUsesScreenDefaultVisual for Flash (Take 2) (11.67 KB, patch)
2009-10-06 04:32 PDT, Girish Ramakrishnan
hausmann: review+
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 PDT, Girish Ramakrishnan
hausmann: review+
Details | Formatted Diff | Diff
Enable printing (2.31 KB, patch)
2009-10-06 04:40 PDT, 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 marcoil 2008-07-17 10:04:43 PDT
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 Chris Kurecka 2008-07-18 12:04:56 PDT
Created attachment 22367 [details]
Zattoo's modified PluginViewWin.cpp file
Comment 2 Chris Kurecka 2008-07-18 12:05:28 PDT
Created attachment 22368 [details]
Zattoo's modified PluginView.h file
Comment 3 Chris Kurecka 2008-07-18 12:08:31 PDT
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 marcoil 2008-07-18 18:41:20 PDT
Me neither, but don't worry about it.
Comment 5 marcoil 2008-07-31 09:51:19 PDT
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 marcoil 2009-03-26 03:13:12 PDT
I'm no longer working on this, so taking me out.
Comment 7 Yael 2009-07-28 13:34:05 PDT
Created attachment 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 Kenneth Rohde Christiansen 2009-08-03 14:12:04 PDT
Hi Yael, I cleaned up some of your event handling code. Please consult my upcoming patch.
Comment 9 Kenneth Rohde Christiansen 2009-08-03 14:14:27 PDT
Created attachment 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 Kenneth Rohde Christiansen 2009-08-03 14:29:07 PDT
Created attachment 34004 [details]
Implement keyboard event forwarding for windowless plugins

ChangeLog fixes
Comment 11 Simon Hausmann 2009-08-03 14:37:33 PDT
Comment on attachment 34004 [details]
Implement keyboard event forwarding for windowless plugins

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 Kenneth Rohde Christiansen 2009-08-03 14:58:34 PDT
Comment on attachment 34004 [details]
Implement keyboard event forwarding for windowless plugins

Landed in 46733
Comment 13 Girish Ramakrishnan 2009-09-17 01:07:54 PDT
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 Kenneth Rohde Christiansen 2009-09-17 06:09:50 PDT
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 Girish Ramakrishnan 2009-09-17 07:20:30 PDT
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 Girish Ramakrishnan 2009-09-21 10:17:41 PDT
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 Kenneth Rohde Christiansen 2009-09-21 11:09:57 PDT
(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 Girish Ramakrishnan 2009-09-25 03:02:38 PDT
Created attachment 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 Girish Ramakrishnan 2009-09-25 03:26:17 PDT
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 Simon Hausmann 2009-09-25 03:45:09 PDT
(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 Girish Ramakrishnan 2009-09-25 03:57:21 PDT
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 Girish Ramakrishnan 2009-09-25 05:58:50 PDT
Created attachment 40107 [details]
Windowless patch - makes painting work (2)

* Moved gdk display detection code to PluginViewQt (Simon's suggestion)
* Fixed coding style
Comment 23 Kenneth Rohde Christiansen 2009-09-25 06:50:22 PDT
(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 Girish Ramakrishnan 2009-09-27 22:01:04 PDT
Created attachment 40216 [details]
Windowless patch - makes painting work (3)

Scrolling works now. The rendering is now ready for review.
Comment 25 Girish Ramakrishnan 2009-09-27 22:05:11 PDT
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 Simon Hausmann 2009-09-28 06:12:45 PDT
Comment on attachment 40216 [details]
Windowless patch - makes painting work (3)

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 Girish Ramakrishnan 2009-09-29 04:41:47 PDT
Created attachment 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 Girish Ramakrishnan 2009-09-29 06:40:55 PDT
Created attachment 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 Girish Ramakrishnan 2009-09-29 23:19:43 PDT
Created attachment 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 Girish Ramakrishnan 2009-09-30 01:10:43 PDT
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 Simon Hausmann 2009-09-30 01:55:16 PDT
(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 Girish Ramakrishnan 2009-09-30 02:36:53 PDT
> 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 Simon Hausmann 2009-09-30 02:57:27 PDT
(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 Girish Ramakrishnan 2009-09-30 22:37:03 PDT
Created attachment 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 Girish Ramakrishnan 2009-10-01 05:02:01 PDT
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 Simon Hausmann 2009-10-01 12:33:55 PDT
Comment on attachment 40422 [details]
Windowless patch - painting and event handling (7)

> +#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 Girish Ramakrishnan 2009-10-02 02:06:12 PDT
Created attachment 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 Kenneth Rohde Christiansen 2009-10-04 07:49:22 PDT
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 Antonio Gomes 2009-10-04 20:23:24 PDT
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 Girish Ramakrishnan 2009-10-04 21:40:39 PDT
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 Girish Ramakrishnan 2009-10-05 04:22:38 PDT
Patches with the change log are at http://gitorious.org/~girish/qtwebkit/girishs-clone/commits/windowless_20081
Comment 42 Girish Ramakrishnan 2009-10-05 04:54:44 PDT
Created attachment 40625 [details]
Patch 1/4 - [Qt] Add support for windowless NPAPI plugins
Comment 43 Girish Ramakrishnan 2009-10-05 04:55:28 PDT
Created attachment 40626 [details]
Patch 2/4 - Use X Pixmap instead of QPixmap.
Comment 44 Girish Ramakrishnan 2009-10-05 04:56:33 PDT
Created attachment 40627 [details]
Patch 3/4 - Add PluginQuirkUsesScreenDefaultVisual for Flash.
Comment 45 Girish Ramakrishnan 2009-10-05 04:57:09 PDT
Created attachment 40628 [details]
Patch 4/4 -  Make painting and events work when page is zoomed.
Comment 46 Eric Seidel 2009-10-05 09:15:41 PDT
Comment on attachment 40626 [details]
Patch 2/4 - Use X Pixmap instead of QPixmap.

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 Simon Hausmann 2009-10-06 00:22:45 PDT
Comment on attachment 40626 [details]
Patch 2/4 - Use X Pixmap instead of QPixmap.

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 Simon Hausmann 2009-10-06 03:29:17 PDT
Comment on attachment 40627 [details]
Patch 3/4 - Add PluginQuirkUsesScreenDefaultVisual for Flash.

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 Girish Ramakrishnan 2009-10-06 04:32:07 PDT
Created attachment 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 Girish Ramakrishnan 2009-10-06 04:33:37 PDT
Created attachment 40710 [details]
Patch 4/4 -  Make painting and events work when page is zoomed. (Take 2)

Added "[Qt] Windowless plugins: " prefix
Comment 51 Girish Ramakrishnan 2009-10-06 04:40:44 PDT
Created attachment 40711 [details]
Enable printing

Bug in Qt has been fixed by Samuel. It is already part of 4.6 branch.
Comment 52 Simon Hausmann 2009-10-06 04:43:23 PDT
Comment on attachment 40709 [details]
Patch 3/4 - Add PluginQuirkUsesScreenDefaultVisual for Flash (Take 2)


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

> +        QPixmap* backingStorePixmap = static_cast<QPixmap *>(backingStoreDevice);
Comment 53 Simon Hausmann 2009-10-06 04:54:31 PDT
All patches landed in 49158, 49159, 49169, 49170 and 49171
Comment 54 Girish Ramakrishnan 2009-10-06 05:50:38 PDT
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 Kenneth Rohde Christiansen 2009-10-06 09:39:44 PDT
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 Girish Ramakrishnan 2009-10-06 21:14:47 PDT
(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 Girish Ramakrishnan 2009-10-07 21:00:23 PDT
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 Yael 2009-10-08 10:44:25 PDT
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 Girish Ramakrishnan 2009-10-09 00:42:07 PDT
(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.
Comment 60 Wade Colson 2014-04-18 16:15:14 PDT
*** Bug 260998 has been marked as a duplicate of this bug. ***
Seen from the domain http://volichat.com
Page where seen: http://volichat.com/adult-chat-rooms
Marked for reference. Resolved as fixed @bugzilla.