[webkit-reviews] review denied: [Bug 63256] [GTK] Fix runtime critical warnings in WebKit2 : [Attachment 98350] Patch

[bugzilla-daemon at webkit.org]

Martin Robinson <mrobinson at webkit.org> has denied Carlos Garcia Campos
<cgarcia at igalia.com>'s request for review:
Bug 63256: [GTK] Fix runtime critical warnings in WebKit2
https://bugs.webkit.org/show_bug.cgi?id=63256

Attachment 98350: Patch
https://bugs.webkit.org/attachment.cgi?id=98350&action=review

------- Additional Comments from Martin Robinson <mrobinson at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=98350&action=review

> Source/WebCore/platform/gtk/PlatformScreenGtk.cpp:92
> -FloatRect screenRect(Widget* widget)
> +static gboolean getScreenAndMonitorForWidget(Widget* widget, GdkScreen**
screenOut, gint* monitorOut)

I'd rather this be a smaller helper (getScreen) like getVisual. In fact,
getVisual should just use this helper to get the screen.

> Source/WebCore/platform/gtk/PlatformScreenGtk.cpp:127
> +    if (!widget)
> +	   return FloatRect();
> +
> +    GdkScreen* screen;
> +    gint monitor;
> +    if (!getScreenAndMonitorForWidget(widget, &screen, &monitor))
>	   return FloatRect();
>  
> -    gint monitor = gdk_screen_get_monitor_at_window(screen,
gtk_widget_get_window(GTK_WIDGET(container)));
>      GdkRectangle geometry;
>      gdk_screen_get_monitor_geometry(screen, monitor, &geometry);
> -    
> +

I think this could be a bit more straightforward. The use of out-parameters is
a bit cludgy here. Why not just maintain the structure of screenRect and then
do an early return if getScreenForWidget/getScreen fails?