[webkit-reviews] review requested: [Bug 111434] [GTK] WebKit2 test TestWebKitFaviconDatabase times out with recent glib : [Attachment 206119] Fixing according to Martin's comments

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has asked  for review:
Bug 111434: [GTK] WebKit2 test TestWebKitFaviconDatabase times out with recent
glib
https://bugs.webkit.org/show_bug.cgi?id=111434

Attachment 206119: Fixing according to Martin's comments
https://bugs.webkit.org/attachment.cgi?id=206119&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.