2055309 - Make WebVTT.processCues robust to exceptions with try/finally

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, task, P4)

Description

[Alastor Wu [:alwu]]

Filing as a good first bug to learn workflows.

WebVTT.processCues in dom/media/webvtt/vtt.sys.mjs guards against re-entrancy with an isProcessingCues flag, but it has no try/finally. It sets the flag to true, calls processCuesInternal(), then sets it back to false:

WebVTT.processCues = function(window, cues, overlay, controls) {
  if (this.isProcessingCues) {
    return;
  }
  this.isProcessingCues = true;
  processCuesInternal(window, cues, overlay, controls);
  this.isProcessingCues = false;
};

If processCuesInternal() throws, isProcessingCues is left stuck at true, and every later call to processCues() returns early at the guard, permanently disabling WebVTT cue (subtitle/caption) rendering for that document.

Please wrap the call in try/finally so the flag is always reset:

this.isProcessingCues = true;
try {
  processCuesInternal(window, cues, overlay, controls);
} finally {
  this.isProcessingCues = false;
}

Link to the code:
https://searchfox.org/mozilla-central/source/dom/media/webvtt/vtt.sys.mjs#1352

To verify the fix:

./mach mochitest dom/media/webvtt/test/mochitest/
./mach wpt testing/web-platform/tests/webvtt/

A regression test that makes processCuesInternal throw once and asserts that a later cue still renders would be a welcome addition, but is optional.

This was surfaced while investigating bug 2047835 (a duplicate of bug 2047708), where an exception thrown inside processCuesInternal turned a single-cue failure into a total, persistent caption blackout.

Tutorial to contribute:
https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html
https://firefox-source-docs.mozilla.org/contributing/stack_quickref.html

Please don't ask for the bug to be assigned. It will be automatically assigned to the first patch.

Comment 1

[munawiki]

Attachment: Bug 2055309 - Restore WebVTT processing state when cue rendering throws. r=alwu

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d25517baca1e
https://hg.mozilla.org/integration/autoland/rev/e9df57b55441
Restore WebVTT processing state when cue rendering throws. r=alwu

Comment 3

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/e9df57b55441