Bug 195922

Summary: media/track/track-in-band-style.html is flaky
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, eric.carlson, jer.noble, tsavell, webkit-bot-watchers-bugzilla, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=167516
Bug Depends on:    
Bug Blocks: 195758    
Attachments:
Description Flags
Patch none

Description Chris Dumez 2019-03-18 16:56:58 PDT
media/track/track-in-band-style.html is flaky:
Diff:
--- /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/track/track-in-band-style-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/media/track/track-in-band-style-actual.txt
@@ -14,6 +14,6 @@
 
 ** Test current cue colors
 EXPECTED (getComputedStyle(cueDisplayElement).color == 'rgb(255, 255, 255)') OK
-EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)') OK
+EXPECTED (getComputedStyle(cueNode).backgroundColor == 'rgb(0, 0, 0)'), OBSERVED 'rgba(0, 0, 0, 0)' FAIL
 END OF TEST

It started happening after https://trac.webkit.org/changeset/243094 although I bet this change merely impacted the timing.
Comment 1 Chris Dumez 2019-03-18 16:57:13 PDT
I can reproduce like so:
run-webkit-tests -gf --repeat-each=100 media/track/track-in-band-style.html
Comment 2 Chris Dumez 2019-03-18 20:20:06 PDT
If I add logging to InbandTextTrackPrivateAVF::processCueAttributes() and run the test, I see that cueData.setForegroundColor(color) gets called with 'rgb(255, 255, 255)'.

However, I never see any call to cueData.setBackgroundColor(color), no matter if the test passes or fail.

Therefore, I am unsure where 'rgb(0, 0, 0)' and 'rgba(0, 0, 0, 0)' come from. Maybe some default values?
Comment 3 Chris Dumez 2019-03-18 20:34:36 PDT
It looks like Color().cssText() returns "rgba(0, 0, 0, 0)" so it would make sense for the background color to be rgba(0, 0, 0, 0) if it does not get set.
Comment 4 Chris Dumez 2019-03-19 08:41:38 PDT
(In reply to Chris Dumez from comment #3)
> It looks like Color().cssText() returns "rgba(0, 0, 0, 0)" so it would make
> sense for the background color to be rgba(0, 0, 0, 0) if it does not get set.

Although I do not see the AV code setting the cue's backgroundColor, it does set the highlightColor. The highlightColor ends up being set as css background-color for the element.
Comment 5 Chris Dumez 2019-03-19 10:05:11 PDT
Created attachment 365177 [details]
Patch
Comment 6 Eric Carlson 2019-03-19 12:45:06 PDT
Comment on attachment 365177 [details]
Patch

thanks cdumez!
Comment 7 Chris Dumez 2019-03-19 13:05:40 PDT
Comment on attachment 365177 [details]
Patch

Clearing flags on attachment: 365177

Committed r243164: <https://trac.webkit.org/changeset/243164>
Comment 8 Chris Dumez 2019-03-19 13:05:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2019-03-19 13:11:16 PDT
<rdar://problem/49031582>
Comment 10 Alexey Proskuryakov 2019-03-19 15:08:12 PDT
We have a flaky timeout expectation for this test. I don't imagine this is likely to fix it, is it?

LayoutTests/platform/mac/TestExpectations:1370:webkit.org/b/171465 media/track/track-in-band-style.html [ Pass Timeout ]
Comment 11 Chris Dumez 2019-03-19 15:10:33 PDT
(In reply to Alexey Proskuryakov from comment #10)
> We have a flaky timeout expectation for this test. I don't imagine this is
> likely to fix it, is it?
> 
> LayoutTests/platform/mac/TestExpectations:1370:webkit.org/b/171465
> media/track/track-in-band-style.html [ Pass Timeout ]

No sorry, I don't think my change would help address a timeout.