[Webkit-unassigned] [Bug 48509] [GTK] Implement UpdateChunk, ChunkedUpdateDrawingArea/Proxy, WebView classes for WebKit2 and add WKAPI

[bugzilla-daemon at webkit.org]

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





--- Comment #18 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-01-14 09:29:13 PST ---
(In reply to comment #16) 
> > You can use G_DEFINE_TYPE() macro that already does this and other things for you.
> As per discussion with mrobinson, we decided not to use G_DEFINE_TYPE so that we can be in sync with WebKit style rather than glib.

I disagree, but at least you could use the current implementation of the G_DEFINE_TYPE macro, using the webkit coding style it would be something like:

GType webkitWebViewGetType()
{
    static volatile gsize gDefineTypeIdVolatile = 0;
    if (g_once_init_enter(&gDefineTypeIdVolatile)) {
        GType gDefineTypeId = g_type_register_static_simple(GTK_TYPE_CONTAINER,
                                                                                            g_intern_static_string("WebKitWebView"),
                                                                                            sizeof(WebKitWebViewClass),
                                                                                            reinterpret_cast<GClassInitFunc>(webkitWebViewClassInit),
                                                                                            sizeof(WebKitWebView),
                                                                                            reinterpret_cast<GInstanceInitFunc>(webkitWebViewInit),
                                                                                            static_cast<GTypeFlags>(0));
        g_once_init_leave(&gDefineTypeIdVolatile, gDefineTypeId);
    }
    return gDefineTypeIdVolatile;
}

> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:35
> > > +    GTK_WIDGET_SET_FLAGS(widget, GTK_REALIZED);
> > 
> > GTK_WIDGET_SET_FLAGS is deprecated, use gtk_widget_set_realized() instead.
> Done.
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:42
> > > +    attributes.x = widget->allocation.x;
> > > +    attributes.y = widget->allocation.y;
> > > +    attributes.width = widget->allocation.width;
> > > +    attributes.height = widget->allocation.height;
> > 
> > The widget inherits from GtkContainer so I think we should take into account the border width. Also GtkWidget::allocation is private now, you should use gtk_widget_get_allocation(). This should be something like:
> > 
> > borderWidth = gtk_container_get_border_width(GTK_CONTAINER(widget));
> > gtk_widget_get_allocation(widget, &allocation);
> > attributes.x = allocation.x + borderWidth;
> > attributes.y = allocation.y + borderWidth;
> > attributes.width = allocation.width - borderWidth * 2;
> > attributes.height = allocation.height - borderWidth * 2;
> Done.
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:52
> > 
> > 
> > This is deprecated too because GtkWidget::style is private, use gtk_widget_style_attach() instead
> Done.
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:53
> > > +    gtk_style_set_background(widget->style, widget->window, GTK_STATE_NORMAL);
> > 
> > Use gtk_widget_get_style() here
> Done.
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:62
> > 
> > 
> > GTK_WIDGET_REALIZED() is deprecated, use gtk_widget_get_realized() instead
> Done.
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:63
> > 
> > 
> > Use gtk_widget_get_window() here
> Done.
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:64
> > > +    gtk_widget_set_parent(widget, GTK_WIDGET(container));
> > 
> > I think this is enough, why do you need to call set_parent_window too?
> Done.
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:83
> > > +static void
> > > +webkitWidgetViewFinalize(GObject* obj)
> > > +{
> > > +    G_OBJECT_CLASS(parentClass)->finalize(obj);
> > > +}
> > 
> > You don't need this, the parent finalize will be called anyway if you don't implement finalize()
> Done. Since we have no other member to be freed in finalize, removed the callback itself.
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:85
> > > +static void webkitWidgetViewClassInit(gpointer widgetViewParentClass, __attribute__((unused)) gpointer widgetViewClassData)
> > 
> > This can simply be webkitWidgetViewClassInit(WebKitWidgetViewClass* klass)
> Done.
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:93
> > > +    widgetClass->expose_event = WebKit::WebView::webViewOnExpose;
> > 
> > Expose is deprecated too, we should use #ifdefs here to use expose_event in gtk2 and draw in gtk3
> Done. The draw is right now stubbed. We will try to put the properly tested draw() implementation ASAP (probably along with reworked UpdateChunk patch) 
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:114
> > > +static void webkitWidgetViewInit(GTypeInstance* instance, __attribute__((unused)) gpointer widgetViewClass)
> > 
> > this can simply be static void webkitWidgetViewInit(WebKitWidgetView* webView)
> Done.
> > 
> > > WebKit2/UIProcess/gtk/WebViewInternal.cpp:126
> > > +GType
> > > +webkitWidgetViewGetType(void)
> > 
> > Use G_DEFINE_TYPE macro instead
> As explained earlier, decided not to use this macro.
> > 
> > > WebKit2/UIProcess/gtk/WebView.cpp:36
> > > +#include "WebViewInternal.h"
> > 
> > Why WebView and WebViewInternal?
> Basically, WebView class implements interface PageClient and holds the native window handle. WebViewInternal is the gobject which captures the actual WebView (GtkWidget). I have now renamed WebViewInternal to WebKitWebView (as in WebKit1 since it does the same job as WebKitWebView).

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