New issue
Advanced search Search tips

Issue 826039 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Add tests for CdmService

Project Member Reported by xhw...@chromium.org, Mar 26 2018

Issue description

Previous CdmService was a alias of MediaService and we have some basic testing coverage (e.g. media_service_unittests). Now CdmService has been split from MediaService and we lose that coverage. It's not too bad now since they share almost the same code pattern. But over time, when they inevitably diverge, we should have test coverage on both.

This issue is filed to track test coverage for CdmService, and it's related classes such as CdmFactoryImpl.
 
Project Member

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

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

commit f4196ee36308150cc55e7df0f48a526b848e43af
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Wed Apr 04 01:59:36 2018

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}
[modify] https://crrev.com/f4196ee36308150cc55e7df0f48a526b848e43af/media/mojo/services/BUILD.gn
[modify] https://crrev.com/f4196ee36308150cc55e7df0f48a526b848e43af/media/mojo/services/OWNERS
[modify] https://crrev.com/f4196ee36308150cc55e7df0f48a526b848e43af/media/mojo/services/cdm_service.cc
[add] https://crrev.com/f4196ee36308150cc55e7df0f48a526b848e43af/media/mojo/services/cdm_service_unittest.cc
[add] https://crrev.com/f4196ee36308150cc55e7df0f48a526b848e43af/media/mojo/services/cdm_service_unittest_manifest.json

Project Member

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

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

commit ce02098e551bc75c994d819623fa80d707e56594
Author: Findit <findit-for-me@appspot.gserviceaccount.com>
Date: Wed Apr 04 02:38:17 2018

Revert "media: Add CdmServiceTest"

This reverts commit f4196ee36308150cc55e7df0f48a526b848e43af.

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}
[modify] https://crrev.com/ce02098e551bc75c994d819623fa80d707e56594/media/mojo/services/BUILD.gn
[modify] https://crrev.com/ce02098e551bc75c994d819623fa80d707e56594/media/mojo/services/OWNERS
[modify] https://crrev.com/ce02098e551bc75c994d819623fa80d707e56594/media/mojo/services/cdm_service.cc
[delete] https://crrev.com/f75b204be5dd86dce3a19e25986a7a9c1d2d6a56/media/mojo/services/cdm_service_unittest.cc
[delete] https://crrev.com/f75b204be5dd86dce3a19e25986a7a9c1d2d6a56/media/mojo/services/cdm_service_unittest_manifest.json

Project Member

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

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

commit abad65cc905a2894dab0dd8618db6436ace8588c
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Thu Apr 05 00:03:48 2018

(reland) media: Add CdmServiceTest

Reland of the original CL with fix on Mac.

Original CL Description:

This is a ServiceTest tha runs CdmService in-process (packaged) which
covers some basic interaction between CdmService and CdmFactoryImpl.

TBR=jrummell@chromium.org,tsepez@chromium.org

Bug:  826039 
Test: Adds new tests to media_service_unittests
Change-Id: Ieb087d7d4690d80965b4c2d3b04d7c497bf9560b
Reviewed-on: https://chromium-review.googlesource.com/994460
Reviewed-by: John Rummell <jrummell@chromium.org>
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548260}
[modify] https://crrev.com/abad65cc905a2894dab0dd8618db6436ace8588c/media/mojo/services/BUILD.gn
[modify] https://crrev.com/abad65cc905a2894dab0dd8618db6436ace8588c/media/mojo/services/OWNERS
[modify] https://crrev.com/abad65cc905a2894dab0dd8618db6436ace8588c/media/mojo/services/cdm_service.cc
[add] https://crrev.com/abad65cc905a2894dab0dd8618db6436ace8588c/media/mojo/services/cdm_service_unittest.cc
[add] https://crrev.com/abad65cc905a2894dab0dd8618db6436ace8588c/media/mojo/services/cdm_service_unittest_manifest.json

Project Member

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

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

commit d3737943a63437bb5fd3bcdfb5a0ebbb775dd65b
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Thu Apr 12 03:32:22 2018

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/1006530
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550004}
[modify] https://crrev.com/d3737943a63437bb5fd3bcdfb5a0ebbb775dd65b/media/mojo/services/cdm_service.cc
[modify] https://crrev.com/d3737943a63437bb5fd3bcdfb5a0ebbb775dd65b/media/mojo/services/cdm_service.h
[modify] https://crrev.com/d3737943a63437bb5fd3bcdfb5a0ebbb775dd65b/media/mojo/services/cdm_service_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Apr 12 2018

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

