[Webkit-unassigned] [Bug 67990] [GTK] Use WebKitWebContext in WebKitWebView

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

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




--- Comment #4 from Martin Robinson <mrobinson at webkit.org>  2011-09-20 15:24:37 PST ---
(From update of attachment 107986)
View in context: https://bugs.webkit.org/attachment.cgi?id=107986&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:153
> +/* Private API */

it should be apparent because it's in WebKit coding style.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp:154
> +WKContextRef webkitWebContextGetContext(WebKitWebContext* context)

Probably better to call it something like: webkitWebContextGetWKContextRef to eliminate ambiguity.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:49
> +    webkitWebViewBaseCreateWebPage(WEBKIT_WEB_VIEW_BASE(webView), webkitWebContextGetContext(webView->priv->context), 0);

Might as well do:
webkitWebViewBaseCreateWebPage(WEBKIT_WEB_VIEW_BASE(webView), 
                               webkitWebContextGetContext(WEBKIT_WEB_VIEW(webView->priv->context)), 0);

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:113
> + * Creates a new #WebKitWebView with the default context.
> + * See also webkit_web_view_new_with_context.
> + *

Somewhere we need to explain what a web context represents. I think it's sufficient to link to #WebKitWebContext here and ensure the documentation there is good.

i.e. Creates a new #WebKitWebView with the default #WebKitWebContext.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:127
> + * @context: the #WebKitWebContext to be used by the #WebKitWebView
> + *
> + * Creates a new #WebKitWebView with the given context.
> + *
> + * Returns: The newly created #WebKitWebView widget

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:140
> + * Gets the web context of @web_view.

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h:-63
>  };
>  
>  WK_EXPORT GType
> -webkit_web_view_get_type   (void);

This is why I think we should stop lining up the argument lists here. It's standard in GLib headers, but this kind of churn hurts patch readability and makes "git blame" useless.

> Source/WebKit2/UIProcess/API/gtk/tests/testwebview.c:40
> +    WebKitWebView *view = WEBKIT_WEB_VIEW(webkit_web_view_new());
> +    g_object_ref_sink(view);
> +    g_assert(webkit_web_view_get_context(view) == webkit_web_context_get_default());
> +    g_object_unref(view);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_thread_init(NULL);
> +    gtk_test_init(&argc, &argv, NULL);
> +
> +    g_test_bug_base("https://bugs.webkit.org/");
> +    g_test_add_func("/webkit2/webview/default_context",
> +                    testWebViewDefaultContext);
> +

I'm loving these unit tests! The asterisks are on the wrong side here though.

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