1427699 - A cloned resource fails to notify the 'suspend' event

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, enhancement, P3)

Comment 1

[JW Wang [:jwwang] [:jw_wang]]

Attachment: test_suspend.html

Comment 2

[JW Wang [:jwwang] [:jw_wang]]

Repro steps:
1. History -> Clear recent history -> Everything
   to clear all cache data.
2. open the link: https://bugzilla.mozilla.org/attachment.cgi?id=8939505

Expected result:
'suspend' should be fired for v2 since it is not fetching data.

Actual result:
'stalled' is fired which means the user agent is trying to fetch media data, but data is unexpectedly not forthcoming.

Comment 3

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1427699. P1 - allow element->DownloadSuspended() to be called when mChannel is null.

This is required by P2 where we want to notify the 'suspend' event for a cloned
resource whose mChannel is initially null.

Review commit: https://reviewboard.mozilla.org/r/210762/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210762/

Comment 4

[JW Wang [:jwwang] [:jw_wang]]

Attachment: Bug 1427699. P2 - rewrite InitAsCloneInternal() to mimic the case where data is downloaded from a channel.

Note we add mClient->CacheClientSuspend() so the network state of the element
is changed to IDLE because we have no channel to fetch data initially.

Review commit: https://reviewboard.mozilla.org/r/210764/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/210764/

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8940567 [details]
Bug 1427699. P1 - allow element->DownloadSuspended() to be called when mChannel is null.

https://reviewboard.mozilla.org/r/210762/#review216518

Comment 6

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8940568 [details]
Bug 1427699. P2 - rewrite InitAsCloneInternal() to mimic the case where data is downloaded from a channel.

https://reviewboard.mozilla.org/r/210764/#review216522

::: dom/media/MediaCache.cpp:2893
(Diff revision 1)
>  {
>    MOZ_ASSERT(OwnerThread()->IsOnCurrentThread());
> -  AutoLock lock(aOriginal->mMediaCache->Monitor());
> +  AutoLock lock(mMediaCache->Monitor());
>  
> -  mResourceID = aOriginal->mResourceID;
> +  // Download data and notify events if necessary. Note the order is important
> +  // in order to mimic the behavior of data is downloaded from the channel.

'is' -> 'being'

::: dom/media/MediaCache.cpp:2940
(Diff revision 1)
> +  mClientSuspended = true;
> +  mCacheSuspended = true;
> +  mChannelEnded = true;
> +  mClient->CacheClientSuspend();
>  
> +  // Step 5: add the steam to be managed by the cache.

'steam' -> 'stream'

Comment 7

[JW Wang [:jwwang] [:jw_wang]]

Comment on attachment 8940568 [details]
Bug 1427699. P2 - rewrite InitAsCloneInternal() to mimic the case where data is downloaded from a channel.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/210764/diff/1-2/

Comment 8

[JW Wang [:jwwang] [:jw_wang]]

Thanks for the review.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15c31a08d6e3
P1 - allow element->DownloadSuspended() to be called when mChannel is null. r=gerald
https://hg.mozilla.org/integration/autoland/rev/8932f0853269
P2 - rewrite InitAsCloneInternal() to mimic the case where data is downloaded from a channel. r=gerald

Comment 10

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/15c31a08d6e3
https://hg.mozilla.org/mozilla-central/rev/8932f0853269