Bug 204762 - WPT test MediaStream-MediaElement-srcObject.https.html times out
Summary: WPT test MediaStream-MediaElement-srcObject.https.html times out
Status: REOPENED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebRTC (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Eric Carlson
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-12-02 12:56 PST by Eric Carlson
Modified: 2019-12-09 09:20 PST (History)
16 users (show)

See Also:


Attachments
Patch (20.59 KB, patch)
2019-12-03 10:25 PST, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch (21.11 KB, patch)
2019-12-03 13:05 PST, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing (21.18 KB, patch)
2019-12-03 13:22 PST, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing (21.18 KB, patch)
2019-12-03 13:56 PST, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing (22.27 KB, patch)
2019-12-04 12:10 PST, Eric Carlson
no flags Details | Formatted Diff | Diff
Patch for landing (22.27 KB, patch)
2019-12-04 12:49 PST, Eric Carlson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Carlson 2019-12-02 12:56:21 PST
Most of MediaStream-MediaElement-srcObject.https.html isn't run because one of the sub tests times out.
Comment 1 Radar WebKit Bug Importer 2019-12-02 12:56:34 PST
<rdar://problem/57567671>
Comment 2 Eric Carlson 2019-12-03 10:25:37 PST
Created attachment 384722 [details]
Patch
Comment 3 youenn fablet 2019-12-03 10:58:54 PST
Comment on attachment 384722 [details]
Patch

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

> Source/WebCore/html/HTMLMediaElement.cpp:4858
> +        if (loop() && !m_mediaController && playbackRate > 0 && !hasMediaStreamSrcObject()) {

Should loop() (or a hasLoop() variant) always return false if there is a media stream src object?

> Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaStreamAVFObjC.mm:830
> +                    m_player->durationChanged();

Should we introduce a player routine to do all these things together (if player knows it is ended)?

> LayoutTests/TestExpectations:3880
> +imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-MediaElement-srcObject.https.html [ Failure ]

We could update the test to do assert_true( ===). instead of assert_equal(, ).
That would be good to ensure we do not regress this test.
Either as part of this patch or as a follow-up that we would upstream to WPT.
Comment 4 Eric Carlson 2019-12-03 13:05:54 PST
Created attachment 384740 [details]
Patch
Comment 5 Eric Carlson 2019-12-03 13:22:18 PST
Created attachment 384745 [details]
Patch for landing
Comment 6 Eric Carlson 2019-12-03 13:56:34 PST
Created attachment 384756 [details]
Patch for landing
Comment 7 Alexey Proskuryakov 2019-12-04 00:09:08 PST
Comment on attachment 384756 [details]
Patch for landing

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

> LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-MediaElement-srcObject.https-expected.txt:16
> +FAIL Tests that a media element with an assigned MediaStream reports the played attribute as expected assert_equals: A MediaStream's end MUST return the last known currentTime expected 0.252313446 but got 0.252291624

This doesn't look like it can possibly be a stable result. Or can it?
Comment 8 youenn fablet 2019-12-04 00:19:38 PST
(In reply to Alexey Proskuryakov from comment #7)
> Comment on attachment 384756 [details]
> Patch for landing
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=384756&action=review
> 
> > LayoutTests/imported/w3c/web-platform-tests/mediacapture-streams/MediaStream-MediaElement-srcObject.https-expected.txt:16
> > +FAIL Tests that a media element with an assigned MediaStream reports the played attribute as expected assert_equals: A MediaStream's end MUST return the last known currentTime expected 0.252313446 but got 0.252291624
> 
> This doesn't look like it can possibly be a stable result. Or can it?

The test is marked as Failed for now.
Plan is to either fix the implementation quickly to pass this assert or change the test to do assert_true instead of assert_equals to have a stable assertion failure message.
Comment 9 Eric Carlson 2019-12-04 12:10:49 PST
Created attachment 384831 [details]
Patch for landing
Comment 10 Eric Carlson 2019-12-04 12:49:48 PST
Created attachment 384835 [details]
Patch for landing
Comment 11 WebKit Commit Bot 2019-12-05 06:27:37 PST
Comment on attachment 384835 [details]
Patch for landing

Clearing flags on attachment: 384835

Committed r253148: <https://trac.webkit.org/changeset/253148>
Comment 12 WebKit Commit Bot 2019-12-05 06:27:39 PST
All reviewed patches have been landed.  Closing bug.
Comment 13 Truitt Savell 2019-12-05 15:53:27 PST
It looks like the changes in https://trac.webkit.org/changeset/253148/webkit

has caused fast/mediastream/stream-switch.html to timeout on Mac.

History:
https://results.webkit.org/?suite=layout-tests&test=fast%2Fmediastream%2Fstream-switch.html

I was able to reproduce this locally by just running the test in iterations.
Comment 14 Truitt Savell 2019-12-09 09:19:33 PST
This was rolled out in https://trac.webkit.org/changeset/253215/webkit
Comment 15 Truitt Savell 2019-12-09 09:20:06 PST
Reopening