7db3f95b0a4cd7f25d5e0e8a576523cc3d85a3c9  [WebCodecs] Fix unnecessary call to Decoder::Reset() during shutdown

The bug at a glance

Watch-tier because the change tightens a shutdown/close-path lifetime interaction in WebCodecs rather than fixing a proven memory-corruption bug. The pre-fix code already handled the shutdown case: during Shutdown() it detected a pending reset request, released it, and ended its trace explicitly, precisely because OnResetDone() would never fire once the decoder was reset and the codec moved to kClosed. The commit reorganizes this so that a reset request is short-circuited at the top of ProcessReset() while shutting_down_ is set, meaning no reset work touches decoder(), no callback is scheduled, and pending_request_ is guaranteed empty at the release point (now a DCHECK(!pending_request_)). This removes a redundant Decoder::Reset() and closes the window in which a reset callback and a being-deleted decoder could interact during teardown. It rates low/CVSS 2.5 because it is a latent-safety/robustness fix with a plausible but undemonstrated UAF shape on the close path; the author frames it as avoiding ’extra work,’ and no crash or exploit is shown.

Treat this as decoder-lifetime hardening on the WebCodecs close path, driven by bug 474398415. When a VideoDecoder/AudioDecoder is closed, Blink’s DecoderTemplate must abort outstanding callbacks without dispatching more work into a decoder it is about to DeleteSoon. The old logic let a reset be queued as part of ResetAlgorithm during shutdown and then cleaned it up after the fact; the new logic refuses to process the reset at all once shutting_down_ is true, so the codec-implementation Reset() is never issued and there is no dangling reset callback to reconcile against a decoder that has been handed to DeleteSoon. Converting the post-hoc pending_request_ release into DCHECK(!pending_request_) turns a runtime cleanup into an invariant the code now maintains by construction - the classic signature of tightening an object-lifetime edge case rather than patching an in-the-wild bug.

Root cause

DecoderTemplate::ProcessReset() handles a Request of type kReset: it signals the codec implementation to stop producing output and calls Reset() on the underlying media decoder, whose completion runs OnResetDone(). Separately, Shutdown() tears the codec down: it DeleteSoon()s decoder_ and moves to kClosed. In the pre-fix code, if a reset had been enqueued (via ResetAlgorithm) as part of shutdown, Shutdown() had to explicitly release pending_request_ - asserting it was a kReset - and end its trace with shutting_down=true, precisely because OnResetDone() would never execute after the decoder was reset and the state became kClosed.

The root cause addressed is that processing a reset during shutdown is both unnecessary and lifetime-fragile: it issues a Decoder::Reset() on a decoder that is about to be destroyed and creates a pending reset callback that must be manually cleaned up, opening a window where the reset path and the teardown path race over decoder_/callback lifetime. The fix inserts an early return at the top of ProcessReset() when shutting_down_ is true - it returns true (request handled) without touching decoder() or scheduling any callback. Because no reset request is ever left outstanding through shutdown, Shutdown() no longer needs to release pending_request_; the manual release-and-EndTracing block is replaced by DCHECK(!pending_request_), asserting the new invariant. The shipped video_decoder_test.cc change adds an EXPECT_CALL that models Reset_ posting its closure on the task runner, exercising the reset-then-shutdown ordering so the new short-circuit is covered.

Key code

decoder_template.cc: short-circuit reset during shutdown; replace post-hoc pending_request release with an invariant DCHECK

  DCHECK_EQ(request->type, Request::Type::kReset);
  DCHECK_GT(reset_generation_, 0u);

  if (shutting_down_) {
    // No need to process reset during shutdown.
    return true;
  }

  // ... signal codec implementation, then in Shutdown():
  main_thread_task_runner_->DeleteSoon(FROM_HERE, std::move(decoder_));

-  if (pending_request_) {
-    DCHECK_EQ(pending_request_->type, Request::Type::kReset);
-    pending_request_.Release()->EndTracing(/*shutting_down=*/true);
-  }
+  DCHECK(!pending_request_);

Patch walkthrough

• third_party/blink/renderer/modules/webcodecs/decoder_template.cc — Two edits. (1) In ProcessReset(), after the type/generation DCHECKs, add an early if (shutting_down_) return true; so a reset request is acknowledged but no codec Reset() is issued and no reset callback is scheduled during shutdown. (2) In Shutdown(), after DeleteSoon(decoder_), remove the block that released pending_request_ and ended its trace, replacing it with DCHECK(!pending_request_). Together these ensure no reset work touches a decoder queued for deletion and no reset callback outlives the decoder, and they encode the resulting no-pending-request state as an assertion instead of a cleanup.
• third_party/blink/renderer/modules/webcodecs/video_decoder_test.cc — Adds an EXPECT_CALL on the mock decoder’s Reset_ that, when invoked, posts the completion closure on the sequenced test task runner. This drives the reset-during-teardown ordering through the new short-circuit path so the reset/shutdown interaction is regression-covered.

