Bug 168660

Summary: REGRESSION(~r212322): LayoutTest media/track/track-cue-container-rendering-position.html is a flaky timeout
Product: WebKit Reporter: Ryan Haddad <ryanhaddad>
Component: MediaAssignee: Per Arne Vollan <pvollan>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, bfulgham, eric.carlson, jer.noble, pvollan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
eric.carlson: review+
Patch none

Comment 1 Alexey Proskuryakov 2017-02-26 18:05:37 PST
First occurrence in r212322. Not sure if it can be the culprit, unless it messed up some feature initialization flags.

But even if this change is not to blame, this was definitely broken not long before - the test was absolutely stable earlier, and fails very often now. The test fails especially often on slower bots (ASan, leaks, GuardMalloc).

@@ -1,7 +1,7 @@
+CONSOLE MESSAGE: line 83: No text track cue with display id '-webkit-media-text-track-display' is currently visible
+FAIL: Timed out waiting for notifyDone to be called
 
 The top of the text track container should be in the bottom 30% of the video element.
 EVENT(canplaythrough)
 
-EXPECTED (cueDisplayElement.offsetTop > (video.videoHeight * .70) == 'true') OK
-END OF TEST
Comment 2 Radar WebKit Bug Importer 2017-02-26 18:06:11 PST
<rdar://problem/30726032>
Comment 3 Per Arne Vollan 2017-03-27 03:51:46 PDT
I am unable to reproduce this timeout with a debug ASAN build.
Comment 4 Per Arne Vollan 2017-03-29 05:38:51 PDT
Created attachment 305728 [details]
Patch
Comment 5 Per Arne Vollan 2017-03-29 06:38:48 PDT
Created attachment 305729 [details]
Patch
Comment 6 Eric Carlson 2017-03-29 06:43:58 PDT
Comment on attachment 305729 [details]
Patch

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

> LayoutTests/media/track/track-cue-container-rendering-position.html:25
> +            } catch (exception) {

I think it would be better to try this a finite number of times and fail with an error message instead of having the test timeout in the event of an error.
Comment 7 Per Arne Vollan 2017-03-29 22:16:21 PDT
Created attachment 305834 [details]
Patch
Comment 8 Per Arne Vollan 2017-03-29 22:55:31 PDT
(In reply to Eric Carlson from comment #6)
> Comment on attachment 305729 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=305729&action=review
> 
> > LayoutTests/media/track/track-cue-container-rendering-position.html:25
> > +            } catch (exception) {
> 
> I think it would be better to try this a finite number of times and fail
> with an error message instead of having the test timeout in the event of an
> error.

Thanks for reviewing! I will update the patch.
Comment 9 Per Arne Vollan 2017-03-29 23:37:33 PDT
Committed <https://trac.webkit.org/changeset/214600/webkit>.
Comment 10 Eric Carlson 2017-03-30 08:52:46 PDT
Comment on attachment 305834 [details]
Patch

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

> LayoutTests/media/track/track-cue-container-rendering-position.html:14
> +        var maxRetries = 256;

This means the test won't fail for 25 seconds. What made you choose such a long time, it seems *extremely* unlikely that it could take that long for the cue display element to be created.