There's common code for X11, but the GTK specific part is still missing.
Created attachment 92577 [details] Patch
Comment on attachment 92577 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=92577&action=review Looking good. Just a couple comments. > Source/WebKit2/WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:224 > +#elif PLATFORM(GTK) > + RefPtr<cairo_surface_t> drawableSurface = adoptRef(cairo_xlib_surface_create(x11Display(), > + m_drawable, > + static_cast<NPSetWindowCallbackStruct*>(m_npWindow.ws_info)->visual, > + m_frameRect.width(), > + m_frameRect.height())); Wouldn't it be better to wrap this surface in a Cairo surface after the calls to NPP_HandleEvent and XSync, like Qt? > Source/WebKit2/WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:267 > + cairo_rectangle(cr, > + m_frameRect.x() + exposedRect.x(), m_frameRect.y() + exposedRect.y(), > + exposedRect.width(), exposedRect.height()); Do you mind just putting this one on one line? 120 character is generally okay. Sorry for the nit. :)
(In reply to comment #2) > (From update of attachment 92577 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=92577&action=review > > Looking good. Just a couple comments. > > > Source/WebKit2/WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:224 > > +#elif PLATFORM(GTK) > > + RefPtr<cairo_surface_t> drawableSurface = adoptRef(cairo_xlib_surface_create(x11Display(), > > + m_drawable, > > + static_cast<NPSetWindowCallbackStruct*>(m_npWindow.ws_info)->visual, > > + m_frameRect.width(), > > + m_frameRect.height())); > > Wouldn't it be better to wrap this surface in a Cairo surface after the calls to NPP_HandleEvent and XSync, like Qt? I'm not sure I understand what you mean by "wrap this surface in a Cairo surface". I tried to follow the current webkit1 approach. > > Source/WebKit2/WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:267 > > + cairo_rectangle(cr, > > + m_frameRect.x() + exposedRect.x(), m_frameRect.y() + exposedRect.y(), > > + exposedRect.width(), exposedRect.height()); > > Do you mind just putting this one on one line? 120 character is generally okay. Sorry for the nit. :) Sure, no problem
(In reply to comment #3) > > Wouldn't it be better to wrap this surface in a Cairo surface after the calls to NPP_HandleEvent and XSync, like Qt? > I'm not sure I understand what you mean by "wrap this surface in a Cairo surface". I tried to follow the current webkit1 approach. Sorry. I should have said wrap m_drawable in a Cairo surface. In the Qt port they wait until right before blitting to do this. Perhaps it isn't possible with the XLib backend, but couldn't the surface be out of sync according to the API. In any case, I think it's better to mirror the Qt port for clarity, unless it's specifically an error in this case. You could use m_pluginDisplay instead of x11Display() if you did.
Created attachment 92591 [details] Updated patch
Comment on attachment 92591 [details] Updated patch Thanks!
Committed r85953: <http://trac.webkit.org/changeset/85953>