Skip to content

Commit

Permalink
[webcodecs] Various DecoderTemplate shutdown cleanups
Browse files Browse the repository at this point in the history
1. Use DeleteSoon() when destroying the |decoder_| to avoid destructing
   in error conditions where it's callback (OnDecodeDone()) is actively
   executing.

2. Call Shutdown() from ContextDestroyed() to invoke full reset
   algorithm and tidily clean up all state.

Bug: 1267426
Change-Id: I927d8a3bd245b3f833e73c74103cbaacf6d73406
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3277273
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Auto-Submit: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/main@{#941390}
  • Loading branch information
chcunningham authored and Chromium LUCI CQ committed Nov 13, 2021
1 parent 4690c39 commit b5e609d
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 10 deletions.
17 changes: 10 additions & 7 deletions third_party/blink/renderer/modules/webcodecs/decoder_template.cc
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,10 @@ DecoderTemplate<Traits>::DecoderTemplate(ScriptState* script_state,
ExecutionContext* context = GetExecutionContext();
DCHECK(context);

logger_ = std::make_unique<CodecLogger>(
context, context->GetTaskRunner(TaskType::kInternalMedia));
main_thread_task_runner_ =
context->GetTaskRunner(TaskType::kInternalMediaRealTime);

logger_ = std::make_unique<CodecLogger>(context, main_thread_task_runner_);

logger_->log()->SetProperty<media::MediaLogProperty::kFrameUrl>(
context->Url().GetString().Ascii());
Expand Down Expand Up @@ -506,8 +508,10 @@ void DecoderTemplate<Traits>::Shutdown(DOMException* exception) {
// Prevent any further logging from being reported.
logger_->Neuter();

// Clear decoding and JS-visible queue state.
decoder_.reset();
// Clear decoding and JS-visible queue state. Use DeleteSoon() to avoid
// deleting decoder_ when its callback (e.g. OnDecodeDone()) may be below us
// in the stack.
main_thread_task_runner_->DeleteSoon(FROM_HERE, std::move(decoder_));

if (pending_request_) {
// This request was added as part of calling ResetAlgorithm above. However,
Expand Down Expand Up @@ -724,9 +728,8 @@ void DecoderTemplate<Traits>::TraceQueueSizes() const {

template <typename Traits>
void DecoderTemplate<Traits>::ContextDestroyed() {
state_ = V8CodecState(V8CodecState::Enum::kClosed);
logger_->Neuter();
decoder_.reset();
// Deallocate resources and supress late callbacks from media thread.
Shutdown();
}

template <typename Traits>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@
#include "third_party/blink/renderer/platform/heap/heap_allocator.h"
#include "third_party/blink/renderer/platform/heap/member.h"

namespace base {
class SingleThreadTaskRunner;
}

namespace media {
class GpuVideoAcceleratorFactories;
class ScopedDecodeTrace;
Expand Down Expand Up @@ -227,6 +231,9 @@ class MODULES_EXPORT DecoderTemplate

// Keyframes are required after configure(), flush(), and reset().
bool require_key_frame_ = true;

// Task runner for main thread.
scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
};

} // namespace blink
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,6 @@ TYPED_TEST(DecoderTemplateTest, MAYBE_CodecReclamation) {
testing::Mock::VerifyAndClearExpectations(error_callback);
}

// Note: AudioDecoder and VideoDecoder specific tests should be put in
// audio_decoder_test.cc and video_decoder_test.cc respectively.

} // namespace

} // namespace blink

0 comments on commit b5e609d

Please sign in to comment.