From e2f094b952dcc91519eb8c7122c0b3aed203613d Mon Sep 17 00:00:00 2001
From: Andrew Paseltiner <apaseltiner@chromium.org>
Date: Thu, 17 Sep 2026 09:09:26 -0700
Subject: [PATCH] Ensure callback outlives decryptor in BackendDecryptor

Reorder class members in BackendDecryptor so that decrypt_cb_ and
task_runner_ are declared before decryptor_. Because member destruction
proceeds in reverse declaration order, this ensures that callback and
task runner dependencies remain valid if the decryptor aborts pending
operations and synchronously notifies its delegate during destruction.

Additionally, add checks before executing decrypt_cb_, validate that
pending_buffers_ is not empty in OnDecryptComplete, remove a redundant
TaskRunnerImpl allocation in the constructor, and add unit tests.

Fixed: 534997484
Change-Id: Ie4a45392bb55c7b48e108136c87b35427f1f4d2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8349791
Commit-Queue: Andrew Paseltiner <apaseltiner@chromium.org>
Reviewed-by: Shawn Quereshi <shawnq@google.com>
Cr-Commit-Position: refs/heads/main@{#1699864}
---
diff --git a/chromecast/media/cma/pipeline/backend_decryptor.cc b/chromecast/media/cma/pipeline/backend_decryptor.cc
index 59e4843..19f32fc 100644
--- a/chromecast/media/cma/pipeline/backend_decryptor.cc
+++ b/chromecast/media/cma/pipeline/backend_decryptor.cc
@@ -17,10 +17,9 @@
 BackendDecryptor::BackendDecryptor(EncryptionScheme scheme)
     : decrypt_success_(true),
       wait_eos_(false),
-      task_runner_(new TaskRunnerImpl) {
+      task_runner_(std::make_unique<TaskRunnerImpl>()) {
   DCHECK(MediaPipelineBackend::CreateAudioDecryptor);
 
-  task_runner_ = std::make_unique<TaskRunnerImpl>();
   decryptor_ = base::WrapUnique(
       MediaPipelineBackend::CreateAudioDecryptor(scheme, task_runner_.get()));
 
@@ -43,32 +42,38 @@
   // also needed so that the last buffer can be flushed.
   pending_buffers_.push(buffer);
 
-  if (buffer->end_of_stream())
+  if (buffer->end_of_stream()) {
     wait_eos_ = true;
+  }
 
   MediaPipelineBackend::BufferStatus status = decryptor_->PushBufferForDecrypt(
       buffer.get(),
       buffer->end_of_stream() ? nullptr : buffer->writable_data());
 
-  if (status != MediaPipelineBackend::kBufferPending)
+  if (status != MediaPipelineBackend::kBufferPending) {
     OnPushBufferForDecryptComplete(status);
+  }
 }
 
 void BackendDecryptor::OnPushBufferForDecryptComplete(
     MediaPipelineBackend::BufferStatus status) {
   // If the pushed buffer is EOS, the callback should be called when all the
   // buffers are decrypted.
-  if (wait_eos_)
+  if (wait_eos_) {
     return;
+  }
 
-  DCHECK(decrypt_cb_);
-  decrypt_cb_.Run(
-      decrypt_success_ && status == MediaPipelineBackend::kBufferSuccess,
-      std::move(ready_buffers_));
+  if (decrypt_cb_) {
+    decrypt_cb_.Run(
+        decrypt_success_ && status == MediaPipelineBackend::kBufferSuccess,
+        std::move(ready_buffers_));
+  }
 }
 
 void BackendDecryptor::OnDecryptComplete(bool success) {
-  DCHECK(!pending_buffers_.empty());
+  if (pending_buffers_.empty()) {
+    return;
+  }
 
   // Cache the success value and return it in OnPushBufferForDecryptComplete.
   decrypt_success_ &= success;
@@ -83,10 +88,11 @@
   if (wait_eos_ && buffer->end_of_stream()) {
     // Last frame, all the buffers should be decrypted.
     DCHECK(pending_buffers_.empty());
-    DCHECK(decrypt_cb_);
-    LOG(INFO) << "Return all the ready buffers, size = "
-              << ready_buffers_.size();
-    decrypt_cb_.Run(decrypt_success_, std::move(ready_buffers_));
+    if (decrypt_cb_) {
+      LOG(INFO) << "Return all the ready buffers, size = "
+                << ready_buffers_.size();
+      decrypt_cb_.Run(decrypt_success_, std::move(ready_buffers_));
+    }
   }
 }
 
diff --git a/chromecast/media/cma/pipeline/backend_decryptor.h b/chromecast/media/cma/pipeline/backend_decryptor.h
index 222ead29..65a9977 100644
--- a/chromecast/media/cma/pipeline/backend_decryptor.h
+++ b/chromecast/media/cma/pipeline/backend_decryptor.h
@@ -45,11 +45,11 @@
   bool decrypt_success_;
   bool wait_eos_;
 
-  // |task_runner_| should have a longer life than |decryptor_|.
+  // |decrypt_cb_| and |task_runner_| should have a longer life than
+  // |decryptor_|.
+  DecryptCB decrypt_cb_;
   std::unique_ptr<TaskRunnerImpl> task_runner_;
   std::unique_ptr<MediaPipelineBackend::AudioDecryptor> decryptor_;
-
-  DecryptCB decrypt_cb_;
 };
 
 }  // namespace media
