Bug 133351 – Viewport percentage tests that resize the viewport are flaky

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Bem Jones-Bey]

Viewport percentage tests that resize the viewport are flaky

[Bem Jones-Bey]

Created attachment 232207 [details]
Patch

[Martin Hock]

Committed r169434: <http://trac.webkit.org/changeset/169434>

[Mark Lam]

(In reply to comment #2)
> Committed r169434: <http://trac.webkit.org/changeset/169434>

The test was marked as flaky in r169434.  Please revert the TestExpectation once this issue is resolved.

[Darin Adler]

Comment on attachment 232207 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=232207&action=review

> LayoutTests/css3/viewport-percentage-lengths/resources/resize-test.js:22
> +        // FIXME: It would be nice to verify that the window resized to the
> +        // given dimensions. Unfortunately, it looks like the window doesn't
> +        // always resize to the specified dimensions, so we can't do the
> +        // straightforward test, as it makes tests written using this script
> +        // flaky.

I think the issue here is a race condition. The problem is using a timeout/0. We should fix by waiting for the resize to happen.

[Benjamin Poulain]

Comment on attachment 232207 [details]
Patch

I agree with Darin. We have a hard condition that the state must be correct when we send the resize event. The test should wait for the resize.

[Bem Jones-Bey]

(In reply to comment #5)
> (From update of attachment 232207 [details])
> I agree with Darin. We have a hard condition that the state must be correct when we send the resize event. The test should wait for the resize.

When I originally wrote the test, I tried to use a window.onresize handler, and I had trouble getting it to work. Is that the correct thing to use here, or is there a better event to check for the resize to be finished?

[Bem Jones-Bey]

Created attachment 232250 [details]
Patch with onresize event handler

So I tried again with the resize event handler, and am running into the same problems I ran into before: when I load it in my build of Safari, it runs, but if I reload the page, it doesn't run until I manually resize the window a little to kick it off. If I run it using RWT (WTR, not DRT), the resize handler is never triggered, and the test just times out. Am I doing something wrong with the way I am setting up the resize handler, or is there a bug in the interaction between resize handlers and programatically causing a resize?

[Darin Adler]

(In reply to comment #7)
> So I tried again with the resize event handler, and am running into the same problems I ran into before: when I load it in my build of Safari, it runs, but if I reload the page, it doesn't run until I manually resize the window a little to kick it off. If I run it using RWT (WTR, not DRT), the resize handler is never triggered, and the test just times out. Am I doing something wrong with the way I am setting up the resize handler, or is there a bug in the interaction between resize handlers and programatically causing a resize?

Yes, we will have to debug to find out why there’s no resize event under WebKit2. I can’t tell just from your description, but we can set breakpoints to find out why it’s failing.

[Darin Adler]

(In reply to comment #7)
> when I load it in my build of Safari, it runs, but if I reload the page, it doesn't run until I manually resize the window a little to kick it off.

I tried making a simple test, and I do not see this problem in stock Safari 7.0.4. Waiting for my build of TOT WebKit to see if I can reproduce the problem with the latest, or with WebKitTestRunner.

[Darin Adler]

Oops, wait, my test was with the "timeout 0" version, not the resize event version!

[Darin Adler]

(In reply to comment #8)
> if I reload the page, it doesn't run until I manually resize the window a little to kick it off

That’s because once we run the test the window is 800x600. So when we run the test again, the code sets the size to 800x600. That is not going to send a resize event, because the window is already that size!

Same problem under the test runner. The window’s default size is 800x600, and so setting it to that size again will not trigger a resize event.

One simple way to fix this is to “warm up” the test by resizing relative to the starting size, perhaps 10 pixels larger in each dimension, and then continue with the rest of the test.

[Darin Adler]

After fixing the test to warm up properly, the resize events are firing, but under old Safari (not latest WebKit), I am still seeing flakiness; some kind of race condition where at the time of the resize event outerWidth and outerHeight sometimes have not yet been updated to the new size.

[Darin Adler]

Those failures only happened with the older WebKit. With TOT WebKit everything seems to work fine.

[Darin Adler]

If you look closely at the expected results, you’ll see that the resizing hasn’t been working in DumpRenderTree. The test is being run repeatedly at the same size. I’ve been working on this for a few hours today. There are many bugs here.

[Darin Adler]

Created attachment 232325 [details]
Patch

[Darin Adler]

Committed r169505: <http://trac.webkit.org/changeset/169505>

[Alexey Proskuryakov]

svg/filters/filter-hidden-content.svg started to fail every time on all WebKit2 bots after this change:

http://build.webkit.org/results/Apple%20Mavericks%20Release%20WK2%20(Tests)/r169508%20(6071)/svg/filters/filter-hidden-content-diff.txt

[Alexey Proskuryakov]

Actually, two more tests are failing (it's difficult to see because WebKit2 testers still retry failures, hiding many of those that depend on previous tests):

fast/dynamic/window-resize-scrollbars-test.html
fast/dom/rtl-scroll-to-leftmost-and-resize.html

[Darin Adler]

(In reply to comment #17)
> svg/filters/filter-hidden-content.svg started to fail every time on all WebKit2 bots after this change:

I noticed, and my change in <http://trac.webkit.org/changeset/169509> was motivated by that failure.

[Darin Adler]

(In reply to comment #18)
> fast/dynamic/window-resize-scrollbars-test.html

http://trac.webkit.org/changeset/169508

> fast/dom/rtl-scroll-to-leftmost-and-resize.html

An expected failure as mentioned in the change log of http://trac.webkit.org/changeset/169505

These tests were “succeeding” before, but they weren’t really testing anything because they weren’t doing any resizing. We could skip them but I think it’s fine to run them as expected failures. Actually fixing the bugs they are testing for could be a big project. The RTL scroll bug is something that has never worked in any Mac port at least; I think it was fixed only for Chromium. The scrollbars bug has never worked on WebKit2, but the test wasn’t revealing that before because the test wasn’t doing any resizing.

[Darin Adler]

(In reply to comment #19)
> (In reply to comment #17)
> > svg/filters/filter-hidden-content.svg started to fail every time on all WebKit2 bots after this change:
> 
> I noticed, and my change in <http://trac.webkit.org/changeset/169509> was motivated by that failure.

This reflects a problem in WebKitTestRunner. Might be hard to fix. I believe it’s a race condition where WebKitTestRunner resizes the view for a test and then runs the test but there is no guarantee the resizing has completed before the test is run.

What’s frustrating is that the thing that’s causing “failure” is not really part of the test at all. Just a layer that’s sized to the size of the view, which is incorrectly configured.