Revert "media: Add CdmServiceTest"

This reverts commit f4196ee3.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 547943 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2Y0MTk2ZWUzNjMwODE1MGNjNTVlN2RmMGY0OGE1MjZiODQ4ZTQzYWYM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium/Mac/40051

Sample Failed Step: compile

Original change's description:
> media: Add CdmServiceTest
> 
> This is a ServiceTest tha runs CdmService in-process (packaged) which
> covers some basic interaction between CdmService and CdmFactoryImpl.
> 
> Bug: 826039
> Test: Adds new tests to media_service_unittests
> Change-Id: I85e99c22dba375d61de1a4be327953e0bf02600d
> Reviewed-on: https://chromium-review.googlesource.com/989212
> Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: John Rummell <jrummell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#547943}

Change-Id: Ifa173b1917ddcde24b6e6d57ea54fb7d22aa4d5f
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 826039
Reviewed-on: https://chromium-review.googlesource.com/994713
Cr-Commit-Position: refs/heads/master@{#547958}

media/mojo/services/BUILD.gn

@@ -140,62 +140,6 @@ component("services") {
   }
 }
 
-service_manifest("cdm_manifest") {
-  name = "cdm"
-  source = "cdm_manifest.json"
-}
-
-service_manifest("media_manifest") {
-  name = "media"
-  source = "media_manifest.json"
-}
-
-# Unit Tests
-
-source_set("unit_tests") {
-  testonly = true
-
-  sources = [
-    "media_metrics_provider_unittest.cc",
-    "mojo_audio_input_stream_unittest.cc",
-    "mojo_audio_output_stream_provider_unittest.cc",
-    "mojo_audio_output_stream_unittest.cc",
-    "mojo_jpeg_decode_accelerator_service_unittest.cc",
-    "mojo_video_encode_accelerator_service_unittest.cc",
-    "video_decode_perf_history_unittest.cc",
-    "watch_time_recorder_unittest.cc",
-  ]
-
-  deps = [
-    "//base",
-    "//base/test:test_support",
-    "//components/ukm:test_support",
-    "//media:test_support",
-    "//media/mojo:test_support",
-    "//mojo/edk",
-    "//services/metrics/public/cpp:ukm_builders",
-    "//testing/gmock",
-    "//testing/gtest",
-  ]
-
-  if (enable_library_cdms) {
-    sources += [
-      "mojo_cdm_allocator_unittest.cc",
-      "mojo_cdm_file_io_unittest.cc",
-      "mojo_cdm_helper_unittest.cc",
-      "mojo_cdm_proxy_unittest.cc",
-    ]
-
-    deps += [ "//media/cdm:cdm_api" ]
-  }
-}
-
-# Service Tests
-
-# MediaService is tested by using a standalone "media" service, which runs the
-# service out-of-process. While CdmService is tested as a packaged service,
-# which runs the service in-process.
-
 service("media") {
   testonly = true
 
@@ -222,11 +166,7 @@ service_test("media_service_unittests") {
     "media_service_unittest.cc",
   ]
 
-  if (enable_library_cdms) {
-    sources += [ "cdm_service_unittest.cc" ]
-  }
-
-  catalog = ":service_tests_catalog"
+  catalog = ":media_service_unittests_catalog"
 
   deps = [
     ":services",
@@ -246,37 +186,21 @@ service_test("media_service_unittests") {
   ]
 }
 
-service_manifest("media_service_unittest_manifest") {
-  name = "media_service_unittests"
-  source = "test_manifest.json"
-}
-
-catalog("media_service_unittest_catalog") {
-  testonly = true
-  embedded_services = [ ":media_service_unittest_manifest" ]
-  standalone_services = [ ":media_manifest" ]
-}
-
-service_manifest("cdm_service_unittest_manifest") {
-  name = "cdm_service_unittest"
-  source = "cdm_service_unittest_manifest.json"
-  packaged_services = [ ":cdm_manifest" ]
+service_manifest("cdm_manifest") {
+  name = "cdm"
+  source = "cdm_manifest.json"
 }
 
-catalog("cdm_service_unittest_catalog") {
-  testonly = true
-  embedded_services = [ ":cdm_service_unittest_manifest" ]
+service_manifest("media_manifest") {
+  name = "media"
+  source = "media_manifest.json"
 }
 
-catalog("service_tests_catalog") {
-  testonly = true
-  catalog_deps = [
-    ":cdm_service_unittest_catalog",
-    ":media_service_unittest_catalog",
-  ]
+service_manifest("test_manifest") {
+  name = "media_service_unittests"
+  source = "test_manifest.json"
 }
 
-# media_pipeline_integration_unittests is out of date and disabled by default.
 service_test("media_pipeline_integration_unittests") {
   testonly = true
 
@@ -296,7 +220,50 @@ service_manifest("pipeline_test_manifest") {
   source = "pipeline_test_manifest.json"
 }
 
+catalog("media_service_unittests_catalog") {
+  embedded_services = [ ":test_manifest" ]
+  standalone_services = [ ":media_manifest" ]
+}
+
 catalog("media_pipeline_integration_unittests_catalog") {
   embedded_services = [ ":pipeline_test_manifest" ]
   standalone_services = [ ":media_manifest" ]
 }
+
+source_set("unit_tests") {
+  testonly = true
+
+  sources = [
+    "media_metrics_provider_unittest.cc",
+    "mojo_audio_input_stream_unittest.cc",
+    "mojo_audio_output_stream_provider_unittest.cc",
+    "mojo_audio_output_stream_unittest.cc",
+    "mojo_jpeg_decode_accelerator_service_unittest.cc",
+    "mojo_video_encode_accelerator_service_unittest.cc",
+    "video_decode_perf_history_unittest.cc",
+    "watch_time_recorder_unittest.cc",
+  ]
+
+  deps = [
+    "//base",
+    "//base/test:test_support",
+    "//components/ukm:test_support",
+    "//media:test_support",
+    "//media/mojo:test_support",
+    "//mojo/edk",
+    "//services/metrics/public/cpp:ukm_builders",
+    "//testing/gmock",
+    "//testing/gtest",
+  ]
+
+  if (enable_library_cdms) {
+    sources += [
+      "mojo_cdm_allocator_unittest.cc",
+      "mojo_cdm_file_io_unittest.cc",
+      "mojo_cdm_helper_unittest.cc",
+      "mojo_cdm_proxy_unittest.cc",
+    ]
+
+    deps += [ "//media/cdm:cdm_api" ]
+  }
+}

media/mojo/services/OWNERS

 per-file cdm_manifest.json=set noparent
 per-file cdm_manifest.json=file://ipc/SECURITY_OWNERS
 
-per-file cdm_service_unittest_manifest.json=set noparent
-per-file cdm_service_unittest_manifest.json=file://ipc/SECURITY_OWNERS
-
 per-file media_manifest.json=set noparent
 per-file media_manifest.json=file://ipc/SECURITY_OWNERS
 

media/mojo/services/cdm_service.cc

@@ -67,17 +67,13 @@ class CdmFactoryImpl : public mojom::CdmFactory {
       : client_(client),
         interfaces_(std::move(interfaces)),
         connection_ref_(std::make_unique<DelayedReleaseServiceContextRef>(
-            std::move(connection_ref))) {
-    DVLOG(1) << __func__;
-  }
+            std::move(connection_ref))) {}
 
-  ~CdmFactoryImpl() final { DVLOG(1) << __func__; }
+  ~CdmFactoryImpl() final {}
 
   // mojom::CdmFactory implementation.
   void CreateCdm(const std::string& key_system,
                  mojom::ContentDecryptionModuleRequest request) final {
-    DVLOG(2) << __func__;
-
     auto* cdm_factory = GetCdmFactory();
     if (!cdm_factory)
       return;
@@ -114,15 +110,12 @@ class CdmFactoryImpl : public mojom::CdmFactory {
 
 CdmService::CdmService(std::unique_ptr<Client> client)
     : client_(std::move(client)) {
-  DVLOG(1) << __func__;
   DCHECK(client_);
   registry_.AddInterface<mojom::CdmService>(
       base::BindRepeating(&CdmService::Create, base::Unretained(this)));
 }
 
-CdmService::~CdmService() {
-  DVLOG(1) << __func__;
-}
+CdmService::~CdmService() = default;
 
 void CdmService::OnStart() {
   DVLOG(1) << __func__;
@@ -189,9 +182,11 @@ void CdmService::LoadCdm(const base::FilePath& cdm_path) {
 #if BUILDFLAG(ENABLE_CDM_HOST_VERIFICATION)
   std::vector<CdmHostFilePath> cdm_host_file_paths;
   client_->AddCdmHostFilePaths(&cdm_host_file_paths);
-  bool success = instance->Initialize(cdm_path, cdm_host_file_paths);
+  if (!instance->Initialize(cdm_path, cdm_host_file_paths))
+    return;
 #else
-  bool success = instance->Initialize(cdm_path);
+  if (!instance->Initialize(cdm_path))
+    return;
 #endif  // BUILDFLAG(ENABLE_CDM_HOST_VERIFICATION)
 
   // This may trigger the sandbox to be sealed.
@@ -203,8 +198,7 @@ void CdmService::LoadCdm(const base::FilePath& cdm_path) {
 #endif  // defined(OS_MACOSX)
 
   // Always called within the sandbox.
-  if (success)
-    instance->InitializeCdmModule();
+  instance->InitializeCdmModule();
 }
 
 void CdmService::CreateCdmFactory(

media/mojo/services/cdm_service_unittest.cc

-// Copyright 2018 The Chromium Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style license that can be
-// found in the LICENSE file.
-
-#include <memory>
-
-#include "base/callback.h"
-#include "base/files/file_path.h"
-#include "base/path_service.h"
-#include "base/run_loop.h"
-#include "media/cdm/default_cdm_factory.h"
-#include "media/media_buildflags.h"
-#include "media/mojo/interfaces/constants.mojom.h"
-#include "media/mojo/services/cdm_service.h"
-#include "media/mojo/services/media_interface_provider.h"
-#include "mojo/public/cpp/bindings/binding_set.h"
-#include "mojo/public/cpp/system/platform_handle.h"
-#include "services/service_manager/public/cpp/binder_registry.h"
-#include "services/service_manager/public/cpp/service_context.h"
-#include "services/service_manager/public/cpp/service_test.h"
-#include "services/service_manager/public/mojom/service_factory.mojom.h"
-#include "testing/gmock/include/gmock/gmock.h"
-#include "testing/gtest/include/gtest/gtest.h"
-#include "url/gurl.h"
-#include "url/origin.h"
-
-using testing::Invoke;
-using testing::InvokeWithoutArgs;
-
-namespace media {
-
-namespace {
-
-const char kClearKeyKeySystem[] = "org.w3.clearkey";
-const char kInvalidKeySystem[] = "invalid.key.system";
-const char kSecurityOrigin[] = "https://foo.com";
-
-class MockCdmServiceClient : public media::CdmService::Client {
- public:
-  MockCdmServiceClient() = default;
-  ~MockCdmServiceClient() override = default;
-
-  // media::CdmService::Client implementation.
-  MOCK_METHOD0(EnsureSandboxed, void());
-
-  std::unique_ptr<media::CdmFactory> CreateCdmFactory(
-      service_manager::mojom::InterfaceProvider* host_interfaces) override {
-    return std::make_unique<media::DefaultCdmFactory>();
-  }
-
-#if BUILDFLAG(ENABLE_CDM_HOST_VERIFICATION)
-  void OnMetadata(std::vector<media::CdmHostFilePath>*) override {}
-#endif  // BUILDFLAG(ENABLE_CDM_HOST_VERIFICATION)
-};
-
-class ServiceTestClient : public service_manager::test::ServiceTestClient,
-                          public service_manager::mojom::ServiceFactory {
- public:
-  explicit ServiceTestClient(service_manager::test::ServiceTest* test)
-      : service_manager::test::ServiceTestClient(test) {
-    registry_.AddInterface<service_manager::mojom::ServiceFactory>(
-        base::BindRepeating(&ServiceTestClient::Create,
-                            base::Unretained(this)));
-  }
-  ~ServiceTestClient() override {}
-
-  // service_manager::Service implementation.
-  void OnBindInterface(const service_manager::BindSourceInfo& source_info,
-                       const std::string& interface_name,
-                       mojo::ScopedMessagePipeHandle interface_pipe) override {
-    registry_.BindInterface(interface_name, std::move(interface_pipe));
-  }
-
-  // service_manager::mojom::ServiceFactory implementation.
-  void CreateService(
-      service_manager::mojom::ServiceRequest request,
-      const std::string& name,
-      service_manager::mojom::PIDReceiverPtr pid_receiver) override {
-    if (name != mojom::kCdmServiceName)
-      return;
-
-    auto mock_cdm_service_client = std::make_unique<MockCdmServiceClient>();
-    mock_cdm_service_client_ = mock_cdm_service_client.get();
-    service_context_ = std::make_unique<service_manager::ServiceContext>(
-        std::make_unique<CdmService>(std::move(mock_cdm_service_client)),
-        std::move(request));
-  }
-
-  void DestroyService() { service_context_.reset(); }
-
-  MockCdmServiceClient* mock_cdm_service_client() {
-    return mock_cdm_service_client_;
-  }
-
- private:
-  void Create(service_manager::mojom::ServiceFactoryRequest request) {
-    service_factory_bindings_.AddBinding(this, std::move(request));
-  }
-
-  service_manager::BinderRegistry registry_;
-  mojo::BindingSet<service_manager::mojom::ServiceFactory>
-      service_factory_bindings_;
-  std::unique_ptr<service_manager::ServiceContext> service_context_;
-  MockCdmServiceClient* mock_cdm_service_client_;
-};
-
-}  // namespace
-
-class CdmServiceTest : public service_manager::test::ServiceTest {
- public:
-  CdmServiceTest() : ServiceTest("cdm_service_unittest") {}
-  ~CdmServiceTest() override {}
-
-  MOCK_METHOD0(CdmFactoryConnectionClosed, void());
-  MOCK_METHOD0(CdmConnectionClosed, void());
-
-  // service_manager::test::ServiceTest:
-  void SetUp() override {
-    ServiceTest::SetUp();
-
-    connector()->BindInterface(media::mojom::kCdmServiceName, &cdm_service_);
-
-    service_manager::mojom::InterfaceProviderPtr interfaces;
-    auto provider = std::make_unique<MediaInterfaceProvider>(
-        mojo::MakeRequest(&interfaces));
-
-    ASSERT_FALSE(cdm_factory_);
-    cdm_service_->CreateCdmFactory(mojo::MakeRequest(&cdm_factory_),
-                                   std::move(interfaces));
-    cdm_service_.FlushForTesting();
-    ASSERT_TRUE(cdm_factory_);
-    cdm_factory_.set_connection_error_handler(base::BindRepeating(
-        &CdmServiceTest::CdmFactoryConnectionClosed, base::Unretained(this)));
-  }
-
-  // MOCK_METHOD* doesn't support move-only types. Work around this by having
-  // an extra method.
-  MOCK_METHOD1(OnCdmInitializedInternal, void(bool result));
-  void OnCdmInitialized(mojom::CdmPromiseResultPtr result,
-                        int cdm_id,
-                        mojom::DecryptorPtr decryptor) {
-    OnCdmInitializedInternal(result->success);
-  }
-
-  void InitializeCdm(const std::string& key_system, bool expected_result) {
-    base::RunLoop run_loop;
-    cdm_factory_->CreateCdm(key_system, mojo::MakeRequest(&cdm_));
-    cdm_.set_connection_error_handler(base::BindRepeating(
-        &CdmServiceTest::CdmConnectionClosed, base::Unretained(this)));
-    EXPECT_CALL(*this, OnCdmInitializedInternal(expected_result))
-        .WillOnce(InvokeWithoutArgs(&run_loop, &base::RunLoop::Quit));
-    cdm_->Initialize(key_system, url::Origin::Create(GURL(kSecurityOrigin)),
-                     CdmConfig(),
-                     base::BindRepeating(&CdmServiceTest::OnCdmInitialized,
-                                         base::Unretained(this)));
-    run_loop.Run();
-  }
-
-  std::unique_ptr<service_manager::Service> CreateService() override {
-    auto service_test_client = std::make_unique<ServiceTestClient>(this);
-    service_test_client_ = service_test_client.get();
-    return service_test_client;
-  }
-
-  mojom::CdmServicePtr cdm_service_;
-  mojom::CdmFactoryPtr cdm_factory_;
-  mojom::ContentDecryptionModulePtr cdm_;
-  ServiceTestClient* service_test_client_;
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(CdmServiceTest);
-};
-
-TEST_F(CdmServiceTest, LoadCdm) {
-  base::FilePath cdm_path(FILE_PATH_LITERAL("dummy path"));
-
-  // Even with a dummy path where the CDM cannot be loaded, EnsureSandboxed()
-  // should still be called to ensure the process is sandboxed.
-  EXPECT_CALL(*service_test_client_->mock_cdm_service_client(),
-              EnsureSandboxed());
-
-  cdm_service_->LoadCdm(cdm_path);
-  cdm_service_.FlushForTesting();
-}
-
-TEST_F(CdmServiceTest, InitializeCdm_Success) {
-  InitializeCdm(kClearKeyKeySystem, true);
-}
-
-TEST_F(CdmServiceTest, InitializeCdm_InvalidKeySystem) {
-  InitializeCdm(kInvalidKeySystem, false);
-}
-
-TEST_F(CdmServiceTest, DestroyAndRecreateCdm) {
-  InitializeCdm(kClearKeyKeySystem, true);
-  cdm_.reset();
-  InitializeCdm(kClearKeyKeySystem, true);
-}
-
-// CdmFactory connection error will destroy all CDMs.
-TEST_F(CdmServiceTest, DestroyCdmFactory) {
-  InitializeCdm(kClearKeyKeySystem, true);
-
-  base::RunLoop run_loop;
-  EXPECT_CALL(*this, CdmConnectionClosed())
-      .WillOnce(Invoke(&run_loop, &base::RunLoop::Quit));
-  cdm_factory_.reset();
-  run_loop.Run();
-}
-
-// Destroy service will destroy the CdmFactory and all CDMs.
-TEST_F(CdmServiceTest, DestroyCdmService) {
-  InitializeCdm(kClearKeyKeySystem, true);
-
-  base::RunLoop run_loop;
-  // Ideally we should not care about order, and should only quit the loop when
-  // both connections are closed.
-  EXPECT_CALL(*this, CdmFactoryConnectionClosed());
-  EXPECT_CALL(*this, CdmConnectionClosed())
-      .WillOnce(Invoke(&run_loop, &base::RunLoop::Quit));
-  service_test_client_->DestroyService();
-  run_loop.Run();
-}
-
-}  // namespace media

media/mojo/services/cdm_service_unittest_manifest.json

-{
-  "name": "cdm_service_unittest",
-  "display_name": "CDM Service Unittest",
-  "interface_provider_specs": {
-    "service_manager:connector": {
-      "provides": {
-        "service_manager:service_factory": [
-          "service_manager::mojom::ServiceFactory"
-        ]
-      },
-      "requires": {
-        "*": [ "app" ],
-        "cdm": [ "media:cdm" ]
-      }
-    }
-  }
-}