Convert media::mojom::CdmFile to new Mojo types

This CL converts media::mojom::CdmFile from
media/mojo/mojom/cdm_storage.mojom to new Mojo types using
AssociatedRemote, PendingAssociatedRemote, and
UniqueAssociatedReceiverSet.

Bug: 955171
Change-Id: I819ae334b8de62ab68c4ee85cefdd7ef4c2f41b5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1903237Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: John Rummell <jrummell@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#714186}

content/browser/media/cdm_storage_impl.cc

@@ -17,6 +17,8 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/storage_partition.h"
+#include "mojo/public/cpp/bindings/associated_remote.h"
+#include "mojo/public/cpp/bindings/pending_associated_remote.h"
 #include "ppapi/shared_impl/ppapi_constants.h"
 #include "storage/browser/file_system/file_system_context.h"
 #include "storage/browser/file_system/file_system_operation_context.h"
@@ -89,20 +91,20 @@ void CdmStorageImpl::Open(const std::string& file_name, OpenCallback callback) {
 
   if (!IsValidCdmFileSystemId(cdm_file_system_id_)) {
     DVLOG(1) << "CdmStorageImpl not initialized properly.";
-    std::move(callback).Run(Status::kFailure, nullptr);
+    std::move(callback).Run(Status::kFailure, mojo::NullAssociatedRemote());
     return;
   }
 
   if (file_name.empty()) {
     DVLOG(1) << "No file specified.";
-    std::move(callback).Run(Status::kFailure, nullptr);
+    std::move(callback).Run(Status::kFailure, mojo::NullAssociatedRemote());
     return;
   }
 
   // The file system should only be opened once. If it has been attempted and
   // failed, we can't create the CdmFile objects.
   if (file_system_state_ == FileSystemState::kError) {
-    std::move(callback).Run(Status::kFailure, nullptr);
+    std::move(callback).Run(Status::kFailure, mojo::NullAssociatedRemote());
     return;
   }
 
@@ -158,7 +160,8 @@ void CdmStorageImpl::OnFileSystemOpened(base::File::Error error) {
     file_system_state_ = FileSystemState::kError;
     // All pending calls will fail.
     for (auto& pending : pending_open_calls_) {
-      std::move(pending.second).Run(Status::kFailure, nullptr);
+      std::move(pending.second)
+          .Run(Status::kFailure, mojo::NullAssociatedRemote());
     }
     pending_open_calls_.clear();
     return;
@@ -183,7 +186,7 @@ void CdmStorageImpl::CreateCdmFile(const std::string& file_name,
   // initialize it (which only grabs the lock to prevent any other access to the
   // file except through this object).
   if (!CdmFileImpl::IsValidName(file_name)) {
-    std::move(callback).Run(Status::kFailure, nullptr);
+    std::move(callback).Run(Status::kFailure, mojo::NullAssociatedRemote());
     return;
   }
 
@@ -193,14 +196,14 @@ void CdmStorageImpl::CreateCdmFile(const std::string& file_name,
 
   if (!cdm_file_impl->Initialize()) {
     // Unable to initialize with the file requested.
-    std::move(callback).Run(Status::kInUse, nullptr);
+    std::move(callback).Run(Status::kInUse, mojo::NullAssociatedRemote());
     return;
   }
 
   // File was opened successfully, so create the binding and return success.
-  media::mojom::CdmFileAssociatedPtrInfo cdm_file;
-  cdm_file_bindings_.AddBinding(std::move(cdm_file_impl),
-                                mojo::MakeRequest(&cdm_file));
+  mojo::PendingAssociatedRemote<media::mojom::CdmFile> cdm_file;
+  cdm_file_receivers_.Add(std::move(cdm_file_impl),
+                          cdm_file.InitWithNewEndpointAndPassReceiver());
   std::move(callback).Run(Status::kSuccess, std::move(cdm_file));
 }
 

content/browser/media/cdm_storage_impl.h

@@ -16,7 +16,7 @@
 #include "content/common/content_export.h"
 #include "content/public/browser/frame_service_base.h"
 #include "media/mojo/mojom/cdm_storage.mojom.h"
-#include "mojo/public/cpp/bindings/strong_associated_binding_set.h"
+#include "mojo/public/cpp/bindings/unique_associated_receiver_set.h"
 
 namespace storage {
 class FileSystemContext;
@@ -90,10 +90,10 @@ class CONTENT_EXPORT CdmStorageImpl final
   // returned, and it needs permission to access the file(s).
   const int child_process_id_;
 
-  // Keep track of all media::mojom::CdmFile bindings, as each CdmFileImpl
+  // Keep track of all media::mojom::CdmFile receivers, as each CdmFileImpl
   // object keeps a reference to |this|. If |this| goes away unexpectedly,
-  // all remaining CdmFile bindings will be closed.
-  mojo::StrongAssociatedBindingSet<media::mojom::CdmFile> cdm_file_bindings_;
+  // all remaining CdmFile receivers will be closed.
+  mojo::UniqueAssociatedReceiverSet<media::mojom::CdmFile> cdm_file_receivers_;
 
   base::WeakPtrFactory<CdmStorageImpl> weak_factory_{this};
 

content/browser/media/cdm_storage_impl_unittest.cc

@@ -14,14 +14,13 @@
 #include "content/public/test/navigation_simulator.h"
 #include "content/public/test/test_renderer_host.h"
 #include "media/mojo/mojom/cdm_storage.mojom.h"
-#include "mojo/public/cpp/bindings/binding.h"
+#include "mojo/public/cpp/bindings/associated_remote.h"
+#include "mojo/public/cpp/bindings/pending_associated_remote.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
 using media::mojom::CdmFile;
-using media::mojom::CdmFileAssociatedPtr;
-using media::mojom::CdmFileAssociatedPtrInfo;
 using media::mojom::CdmStorage;
 using media::mojom::CdmStoragePtr;
 
@@ -99,7 +98,7 @@ class CdmStorageTest : public RenderViewHostTestHarness {
   // Open the file |name|. Returns true if the file returned is valid, false
   // otherwise. On success |cdm_file| is bound to the CdmFileImpl object.
   bool Open(const std::string& name,
-            CdmFileAssociatedPtr* cdm_file) {
+            mojo::AssociatedRemote<CdmFile>* cdm_file) {
     DVLOG(3) << __func__;
 
     CdmStorage::Status status;
@@ -169,17 +168,21 @@ class CdmStorageTest : public RenderViewHostTestHarness {
 
  private:
   void OpenDone(CdmStorage::Status* status,
-                CdmFileAssociatedPtr* cdm_file,
+                mojo::AssociatedRemote<CdmFile>* cdm_file,
                 CdmStorage::Status actual_status,
-                CdmFileAssociatedPtrInfo actual_cdm_file) {
+                mojo::PendingAssociatedRemote<CdmFile> actual_cdm_file) {
     DVLOG(3) << __func__;
     *status = actual_status;
 
-    // Open() returns a CdmFileAssociatedPtrInfo, so bind it to the
-    // CdmFileAssociatedPtr provided.
-    CdmFileAssociatedPtr cdm_file_ptr;
-    cdm_file_ptr.Bind(std::move(actual_cdm_file));
-    *cdm_file = std::move(cdm_file_ptr);
+    if (!actual_cdm_file) {
+      run_loop_with_count_->Quit();
+      return;
+    }
+    // Open() returns a mojo::PendingAssociatedRemote<CdmFile>, so bind it to
+    // the mojo::AssociatedRemote<CdmFileAssociated> provided.
+    mojo::AssociatedRemote<CdmFile> cdm_file_remote;
+    cdm_file_remote.Bind(std::move(actual_cdm_file));
+    *cdm_file = std::move(cdm_file_remote);
     run_loop_with_count_->Quit();
   }
 
@@ -214,7 +217,7 @@ TEST_F(CdmStorageTest, InvalidFileSystemIdWithSlash) {
   Initialize("name/");
 
   const char kFileName[] = "valid_file_name";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_FALSE(Open(kFileName, &cdm_file));
   EXPECT_FALSE(cdm_file.is_bound());
 }
@@ -223,7 +226,7 @@ TEST_F(CdmStorageTest, InvalidFileSystemIdWithBackSlash) {
   Initialize("name\\");
 
   const char kFileName[] = "valid_file_name";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_FALSE(Open(kFileName, &cdm_file));
   EXPECT_FALSE(cdm_file.is_bound());
 }
@@ -232,7 +235,7 @@ TEST_F(CdmStorageTest, InvalidFileSystemIdEmpty) {
   Initialize("");
 
   const char kFileName[] = "valid_file_name";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_FALSE(Open(kFileName, &cdm_file));
   EXPECT_FALSE(cdm_file.is_bound());
 }
@@ -243,7 +246,7 @@ TEST_F(CdmStorageTest, InvalidFileName) {
   // Anything other than ASCII letter, digits, and -._ will fail. Add a
   // Unicode character to the name.
   const char kFileName[] = "openfile\u1234";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_FALSE(Open(kFileName, &cdm_file));
   EXPECT_FALSE(cdm_file.is_bound());
 }
@@ -252,7 +255,7 @@ TEST_F(CdmStorageTest, InvalidFileNameEmpty) {
   Initialize(kTestFileSystemId);
 
   const char kFileName[] = "";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_FALSE(Open(kFileName, &cdm_file));
   EXPECT_FALSE(cdm_file.is_bound());
 }
@@ -261,7 +264,7 @@ TEST_F(CdmStorageTest, InvalidFileNameStartWithUnderscore) {
   Initialize(kTestFileSystemId);
 
   const char kFileName[] = "_invalid";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_FALSE(Open(kFileName, &cdm_file));
   EXPECT_FALSE(cdm_file.is_bound());
 }
@@ -271,7 +274,7 @@ TEST_F(CdmStorageTest, InvalidFileNameTooLong) {
 
   // Limit is 256 characters, so try a file name with 257.
   const std::string kFileName(257, 'a');
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_FALSE(Open(kFileName, &cdm_file));
   EXPECT_FALSE(cdm_file.is_bound());
 }
@@ -280,7 +283,7 @@ TEST_F(CdmStorageTest, OpenFile) {
   Initialize(kTestFileSystemId);
 
   const char kFileName[] = "test_file_name";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_TRUE(Open(kFileName, &cdm_file));
   EXPECT_TRUE(cdm_file.is_bound());
 }
@@ -289,19 +292,19 @@ TEST_F(CdmStorageTest, OpenFileLocked) {
   Initialize(kTestFileSystemId);
 
   const char kFileName[] = "test_file_name";
-  CdmFileAssociatedPtr cdm_file1;
+  mojo::AssociatedRemote<CdmFile> cdm_file1;
   EXPECT_TRUE(Open(kFileName, &cdm_file1));
   EXPECT_TRUE(cdm_file1.is_bound());
 
   // Second attempt on the same file should fail as the file is locked.
-  CdmFileAssociatedPtr cdm_file2;
+  mojo::AssociatedRemote<CdmFile> cdm_file2;
   EXPECT_FALSE(Open(kFileName, &cdm_file2));
   EXPECT_FALSE(cdm_file2.is_bound());
 
   // Now close the first file and try again. It should be free now.
   cdm_file1.reset();
 
-  CdmFileAssociatedPtr cdm_file3;
+  mojo::AssociatedRemote<CdmFile> cdm_file3;
   EXPECT_TRUE(Open(kFileName, &cdm_file3));
   EXPECT_TRUE(cdm_file3.is_bound());
 }
@@ -310,17 +313,17 @@ TEST_F(CdmStorageTest, MultipleFiles) {
   Initialize(kTestFileSystemId);
 
   const char kFileName1[] = "file1";
-  CdmFileAssociatedPtr cdm_file1;
+  mojo::AssociatedRemote<CdmFile> cdm_file1;
   EXPECT_TRUE(Open(kFileName1, &cdm_file1));
   EXPECT_TRUE(cdm_file1.is_bound());
 
   const char kFileName2[] = "file2";
-  CdmFileAssociatedPtr cdm_file2;
+  mojo::AssociatedRemote<CdmFile> cdm_file2;
   EXPECT_TRUE(Open(kFileName2, &cdm_file2));
   EXPECT_TRUE(cdm_file2.is_bound());
 
   const char kFileName3[] = "file3";
-  CdmFileAssociatedPtr cdm_file3;
+  mojo::AssociatedRemote<CdmFile> cdm_file3;
   EXPECT_TRUE(Open(kFileName3, &cdm_file3));
   EXPECT_TRUE(cdm_file3.is_bound());
 }
@@ -329,7 +332,7 @@ TEST_F(CdmStorageTest, WriteThenReadFile) {
   Initialize(kTestFileSystemId);
 
   const char kFileName[] = "test_file_name";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_TRUE(Open(kFileName, &cdm_file));
   EXPECT_TRUE(cdm_file.is_bound());
 
@@ -346,7 +349,7 @@ TEST_F(CdmStorageTest, ReadThenWriteEmptyFile) {
   Initialize(kTestFileSystemId);
 
   const char kFileName[] = "empty_file_name";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_TRUE(Open(kFileName, &cdm_file));
   EXPECT_TRUE(cdm_file.is_bound());
 
@@ -367,7 +370,7 @@ TEST_F(CdmStorageTest, ParallelRead) {
   Initialize(kTestFileSystemId);
 
   const char kFileName[] = "duplicate_read_file_name";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_TRUE(Open(kFileName, &cdm_file));
   EXPECT_TRUE(cdm_file.is_bound());
 
@@ -386,7 +389,7 @@ TEST_F(CdmStorageTest, ParallelWrite) {
   Initialize(kTestFileSystemId);
 
   const char kFileName[] = "duplicate_write_file_name";
-  CdmFileAssociatedPtr cdm_file;
+  mojo::AssociatedRemote<CdmFile> cdm_file;
   EXPECT_TRUE(Open(kFileName, &cdm_file));
   EXPECT_TRUE(cdm_file.is_bound());
 

media/mojo/mojom/cdm_storage.mojom

@@ -25,7 +25,7 @@ interface CdmStorage {
   //   and must not start with an underscore ('_'). If this happens,
   //   |status| == kFailure is returned.
   Open(string file_name)
-    => (Status status, associated CdmFile? cdm_file);
+    => (Status status, pending_associated_remote<CdmFile>? cdm_file);
 };
 
 // Provides a way to access the contents of the file opened. When the connection
@@ -50,4 +50,4 @@ interface CdmFile {
   // save space. The contents of the file are unknown if Write()
   // fails.
   Write(array<uint8> data) => (Status status);
-};
\ No newline at end of file
+};

media/mojo/services/mojo_cdm_file_io.cc

@@ -80,12 +80,13 @@ void MojoCdmFileIO::Open(const char* file_name, uint32_t file_name_size) {
   // Open() failed.
   auto callback = mojo::WrapCallbackWithDefaultInvokeIfNotRun(
       base::BindOnce(&MojoCdmFileIO::OnFileOpened, weak_factory_.GetWeakPtr()),
-      StorageStatus::kFailure, nullptr);
+      StorageStatus::kFailure, mojo::NullAssociatedRemote());
   cdm_storage_->Open(file_name_string, std::move(callback));
 }
 
-void MojoCdmFileIO::OnFileOpened(StorageStatus status,
-                                 mojom::CdmFileAssociatedPtrInfo cdm_file) {
+void MojoCdmFileIO::OnFileOpened(
+    StorageStatus status,
+    mojo::PendingAssociatedRemote<mojom::CdmFile> cdm_file) {
   DVLOG(3) << __func__ << " file: " << file_name_ << ", status: " << status;
 
   // This logs the end of the async Open() request, and separately logs

media/mojo/services/mojo_cdm_file_io.h

@@ -16,6 +16,8 @@
 #include "media/cdm/api/content_decryption_module.h"
 #include "media/mojo/mojom/cdm_storage.mojom.h"
 #include "media/mojo/services/media_mojo_export.h"
+#include "mojo/public/cpp/bindings/associated_remote.h"
+#include "mojo/public/cpp/bindings/pending_associated_remote.h"
 
 namespace media {
 
@@ -68,7 +70,7 @@ class MEDIA_MOJO_EXPORT MojoCdmFileIO : public cdm::FileIO {
 
   // Called when the file is opened (or not).
   void OnFileOpened(mojom::CdmStorage::Status status,
-                    mojom::CdmFileAssociatedPtrInfo cdm_file);
+                    mojo::PendingAssociatedRemote<mojom::CdmFile> cdm_file);
 
   // Called when the read operation is done.
   void OnFileRead(mojom::CdmFile::Status status,
@@ -101,7 +103,7 @@ class MEDIA_MOJO_EXPORT MojoCdmFileIO : public cdm::FileIO {
   // |cdm_file_| is used to read and write the file and is released when the
   // file is closed so that CdmStorage can tell that the file is no longer being
   // used.
-  mojom::CdmFileAssociatedPtr cdm_file_;
+  mojo::AssociatedRemote<mojom::CdmFile> cdm_file_;
 
   // Keep track of operations in progress.
   State state_ = State::kUnopened;

media/mojo/services/mojo_cdm_helper_unittest.cc

@@ -10,7 +10,7 @@
 #include "base/test/task_environment.h"
 #include "media/cdm/api/content_decryption_module.h"
 #include "media/mojo/mojom/cdm_storage.mojom.h"
-#include "mojo/public/cpp/bindings/associated_binding.h"
+#include "mojo/public/cpp/bindings/associated_receiver.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "services/service_manager/public/cpp/binder_registry.h"
 #include "services/service_manager/public/mojom/interface_provider.mojom.h"
@@ -52,19 +52,17 @@ class TestCdmFile : public mojom::CdmFile {
 
 class MockCdmStorage : public mojom::CdmStorage {
  public:
-  MockCdmStorage() : client_binding_(&cdm_file_) {}
+  MockCdmStorage() = default;
   ~MockCdmStorage() override = default;
 
   void Open(const std::string& file_name, OpenCallback callback) override {
-    mojom::CdmFileAssociatedPtrInfo client_ptr_info;
-    client_binding_.Bind(mojo::MakeRequest(&client_ptr_info));
     std::move(callback).Run(mojom::CdmStorage::Status::kSuccess,
-                            std::move(client_ptr_info));
+                            client_receiver_.BindNewEndpointAndPassRemote());
   }
 
  private:
   TestCdmFile cdm_file_;
-  mojo::AssociatedBinding<mojom::CdmFile> client_binding_;
+  mojo::AssociatedReceiver<mojom::CdmFile> client_receiver_{&cdm_file_};
 };
 
 void CreateCdmStorage(mojom::CdmStorageRequest request) {