media:: Move FileReadCB to MojoCdmFileIO::Delegate

This is a clean up CL so that we can avoid passing one additional
callback into MojoCdmFileIO.

BUG=721876
TEST=No functionality change

Change-Id: Ifa3b071a5a95437f87b50f888bce72bc51ad3fd9
Reviewed-on: https://chromium-review.googlesource.com/770318
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517324}

media/cdm/cdm_adapter.cc

@@ -497,6 +497,9 @@ CdmAdapter::CdmAdapter(
   DCHECK(!session_keys_change_cb_.is_null());
   DCHECK(!session_expiration_update_cb_.is_null());
   DCHECK(helper_);
+
+  helper_->SetFileReadCB(
+      base::Bind(&CdmAdapter::OnFileRead, weak_factory_.GetWeakPtr()));
 }
 
 CdmAdapter::~CdmAdapter() {
@@ -1152,8 +1155,7 @@ void CdmAdapter::OnDeferredInitializationDone(cdm::StreamType stream_type,
 
 cdm::FileIO* CdmAdapter::CreateFileIO(cdm::FileIOClient* client) {
   DCHECK(task_runner_->BelongsToCurrentThread());
-  return helper_->CreateCdmFileIO(
-      client, base::Bind(&CdmAdapter::OnFileRead, weak_factory_.GetWeakPtr()));
+  return helper_->CreateCdmFileIO(client);
 }
 
 void CdmAdapter::RequestStorageId(uint32_t version) {

media/cdm/cdm_auxiliary_helper.cc

@@ -11,8 +11,9 @@ namespace media {
 CdmAuxiliaryHelper::CdmAuxiliaryHelper() {}
 CdmAuxiliaryHelper::~CdmAuxiliaryHelper() {}
 
-cdm::FileIO* CdmAuxiliaryHelper::CreateCdmFileIO(cdm::FileIOClient* client,
-                                                 FileReadCB file_read_cb) {
+void CdmAuxiliaryHelper::SetFileReadCB(FileReadCB file_read_cb) {}
+
+cdm::FileIO* CdmAuxiliaryHelper::CreateCdmFileIO(cdm::FileIOClient* client) {
   return nullptr;
 }
 

media/cdm/cdm_auxiliary_helper.h

@@ -35,15 +35,15 @@ class MEDIA_EXPORT CdmAuxiliaryHelper : public CdmAllocator,
   CdmAuxiliaryHelper();
   ~CdmAuxiliaryHelper() override;
 
-  // Callback to report the size of file read by |this|.
+  // Callback to report the size of file read by cdm::FileIO created by |this|.
   using FileReadCB = base::RepeatingCallback<void(int)>;
+  virtual void SetFileReadCB(FileReadCB file_read_cb);
 
   // Given |client|, creates a cdm::FileIO object and returns it.
   // The caller does not own the returned object and should not delete it
   // directly. Instead, it should call cdm::FileIO::Close() after it's not
   // needed anymore.
-  virtual cdm::FileIO* CreateCdmFileIO(cdm::FileIOClient* client,
-                                       FileReadCB file_read_cb);
+  virtual cdm::FileIO* CreateCdmFileIO(cdm::FileIOClient* client);
 
   // CdmAllocator implementation.
   cdm::Buffer* CreateCdmBuffer(size_t capacity) override;

media/cdm/mock_helpers.cc

@@ -12,8 +12,10 @@ MockCdmAuxiliaryHelper::MockCdmAuxiliaryHelper(
 
 MockCdmAuxiliaryHelper::~MockCdmAuxiliaryHelper() {}
 
-cdm::FileIO* MockCdmAuxiliaryHelper::CreateCdmFileIO(cdm::FileIOClient* client,
-                                                     FileReadCB file_read_cb) {
+void MockCdmAuxiliaryHelper::SetFileReadCB(FileReadCB file_read_cb) {}
+
+cdm::FileIO* MockCdmAuxiliaryHelper::CreateCdmFileIO(
+    cdm::FileIOClient* client) {
   NOTREACHED();
   return nullptr;
 }

media/cdm/mock_helpers.h

@@ -25,8 +25,8 @@ class MockCdmAuxiliaryHelper : public CdmAuxiliaryHelper {
   ~MockCdmAuxiliaryHelper() override;
 
   // CdmAuxiliaryHelper implementation.
-  cdm::FileIO* CreateCdmFileIO(cdm::FileIOClient* client,
-                               FileReadCB file_read_cb) override;
+  void SetFileReadCB(FileReadCB file_read_cb) override;
+  cdm::FileIO* CreateCdmFileIO(cdm::FileIOClient* client) override;
 
   cdm::Buffer* CreateCdmBuffer(size_t capacity) override;
   std::unique_ptr<VideoFrameImpl> CreateCdmVideoFrame() override;

media/mojo/services/mojo_cdm_file_io.cc

@@ -50,18 +50,15 @@ bool IsValidFileName(const std::string& name) {
 
 MojoCdmFileIO::MojoCdmFileIO(Delegate* delegate,
                              cdm::FileIOClient* client,
-                             mojom::CdmStorage* cdm_storage,
-                             FileReadCB file_read_cb)
+                             mojom::CdmStorage* cdm_storage)
     : delegate_(delegate),
       client_(client),
       cdm_storage_(cdm_storage),
-      file_read_cb_(std::move(file_read_cb)),
       weak_factory_(this) {
   DVLOG(1) << __func__;
   DCHECK(delegate_);
   DCHECK(client_);
   DCHECK(cdm_storage_);
-  // |file_read_cb_| may be null in tests.
 }
 
 MojoCdmFileIO::~MojoCdmFileIO() {
@@ -216,8 +213,7 @@ void MojoCdmFileIO::DoRead(int64_t num_bytes) {
 
   // Call this before OnReadComplete() so that we always have the latest file
   // size before CDM fires errors.
-  if (file_read_cb_)
-    file_read_cb_.Run(bytes_to_read);
+  delegate_->ReportFileReadSize(bytes_to_read);
 
   state_ = State::kOpened;
   client_->OnReadComplete(ClientStatus::kSuccess, buffer.data(), buffer.size());

media/mojo/services/mojo_cdm_file_io.h

@@ -29,14 +29,13 @@ namespace media {
 // Implements a cdm::FileIO that communicates with mojom::CdmStorage.
 class MEDIA_MOJO_EXPORT MojoCdmFileIO : public cdm::FileIO {
  public:
-  // Callback to report the size of file read by |this|.
-  // TODO(xhwang): Move |file_read_cb| into |delegate| as well.
-  using FileReadCB = base::RepeatingCallback<void(int)>;
-
   class Delegate {
    public:
-    // Notify the delegate to close |cdm_file_io|.
+    // Notifies the delegate to close |cdm_file_io|.
     virtual void CloseCdmFileIO(MojoCdmFileIO* cdm_file_io) = 0;
+
+    // Reports the size of file read by MojoCdmFileIO.
+    virtual void ReportFileReadSize(int file_size_bytes) = 0;
   };
 
   // The constructor and destructor of cdm::FileIO are protected so that the CDM
@@ -45,8 +44,7 @@ class MEDIA_MOJO_EXPORT MojoCdmFileIO : public cdm::FileIO {
   // management.
   MojoCdmFileIO(Delegate* delegate,
                 cdm::FileIOClient* client,
-                mojom::CdmStorage* cdm_storage,
-                FileReadCB file_read_cb);
+                mojom::CdmStorage* cdm_storage);
   ~MojoCdmFileIO() override;
 
   // cdm::FileIO implementation.
@@ -105,9 +103,6 @@ class MEDIA_MOJO_EXPORT MojoCdmFileIO : public cdm::FileIO {
 
   mojom::CdmStorage* cdm_storage_ = nullptr;
 
-  // Callback to report the size of a file that was read.
-  FileReadCB file_read_cb_;
-
   // Keep track of the file being used. As this class can only be used for
   // accessing a single file, once |file_name_| is set it shouldn't be changed.
   // |file_name_| is only saved for logging purposes.

media/mojo/services/mojo_cdm_file_io_unittest.cc

@@ -76,8 +76,8 @@ class MojoCdmFileIOTest : public testing::Test, public MojoCdmFileIO::Delegate {
     client_ = std::make_unique<MockFileIOClient>();
     cdm_storage_ = std::make_unique<MockCdmStorage>();
     ASSERT_TRUE(cdm_storage_->SetUp());
-    file_io_ = std::make_unique<MojoCdmFileIO>(
-        this, client_.get(), cdm_storage_.get(), MojoCdmFileIO::FileReadCB());
+    file_io_ = std::make_unique<MojoCdmFileIO>(this, client_.get(),
+                                               cdm_storage_.get());
   }
 
   // MojoCdmFileIO::Delegate implementation.
@@ -86,6 +86,8 @@ class MojoCdmFileIOTest : public testing::Test, public MojoCdmFileIO::Delegate {
     file_io_.reset();
   }
 
+  void ReportFileReadSize(int file_size_bytes) override {}
+
   base::test::ScopedTaskEnvironment scoped_task_environment_;
   std::unique_ptr<MojoCdmFileIO> file_io_;
   std::unique_ptr<MockCdmStorage> cdm_storage_;

media/mojo/services/mojo_cdm_helper.cc

@@ -19,13 +19,16 @@ MojoCdmHelper::MojoCdmHelper(
 
 MojoCdmHelper::~MojoCdmHelper() {}
 
-cdm::FileIO* MojoCdmHelper::CreateCdmFileIO(cdm::FileIOClient* client,
-                                            FileReadCB file_read_cb) {
+void MojoCdmHelper::SetFileReadCB(FileReadCB file_read_cb) {
+  file_read_cb_ = std::move(file_read_cb);
+}
+
+cdm::FileIO* MojoCdmHelper::CreateCdmFileIO(cdm::FileIOClient* client) {
   ConnectToCdmStorage();
 
   // Pass a reference to CdmStorage so that MojoCdmFileIO can open a file.
-  auto mojo_cdm_file_io = std::make_unique<MojoCdmFileIO>(
-      this, client, cdm_storage_ptr_.get(), std::move(file_read_cb));
+  auto mojo_cdm_file_io =
+      std::make_unique<MojoCdmFileIO>(this, client, cdm_storage_ptr_.get());
 
   cdm::FileIO* cdm_file_io = mojo_cdm_file_io.get();
   DVLOG(3) << __func__ << ": cdm_file_io = " << cdm_file_io;
@@ -83,6 +86,12 @@ void MojoCdmHelper::CloseCdmFileIO(MojoCdmFileIO* cdm_file_io) {
                 });
 }
 
+void MojoCdmHelper::ReportFileReadSize(int file_size_bytes) {
+  DVLOG(3) << __func__ << ": file_size_bytes = " << file_size_bytes;
+  if (file_read_cb_)
+    file_read_cb_.Run(file_size_bytes);
+}
+
 void MojoCdmHelper::ConnectToCdmStorage() {
   if (!cdm_storage_ptr_) {
     service_manager::GetInterface<mojom::CdmStorage>(interface_provider_,

media/mojo/services/mojo_cdm_helper.h

@@ -37,8 +37,8 @@ class MEDIA_MOJO_EXPORT MojoCdmHelper final : public CdmAuxiliaryHelper,
   ~MojoCdmHelper() final;
 
   // CdmAuxiliaryHelper implementation.
-  cdm::FileIO* CreateCdmFileIO(cdm::FileIOClient* client,
-                               FileReadCB file_read_cb) final;
+  void SetFileReadCB(FileReadCB file_read_cb) final;
+  cdm::FileIO* CreateCdmFileIO(cdm::FileIOClient* client) final;
   cdm::Buffer* CreateCdmBuffer(size_t capacity) final;
   std::unique_ptr<VideoFrameImpl> CreateCdmVideoFrame() final;
   void QueryStatus(QueryStatusCB callback) final;
@@ -51,6 +51,7 @@ class MEDIA_MOJO_EXPORT MojoCdmHelper final : public CdmAuxiliaryHelper,
 
   // MojoCdmFileIO::Delegate implementation.
   void CloseCdmFileIO(MojoCdmFileIO* cdm_file_io) final;
+  void ReportFileReadSize(int file_size_bytes) final;
 
  private:
   // All services are created lazily.
@@ -71,6 +72,8 @@ class MEDIA_MOJO_EXPORT MojoCdmHelper final : public CdmAuxiliaryHelper,
   mojom::OutputProtectionPtr output_protection_ptr_;
   mojom::PlatformVerificationPtr platform_verification_ptr_;
 
+  FileReadCB file_read_cb_;
+
   // A list of open cdm::FileIO objects.
   std::vector<std::unique_ptr<MojoCdmFileIO>> cdm_file_io_set_;
 

media/mojo/services/mojo_cdm_helper_unittest.cc

@@ -90,8 +90,7 @@ class MojoCdmHelperTest : public testing::Test {
 };
 
 TEST_F(MojoCdmHelperTest, CreateCdmFileIO_OpenClose) {
-  cdm::FileIO* file_io = helper_.CreateCdmFileIO(
-      &file_io_client_, CdmAuxiliaryHelper::FileReadCB());
+  cdm::FileIO* file_io = helper_.CreateCdmFileIO(&file_io_client_);
   const std::string kFileName = "openfile";
   EXPECT_CALL(file_io_client_, OnOpenComplete(Status::kSuccess));
   file_io->Open(kFileName.data(), kFileName.length());
@@ -106,8 +105,7 @@ TEST_F(MojoCdmHelperTest, CreateCdmFileIO_OpenClose) {
 // should not leak the cdm::FileIO object. LeakSanitizer bots should be able to
 // catch such issues.
 TEST_F(MojoCdmHelperTest, CreateCdmFileIO_OpenWithoutClose) {
-  cdm::FileIO* file_io = helper_.CreateCdmFileIO(
-      &file_io_client_, CdmAuxiliaryHelper::FileReadCB());
+  cdm::FileIO* file_io = helper_.CreateCdmFileIO(&file_io_client_);
   const std::string kFileName = "openfile";
   EXPECT_CALL(file_io_client_, OnOpenComplete(Status::kSuccess));
   file_io->Open(kFileName.data(), kFileName.length());