diff --git a/chromecast/media/cma/pipeline/backend_decryptor_unittest.cc b/chromecast/media/cma/pipeline/backend_decryptor_unittest.cc
new file mode 100644
index 0000000..7890c28
--- /dev/null
+++ b/chromecast/media/cma/pipeline/backend_decryptor_unittest.cc
@@ -0,0 +1,227 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromecast/media/cma/pipeline/backend_decryptor.h"
+
+#include <memory>
+#include <utility>
+
+#include "base/functional/bind.h"
+#include "base/test/task_environment.h"
+#include "chromecast/media/base/cast_decoder_buffer_impl.h"
+#include "chromecast/public/media/media_pipeline_backend.h"
+#include "chromecast/public/task_runner.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromecast {
+namespace media {
+
+namespace {
+
+enum class DecryptorMode {
+  kPendingAndAbortOnDestruction,
+  kImmediateSuccess,
+};
+
+DecryptorMode g_decryptor_mode = DecryptorMode::kPendingAndAbortOnDestruction;
+
+class TestAudioDecryptor;
+TestAudioDecryptor* g_last_decryptor = nullptr;
+
+// AudioDecryptor test implementation whose behavior is configurable for tests.
+class TestAudioDecryptor : public MediaPipelineBackend::AudioDecryptor {
+ public:
+  TestAudioDecryptor() { g_last_decryptor = this; }
+
+  ~TestAudioDecryptor() override {
+    if (g_last_decryptor == this) {
+      g_last_decryptor = nullptr;
+    }
+    if (g_decryptor_mode == DecryptorMode::kPendingAndAbortOnDestruction &&
+        has_pending_ && delegate_) {
+      delegate_->OnPushBufferForDecryptComplete(
+          MediaPipelineBackend::kBufferFailed);
+    }
+  }
+
+  void SetDelegate(Delegate* delegate) override { delegate_ = delegate; }
+
+  BufferStatus PushBufferForDecrypt(CastDecoderBuffer* buffer,
+                                    uint8_t* output) override {
+    if (g_decryptor_mode == DecryptorMode::kPendingAndAbortOnDestruction) {
+      has_pending_ = true;
+      return MediaPipelineBackend::kBufferPending;
+    }
+
+    if (g_decryptor_mode == DecryptorMode::kImmediateSuccess) {
+      if (delegate_) {
+        delegate_->OnDecryptComplete(true);
+      }
+      return MediaPipelineBackend::kBufferSuccess;
+    }
+
+    return MediaPipelineBackend::kBufferSuccess;
+  }
+
+  Delegate* delegate() const { return delegate_; }
+
+ private:
+  Delegate* delegate_ = nullptr;
+  bool has_pending_ = false;
+};
+
+void OnDecrypted(bool* called,
+                 bool* out_success,
+                 size_t* out_buffer_count,
+                 bool success,
+                 StreamDecryptor::BufferQueue buffers) {
+  if (called) {
+    *called = true;
+  }
+  if (out_success) {
+    *out_success = success;
+  }
+  if (out_buffer_count) {
+    *out_buffer_count = buffers.size();
+  }
+}
+
+}  // namespace
+
+// Provide the factory function so BackendDecryptor creates TestAudioDecryptor.
+MediaPipelineBackend::AudioDecryptor*
+MediaPipelineBackend::CreateAudioDecryptor(EncryptionScheme scheme,
+                                           TaskRunner* task_runner) {
+  return new TestAudioDecryptor();
+}
+
+class BackendDecryptorTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    g_decryptor_mode = DecryptorMode::kPendingAndAbortOnDestruction;
+    g_last_decryptor = nullptr;
+  }
+
+  void TearDown() override { g_last_decryptor = nullptr; }
+
+  base::test::TaskEnvironment task_environment_;
+};
+
+TEST_F(BackendDecryptorTest, DelegateNotificationOnDestruction) {
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  bool success = true;
+  decryptor->Init(
+      base::BindRepeating(&OnDecrypted, &called, &success, nullptr));
+
+  // Push a buffer; decryptor returns kBufferPending so completion is deferred.
+  decryptor->Decrypt(base::MakeRefCounted<CastDecoderBufferImpl>(16));
+
+  // Reset decryptor. During destruction, AudioDecryptor notifies delegate.
+  decryptor.reset();
+
+  EXPECT_TRUE(called);
+  EXPECT_FALSE(success);
+}
+
+TEST_F(BackendDecryptorTest, DestructionWithoutInit) {
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  // Push buffer without calling Init().
+  decryptor->Decrypt(base::MakeRefCounted<CastDecoderBufferImpl>(16));
+
+  // Reset decryptor; should complete cleanly without initialized callback.
+  decryptor.reset();
+}
+
+TEST_F(BackendDecryptorTest, DestructionWithoutPendingOperation) {
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  decryptor->Init(base::BindRepeating(&OnDecrypted, &called, nullptr, nullptr));
+
+  // Reset decryptor without pushing any buffers.
+  decryptor.reset();
+
+  EXPECT_FALSE(called);
+}
+
+TEST_F(BackendDecryptorTest, ImmediateSuccessfulDecryption) {
+  g_decryptor_mode = DecryptorMode::kImmediateSuccess;
+
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  bool success = false;
+  size_t buffer_count = 0;
+  decryptor->Init(
+      base::BindRepeating(&OnDecrypted, &called, &success, &buffer_count));
+
+  // Push a buffer; decryptor completes immediately.
+  decryptor->Decrypt(base::MakeRefCounted<CastDecoderBufferImpl>(16));
+
+  EXPECT_TRUE(called);
+  EXPECT_TRUE(success);
+  EXPECT_EQ(buffer_count, 1u);
+}
+
+TEST_F(BackendDecryptorTest, EmptyPendingBuffersOnDecryptComplete) {
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  decryptor->Init(base::BindRepeating(&OnDecrypted, &called, nullptr, nullptr));
+
+  // Invoke OnDecryptComplete directly when no buffer is pending.
+  EXPECT_TRUE(g_last_decryptor);
+  if (g_last_decryptor && g_last_decryptor->delegate()) {
+    g_last_decryptor->delegate()->OnDecryptComplete(false);
+  }
+
+  EXPECT_FALSE(called);
+}
+
+TEST_F(BackendDecryptorTest, EndOfStreamDecryption) {
+  g_decryptor_mode = DecryptorMode::kImmediateSuccess;
+
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  bool success = false;
+  size_t buffer_count = 0;
+  decryptor->Init(
+      base::BindRepeating(&OnDecrypted, &called, &success, &buffer_count));
+
+  // Push an EOS buffer; decryptor completes and flushes all ready buffers.
+  decryptor->Decrypt(CastDecoderBufferImpl::CreateEOSBuffer());
+
+  EXPECT_TRUE(called);
+  EXPECT_TRUE(success);
+  EXPECT_EQ(buffer_count, 1u);
+}
+
+TEST_F(BackendDecryptorTest, DestructionWithEosPending) {
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  decryptor->Init(base::BindRepeating(&OnDecrypted, &called, nullptr, nullptr));
+
+  // Push an EOS buffer with pending completion.
+  decryptor->Decrypt(CastDecoderBufferImpl::CreateEOSBuffer());
+
+  // Reset decryptor while waiting for EOS.
+  decryptor.reset();
+
+  EXPECT_FALSE(called);
+}
+
+}  // namespace media
+}  // namespace chromecast
diff --git a/chromecast/media/cma/BUILD.gn b/chromecast/media/cma/BUILD.gn
index fd90cc3..4df96eb 100644
--- a/chromecast/media/cma/BUILD.gn
+++ b/chromecast/media/cma/BUILD.gn
@@ -54,6 +54,7 @@
     "base/demuxer_stream_for_test.h",
     "base/multi_demuxer_stream_adapter_unittest.cc",
     "pipeline/audio_video_pipeline_impl_unittest.cc",
