media: Disable DelayedReleaseServiceContextRef in CdmServiceTest

The delayed CdmService release will cause a delayed task to be posted
which will not block *.RunUntilIdle(), and cause a memory leak of the
ServiceContextRef in some test cases.

This CL disables DelayedReleaseServiceContextRef so that the ref will be
deleted immediately.

In a future CL, we'll add test cases that actually covers
DelayedReleaseServiceContextRef.

Bug: 826039
Test: Fix tests.
Change-Id: I1d9217046c7ed66459fd578976b545aa0fe0d3c6
Reviewed-on: https://chromium-review.googlesource.com/1006530Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550004}

media/mojo/services/cdm_service.cc

@@ -35,14 +35,15 @@ void DeleteServiceContextRef(service_manager::ServiceContextRef* ref) {
 // destruction by the same delay as well. This helps reduce the chance of
 // destroying the CdmService and immediately creates it (in another process) in
 // cases like navigation, which could cause long service connection delays.
-class DelayedReleaseServiceContextRef {
+class DelayedReleaseServiceContextRef
+    : public service_manager::ServiceContextRef {
  public:
   explicit DelayedReleaseServiceContextRef(
       std::unique_ptr<service_manager::ServiceContextRef> ref)
       : ref_(std::move(ref)),
         task_runner_(base::ThreadTaskRunnerHandle::Get()) {}
 
-  ~DelayedReleaseServiceContextRef() {
+  ~DelayedReleaseServiceContextRef() override {
     service_manager::ServiceContextRef* ref_ptr = ref_.release();
     if (!task_runner_->PostNonNestableDelayedTask(
             FROM_HERE, base::BindOnce(&DeleteServiceContextRef, ref_ptr),
@@ -51,6 +52,12 @@ class DelayedReleaseServiceContextRef {
     }
   }
 
+  // service_manager::ServiceContextRef implementation.
+  std::unique_ptr<ServiceContextRef> Clone() override {
+    NOTIMPLEMENTED();
+    return nullptr;
+  }
+
  private:
   std::unique_ptr<service_manager::ServiceContextRef> ref_;
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
@@ -81,11 +88,10 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> {
   CdmFactoryImpl(
       CdmService::Client* client,
       service_manager::mojom::InterfaceProviderPtr interfaces,
-      std::unique_ptr<service_manager::ServiceContextRef> connection_ref)
+      std::unique_ptr<service_manager::ServiceContextRef> service_context_ref)
       : client_(client),
         interfaces_(std::move(interfaces)),
-        connection_ref_(std::make_unique<DelayedReleaseServiceContextRef>(
-            std::move(connection_ref))) {
+        service_context_ref_(std::move(service_context_ref)) {
     DVLOG(1) << __func__;
 
     // base::Unretained is safe because |cdm_bindings_| is owned by |this|. If
@@ -141,7 +147,7 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> {
   CdmService::Client* client_;
   service_manager::mojom::InterfaceProviderPtr interfaces_;
   mojo::StrongBindingSet<mojom::ContentDecryptionModule> cdm_bindings_;
-  std::unique_ptr<DelayedReleaseServiceContextRef> connection_ref_;
+  std::unique_ptr<service_manager::ServiceContextRef> service_context_ref_;
   std::unique_ptr<media::CdmFactory> cdm_factory_;
   base::OnceClosure destroy_cb_;
 
@@ -254,9 +260,16 @@ void CdmService::CreateCdmFactory(
   if (!client_)
     return;
 
+  std::unique_ptr<service_manager::ServiceContextRef> service_context_ref =
+      is_delayed_service_release_enabled
+          ? std::make_unique<DelayedReleaseServiceContextRef>(
+                ref_factory_->CreateRef())
+          : ref_factory_->CreateRef();
+
   cdm_factory_bindings_.AddBinding(
-      std::make_unique<CdmFactoryImpl>(
-          client_.get(), std::move(host_interfaces), ref_factory_->CreateRef()),
+      std::make_unique<CdmFactoryImpl>(client_.get(),
+                                       std::move(host_interfaces),
+                                       std::move(service_context_ref)),
       std::move(request));
 }
 

media/mojo/services/cdm_service.h

@@ -53,6 +53,10 @@ class MEDIA_MOJO_EXPORT CdmService : public service_manager::Service,
   explicit CdmService(std::unique_ptr<Client> client);
   ~CdmService() final;
 
+  void DisableDelayedServiceReleaseForTesting() {
+    is_delayed_service_release_enabled = false;
+  }
+
   size_t BoundCdmFactorySizeForTesting() const {
     return cdm_factory_bindings_.size();
   }
@@ -88,6 +92,7 @@ class MEDIA_MOJO_EXPORT CdmService : public service_manager::Service,
   DeferredDestroyStrongBindingSet<mojom::CdmFactory> cdm_factory_bindings_;
   service_manager::BinderRegistry registry_;
   mojo::BindingSet<mojom::CdmService> bindings_;
+  bool is_delayed_service_release_enabled = true;
 };
 
 }  // namespace media

media/mojo/services/cdm_service_unittest.cc

@@ -87,6 +87,10 @@ class ServiceTestClient : public service_manager::test::ServiceTestClient,
         std::make_unique<CdmService>(std::move(mock_cdm_service_client));
     cdm_service_ = cdm_service.get();
 
+    // Delayed service release involves a posted delayed task which will not
+    // block *.RunUntilIdle() and hence cause a memory leak in the test.
+    cdm_service_->DisableDelayedServiceReleaseForTesting();
+
     service_context_ = std::make_unique<service_manager::ServiceContext>(
         std::move(cdm_service), std::move(request));
   }