[chromecast] Track MediaPipelineBackend used by CastRenderer.

BUG=571155

Review-Url: https://codereview.chromium.org/2348603002
Cr-Commit-Position: refs/heads/master@{#418993}

chromecast/browser/cast_content_browser_client.cc

@@ -83,7 +83,8 @@ static std::unique_ptr<::shell::Service> CreateMojoMediaApplication(
                      base::Unretained(browser_client)),
           base::Bind(&CastContentBrowserClient::CreateCdmFactory,
                      base::Unretained(browser_client)),
-          browser_client->GetVideoResolutionPolicy()));
+          browser_client->GetVideoResolutionPolicy(),
+          browser_client->media_resource_tracker()));
   return std::unique_ptr<::shell::Service>(
       new ::media::MojoMediaApplication(std::move(mojo_media_client),
                                         quit_closure));

chromecast/browser/media/cast_mojo_media_client.cc

@@ -55,10 +55,12 @@ class CastRendererFactory : public ::media::RendererFactory {
  public:
   CastRendererFactory(const CreateMediaPipelineBackendCB& create_backend_cb,
                       const scoped_refptr<::media::MediaLog>& media_log,
-                      VideoResolutionPolicy* video_resolution_policy)
+                      VideoResolutionPolicy* video_resolution_policy,
+                      MediaResourceTracker* media_resource_tracker)
       : create_backend_cb_(create_backend_cb),
         media_log_(media_log),
-        video_resolution_policy_(video_resolution_policy) {}
+        video_resolution_policy_(video_resolution_policy),
+        media_resource_tracker_(media_resource_tracker) {}
   ~CastRendererFactory() final {}
 
   std::unique_ptr<::media::Renderer> CreateRenderer(
@@ -72,13 +74,14 @@ class CastRendererFactory : public ::media::RendererFactory {
     return base::MakeUnique<CastRenderer>(
         create_backend_cb_, media_task_runner,
         audio_renderer_sink->GetOutputDeviceInfo().device_id(),
-        video_resolution_policy_);
+        video_resolution_policy_, media_resource_tracker_);
   }
 
  private:
   const CreateMediaPipelineBackendCB create_backend_cb_;
   scoped_refptr<::media::MediaLog> media_log_;
   VideoResolutionPolicy* video_resolution_policy_;
+  MediaResourceTracker* media_resource_tracker_;
   DISALLOW_COPY_AND_ASSIGN(CastRendererFactory);
 };
 }  // namespace
@@ -86,10 +89,12 @@ class CastRendererFactory : public ::media::RendererFactory {
 CastMojoMediaClient::CastMojoMediaClient(
     const CreateMediaPipelineBackendCB& create_backend_cb,
     const CreateCdmFactoryCB& create_cdm_factory_cb,
-    VideoResolutionPolicy* video_resolution_policy)
+    VideoResolutionPolicy* video_resolution_policy,
+    MediaResourceTracker* media_resource_tracker)
     : create_backend_cb_(create_backend_cb),
       create_cdm_factory_cb_(create_cdm_factory_cb),
-      video_resolution_policy_(video_resolution_policy) {}
+      video_resolution_policy_(video_resolution_policy),
+      media_resource_tracker_(media_resource_tracker) {}
 
 CastMojoMediaClient::~CastMojoMediaClient() {}
 
