Bug 113114 - REGRESSION(r146647): media/video-controls-captions.html is timing out
Summary: REGRESSION(r146647): media/video-controls-captions.html is timing out
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P1 Normal
Assignee: Ahmad Saleem
URL:
Keywords: LayoutTestFailure, Regression
Depends on:
Blocks: 113062
  Show dependency treegraph
 
Reported: 2013-03-22 16:39 PDT by Peter Kasting
Modified: 2023-10-27 09:00 PDT (History)
9 users (show)

See Also:


Attachments
Patch (21.48 KB, patch)
2013-04-19 17:54 PDT, Victor Carbune
darin: review-
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2 (506.21 KB, application/zip)
2013-04-19 19:34 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (745.62 KB, application/zip)
2013-04-20 05:30 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Kasting 2013-03-22 16:39:41 PDT
See http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&showExpectations=true&tests=media%2Fvideo-controls-captions.html .

The Chromium bots seem to be getting killed before they print anything, but the EFL Linux bots have a longer timeout, and print:

CONSOLE MESSAGE: line 97: TypeError: 'undefined' is not an object (evaluating 'captionsButtonCoordinates[0]')
FAIL: Timed out waiting for notifyDone to be called

Jer Noble pointed out bug 95428 which suggests that the test has been previously failing on EFL, but the dashboard shows GTK as broken and EFL not, so I'm thinking that bug is out-of-date w.r.t. EFL.

For now I'm going to mark this as expected to fail in Chromium as opposed to just rolling the change out, but please investigate.  CCing scherkus on the Chromium side as a knowledgeable Media guy in case he wants some particular action taken.
Comment 1 Peter Kasting 2013-03-22 22:50:14 PDT
*** Bug 113127 has been marked as a duplicate of this bug. ***
Comment 2 Peter Kasting 2013-03-22 22:53:10 PDT
According to the link from bug 113127 ( http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=media%2Ftrack%2Ftrack-prefer-captions.html ), at least some GTK bots were passing and are now newly timing out as well.  And even the Apple bots, which were already flaky, seem to be more consistently timing out.  Widening summary.

