[webkit-reviews] review canceled: [Bug 111434] [GTK] WebKit2 test TestWebKitFaviconDatabase times out with recent glib : [Attachment 206116] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jul 4 19:31:37 PDT 2013
Xabier Rodríguez Calvar <calvaris at igalia.com> has canceled Xabier Rodríguez
Calvar <calvaris at igalia.com>'s request for review:
Bug 111434: [GTK] WebKit2 test TestWebKitFaviconDatabase times out with recent
glib
https://bugs.webkit.org/show_bug.cgi?id=111434
Attachment 206116: Patch
https://bugs.webkit.org/attachment.cgi?id=206116&action=review
------- Additional Comments from Xabier Rodríguez Calvar <calvaris at igalia.com>
(In reply to comment #28)
> > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFaviconDatabase.cpp:156
> > + GOwnPtr<char>
iconURI(webkit_favicon_database_get_favicon_uri(database,
kServer->getURIForPath("/foo").data()));
>
> I'm not sure exactly why this change is necessary.
The problem with this is that the clear database test was phony because it is
not actually testing for an already existing icon so it could never find it.
> > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFaviconDatabase.cpp:220
> > + test->m_faviconURI = static_cast<char*>(0);
>
> Do you really need to cast 0 to a pointer?
Changed.
> > Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitFaviconDatabase.cpp:242
> > + // This tests depend on this order to run properly so we declare
> > + // them in a single one
> > + testNotInitialized(test, 0);
>
> English nit: "This tests" -> "These tests" It would probably be good to
include a link to this bug.
Changed.
I also removed the code to slice the testGetFavicon in three as this could be a
subject of improving the tests to make them actually independent from each
other and not needing to run them sorted.
More information about the webkit-reviews
mailing list