@@ -103,7 +108,8 @@ std::unique_ptr<::media::RendererFactory>
 CastMojoMediaClient::CreateRendererFactory(
     const scoped_refptr<::media::MediaLog>& media_log) {
   return base::MakeUnique<CastRendererFactory>(create_backend_cb_, media_log,
-                                               video_resolution_policy_);
+                                               video_resolution_policy_,
+                                               media_resource_tracker_);
 }
 
 std::unique_ptr<::media::CdmFactory> CastMojoMediaClient::CreateCdmFactory(

chromecast/browser/media/cast_mojo_media_client.h

@@ -11,6 +11,7 @@
 namespace chromecast {
 namespace media {
 
+class MediaResourceTracker;
 class VideoResolutionPolicy;
 
 class CastMojoMediaClient : public ::media::MojoMediaClient {
@@ -20,7 +21,8 @@ class CastMojoMediaClient : public ::media::MojoMediaClient {
 
   CastMojoMediaClient(const CreateMediaPipelineBackendCB& create_backend_cb,
                       const CreateCdmFactoryCB& create_cdm_factory_cb,
-                      VideoResolutionPolicy* video_resolution_policy);
+                      VideoResolutionPolicy* video_resolution_policy,
+                      MediaResourceTracker* media_resource_tracker);
   ~CastMojoMediaClient() override;
 
   // MojoMediaClient overrides.
@@ -35,6 +37,7 @@ class CastMojoMediaClient : public ::media::MojoMediaClient {
   const CreateMediaPipelineBackendCB create_backend_cb_;
   const CreateCdmFactoryCB create_cdm_factory_cb_;
   VideoResolutionPolicy* video_resolution_policy_;
+  MediaResourceTracker* media_resource_tracker_;
 
   DISALLOW_COPY_AND_ASSIGN(CastMojoMediaClient);
 };

chromecast/browser/media/cast_renderer.cc

@@ -35,11 +35,13 @@ CastRenderer::CastRenderer(
     const CreateMediaPipelineBackendCB& create_backend_cb,
     const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
     const std::string& audio_device_id,
-    VideoResolutionPolicy* video_resolution_policy)
+    VideoResolutionPolicy* video_resolution_policy,
+    MediaResourceTracker* media_resource_tracker)
     : create_backend_cb_(create_backend_cb),
       task_runner_(task_runner),
       audio_device_id_(audio_device_id),
       video_resolution_policy_(video_resolution_policy),
+      media_resource_tracker_(media_resource_tracker),
       client_(nullptr),
       cast_cdm_context_(nullptr),
       media_task_runner_factory_(
@@ -67,6 +69,8 @@ void CastRenderer::Initialize(
   DCHECK(task_runner_->BelongsToCurrentThread());
 
   // Create pipeline backend.
+  media_resource_usage_.reset(
+      new MediaResourceTracker::ScopedUsage(media_resource_tracker_));
   backend_task_runner_.reset(new TaskRunnerImpl());
   // TODO(erickung): crbug.com/443956. Need to provide right LoadType.
   LoadType load_type = kLoadTypeMediaSource;

chromecast/browser/media/cast_renderer.h

@@ -8,6 +8,7 @@
 #include "base/memory/weak_ptr.h"
 #include "chromecast/browser/media/media_pipeline_backend_factory.h"
 #include "chromecast/browser/media/video_resolution_policy.h"
+#include "chromecast/media/base/media_resource_tracker.h"
 #include "media/base/renderer.h"
 #include "ui/gfx/geometry/size.h"
 
@@ -33,7 +34,8 @@ class CastRenderer : public ::media::Renderer,
   CastRenderer(const CreateMediaPipelineBackendCB& create_backend_cb,
                const scoped_refptr<base::SingleThreadTaskRunner>& task_runner,
                const std::string& audio_device_id,
-               VideoResolutionPolicy* video_resolution_policy);
+               VideoResolutionPolicy* video_resolution_policy,
+               MediaResourceTracker* media_resource_tracker);
   ~CastRenderer() final;
 
   // ::media::Renderer implementation.
@@ -68,6 +70,10 @@ class CastRenderer : public ::media::Renderer,
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
   std::string audio_device_id_;
   VideoResolutionPolicy* video_resolution_policy_;
+  MediaResourceTracker* media_resource_tracker_;
+  // Must outlive |pipeline_| to properly count resource usage.
+  std::unique_ptr<MediaResourceTracker::ScopedUsage> media_resource_usage_;
+
   ::media::RendererClient* client_;
   CastCdmContext* cast_cdm_context_;
   scoped_refptr<BalancedMediaTaskRunnerFactory> media_task_runner_factory_;
@@ -75,8 +81,8 @@ class CastRenderer : public ::media::Renderer,
   std::unique_ptr<MediaPipelineImpl> pipeline_;
   bool eos_[2];
   gfx::Size video_res_;
-  base::WeakPtrFactory<CastRenderer> weak_factory_;
 
+  base::WeakPtrFactory<CastRenderer> weak_factory_;
   DISALLOW_COPY_AND_ASSIGN(CastRenderer);
 };
 

chromecast/media/base/media_resource_tracker.cc

@@ -12,6 +12,18 @@
 namespace chromecast {
 namespace media {
 
+MediaResourceTracker::ScopedUsage::ScopedUsage(MediaResourceTracker* tracker)
+    : tracker_(tracker) {
+  DCHECK(tracker_);
+  DCHECK(tracker_->media_task_runner_->BelongsToCurrentThread());
+  tracker_->IncrementUsageCount();
+}
+
+MediaResourceTracker::ScopedUsage::~ScopedUsage() {
+  DCHECK(tracker_->media_task_runner_->BelongsToCurrentThread());
+  tracker_->DecrementUsageCount();
+}
+
 MediaResourceTracker::MediaResourceTracker(
     const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner,
     const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner)

chromecast/media/base/media_resource_tracker.h

@@ -32,6 +32,21 @@ namespace media {
 //   thread.
 class MediaResourceTracker {
  public:
+  // Helper class to manage media resource usage count.
+  // Create an instance of this class when a media resource is created.
+  // Delete the instance *after* the media resource is deleted.
+  // This class is not thread-safe. It must be created and deleted on
+  // |MediaResourceTracker::media_task_runner_|.
+  class ScopedUsage {
+   public:
+    ScopedUsage(MediaResourceTracker* tracker);
+    ~ScopedUsage();
+
+   private:
+    MediaResourceTracker* tracker_;
+    DISALLOW_COPY_AND_ASSIGN(ScopedUsage);
+  };
+
   MediaResourceTracker(
       const scoped_refptr<base::SingleThreadTaskRunner>& ui_task_runner,
       const scoped_refptr<base::SingleThreadTaskRunner>& media_task_runner);

chromecast/media/base/media_resource_tracker_unittest.cc

@@ -7,28 +7,14 @@
 #include <memory>
 
 #include "base/bind.h"
-#include "base/location.h"
+#include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
-#include "base/single_thread_task_runner.h"
-#include "base/synchronization/waitable_event.h"
-#include "base/task_runner.h"
-#include "base/threading/thread.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace chromecast {
 namespace media {
 
-void RunUntilIdle(base::TaskRunner* task_runner) {
-  base::WaitableEvent completion_event(
-      base::WaitableEvent::ResetPolicy::AUTOMATIC,
-      base::WaitableEvent::InitialState::NOT_SIGNALED);
-  task_runner->PostTask(FROM_HERE,
-                        base::Bind(&base::WaitableEvent::Signal,
-                                   base::Unretained(&completion_event)));
-  completion_event.Wait();
-}
-
 // Collection of mocks to verify MediaResourceTracker takes the correct actions.
 class MediaResourceTrackerTestMocks {
  public:
@@ -60,6 +46,8 @@ class TestMediaResourceTracker : public MediaResourceTracker {
     test_mocks_->Finalize();
   }
 
+  size_t media_use_count() const { return media_use_count_; }
+
  private:
   MediaResourceTrackerTestMocks* test_mocks_;
 };
@@ -71,36 +59,22 @@ class MediaResourceTrackerTest : public ::testing::Test {
 
  protected:
   void SetUp() override {
-    message_loop_.reset(new base::MessageLoop());
-    media_thread_.reset(new base::Thread("MediaThread"));
-    media_thread_->Start();
-    media_task_runner_ = media_thread_->task_runner();
-
     test_mocks_.reset(new MediaResourceTrackerTestMocks());
 
     resource_tracker_ = new TestMediaResourceTracker(
-        test_mocks_.get(), message_loop_->task_runner(),
-        media_task_runner_);
+        test_mocks_.get(), message_loop_.task_runner(),
+        message_loop_.task_runner());
   }
 
-  void TearDown() override { media_thread_.reset(); }
-
-  void IncrementMediaUsageCount() {
-    media_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&MediaResourceTracker::IncrementUsageCount,
-                              base::Unretained(resource_tracker_)));
-  }
-  void DecrementMediaUsageCount() {
-    media_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&MediaResourceTracker::DecrementUsageCount,
-                              base::Unretained(resource_tracker_)));
+  void InitializeMediaLib() {
+    EXPECT_CALL(*test_mocks_, Initialize()).Times(1);
+    resource_tracker_->InitializeMediaLib();
+    base::RunLoop().RunUntilIdle();
   }
 
-  MediaResourceTracker* resource_tracker_;
+  base::MessageLoop message_loop_;
+  TestMediaResourceTracker* resource_tracker_;
   std::unique_ptr<MediaResourceTrackerTestMocks> test_mocks_;
-  std::unique_ptr<base::MessageLoop> message_loop_;
-  std::unique_ptr<base::Thread> media_thread_;
-  scoped_refptr<base::SingleThreadTaskRunner> media_task_runner_;
 
   DISALLOW_COPY_AND_ASSIGN(MediaResourceTrackerTest);
 };
@@ -113,8 +87,6 @@ TEST_F(MediaResourceTrackerTest, BasicLifecycle) {
 
   resource_tracker_->InitializeMediaLib();
   resource_tracker_->FinalizeAndDestroy();
-
-  RunUntilIdle(media_task_runner_.get());
   base::RunLoop().RunUntilIdle();
 }
 
@@ -126,8 +98,6 @@ TEST_F(MediaResourceTrackerTest, InitializeTwice) {
   resource_tracker_->InitializeMediaLib();
   resource_tracker_->InitializeMediaLib();
   resource_tracker_->FinalizeAndDestroy();
-
-  RunUntilIdle(media_task_runner_.get());
   base::RunLoop().RunUntilIdle();
 }
 
@@ -137,16 +107,13 @@ TEST_F(MediaResourceTrackerTest, FinalizeWithoutInitialize) {
   EXPECT_CALL(*test_mocks_, Destroyed()).Times(1);
 
   resource_tracker_->FinalizeAndDestroy();
-
-  RunUntilIdle(media_task_runner_.get());
   base::RunLoop().RunUntilIdle();
 }
 
+// Check FinalizeCastMediaShlib works correctly with no users of
+// media resource.
 TEST_F(MediaResourceTrackerTest, FinalizeResourceNotInUse) {
-  // Check FinalizeCastMediaShlib works correctly with no users of
-  // media resource.
-  EXPECT_CALL(*test_mocks_, Initialize()).Times(1);
-  resource_tracker_->InitializeMediaLib();
+  InitializeMediaLib();
 
   EXPECT_CALL(*test_mocks_, Finalize()).Times(1);
   EXPECT_CALL(*test_mocks_, Destroyed()).Times(0);
@@ -154,23 +121,18 @@ TEST_F(MediaResourceTrackerTest, FinalizeResourceNotInUse) {
   resource_tracker_->FinalizeMediaLib(
       base::Bind(&MediaResourceTrackerTestMocks::FinalizeCallback,
                  base::Unretained(test_mocks_.get())));
-  RunUntilIdle(media_task_runner_.get());
   base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(*test_mocks_, Destroyed()).Times(1);
   resource_tracker_->FinalizeAndDestroy();
-
-  RunUntilIdle(media_task_runner_.get());
   base::RunLoop().RunUntilIdle();
 }
 
+// Check FinalizeCastMediaShlib waits for resource to not be in use.
 TEST_F(MediaResourceTrackerTest, FinalizeResourceInUse) {
-  // Check FinalizeCastMediaShlib waits for resource to not be in use.
-  EXPECT_CALL(*test_mocks_, Initialize()).Times(1);
-  resource_tracker_->InitializeMediaLib();
+  InitializeMediaLib();
 
-  IncrementMediaUsageCount();
-  RunUntilIdle(media_task_runner_.get());
+  resource_tracker_->IncrementUsageCount();
   base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(*test_mocks_, Finalize()).Times(0);
@@ -178,52 +140,42 @@ TEST_F(MediaResourceTrackerTest, FinalizeResourceInUse) {
   resource_tracker_->FinalizeMediaLib(
       base::Bind(&MediaResourceTrackerTestMocks::FinalizeCallback,
                  base::Unretained(test_mocks_.get())));
-  RunUntilIdle(media_task_runner_.get());
+  base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(*test_mocks_, Finalize()).Times(1);
-  DecrementMediaUsageCount();
-
   EXPECT_CALL(*test_mocks_, FinalizeCallback()).Times(1);
-  RunUntilIdle(media_task_runner_.get());
+  resource_tracker_->DecrementUsageCount();
   base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(*test_mocks_, Destroyed()).Times(1);
   resource_tracker_->FinalizeAndDestroy();
-
-  RunUntilIdle(media_task_runner_.get());
   base::RunLoop().RunUntilIdle();
 }
 
+// Check FinalizeAndDestroy waits for resource to not be in use.
 TEST_F(MediaResourceTrackerTest, DestroyWaitForNoUsers) {
-  // Check FinalizeAndDestroy waits for resource to not be in use.
-  EXPECT_CALL(*test_mocks_, Initialize()).Times(1);
-  resource_tracker_->InitializeMediaLib();
+  InitializeMediaLib();
 
-  IncrementMediaUsageCount();
-  RunUntilIdle(media_task_runner_.get());
+  resource_tracker_->IncrementUsageCount();
   base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(*test_mocks_, Finalize()).Times(0);
   EXPECT_CALL(*test_mocks_, Destroyed()).Times(0);
   resource_tracker_->FinalizeAndDestroy();
-  RunUntilIdle(media_task_runner_.get());
+  base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(*test_mocks_, Finalize()).Times(1);
   EXPECT_CALL(*test_mocks_, Destroyed()).Times(1);
-  DecrementMediaUsageCount();
-
-  RunUntilIdle(media_task_runner_.get());
+  resource_tracker_->DecrementUsageCount();
   base::RunLoop().RunUntilIdle();
 }
 
+// Check finalize callback still made if FinalizeAndDestroy called
+// while waiting for resource usage to end.
 TEST_F(MediaResourceTrackerTest, DestroyWithPendingFinalize) {
-  // Check finalize callback still made if FinalizeAndDestroy called
-  // while waiting for resource usage to end.
-  EXPECT_CALL(*test_mocks_, Initialize()).Times(1);
-  resource_tracker_->InitializeMediaLib();
+  InitializeMediaLib();
 
-  IncrementMediaUsageCount();
-  RunUntilIdle(media_task_runner_.get());
+  resource_tracker_->IncrementUsageCount();
   base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(*test_mocks_, Finalize()).Times(0);
@@ -232,16 +184,25 @@ TEST_F(MediaResourceTrackerTest, DestroyWithPendingFinalize) {
       base::Bind(&MediaResourceTrackerTestMocks::FinalizeCallback,
                  base::Unretained(test_mocks_.get())));
   resource_tracker_->FinalizeAndDestroy();
-  RunUntilIdle(media_task_runner_.get());
+  base::RunLoop().RunUntilIdle();
 
   EXPECT_CALL(*test_mocks_, Finalize()).Times(1);
   EXPECT_CALL(*test_mocks_, Destroyed()).Times(1);
   EXPECT_CALL(*test_mocks_, FinalizeCallback()).Times(1);
+  resource_tracker_->DecrementUsageCount();
+  base::RunLoop().RunUntilIdle();
+}
 
-  DecrementMediaUsageCount();
+TEST_F(MediaResourceTrackerTest, ScopedUsage) {
+  InitializeMediaLib();
 
-  RunUntilIdle(media_task_runner_.get());
-  base::RunLoop().RunUntilIdle();
+  EXPECT_EQ(0u, resource_tracker_->media_use_count());
+  {
+    std::unique_ptr<MediaResourceTracker::ScopedUsage> scoped_usage(
+        new MediaResourceTracker::ScopedUsage(resource_tracker_));
+    EXPECT_EQ(1u, resource_tracker_->media_use_count());
+  }
+  EXPECT_EQ(0u, resource_tracker_->media_use_count());
 }
 
 }  // namespace media