1580015 - [webvtt] Video with min-width AND captions locks up browser tab

Status: VERIFIED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[dstanich]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:69.0) Gecko/20100101 Firefox/69.0

Steps to reproduce:

This is a problem with the <video> tag locking up the browser tab and spiking the CPU.

Steps in the HTML:

1. Created a parent container with a defined height/width
2. Apply a min-width CSS property to the video tag
3. Provide a caption track for the video file for closed captioning
4. Attempt to view the video with closed captioning enabled

You can recreate this by:

1. Clone this repo: https://github.com/dstanich/ff-video-caption-bug
2. npm install
3. npm start
4. Go to http://localhost:8080 and attempt to watch the video

Actual results:

As of Firefox 69, if BOTH min-width and closed captioning is enabled on the video element, then the browser tab will lock up and requires it to sometimes be force quit.

I tested in the latest FF ESR 68.1.0 to compare and this works fine here.

Expected results:

Video should have been properly positioned with the min-width and the video should have played showing the captioning as it played.

Comment 1

[Alex Chronopoulos [:achronop]]

I see from the profile [1] that most of the time is spent in the WeVTT.processCues which is called repeatedly. Alastor can you please have a look and redirect accordingly.

[1] https://perfht.ml/2PYXvkP

Comment 2

[Alastor Wu [:alwu]]

Will take a look.

Comment 3

[Alastor Wu [:alwu]]

Attachment: Bug 1580015 - part1 : do not reentry function 'processCues()'.

Accessing offsetXXX attributes of element, it would trigger reflow, and sometime it would result in a re-entry of this function because we would call processCue() when resizecaption occurs.

One specifical case is that, assigning CSS properties min-width and max-width at the same video element, in thise case, when we re-enter the 'processCues()' and access offsetXXX. It seems that the layout system would reprocess those properites again and again and dispatch resizecaption when we access offsetXXX, which causes an infinite loop, starting from receving resizecaption, then calling processCue(), accessing offSetXXX, triggering reflow, sending resizecaption` event again.

Therefore, we should stop revisiting processCues() if we're already running a processing, we should run a processing once at a time.

Comment 4

[Alastor Wu [:alwu]]

Attachment: Bug 1580015 - part2 : add test 'test_webvtt_infinite_processing_loop.html'.

Comment 5

[Alastor Wu [:alwu]]

Add NI as a reminder to uplift the change to beta.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cf45c1ec28a2
part1 : do not reentry function 'processCues()'. r=heycam
https://hg.mozilla.org/integration/autoland/rev/6bd2baa12b99
part2 : add test 'test_webvtt_infinite_processing_loop.html'. r=heycam

Comment 7

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/cf45c1ec28a2
https://hg.mozilla.org/mozilla-central/rev/6bd2baa12b99

Comment 8

[Alastor Wu [:alwu]]

Comment on attachment 9091861 [details]
Bug 1580015 - part1 : do not reentry function 'processCues()'.

Beta/Release Uplift Approval Request

• User impact if declined: Page will freeze forever and reach a CPU spike.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): In this patch, we prevent to run the same task again while there is another task running, we didn't change or add any feature or behavior changes.
• String changes made/needed: no

Comment 9

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Comment on attachment 9091861 [details]
Bug 1580015 - part1 : do not reentry function 'processCues()'.

Fix for regression from 69, avoids CPU freeze when video captions are on.
Let's take this for beta 7.

Comment 10

[Oana Pop-Rus]

https://hg.mozilla.org/releases/mozilla-beta/rev/729169db4c46
https://hg.mozilla.org/releases/mozilla-beta/rev/11d82c144152

Comment 11

[Rares Doghi, Desktop QA]

This issue is verified as fixed on our latest Nightly build as well as beta 70.0b7 on Windows 10, Mac 10.14 and Ubuntu 18.04

Comment 12

[Rares Doghi, Desktop QA]

Since this is a Wont Fix for 69 I will go ahead and change the resolution for it to Verified.