[Webkit-unassigned] [Bug 70594] [GTK] Add helper function to set the loader client in WebKitWebView

[bugzilla-daemon at webkit.org]

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


Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #111934|review?                     |review+
               Flag|                            |




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

Okay. Looks fine, but I have a minor change requested below. Thank for cleaning this up.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:60
> +    webView->priv->loaderClient = loaderClient;
> +    webkitWebLoaderClientAttachLoaderClientToPage(loaderClient, wkPage);

Instead of passing the page as an argument, I think it'd be better to get it here with:

WebPageProxy* page = webkitWebViewBaseGetPage(WEBKIT_WEB_VIEW_BASE(webView));

>>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:68
>>>  
>> 
>> It seems like instead of doing this manually we should be chaining up to the parent constructor. It's mentioned here that you should always chain up to the parent constructor: http://developer.gnome.org/gobject/stable/chapter-gobject.html#gobject-instantiation
> 
> This method is constructed, not constructor.

Still the docs suggest chaining up: "the constructed function is called by g_object_new() as the final step of the object creation process. At the point of the call, all construction properties have been set on the object. The purpose of this call is to allow for object initialisation steps that can only be performed after construction properties have been set. constructed implementors should chain up to the constructed call of their parent class to allow it to complete its initialisation."

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:439
> -void webkitWebViewBaseCreateWebPage(WebKitWebViewBase* webkitWebViewBase, WKContextRef context, WKPageGroupRef pageGroup)
> +WebPageProxy* webkitWebViewBaseCreateWebPage(WebKitWebViewBase* webkitWebViewBase, WKContextRef context, WKPageGroupRef pageGroup)
>  {
>      WebKitWebViewBasePrivate* priv = webkitWebViewBase->priv;
>  

I dislike this change because we ignore the return value in WebKitWebViewBase. Do you mind just getting the Page in WebKitWebViewSetLoader client?

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