Bug 234066

Summary: REGRESSION (Safari 15?): Blob videos slow to pause, affects CBS and CNN
Product: WebKit Reporter: Jeff Johnson <opendarwin>
Component: MediaAssignee: Peng Liu <peng.liu6>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, eric.carlson, ews-watchlist, glenn, jer.noble, peng.liu6, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 15   
Hardware: All   
OS: All   
Attachments:
Description Flags
screenshot of web inspector breakpoint
none
Patch
jer.noble: review+, ews-feeder: commit-queue-
Remove some changes to address a test failure none

Description Jeff Johnson 2021-12-08 19:52:43 PST
Created attachment 446491 [details]
screenshot of web inspector breakpoint

This bug affects some HTML <video> elements with a blob: URL src. When JavaScript pause() is called on such a video soon after play(), the video can be very slow to pause, taking several seconds.

I'm not sure when exactly the bug was introduced, but I think it was in some version of Safari 15. I can reproduce the bug on iPadOS 15.1, as well as on macOS 11.6.1 (Intel) and macOS 12.0.1 (Apple Silicon) with Safari 15.1 and Safari Technology Preview 136. It's easier to give steps to reproduce on Mac, so I'll do that.

Steps to reproduce:
1. In Safari Preferences, Websites, set Auto-Play to Allow All Auto-Play.
2. Open the web inspector to the Sources tab.
3. Add an event breakpoint for the play event.
4. Add the following breakpoint condition:

event.target instanceof HTMLVideoElement && event.target.src.startsWith('blob:') && !event.target.paused

5. Add the following breakpoint action:

event.target.pause(); console.log('pause ' + event.target.src);

6. Set the breakpoint option to Automatically continue after evaluating.
7. Open https://detroit.cbslocal.com/2021/12/03/prosecutor-lays-out-disturbing-timeline-explaining-why-oxford-school-shooting-suspects-parents-were-charged/
8. Wait for the video to load (you can watch the Console).

The bug might not happen every time, so you may have to reload. The bug also occurs with https://www.cnn.com/videos/world/2021/12/08/allegra-stratton-boris-johnson-downing-street-party-video-ctw-intl-ldn-vpx.cnn (and many other CNN videos), but you'll have to sit through the ad to wait for the main video.

An alternative for reproducing on macOS and iPadOS would be to create a Safari extension to do the same thing as the breakpoint, i.e., addEventListener for play.
Comment 1 Jeff Johnson 2021-12-09 09:59:03 PST
I've tested with Safari 14.1.2 on macOS 10.15.7, and I can't reproduce on that configuration.

