Bug 159173 – [mac] LayoutTest fast/css/ancestor-of-hovered-element-detached.html is flaky

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Ryan Haddad]

LayoutTest fast/css/ancestor-of-hovered-element-detached.html is flaky

Most recent failure:
<https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20(Tests)/builds/7163>
Flakiness dashboard:
<https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fcss%2Fancestor-of-hovered-element-detached.html>

--- /Volumes/Data/slave/elcapitan-release-tests-wk2/build/layout-test-results/fast/css/ancestor-of-hovered-element-detached-expected.txt
+++ /Volumes/Data/slave/elcapitan-release-tests-wk2/build/layout-test-results/fast/css/ancestor-of-hovered-element-detached-actual.txt
@@ -8,8 +8,8 @@
 PASS elementsWithHoverStyle() is ["html", "body", "prime-ancestor", "group", "element-to-remove", "target"]
 PASS elementsMatchingHoverSelector() is ["html", "body", "prime-ancestor", "group", "element-to-remove", "target"]
 Removing the renderer of #element-to-remove
-PASS elementsWithHoverStyle() is ["html", "body", "prime-ancestor", "interceptor"]
-PASS elementsMatchingHoverSelector() is ["html", "body", "prime-ancestor", "interceptor"]
+FAIL elementsWithHoverStyle() should be html,body,prime-ancestor,interceptor. Was html,body,prime-ancestor,group,element-to-remove,target.
+FAIL elementsMatchingHoverSelector() should be html,body,prime-ancestor,interceptor. Was html,body,prime-ancestor,group,element-to-remove,target.
 PASS successfullyParsed is true
 
 TEST COMPLETE

[Ryan Haddad]

This was added with <http://trac.webkit.org/changeset/202324>

[Alexey Proskuryakov]

Benjamin, do you expect to take a look at this? If not, can we roll out r202324?

[Ryan Haddad]

Marked test as flaky on mac in https://trac.webkit.org/changeset/203136

[Benjamin Poulain]

Created attachment 283835 [details]
Patch

[Alexey Proskuryakov]

Comment on attachment 283835 [details]
Patch

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

> LayoutTests/fast/css/ancestor-of-hovered-element-removed.html:96
> -    }, 125);
> +    }, 17);

Interesting.

I usually say that no timeout between 0ms and 5000ms makes sense in layout tests, because of how much contention for CPU resources there is. Given your explanation, maybe 17ms is right here!

[Benjamin Poulain]

Comment on attachment 283835 [details]
Patch

(In reply to comment #5)
> Comment on attachment 283835 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=283835&action=review
> 
> > LayoutTests/fast/css/ancestor-of-hovered-element-removed.html:96
> > -    }, 125);
> > +    }, 17);
> 
> Interesting.
> 
> I usually say that no timeout between 0ms and 5000ms makes sense in layout
> tests, because of how much contention for CPU resources there is. Given your
> explanation, maybe 17ms is right here!

Thanks for the review Alexey.

In theory we should not need a timer for :hover. We should keep a dirty flag somewhere and update as needed like the other style updates.

In practice, our code for :hover and :active is in a pretty bad shape. The only guarantee right now is that we'll paint it right.

Our test coverage is also bad. It is not the right time to attempt larger changes.

[WebKit Commit Bot]

Comment on attachment 283835 [details]
Patch

Clearing flags on attachment: 283835

Committed r203321: <http://trac.webkit.org/changeset/203321>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.