[Webkit-unassigned] [Bug 116717] [GTK] Do not use X11 WidgetBackingStore implementation in Wayland

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=116717


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #203042|review?                     |review-
               Flag|                            |




--- Comment #9 from Martin Robinson <mrobinson at webkit.org>  2013-05-28 09:52:42 PST ---
(From update of attachment 203042)
View in context: https://bugs.webkit.org/attachment.cgi?id=203042&action=review

Nice work. I have a few small issues with the design, but the general approach looks great.

> Source/WebCore/platform/cairo/WidgetBackingStore.cpp:44
> +namespace WebCore {
> +
> +PassOwnPtr<WidgetBackingStore> WidgetBackingStore::create(PlatformWidget widget, const IntSize& size)
> +{
> +#if PLATFORM(GTK) && !defined(GTK_API_VERSION_2)
> +    GdkDisplay* display = gdk_display_manager_get_default_display(gdk_display_manager_get());
> +    if (GDK_IS_X11_DISPLAY(display))
> +        return WebCore::GtkWidgetBackingStoreX11::create(widget, size);
> +#endif
> +    return WebCore::WidgetBackingStoreCairo::create(widget, size);
> +}

Instead of putting all this GTK+ specific code here, just move it to WebKit. Have the GTK+ implementation do the check and have the EFL implementation just instantiate a Cairo backing store. Since this code is mainly GTK+-specific it probably should not go into the cairo directory.

> Source/WebCore/platform/cairo/WidgetBackingStoreCairo.h:26
> +class WidgetBackingStorePrivate;

You can now move the data that was once stored in WidgetBackingStorePrivate to be a member of this concrete class and ditch the private class entirely.

> Source/WebCore/platform/gtk/GtkWidgetBackingStoreX11.cpp:20
> +#include "GtkWidgetBackingStoreX11.h"

Probably better to call this WidgetBackingStoreGtkX11. I know the filename if funky (that's my fault, feel free to fix it or not), but let's keep the class names a bit more consistent. :)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.