Background

WebCodecs DecoderTemplate — Shared base for VideoDecoder/AudioDecoder in Blink. It queues Requests (decode, reset, flush) and drives an underlying media::Decoder, reconciling asynchronous decoder callbacks with JS-visible state transitions including close().

Reset vs Shutdown/close — reset() aborts outstanding work and calls Decoder::Reset(), completing via OnResetDone(). close()/Shutdown() destroys the codec, moving to kClosed and DeleteSoon-ing the decoder; after that, decoder callbacks like OnResetDone() will never run.

pending_request_ and EndTracing — pending_request_ tracks the in-flight request for async-trace bookkeeping. The removed code released it with shutting_down=true so its trace event closed; the fix guarantees it is never set at that point, so the manual close is unnecessary.

DeleteSoon — Posts destruction of decoder_ to the task runner rather than freeing inline, so any work still referencing the decoder between now and that task runs against a soon-to-be-freed object - the fragile window this change avoids entering.

No CVE / no backport — Bug 474398415; no CVE and no [M###] merge/backport marker; landed to main only (refs/heads/main@{#1572701}), consistent with lifetime hardening rather than a stable security merge.

Vulnerability window

1. Discovery — Bug 474398415 identified unnecessary/redundant Decoder::Reset() work on the WebCodecs shutdown path and the awkward post-hoc pending_request_ release that shadowed a lifetime-fragile reset/teardown interaction.
2. Fix design — Rather than clean up after a reset processed during shutdown, short-circuit reset processing while shutting_down_ so no reset callback is ever outstanding, making the release step provably unnecessary.
3. Landed fix — Committed 2026-01-21 (Dale Curtis), reviewed by Eugene Zemtsov, at refs/heads/main@{#1572701}, with a video_decoder_test.cc case exercising reset-then-shutdown.

Proof of concept

The commit ships a unit-test fragment (not an exploit) that exercises the reset-during-teardown ordering: it programs the mock decoder’s Reset_ to post its completion closure asynchronously, then calls reset() near shutdown so the pending reset callback and the close path overlap. This reproduces the scenario the fix hardens; it demonstrates coverage, not a working exploit. A defender reconstructing the JS-level trigger would create a VideoDecoder, submit a decode to make it active/reclamation-timed, call reset() and immediately close() (or let the page/frame tear down) so a reset is in flight as shutdown runs. No memory-corruption or control-flow-hijack exploit is demonstrated, and none is invented here; the underlying interaction is a latent lifetime edge, not a proven UAF primitive.

  // Reset the codec.
  EXPECT_CALL(*fake_decoder->mock_decoder(), Reset_(_))
      .WillOnce([](base::OnceClosure& closure) {
        scheduler::GetSequencedTaskRunnerForTesting()->PostTask(
            FROM_HERE, std::move(closure));
      });
  fake_decoder->reset(v8_scope.GetExceptionState());

Exploitation

1. Reach — Fully web-reachable: any page can instantiate a WebCodecs VideoDecoder/AudioDecoder and interleave decode(), reset(), and close(), so the reset/shutdown ordering is under attacker control from JavaScript.
2. Trigger — The pre-fix path required a reset to be enqueued as shutdown ran, then relied on manual pending_request_ cleanup. Turning it into a UAF would require the reset callback or codec Reset() to dereference decoder_ after DeleteSoon in an unsafe order - the fix removes that possibility but no reliable trigger was demonstrated beforehand.
3. Impact ceiling — Honest ceiling is a potential use-after-free crash on the close path under precise timing; no allocation-control, info-leak, or code-execution path is shown. The author’s own framing (‘just extra work’) and the DCHECK-based invariant point to defense-in-depth, not a confirmed exploitable bug.

Detection & hunting

For defenders and detection engineers:

• WebCodecs shutdown crashes — In ASAN/BackupRefPtr fleets, use-after-free or reset-callback crashes in DecoderTemplate::OnResetDone / ProcessReset / Shutdown during codec close indicate the pre-fix window; their absence after this change is the expected signal, and a regression would resurface them.
• DCHECK(!pending_request_) failures — A failure of the new DCHECK(!pending_request_) in Shutdown() would mean a reset slipped past the shutting_down_ short-circuit - a strong indicator that a related lifetime invariant regressed and warrants investigation.

Audit directions

• Other async callbacks vs DeleteSoon in DecoderTemplate —
• Encoder and other WebCodecs templates —