[Webkit-unassigned] [Bug 89810] [GTK] Test /webkit2/WebKitFindController/hide always fails in Xvfb

[bugzilla-daemon at webkit.org]

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


Carlos Garcia Campos <cgarcia at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #198694|review?                     |review-
               Flag|                            |




--- Comment #2 from Carlos Garcia Campos <cgarcia at igalia.com>  2013-04-18 02:11:27 PST ---
(From update of attachment 198694)
View in context: https://bugs.webkit.org/attachment.cgi?id=198694&action=review

Looks good in general, thanks!

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFindController.cpp:68
> -        g_signal_connect_after(m_webView, "draw", G_CALLBACK(webViewDraw), this);
> +        webkit_web_view_get_snapshot(m_webView, WEBKIT_SNAPSHOT_REGION_VISIBLE, WEBKIT_SNAPSHOT_OPTIONS_NONE, 0,
> +            reinterpret_cast<GAsyncReadyCallback>(onSnapshotReady), this);

I think we can make this generic so that it can be used by other tests, adding WebViewTest::snapshot(), for example. Instead of the visible region I think you should get the whole document.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFindController.cpp:283
> +    g_assert_cmpint(cairo_image_surface_get_width(firstSurface), ==, cairo_image_surface_get_width(secondSurface));
> +    g_assert_cmpint(cairo_image_surface_get_height(firstSurface), ==, cairo_image_surface_get_height(secondSurface));
> +    g_assert_cmpint(cairo_image_surface_get_stride(firstSurface), ==, cairo_image_surface_get_stride(secondSurface));

You should not assert here, you either call this assertSurfacesEqual and assert when they are different, or you leave it as cairoSurfaceEqual and assert on the result. Note that there's a function for this already in TestWebKitWebView.cpp. Move it to Test.h to make it shareable and use it from both places.

> Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFindController.cpp:285
> +        cairo_image_surface_get_width(firstSurface) * cairo_image_surface_get_stride(firstSurface));

Sorry, I told you wrongly, it's height * stride, not width * stride.

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