[webkit-reviews] review denied: [Bug 89810] [GTK] Test /webkit2/WebKitFindController/hide always fails in Xvfb : [Attachment 198714] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Manuel Rego Casasnovas
<rego at igalia.com>'s request for review:
Bug 89810: [GTK] Test /webkit2/WebKitFindController/hide always fails in Xvfb
https://bugs.webkit.org/show_bug.cgi?id=89810

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

------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=198714&action=review


Much better! Just some minor issues and a memory leak.

> Source/WebKit2/UIProcess/API/gtk/tests/WebViewTest.cpp:437
> +    m_surface = 0;

This leaks the surface, this is not a smart pointer. You should check if the
poiner is valid can call cairo_surface_destroy.

> Source/WebKit2/UIProcess/API/gtk/tests/WebViewTest.cpp:440
> +    return cairo_surface_reference(m_surface);

If you are keeping a reference, it's better to return your reference and let
the caller decide whether she needs to hold a reference or not.

> Source/WebKit2/UIProcess/API/gtk/tests/WebViewTest.cpp:443
> +gboolean WebViewTest::cairoSurfacesEqual(cairo_surface_t* s1,
cairo_surface_t* s2)

Use bool instead of gboolean. This has nothing to do with WebView, you could
add it as a static method to Test, as I proposed before.