Perhaps I should have rolled this out after all...
Comment 3 Peter Kasting 2013-03-22 23:00:42 PDT
(I meant to assign this to Eric originally rather than just CC him)
Comment 4 Ryosuke Niwa 2013-03-22 23:01:22 PDT
(In reply to comment #3)
> (I meant to assign this to Eric originally rather than just CC him)

We don't normally assign bugs on WebKit Bugzilla since virtually everyone ignores it.
Comment 5 Eric Carlson 2013-03-25 07:57:24 PDT
(In reply to comment #2)
> According to the link from bug 113127 ( http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=media%2Ftrack%2Ftrack-prefer-captions.html ), at least some GTK bots were passing and are now newly timing out as well.  And even the Apple bots, which were already flaky, seem to be more consistently timing out.  Widening summary.
> 
> Perhaps I should have rolled this out after all...

track-prefer-captions.html is a different test.
Comment 6 Eric Carlson 2013-03-25 07:59:00 PDT
(In reply to comment #2)
>  And even the Apple bots, which were already flaky, seem to be more consistently timing out. 

This test is not run on the Apple bots, they use the track menu and this tests the "CC" button (which toggles captions on and off).
Comment 7 Eric Carlson 2013-03-25 08:00:01 PDT
The logic in media/video-controls-captions.html is flawed. It assumes the <track> element will not load until the "CC" button is clicked, but the track has a "srclan=en" attribute and the script sets the user's preferred language to "en". HTMLMediaElement::configureTextTrackGroup has always automatically selected a track when it matches the user's preferred language, but the timing of when it runs has changed so this test now fails more consistently.

The logic in this test should be updated.
Comment 8 Peter Kasting 2013-03-25 11:07:14 PDT
(In reply to comment #5)
> (In reply to comment #2)
> > According to the link from bug 113127 ( http://test-results.appspot.com/dashboards/flakiness_dashboard.html#group=%40ToT%20-%20webkit.org&tests=media%2Ftrack%2Ftrack-prefer-captions.html ), at least some GTK bots were passing and are now newly timing out as well.  And even the Apple bots, which were already flaky, seem to be more consistently timing out.  Widening summary.
> > 
> > Perhaps I should have rolled this out after all...
> 
> track-prefer-captions.html is a different test.

Oops, my mistake.  Sorry for the bad dupe there :(
Comment 9 Victor Carbune 2013-04-15 01:55:06 PDT
(In reply to comment #7)
> The logic in media/video-controls-captions.html is flawed. It assumes the <track> element will not load until the "CC" button is clicked, but the track has a "srclan=en" attribute and the script sets the user's preferred language to "en". HTMLMediaElement::configureTextTrackGroup has always automatically selected a track when it matches the user's preferred language, but the timing of when it runs has changed so this test now fails more consistently.
> 
> The logic in this test should be updated.

Indeed, but besides that there is also a bug when there's no srclang specified
for the track - no track gets displayed at all.

I've tried fixing the code and split language behavior in a different test:
https://codereview.chromium.org/14056005/

I'll post the patch here too, but feel free to comment on the codereview there
as well in the meantime.
Comment 10 Victor Carbune 2013-04-19 17:54:16 PDT
Created attachment 198921 [details]
Patch
Comment 11 Eric Carlson 2013-04-19 18:41:36 PDT
Comment on attachment 198921 [details]
Patch

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

> LayoutTests/media/video-controls-captions-load-by-lang.html:31
> +            consoleWrite("** Set the user language preference so that the track will be chosen when the CC button is clicked. **");
> +            run("internals.setUserPreferredLanguages(['ar'])");
> +            clickCCButton();

This and the other test won't work on the Mac because the CC button shows and hides the track menu. I expect that you will want something similar at some point, but you can make the test work by setting the old webkitClosedCaptionsVisible property.
Comment 12 Build Bot 2013-04-19 19:34:45 PDT
Comment on attachment 198921 [details]
Patch

Attachment 198921 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/136221

New failing tests:
media/video-controls-captions-multiple-clicks.html
media/video-controls-captions-load-by-lang.html
Comment 13 Build Bot 2013-04-19 19:34:47 PDT
Created attachment 198924 [details]
Archive of layout-test-results from webkit-ews-14 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-14  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.2
Comment 14 Build Bot 2013-04-20 05:30:56 PDT
Comment on attachment 198921 [details]
Patch

Attachment 198921 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/96655

New failing tests:
media/video-controls-captions-multiple-clicks.html
media/video-controls-captions-load-by-lang.html
Comment 15 Build Bot 2013-04-20 05:30:58 PDT
Created attachment 198940 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.2
Comment 16 Darin Adler 2016-03-09 09:04:00 PST
Comment on attachment 198921 [details]
Patch

Tests failed on the EWS. Please make a new patch that revolves that if you would like to continue working on this fix.
Comment 17 Ahmad Saleem 2023-10-25 13:56:59 PDT
This test case is gone:

https://github.com/WebKit/WebKit/commit/a39bd42ad87d82a3bea0fa6eac107376a973cf7c

>> * media/video-controls-captions.html: Removed.

But do we want to merge: https://src.chromium.org/viewvc/blink?revision=148780&view=revision

As mentioned in Comment 09?
Comment 18 Eric Carlson 2023-10-27 08:57:38 PDT
(In reply to Ahmad Saleem from comment #17)
> This test case is gone:
> 
> https://github.com/WebKit/WebKit/commit/
> a39bd42ad87d82a3bea0fa6eac107376a973cf7c
> 
> >> * media/video-controls-captions.html: Removed.
> 
> But do we want to merge:
> https://src.chromium.org/viewvc/blink?revision=148780&view=revision
> 
> As mentioned in Comment 09?

I don't think we want to merge that change. We may want to implement the same behavior, but we should do that with a new bug and new code if we decide to do that.