[Webkit-unassigned] [Bug 61065] [GTK][WebKit2] Initial windowed plugins implementation

[bugzilla-daemon at webkit.org]

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





--- Comment #12 from Carlos Garcia Campos <cgarcia at igalia.com>  2012-04-24 07:53:54 PST ---
(In reply to comment #11)
> (From update of attachment 117182 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=117182&action=review
> 
> Patch looks good to me apart from some nits listed below.

Thanks for reviewing!

> Also not sure about the Message changes, do they need to be approved by other WebKit2 reviewers? If so please CC the appropriate people :)

I haven't changed any messages, just added a new one needed by ports using X11 plugin arch.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:126
> > +    priv->children.set(widget, childAllocation);
> 
> Would it make sense to convert the GtkAllocation to an IntRect? I know they're quite similar but anyway :)

It's converted, the hashmap stores IntRects.

typedef HashMap<GtkWidget*, IntRect> WebKitWebViewChildrenMap;

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:146
> > +static void webkitWebViewBaseContainerForall(GtkContainer *container, gboolean includeInternals, GtkCallback callback, gpointer callbackData)
> 
> star on the wrong side it seems?

right.

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:231
> > +    IntRect geometry(priv->children.get(child));
> 
> A different assignment style is used in webkitWebViewBaseChildMoveResize. Can the code be more coherent in that regard, if possible?

Sure

> > Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:235
> > +    GtkAllocation childAllocation(geometry);
> 
> Shouldn't a C-style assignment be used here?

why? does the coding style say something regarding assignments? I don't mind using either one, just wondering if there's a reason to prefer C-style one.

> > Source/WebKit2/UIProcess/gtk/WebPageProxyGtk.cpp:86
> > +        GdkRectangle clip(clipRect);
> 
> Ditto about assignment style.
> 
> > Source/WebKit2/WebProcess/Plugins/Netscape/x11/NetscapePluginX11.cpp:164
> > +    // I guess it uses gdk_window_lookup(), se we create a new socket here
> 
> typo: "so we"

Ok.

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