New issue
Advanced search Search tips

Issue 836046 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Detach CdmAdapter from CdmModule

Project Member Reported by xhw...@chromium.org, Apr 23 2018

Issue description

Currently CdmAdapter depends on CdmModule to provide CreateCdmFunc. This makes it harder to test CdmAdapter. For example, today CdmAdapter can only be tested with ClearKeyCdm, which isn't very flexible to test with.

We should detach CdmAdapter from CdmModule, such that we can easily test CdmAdapter with any cdm::ContentDecryptorModule* implementation, e.g. a mock CDM implementing cdm::ContentDecryptorModule* directly without a library CDM binary.

 

Comment 1 by xhw...@chromium.org, Apr 23 2018

Description: Show this description
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 24 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e18a5367f46da16928e852661969eb8a7cea1fd5

commit e18a5367f46da16928e852661969eb8a7cea1fd5
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Tue Apr 24 23:51:53 2018

media: Check CdmConfig in CdmAdapter

We used to have these checks in PpapiCdmAdapter, but they were lost when
we added media::CdmAdapter.

In summary:
- Only allow storage ID when |allow_persistent_state| is true.
- Only allow File IO when |allow_persistent_state| is true.
- Only allow platform challenge when |allow_distinctive_identifier| is
  true.

Also post tasks in CdmAdapter in failure cases to avoid reentrance into
the CDM instance.

this change. Filed crbug.com/836046 to refactor CdmAdapter and tests.

Test: The current CdmAdapterTest makes it harder to add unittest for
Bug: 510088,836046
Change-Id: I5a88c253a75a3af00f553681b65792198f31dba7
Reviewed-on: https://chromium-review.googlesource.com/1025220
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553373}
[modify] https://crrev.com/e18a5367f46da16928e852661969eb8a7cea1fd5/media/cdm/cdm_adapter.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Apr 25 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9e2aa9ec031f9d645aaa783dca488400c315291

commit b9e2aa9ec031f9d645aaa783dca488400c315291
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Wed Apr 25 19:14:12 2018

media: Move CdmHostProxy to media/cdm/library_cdm

This class can be useful for any classes to implement/support multiple
versins of library CDM interface. Hence, move it out of clear_key_cdm
folder. In a later CL, I'll use them for a mock library CDM
implementation.

Bug: 836046
Test: No functionality change.
Change-Id: I7567e405de0251ddce3718eb76d102f07ff8f33e
Reviewed-on: https://chromium-review.googlesource.com/1026970
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553693}
[add] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/BUILD.gn
[rename] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/cdm_host_proxy.h
[rename] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/cdm_host_proxy_impl.h
[modify] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/clear_key_cdm/BUILD.gn
[modify] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/clear_key_cdm/cdm_file_adapter.cc
[modify] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/clear_key_cdm/cdm_proxy_test.cc
[modify] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/clear_key_cdm/clear_key_cdm.cc
[modify] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/clear_key_cdm/clear_key_cdm.h
[modify] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/clear_key_cdm/clear_key_persistent_session_cdm.h
[modify] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/clear_key_cdm/ffmpeg_cdm_audio_decoder.cc
[modify] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/clear_key_cdm/ffmpeg_cdm_video_decoder.cc
[modify] https://crrev.com/b9e2aa9ec031f9d645aaa783dca488400c315291/media/cdm/library_cdm/clear_key_cdm/libvpx_cdm_video_decoder.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Apr 27 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e

commit 9d1ce982b52bf17cc6189ae4666ad7d327a1b81e
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Fri Apr 27 19:40:38 2018

media: Refactor CdmAdapterTest

- Add MockLibraryCdm, a mock implementation of library CDM interface so
  that we can have more control in the test cases.
- Refactor CdmAdapter so that CreateCdmFunc is passed in constructor
- Refactor CdmAdapterTest so that it can take different CreateCdmFuncs
 * CdmAdapterTestWithClearKeyCdm uses Clear Key CDM and uses the
   CreateCdmFunc provided by CdmModule.
 * CdmAdapterTestWithMockCdm uses CreateMockCdmInstance to use
   MockLibraryCdm for testing.
- Add tests for ChallengePlatform, CreateFileIO and RequestStorageId
  using CdmAdapterTestWithMockCdm.

Bug: 836046
Test: Test class refactoring; added more tests.
Change-Id: If79922b26b57793f8e75f86e583109b0495fda58
Reviewed-on: https://chromium-review.googlesource.com/1027003
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#554460}
[modify] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/BUILD.gn
[modify] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/aes_decryptor_unittest.cc
[modify] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/cdm_adapter.cc
[modify] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/cdm_adapter.h
[modify] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/cdm_adapter_factory.cc
[modify] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/cdm_adapter_unittest.cc
[modify] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/library_cdm/BUILD.gn
[add] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/library_cdm/mock_library_cdm.cc
[add] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/library_cdm/mock_library_cdm.h
[modify] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/mock_helpers.cc
[modify] https://crrev.com/9d1ce982b52bf17cc6189ae4666ad7d327a1b81e/media/cdm/mock_helpers.h

Sign in to add a comment