[webkit-reviews] review requested: [Bug 62459] Local files cannot load icons. : [Attachment 101240] Layout test should be skipped for platforms with IconDatabase disabled, and using the test with timeout.

[bugzilla-daemon at webkit.org]

Rafael Brandao <rafael.lobo at openbossa.org> has asked  for review:
Bug 62459: Local files cannot load icons.
https://bugs.webkit.org/show_bug.cgi?id=62459

Attachment 101240: Layout test should be skipped for platforms with
IconDatabase disabled, and using the test with timeout.
https://bugs.webkit.org/attachment.cgi?id=101240&action=review

------- Additional Comments from Rafael Brandao <rafael.lobo at openbossa.org>
The problem with chromium-ews has shown to me that it would be impossible to
make a single layout test for all platforms. So I gave up, and for now I've
marked this test as skipped on chromium (and hopefully did this correctly).
I've decided to use the timeout version of the test because it seems more
stable right now. I've done many tests with the previous setting and I've
noticed one test case that failed among many (flaky test). I've done many tests
with this new one too but didn't see it failing anytime yet (so it doesn't mean
it's 100% safe neither). The problem with favicons instability in tests is
probably related to the fact that the icon may be requested at a time that
IconDatabase's thread can't read from I/O to find out for sure if it already
has the icon data or not, so it leaves the favicon load decision for "later". I
think 400 ms for timeout is reasonably good. Another reason for this change is
that the previous test setting is not working anymore for some reason.

Hopefully we'll find a good solution for this. It seems more reasonable to
leave the test running for all platforms that I don't know yet the output so I
can see them failing and then fixing it later.