+    "pipeline/backend_decryptor_unittest.cc",
     "pipeline/media_pipeline_impl_unittest.cc",
   ]
 

diff --git a/chromecast/media/cma/pipeline/backend_decryptor_unittest.cc b/chromecast/media/cma/pipeline/backend_decryptor_unittest.cc
new file mode 100644
index 0000000..7890c28
--- /dev/null
+++ b/chromecast/media/cma/pipeline/backend_decryptor_unittest.cc
@@ -0,0 +1,227 @@
+// Copyright 2026 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chromecast/media/cma/pipeline/backend_decryptor.h"
+
+#include <memory>
+#include <utility>
+
+#include "base/functional/bind.h"
+#include "base/test/task_environment.h"
+#include "chromecast/media/base/cast_decoder_buffer_impl.h"
+#include "chromecast/public/media/media_pipeline_backend.h"
+#include "chromecast/public/task_runner.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace chromecast {
+namespace media {
+
+namespace {
+
+enum class DecryptorMode {
+  kPendingAndAbortOnDestruction,
+  kImmediateSuccess,
+};
+
+DecryptorMode g_decryptor_mode = DecryptorMode::kPendingAndAbortOnDestruction;
+
+class TestAudioDecryptor;
+TestAudioDecryptor* g_last_decryptor = nullptr;
+
+// AudioDecryptor test implementation whose behavior is configurable for tests.
+class TestAudioDecryptor : public MediaPipelineBackend::AudioDecryptor {
+ public:
+  TestAudioDecryptor() { g_last_decryptor = this; }
+
+  ~TestAudioDecryptor() override {
+    if (g_last_decryptor == this) {
+      g_last_decryptor = nullptr;
+    }
+    if (g_decryptor_mode == DecryptorMode::kPendingAndAbortOnDestruction &&
+        has_pending_ && delegate_) {
+      delegate_->OnPushBufferForDecryptComplete(
+          MediaPipelineBackend::kBufferFailed);
+    }
+  }
+
+  void SetDelegate(Delegate* delegate) override { delegate_ = delegate; }
+
+  BufferStatus PushBufferForDecrypt(CastDecoderBuffer* buffer,
+                                    uint8_t* output) override {
+    if (g_decryptor_mode == DecryptorMode::kPendingAndAbortOnDestruction) {
+      has_pending_ = true;
+      return MediaPipelineBackend::kBufferPending;
+    }
+
+    if (g_decryptor_mode == DecryptorMode::kImmediateSuccess) {
+      if (delegate_) {
+        delegate_->OnDecryptComplete(true);
+      }
+      return MediaPipelineBackend::kBufferSuccess;
+    }
+
+    return MediaPipelineBackend::kBufferSuccess;
+  }
+
+  Delegate* delegate() const { return delegate_; }
+
+ private:
+  Delegate* delegate_ = nullptr;
+  bool has_pending_ = false;
+};
+
+void OnDecrypted(bool* called,
+                 bool* out_success,
+                 size_t* out_buffer_count,
+                 bool success,
+                 StreamDecryptor::BufferQueue buffers) {
+  if (called) {
+    *called = true;
+  }
+  if (out_success) {
+    *out_success = success;
+  }
+  if (out_buffer_count) {
+    *out_buffer_count = buffers.size();
+  }
+}
+
+}  // namespace
+
+// Provide the factory function so BackendDecryptor creates TestAudioDecryptor.
+MediaPipelineBackend::AudioDecryptor*
+MediaPipelineBackend::CreateAudioDecryptor(EncryptionScheme scheme,
+                                           TaskRunner* task_runner) {
+  return new TestAudioDecryptor();
+}
+
+class BackendDecryptorTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    g_decryptor_mode = DecryptorMode::kPendingAndAbortOnDestruction;
+    g_last_decryptor = nullptr;
+  }
+
+  void TearDown() override { g_last_decryptor = nullptr; }
+
+  base::test::TaskEnvironment task_environment_;
+};
+
+TEST_F(BackendDecryptorTest, DelegateNotificationOnDestruction) {
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  bool success = true;
+  decryptor->Init(
+      base::BindRepeating(&OnDecrypted, &called, &success, nullptr));
+
+  // Push a buffer; decryptor returns kBufferPending so completion is deferred.
+  decryptor->Decrypt(base::MakeRefCounted<CastDecoderBufferImpl>(16));
+
+  // Reset decryptor. During destruction, AudioDecryptor notifies delegate.
+  decryptor.reset();
+
+  EXPECT_TRUE(called);
+  EXPECT_FALSE(success);
+}
+
+TEST_F(BackendDecryptorTest, DestructionWithoutInit) {
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  // Push buffer without calling Init().
+  decryptor->Decrypt(base::MakeRefCounted<CastDecoderBufferImpl>(16));
+
+  // Reset decryptor; should complete cleanly without initialized callback.
+  decryptor.reset();
+}
+
+TEST_F(BackendDecryptorTest, DestructionWithoutPendingOperation) {
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  decryptor->Init(base::BindRepeating(&OnDecrypted, &called, nullptr, nullptr));
+
+  // Reset decryptor without pushing any buffers.
+  decryptor.reset();
+
+  EXPECT_FALSE(called);
+}
+
+TEST_F(BackendDecryptorTest, ImmediateSuccessfulDecryption) {
+  g_decryptor_mode = DecryptorMode::kImmediateSuccess;
+
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  bool success = false;
+  size_t buffer_count = 0;
+  decryptor->Init(
+      base::BindRepeating(&OnDecrypted, &called, &success, &buffer_count));
+
+  // Push a buffer; decryptor completes immediately.
+  decryptor->Decrypt(base::MakeRefCounted<CastDecoderBufferImpl>(16));
+
+  EXPECT_TRUE(called);
+  EXPECT_TRUE(success);
+  EXPECT_EQ(buffer_count, 1u);
+}
+
+TEST_F(BackendDecryptorTest, EmptyPendingBuffersOnDecryptComplete) {
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  decryptor->Init(base::BindRepeating(&OnDecrypted, &called, nullptr, nullptr));
+
+  // Invoke OnDecryptComplete directly when no buffer is pending.
+  EXPECT_TRUE(g_last_decryptor);
+  if (g_last_decryptor && g_last_decryptor->delegate()) {
+    g_last_decryptor->delegate()->OnDecryptComplete(false);
+  }
+
+  EXPECT_FALSE(called);
+}
+
+TEST_F(BackendDecryptorTest, EndOfStreamDecryption) {
+  g_decryptor_mode = DecryptorMode::kImmediateSuccess;
+
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  bool success = false;
+  size_t buffer_count = 0;
+  decryptor->Init(
+      base::BindRepeating(&OnDecrypted, &called, &success, &buffer_count));
+
+  // Push an EOS buffer; decryptor completes and flushes all ready buffers.
+  decryptor->Decrypt(CastDecoderBufferImpl::CreateEOSBuffer());
+
+  EXPECT_TRUE(called);
+  EXPECT_TRUE(success);
+  EXPECT_EQ(buffer_count, 1u);
+}
+
+TEST_F(BackendDecryptorTest, DestructionWithEosPending) {
+  auto decryptor =
+      std::make_unique<BackendDecryptor>(EncryptionScheme::kAesCbc);
+
+  bool called = false;
+  decryptor->Init(base::BindRepeating(&OnDecrypted, &called, nullptr, nullptr));
+
+  // Push an EOS buffer with pending completion.
+  decryptor->Decrypt(CastDecoderBufferImpl::CreateEOSBuffer());
+
+  // Reset decryptor while waiting for EOS.
+  decryptor.reset();
+
+  EXPECT_FALSE(called);
+}
+
+}  // namespace media
+}  // namespace chromecast