diff --git a/components/cast_streaming/renderer/BUILD.gn b/components/cast_streaming/renderer/BUILD.gn index 7d470bed1ebc12..f81927c2cc5ddc 100644 --- a/components/cast_streaming/renderer/BUILD.gn +++ b/components/cast_streaming/renderer/BUILD.gn @@ -72,6 +72,7 @@ source_set("unit_tests") { deps = [ "//components/cast_streaming/renderer/common:unit_tests", "//components/cast_streaming/renderer/control:unit_tests", + "//components/cast_streaming/renderer/frame:unit_tests", "//components/cast_streaming/renderer/web_codecs:unit_tests", ] diff --git a/components/cast_streaming/renderer/frame/BUILD.gn b/components/cast_streaming/renderer/frame/BUILD.gn index 119efe0da1c498..b3660096e5e59c 100644 --- a/components/cast_streaming/renderer/frame/BUILD.gn +++ b/components/cast_streaming/renderer/frame/BUILD.gn @@ -25,3 +25,19 @@ source_set("frame") { "//mojo/public/cpp/bindings", ] } + +source_set("unit_tests") { + testonly = true + public = [] + sources = [ "demuxer_connector_unittest.cc" ] + deps = [ + ":frame", + "//base", + "//base/test:test_support", + "//components/cast_streaming/common:mojom", + "//media:test_support", + "//mojo/public/cpp/bindings", + "//testing/gmock", + "//testing/gtest", + ] +} diff --git a/components/cast_streaming/renderer/frame/demuxer_connector.cc b/components/cast_streaming/renderer/frame/demuxer_connector.cc index 547e5ef0d2ca6b..32e2f1130f5882 100644 --- a/components/cast_streaming/renderer/frame/demuxer_connector.cc +++ b/components/cast_streaming/renderer/frame/demuxer_connector.cc @@ -4,7 +4,7 @@ #include "components/cast_streaming/renderer/frame/demuxer_connector.h" -#include "components/cast_streaming/renderer/frame/frame_injecting_demuxer.h" +#include namespace cast_streaming { @@ -14,38 +14,41 @@ DemuxerConnector::~DemuxerConnector() { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); } -void DemuxerConnector::SetDemuxer(FrameInjectingDemuxer* demuxer) { +void DemuxerConnector::SetDemuxer(StreamsInitializedCB on_streams_initialized) { DVLOG(1) << __func__; DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - DCHECK(demuxer); + DCHECK(on_streams_initialized); - if (demuxer_) { + if (has_demuxer_) { // We do not support more than one active FrameInjectingDemuxer in the same // RenderFrame. Return early here. - demuxer->OnStreamsInitialized(mojom::AudioStreamInitializationInfoPtr(), - mojom::VideoStreamInitializationInfoPtr()); + std::move(on_streams_initialized) + .Run(mojom::AudioStreamInitializationInfoPtr(), + mojom::VideoStreamInitializationInfoPtr()); return; } DCHECK(!is_demuxer_initialized_); if (IsBound()) { - demuxer_ = demuxer; + has_demuxer_ = true; + on_streams_initialized_cb_ = std::move(on_streams_initialized); MaybeCallEnableReceiverCallback(); } else { - // The Cast Streaming Sender disconnected after |demuxer| was instantiated - // but before |demuxer| was initialized on the media thread. - demuxer->OnStreamsInitialized(mojom::AudioStreamInitializationInfoPtr(), - mojom::VideoStreamInitializationInfoPtr()); + // The Cast Streaming Sender disconnected after the demuxer was instantiated + // but before it was initialized on the media thread. + std::move(on_streams_initialized) + .Run(mojom::AudioStreamInitializationInfoPtr(), + mojom::VideoStreamInitializationInfoPtr()); } } void DemuxerConnector::OnDemuxerDestroyed() { DVLOG(1) << __func__; DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - DCHECK(demuxer_); - demuxer_ = nullptr; + has_demuxer_ = false; + on_streams_initialized_cb_.Reset(); is_demuxer_initialized_ = false; demuxer_connector_receiver_.reset(); } @@ -75,7 +78,7 @@ void DemuxerConnector::MaybeCallEnableReceiverCallback() { DVLOG(2) << __func__; DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); - if (enable_receiver_callback_ && demuxer_) { + if (enable_receiver_callback_ && has_demuxer_) { std::move(enable_receiver_callback_).Run(); } } @@ -87,7 +90,7 @@ void DemuxerConnector::OnReceiverDisconnected() { demuxer_connector_receiver_.reset(); enable_receiver_callback_.Reset(); - if (demuxer_ && !is_demuxer_initialized_) { + if (has_demuxer_ && !is_demuxer_initialized_) { OnStreamsInitialized(mojom::AudioStreamInitializationInfoPtr(), mojom::VideoStreamInitializationInfoPtr()); } @@ -109,11 +112,11 @@ void DemuxerConnector::OnStreamsInitialized( DVLOG(1) << __func__; DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK(!is_demuxer_initialized_); - DCHECK(demuxer_); + DCHECK(on_streams_initialized_cb_); is_demuxer_initialized_ = true; - demuxer_->OnStreamsInitialized(std::move(audio_stream_info), - std::move(video_stream_info)); + std::move(on_streams_initialized_cb_) + .Run(std::move(audio_stream_info), std::move(video_stream_info)); } } // namespace cast_streaming diff --git a/components/cast_streaming/renderer/frame/demuxer_connector.h b/components/cast_streaming/renderer/frame/demuxer_connector.h index ad0e86facd759b..e6cba8ad7624a4 100644 --- a/components/cast_streaming/renderer/frame/demuxer_connector.h +++ b/components/cast_streaming/renderer/frame/demuxer_connector.h @@ -6,7 +6,6 @@ #define COMPONENTS_CAST_STREAMING_RENDERER_FRAME_DEMUXER_CONNECTOR_H_ #include "base/functional/callback.h" -#include "base/memory/raw_ptr.h" #include "base/sequence_checker.h" #include "components/cast_streaming/common/public/mojom/demuxer_connector.mojom.h" #include "mojo/public/cpp/bindings/associated_receiver.h" @@ -14,8 +13,6 @@ namespace cast_streaming { -class FrameInjectingDemuxer; - // Handles initiating the streaming session between the browser-process sender // and renderer-process receiver of the Cast Streaming Session. Specifically, // this class manages the DemuxerConnector's lifetime in the renderer @@ -25,6 +22,10 @@ class FrameInjectingDemuxer; // triggered its destruction first. class DemuxerConnector final : public mojom::DemuxerConnector { public: + using StreamsInitializedCB = + base::OnceCallback; + DemuxerConnector(); ~DemuxerConnector() override; DemuxerConnector(const DemuxerConnector&) = delete; @@ -33,7 +34,7 @@ class DemuxerConnector final : public mojom::DemuxerConnector { void BindReceiver( mojo::PendingAssociatedReceiver connector); - void SetDemuxer(FrameInjectingDemuxer* demuxer); + void SetDemuxer(StreamsInitializedCB on_streams_initialized); void OnDemuxerDestroyed(); // Returns true if a Mojo connection is active. @@ -54,7 +55,8 @@ class DemuxerConnector final : public mojom::DemuxerConnector { this}; EnableReceiverCallback enable_receiver_callback_; - raw_ptr demuxer_ = nullptr; + StreamsInitializedCB on_streams_initialized_cb_; + bool has_demuxer_ = false; bool is_demuxer_initialized_ = false; SEQUENCE_CHECKER(sequence_checker_); diff --git a/components/cast_streaming/renderer/frame/demuxer_connector_unittest.cc b/components/cast_streaming/renderer/frame/demuxer_connector_unittest.cc new file mode 100644 index 00000000000000..56e94e0a6c534e --- /dev/null +++ b/components/cast_streaming/renderer/frame/demuxer_connector_unittest.cc @@ -0,0 +1,104 @@ +// 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 "components/cast_streaming/renderer/frame/demuxer_connector.h" + +#include +#include + +#include "base/functional/bind.h" +#include "base/run_loop.h" +#include "base/test/task_environment.h" +#include "base/threading/thread.h" +#include "components/cast_streaming/common/public/mojom/demuxer_connector.mojom.h" +#include "components/cast_streaming/renderer/frame/frame_injecting_demuxer.h" +#include "media/base/mock_demuxer_host.h" +#include "media/base/pipeline_status.h" +#include "mojo/public/cpp/bindings/associated_remote.h" +#include "testing/gmock/include/gmock/gmock.h" +#include "testing/gtest/include/gtest/gtest.h" + +namespace cast_streaming { + +class DemuxerConnectorTest : public testing::Test { + public: + DemuxerConnectorTest() : media_thread_("DemuxerConnectorTestMedia") { + media_thread_.Start(); + } + + ~DemuxerConnectorTest() override { media_thread_.Stop(); } + + protected: + void InitializeDemuxerOnMediaThread(media::Demuxer* demuxer) { + base::RunLoop run_loop; + media_thread_.task_runner()->PostTaskAndReply( + FROM_HERE, + base::BindOnce(&media::Demuxer::Initialize, base::Unretained(demuxer), + &demuxer_host_, InitializedCB()), + run_loop.QuitClosure()); + run_loop.Run(); + } + + void DestroyDemuxerOnMediaThread( + std::unique_ptr demuxer) { + base::RunLoop run_loop; + media_thread_.task_runner()->PostTaskAndReply( + FROM_HERE, + base::BindOnce([](std::unique_ptr) {}, + std::move(demuxer)), + run_loop.QuitClosure()); + run_loop.Run(); + } + + media::PipelineStatusCallback InitializedCB() { + return base::BindOnce(&DemuxerConnectorTest::OnDemuxerInitialized, + base::Unretained(this)); + } + + MOCK_METHOD1(OnDemuxerInitialized, void(media::PipelineStatus)); + + base::test::TaskEnvironment task_environment_; + base::Thread media_thread_; + testing::NiceMock demuxer_host_; + DemuxerConnector connector_; +}; + +TEST_F(DemuxerConnectorTest, DisconnectAfterDemuxerDestroyedDuringInit) { + mojo::AssociatedRemote remote; + connector_.BindReceiver(remote.BindNewEndpointAndPassDedicatedReceiver()); + + auto demuxer = std::make_unique( + &connector_, media_thread_.task_runner()); + InitializeDemuxerOnMediaThread(demuxer.get()); + task_environment_.RunUntilIdle(); + + EXPECT_CALL(*this, OnDemuxerInitialized).Times(0); + DestroyDemuxerOnMediaThread(std::move(demuxer)); + task_environment_.RunUntilIdle(); + + remote.reset(); + task_environment_.RunUntilIdle(); +} + +TEST_F(DemuxerConnectorTest, DemuxerDestroyedBeforeRegistration) { + auto demuxer = std::make_unique( + &connector_, media_thread_.task_runner()); + + base::RunLoop run_loop; + media_thread_.task_runner()->PostTask( + FROM_HERE, base::BindOnce(&media::Demuxer::Initialize, + base::Unretained(demuxer.get()), &demuxer_host_, + InitializedCB())); + media_thread_.task_runner()->PostTaskAndReply( + FROM_HERE, + base::BindOnce([](std::unique_ptr) {}, + std::move(demuxer)), + run_loop.QuitClosure()); + run_loop.Run(); + + EXPECT_CALL(*this, OnDemuxerInitialized).Times(0); + task_environment_.RunUntilIdle(); +} + +} // namespace cast_streaming diff --git a/components/cast_streaming/renderer/frame/frame_injecting_demuxer.cc b/components/cast_streaming/renderer/frame/frame_injecting_demuxer.cc index ec9fbd31e356e8..374435fcd06a50 100644 --- a/components/cast_streaming/renderer/frame/frame_injecting_demuxer.cc +++ b/components/cast_streaming/renderer/frame/frame_injecting_demuxer.cc @@ -11,6 +11,7 @@ #include "base/memory/raw_ptr.h" #include "base/memory/scoped_refptr.h" #include "base/sequence_checker.h" +#include "base/task/bind_post_task.h" #include "base/task/sequenced_task_runner.h" #include "base/task/single_thread_task_runner.h" #include "base/time/time.h" @@ -325,24 +326,9 @@ FrameInjectingDemuxer::FrameInjectingDemuxer( FrameInjectingDemuxer::~FrameInjectingDemuxer() { DVLOG(1) << __func__; - if (was_initialization_successful_) { - original_task_runner_->PostTask( - FROM_HERE, base::BindOnce(&DemuxerConnector::OnDemuxerDestroyed, - base::Unretained(demuxer_connector_))); - } -} - -void FrameInjectingDemuxer::OnStreamsInitialized( - mojom::AudioStreamInitializationInfoPtr audio_stream_info, - mojom::VideoStreamInitializationInfoPtr video_stream_info) { - DVLOG(1) << __func__; - DCHECK(!media_task_runner_->RunsTasksInCurrentSequence()); - - media_task_runner_->PostTask( - FROM_HERE, - base::BindOnce(&FrameInjectingDemuxer::OnStreamsInitializedOnMediaThread, - weak_factory_.GetWeakPtr(), std::move(audio_stream_info), - std::move(video_stream_info))); + original_task_runner_->PostTask( + FROM_HERE, base::BindOnce(&DemuxerConnector::OnDemuxerDestroyed, + base::Unretained(demuxer_connector_))); } void FrameInjectingDemuxer::OnStreamsInitializedOnMediaThread( @@ -389,7 +375,6 @@ void FrameInjectingDemuxer::OnStreamInitializationComplete() { return; } - was_initialization_successful_ = true; std::move(initialized_cb_).Run(media::PIPELINE_OK); } @@ -426,10 +411,14 @@ void FrameInjectingDemuxer::Initialize( host_->SetDuration(media::kInfiniteDuration); initialized_cb_ = std::move(status_cb); + auto on_streams_initialized = base::BindPostTask( + media_task_runner_, + base::BindOnce(&FrameInjectingDemuxer::OnStreamsInitializedOnMediaThread, + weak_factory_.GetWeakPtr())); original_task_runner_->PostTask( FROM_HERE, base::BindOnce(&DemuxerConnector::SetDemuxer, base::Unretained(demuxer_connector_), - base::Unretained(this))); + std::move(on_streams_initialized))); } void FrameInjectingDemuxer::AbortPendingReads() { diff --git a/components/cast_streaming/renderer/frame/frame_injecting_demuxer.h b/components/cast_streaming/renderer/frame/frame_injecting_demuxer.h index b6636fc1deb65e..95271c4a19442e 100644 --- a/components/cast_streaming/renderer/frame/frame_injecting_demuxer.h +++ b/components/cast_streaming/renderer/frame/frame_injecting_demuxer.h @@ -21,9 +21,8 @@ class DemuxerConnector; // media::Demuxer implementation for a Cast Streaming Receiver. // This object is instantiated on the main thread, whose task runner is stored -// as |original_task_runner_|. OnStreamsInitialized() is the only method called -// on the main thread. Every other method is called on the media thread, whose -// task runner is |media_task_runner_|. +// as |original_task_runner_|. Every other method is called on the media thread, +// whose task runner is |media_task_runner_|. // TODO(crbug.com/40131115): Simplify the FrameInjectingDemuxer initialization // sequence when the DemuxerConnector Component has been implemented. class FrameInjectingDemuxer final : public media::Demuxer { @@ -36,10 +35,6 @@ class FrameInjectingDemuxer final : public media::Demuxer { FrameInjectingDemuxer(const FrameInjectingDemuxer&) = delete; FrameInjectingDemuxer& operator=(const FrameInjectingDemuxer&) = delete; - void OnStreamsInitialized( - mojom::AudioStreamInitializationInfoPtr audio_stream_info, - mojom::VideoStreamInitializationInfoPtr video_stream_info); - private: void OnStreamsInitializedOnMediaThread( mojom::AudioStreamInitializationInfoPtr audio_stream_info, @@ -82,8 +77,6 @@ class FrameInjectingDemuxer final : public media::Demuxer { std::unique_ptr audio_stream_; std::unique_ptr video_stream_; - // Set to true if the Demuxer was successfully initialized. - bool was_initialization_successful_ = false; media::PipelineStatusCallback initialized_cb_; const raw_ptr demuxer_connector_;