The issue occurs with pretty much any video on https://detroit.cbslocal.com, but you may have to sit through an advertisement on that site first to get to the main video, sorry.
Comment 2 Radar WebKit Bug Importer 2021-12-09 13:14:25 PST
<rdar://problem/86287467>
Comment 3 Jeff Johnson 2021-12-10 07:09:32 PST
Question: I'm trying to debug this, and I enabled Media Logging in the web inspector, but it appears that the ALWAYS_LOG messages in PlatformMediaSession.cpp are not logged, because it has a logClassName() of "PlatformMediaSession", as opposed to "MediaSession", is that correct?
Comment 4 Eric Carlson 2021-12-10 09:15:18 PST
(In reply to opendarwin from comment #3)
> Question: I'm trying to debug this, and I enabled Media Logging in the web
> inspector, but it appears that the ALWAYS_LOG messages in
> PlatformMediaSession.cpp are not logged, because it has a logClassName() of
> "PlatformMediaSession", as opposed to "MediaSession", is that correct?

Those are logged as `MediaElementSession` because that is the most derived class, so its `logClassName()` wins.
Comment 5 Jeff Johnson 2021-12-10 11:59:48 PST
(In reply to Eric Carlson from comment #4)
> (In reply to opendarwin from comment #3)
> > Question: I'm trying to debug this, and I enabled Media Logging in the web
> > inspector, but it appears that the ALWAYS_LOG messages in
> > PlatformMediaSession.cpp are not logged, because it has a logClassName() of
> > "PlatformMediaSession", as opposed to "MediaSession", is that correct?
> 
> Those are logged as `MediaElementSession` because that is the most derived
> class, so its `logClassName()` wins.

Does that mean the messages should appear in the log, or not? I'm not seeing them, unless I misunderstand the source code flow.

Unfortunately I'm finding it very difficult to debug the built WebKit source, because run-safari doesn't work right (sandboxing?), the MiniBrowser runs incredibly slow with video, I'm not sure what it's video autoplay policy is, and it also keeps crashing for some reason.
Comment 6 Peng Liu 2022-01-18 15:39:17 PST
Could you clarify the reproducing steps? Is the video slow to response to user's click after the steps?
Comment 7 Peng Liu 2022-01-18 15:41:00 PST
There is a recent fix regarding the performance of blob video loading (bug #234940). Not sure it is related to this bug though.
Comment 8 Jeff Johnson 2022-01-18 16:13:08 PST
(In reply to Peng Liu from comment #6)
> Could you clarify the reproducing steps? Is the video slow to response to
> user's click after the steps?

I'm not sure what you mean with regard to user clicks. The bug is the overly long time between when HTMLVideoElement.pause() is called and when the video actually pauses. The user could trigger pause() with a click, but that's not essential.
Comment 9 Peng Liu 2022-01-18 16:30:42 PST
OK. Regarding the reproducing steps, I guess you meant the time between the following two event is a few seconds:
- The console outputs "pause blob: ****".
- The video pauses (after auto-playing).

And the expected behavior is: "the video pauses immediately after auto-playing", right?

Without the web inspector things (the breakpoint and action), I did not see any issue after clicking the play/pause button. That's why I would like to clarify the reproducing steps. Thanks!
Comment 10 Jeff Johnson 2022-01-18 17:02:01 PST
(In reply to Peng Liu from comment #9)
> OK. Regarding the reproducing steps, I guess you meant the time between the
> following two event is a few seconds:
> - The console outputs "pause blob: ****".
> - The video pauses (after auto-playing).
> 
> And the expected behavior is: "the video pauses immediately after
> auto-playing", right?
> 
> Without the web inspector things (the breakpoint and action), I did not see
> any issue after clicking the play/pause button. That's why I would like to
> clarify the reproducing steps. Thanks!

Right. The timing of the pause is crucial to reproduce the bug. It might be very difficult to manually pause ASAP after the video starts to play.

I thought you meant something different by the question, and this made me do a little testing. I've now noticed that with the breakpoint and action, the video view and controls become somewhat unresponsive to clicking when the bug occurs. You can expose the <video> element directly to clicking by adding a little CSS to your Safari style sheet:

video ~ div { display:none !important; }

That should hide all of the non-native video controls.
Comment 11 Peng Liu 2022-01-21 14:57:36 PST
Created attachment 449697 [details]
Patch
Comment 12 Peng Liu 2022-01-21 16:50:26 PST
Comment on attachment 449697 [details]
Patch

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

> Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:-230
> -    m_cachedState.paused = false;

Looks like this change will lead to a test (media/track/track-cues-pause-on-exit.html) timeout.
Comment 13 Peng Liu 2022-01-21 18:43:57 PST
(In reply to Peng Liu from comment #12)
> Comment on attachment 449697 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=449697&action=review
> 
> > Source/WebKit/WebProcess/GPU/media/MediaPlayerPrivateRemote.cpp:-230
> > -    m_cachedState.paused = false;
> 
> Looks like this change will lead to a test
> (media/track/track-cues-pause-on-exit.html) timeout.

Filed Bug 235465 to address it.
Comment 14 Peng Liu 2022-01-21 18:45:24 PST
Created attachment 449716 [details]
Remove some changes to address a test failure
Comment 15 EWS 2022-01-22 19:39:16 PST
Committed r288413 (246304@main): <https://commits.webkit.org/246304@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 449716 [details].
Comment 16 Jeff Johnson 2022-02-10 15:05:53 PST
In my testing, this bug appears to be fixed in Safari Technology Preview 140. Thanks!
Comment 17 Brent Fulgham 2022-05-26 14:53:22 PDT
This fix shipped with Safari 15.5 (all platforms).
Comment 18 Jeff Johnson 2022-05-26 14:56:51 PDT
(In reply to Brent Fulgham from comment #17)
> This fix shipped with Safari 15.5 (all platforms).

Confirmed.