commit d6ecc1be8f30d54ae37ef47878353b49085c5783
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Thu Apr 12 19:15:54 2018

media: Enable "media_service_unittests" on more bots

MediaService has been shipped on Android and ChromeCast, to support
remote CDM, AudioDecoder and media Renderer. It will be shipped on Mac
and Windows soon to support hardware audio/video decoders running in the
GPU process.

CdmService (also covered by this test) has been shipped on all desktop
platforms to support CDMs.

Hence, it's getting more important to have proper test/bot coverage on
both MediaService and CdmService.

This CL enables "media_service_unitests" in "chromium_gtests" so we run
the test on more bots. More test cases will come later.

Bug:  826039 
Test: This CL enables the test on more bots.
Change-Id: I4ef9508f47386dfb61a0fefeba4640657aa1b725
Reviewed-on: https://chromium-review.googlesource.com/994873
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550298}
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.android.fyi.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.android.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.chromiumos.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.clang.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.mac.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.memory.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.win.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/test_suite_exceptions.pyl
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/test_suites.pyl

Project Member

Comment 6 by bugdroid1@chromium.org, Apr 13 2018

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

commit d014653a8c07c2ad6e4899d401037a313b4ed095
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Fri Apr 13 16:34:58 2018

media: Add test to cover delayed CdmService release

Also refator CdmService so that we can override service release delay for
testing.

Bug:  826039 
Change-Id: I595a26c019ee760c49a2bac638c4f928101f6423
Reviewed-on: https://chromium-review.googlesource.com/1011404
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550640}
[modify] https://crrev.com/d014653a8c07c2ad6e4899d401037a313b4ed095/media/mojo/services/cdm_service.cc
[modify] https://crrev.com/d014653a8c07c2ad6e4899d401037a313b4ed095/media/mojo/services/cdm_service.h
[modify] https://crrev.com/d014653a8c07c2ad6e4899d401037a313b4ed095/media/mojo/services/cdm_service_unittest.cc

Comment 7 by xhw...@chromium.org, Apr 13 2018

Status: Fixed (was: Assigned)
Project Member

Comment 8 by bugdroid1@chromium.org, Apr 17 2018

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d6ecc1be8f30d54ae37ef47878353b49085c5783

commit d6ecc1be8f30d54ae37ef47878353b49085c5783
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Thu Apr 12 19:15:54 2018

media: Enable "media_service_unittests" on more bots

MediaService has been shipped on Android and ChromeCast, to support
remote CDM, AudioDecoder and media Renderer. It will be shipped on Mac
and Windows soon to support hardware audio/video decoders running in the
GPU process.

CdmService (also covered by this test) has been shipped on all desktop
platforms to support CDMs.

Hence, it's getting more important to have proper test/bot coverage on
both MediaService and CdmService.

This CL enables "media_service_unitests" in "chromium_gtests" so we run
the test on more bots. More test cases will come later.

Bug:  826039 
Test: This CL enables the test on more bots.
Change-Id: I4ef9508f47386dfb61a0fefeba4640657aa1b725
Reviewed-on: https://chromium-review.googlesource.com/994873
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550298}
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.android.fyi.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.android.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.chromiumos.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.clang.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.fyi.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.linux.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.mac.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.memory.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/chromium.win.json
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/test_suite_exceptions.pyl
[modify] https://crrev.com/d6ecc1be8f30d54ae37ef47878353b49085c5783/testing/buildbot/test_suites.pyl

Project Member

Comment 9 by bugdroid1@chromium.org, Apr 17 2018

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

commit d014653a8c07c2ad6e4899d401037a313b4ed095
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Fri Apr 13 16:34:58 2018

media: Add test to cover delayed CdmService release

Also refator CdmService so that we can override service release delay for
testing.

Bug:  826039 
Change-Id: I595a26c019ee760c49a2bac638c4f928101f6423
Reviewed-on: https://chromium-review.googlesource.com/1011404
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550640}
[modify] https://crrev.com/d014653a8c07c2ad6e4899d401037a313b4ed095/media/mojo/services/cdm_service.cc
[modify] https://crrev.com/d014653a8c07c2ad6e4899d401037a313b4ed095/media/mojo/services/cdm_service.h
[modify] https://crrev.com/d014653a8c07c2ad6e4899d401037a313b4ed095/media/mojo/services/cdm_service_unittest.cc

Sign in to add a comment