[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 #12 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-01-12 02:11:18 PST ---
(From update of attachment 78233)
View in context: https://bugs.webkit.org/attachment.cgi?id=78233&action=review

Unofficial review

> WebKit2/ChangeLog:9
> +        * UIProcess/gtk/WebView.cpp: Added. The native GtkWidget handle for each WKViewRef for GTK port.

Shouldn't this be in UIProcess/API/gtk ?

> WebKit2/UIProcess/gtk/WebViewInternal.cpp:31
> +static GObjectClass *parentClass = 0;

You can use G_DEFINE_TYPE() macro that already does this and other things for you.

> 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.

> 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;

> WebKit2/UIProcess/gtk/WebViewInternal.cpp:52


This is deprecated too because GtkWidget::style is private, use gtk_widget_style_attach() instead

> WebKit2/UIProcess/gtk/WebViewInternal.cpp:53
> +    gtk_style_set_background(widget->style, widget->window, GTK_STATE_NORMAL);

Use gtk_widget_get_style() here

> WebKit2/UIProcess/gtk/WebViewInternal.cpp:62


GTK_WIDGET_REALIZED() is deprecated, use gtk_widget_get_realized() instead

> WebKit2/UIProcess/gtk/WebViewInternal.cpp:63


Use gtk_widget_get_window() here

> 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?

> 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()

> WebKit2/UIProcess/gtk/WebViewInternal.cpp:85
> +static void webkitWidgetViewClassInit(gpointer widgetViewParentClass, __attribute__((unused)) gpointer widgetViewClassData)

This can simply be webkitWidgetViewClassInit(WebKitWidgetViewClass* klass)

> 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

> WebKit2/UIProcess/gtk/WebViewInternal.cpp:114
> +static void webkitWidgetViewInit(GTypeInstance* instance, __attribute__((unused)) gpointer widgetViewClass)

this can simply be static void webkitWidgetViewInit(WebKitWidgetView* webView)

> WebKit2/UIProcess/gtk/WebViewInternal.cpp:126
> +GType
> +webkitWidgetViewGetType(void)

Use G_DEFINE_TYPE macro instead

> WebKit2/UIProcess/gtk/WebView.cpp:36
> +#include "WebViewInternal.h"

Why WebView and WebViewInternal?

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