[webkit-reviews] review denied: [Bug 61065] [GTK][WebKit2] Initial windowed plugins implementation : [Attachment 117182] New patch

[bugzilla-daemon at webkit.org]

Philippe Normand <pnormand at igalia.com> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 61065: [GTK][WebKit2] Initial windowed plugins implementation
https://bugs.webkit.org/show_bug.cgi?id=61065

Attachment 117182: New patch
https://bugs.webkit.org/attachment.cgi?id=117182&action=review

------- Additional Comments from Philippe Normand <pnormand at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=117182&action=review


Patch looks good to me apart from some nits listed below. Also not sure about
the Message changes, do they need to be approved by other WebKit2 reviewers? If
so please CC the appropriate people :)

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

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

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

> Source/WebKit2/UIProcess/API/gtk/WebKitWebViewBase.cpp:235
> +    GtkAllocation childAllocation(geometry);

Shouldn't a C-style assignment be used here?

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