New issue
Advanced search Search tips

Issue 771791 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: ----
Type: ----

Blocking:
issue 768926



Sign in to add a comment

Split off mojo CdmService from mojo MediaService

Project Member Reported by xhw...@chromium.org, Oct 4 2017

Issue description

Currently the mojo CDM service uses the same MediaService mojom interface and implementation. This is because the CDM service is a subset of the MediaService: MediaService can provide decoders/renderers, as well as CDMs.

However, it might make sense to have a standalone CDM service now. Here are the reasons:

1. With library CDM support, we are adding more and more library-CDM-specific code into MediaService.
2. MediaService lives in media/, which should not know anything about sandbox, but in the current design, we'll have to add some sandbox dependency in the CDM service. If we have a separate CDM service, we might be able to put it in content/ layer.
3. In MediaService, we have media::mojom::InterfaceFactory to enforce site isolation: decoders/renderers/cdms belonging to the same "frame" are grouped together under one InterfaceFactoryImpl. In the CDM service, since we only have individual CDM services, this seems unnecessary.
4. A lot of frame auxiliary services are only used by the library CDM. By splitting CDM service from MediaService, we avoid over-exposing frame auxiliary services.

Of course, if we do this, we'll end up with some duplicate code. I'll have to think more or prototype it a bit to see whether it's worth it.
 
Description: Show this description
Having a dedicated CDM service will also make it easier to solver 768926. Basically, just let the CDM service owning the CdmModule.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 22 2017

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

commit e0281b79c4b54256e9127e7067f97a1510bb401a
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Fri Dec 22 00:45:28 2017

media: Split CdmService from MediaService

Current CdmService is just an alias of MediaService because the latter
can also support hosting a CDM. This help prevent duplication of code.
However, there are a few issues with this approach:
1. It makes the code harder to understand.
2. CdmService doesn't need all the functionality provided by the
MediaService.
3. CdmService needs some customization (e.g. LoadCdm) which introduces
some complexity to MediaService.

This CL split CdmService from MediaService so that we can do some clean
up. More simplification will follow in future CLs.

BUG= 771791 
TEST=No functionality change.

Change-Id: I0ee627f2d01a77b33610956482197fbdc5425dcd
Reviewed-on: https://chromium-review.googlesource.com/833369
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525875}
[modify] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/content/browser/media/media_interface_proxy.cc
[modify] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/content/utility/utility_service_factory.cc
[modify] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/media/mojo/interfaces/BUILD.gn
[add] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/media/mojo/interfaces/cdm_service.mojom
[rename] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/media/mojo/interfaces/cdm_service_mac.mojom
[modify] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/media/mojo/interfaces/media_service.mojom
[modify] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/media/mojo/services/BUILD.gn
[modify] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/media/mojo/services/cdm_manifest.json
[add] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/media/mojo/services/cdm_service.cc
[add] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/media/mojo/services/cdm_service.h
[modify] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/media/mojo/services/media_service.cc
[modify] https://crrev.com/e0281b79c4b54256e9127e7067f97a1510bb401a/media/mojo/services/media_service.h

Project Member

Comment 4 by bugdroid1@chromium.org, Dec 22 2017

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

commit e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960
Author: Thomas Anderson <thomasanderson@chromium.org>
Date: Fri Dec 22 01:31:31 2017

Revert "media: Split CdmService from MediaService"

This reverts commit e0281b79c4b54256e9127e7067f97a1510bb401a.

Reason for revert: Causing a build failure on the Mac builder:
https://ci.chromium.org/buildbot/chromium/Mac/36060

Original change's description:
> media: Split CdmService from MediaService
> 
> Current CdmService is just an alias of MediaService because the latter
> can also support hosting a CDM. This help prevent duplication of code.
> However, there are a few issues with this approach:
> 1. It makes the code harder to understand.
> 2. CdmService doesn't need all the functionality provided by the
> MediaService.
> 3. CdmService needs some customization (e.g. LoadCdm) which introduces
> some complexity to MediaService.
> 
> This CL split CdmService from MediaService so that we can do some clean
> up. More simplification will follow in future CLs.
> 
> BUG= 771791 
> TEST=No functionality change.
> 
> Change-Id: I0ee627f2d01a77b33610956482197fbdc5425dcd
> Reviewed-on: https://chromium-review.googlesource.com/833369
> Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
> Reviewed-by: Greg Kerr <kerrnel@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: John Rummell <jrummell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#525875}

TBR=dcheng@chromium.org,xhwang@chromium.org,jam@chromium.org,jrummell@chromium.org,kerrnel@chromium.org

Change-Id: I3c50947899216656c3d935b6af43f07d924ec5a1
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  771791 
Reviewed-on: https://chromium-review.googlesource.com/841663
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525884}
[modify] https://crrev.com/e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960/content/browser/media/media_interface_proxy.cc
[modify] https://crrev.com/e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960/content/utility/utility_service_factory.cc
[modify] https://crrev.com/e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960/media/mojo/interfaces/BUILD.gn
[delete] https://crrev.com/08ef0c879f819a2b26227856688d7520eb66adfd/media/mojo/interfaces/cdm_service.mojom
[modify] https://crrev.com/e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960/media/mojo/interfaces/media_service.mojom
[rename] https://crrev.com/e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960/media/mojo/interfaces/media_service_mac.mojom
[modify] https://crrev.com/e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960/media/mojo/services/BUILD.gn
[modify] https://crrev.com/e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960/media/mojo/services/cdm_manifest.json
[delete] https://crrev.com/08ef0c879f819a2b26227856688d7520eb66adfd/media/mojo/services/cdm_service.cc
[delete] https://crrev.com/08ef0c879f819a2b26227856688d7520eb66adfd/media/mojo/services/cdm_service.h
[modify] https://crrev.com/e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960/media/mojo/services/media_service.cc
[modify] https://crrev.com/e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960/media/mojo/services/media_service.h

Project Member

Comment 5 by bugdroid1@chromium.org, Dec 22 2017

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

commit 7303874882599712c0a3933b294f87782127c282
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Dec 22 01:44:25 2017

Revert "media: Split CdmService from MediaService"

This reverts commit e0281b79c4b54256e9127e7067f97a1510bb401a.

Reason for revert: Suspected of breaking Mac:
https://logs.chromium.org/v/?s=chromium%2Fbb%2Fchromium%2FMac%2F36060%2F%2B%2Frecipes%2Fsteps%2Fcompile%2F0%2Fstdout

[42520/57721] CXX obj/media/mojo/services/media_service_unittests/media_service_unittest.o
FAILED: obj/media/mojo/services/media_service_unittests/media_service_unittest.o 
export DEVELOPER_DIR=/b/c/b/mac/src/build/mac_files/Xcode.app;  /b/c/goma_client/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -MMD -MF obj/media/mojo/services/media_service_unittests/media_service_unittest.o.d -DV8_DEPRECATION_WARNINGS -DNO_TCMALLOC -DFULL_SAFE_BROWSING -DSAFE_BROWSING_CSD -DSAFE_BROWSING_DB_LOCAL -DCHROMIUM_BUILD -DFIELDTRIAL_TESTING_ENABLED -DCR_XCODE_VERSION=0832 -DCR_CLANG_REVISION=\"321204-1\" -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D_FORTIFY_SOURCE=2 -D__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORE=0 -DNDEBUG -DNVALGRIND -DDYNAMIC_ANNOTATIONS_ENABLED=0 -DWEBP_EXTERN=extern -DGTEST_API_= -DGTEST_HAS_POSIX_RE=0 -DGTEST_LANG_CXX11=1 -DSK_IGNORE_LINEONLY_AA_CONVEX_PATH_OPTS -DSK_HAS_PNG_LIBRARY -DSK_HAS_WEBP_LIBRARY -DSK_HAS_JPEG_LIBRARY -DSK_SUPPORT_GPU=1 -DSK_BUILD_FOR_MAC -DGOOGLE_PROTOBUF_NO_RTTI -DGOOGLE_PROTOBUF_NO_STATIC_INITIALIZER -DHAVE_PTHREAD -DLEVELDB_PLATFORM_CHROMIUM=1 -DENABLE_IPC_FUZZER -DU_USING_ICU_NAMESPACE=0 -DU_ENABLE_DYLOAD=0 -DU_STATIC_IMPLEMENTATION -DICU_UTIL_DATA_IMPL=ICU_UTIL_DATA_FILE -DUCHAR_TYPE=uint16_t -DMESA_EGL_NO_X11_HEADERS -DWEBRTC_NON_STATIC_TRACE_EVENT_HANDLERS=0 -DFEATURE_ENABLE_VOICEMAIL -DGTEST_RELATIVE_PATH -DWEBRTC_CHROMIUM_BUILD -DWEBRTC_POSIX -DWEBRTC_MAC -DUNIT_TEST -I../.. -Igen -I../../third_party/libwebp/src -I../../third_party/libyuv/include -I../../third_party/khronos -I../../gpu -I../../third_party/googletest/src/googletest/include -I../../third_party/libwebm/source -I../../skia/config -I../../skia/ext -I../../third_party/skia/include/c -I../../third_party/skia/include/config -I../../third_party/skia/include/core -I../../third_party/skia/include/effects -I../../third_party/skia/include/encode -I../../third_party/skia/include/gpu -I../../third_party/skia/include/images -I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops -I../../third_party/skia/include/pdf -I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports -I../../third_party/skia/include/utils -I../../third_party/vulkan/include -I../../third_party/skia/src/gpu -I../../third_party/skia/src/sksl -I../../third_party/protobuf/src -Igen/protoc_out -I../../third_party/protobuf/src -I../../third_party/leveldatabase -I../../third_party/leveldatabase/src -I../../third_party/leveldatabase/src/include -I../../third_party/icu/source/common -I../../third_party/icu/source/i18n -I../../third_party/mesa/src/include -I../../third_party/webrtc_overrides -I../../third_party/webrtc -I../../third_party/webrtc/common_video/include -I../../third_party/ced/src -I../../third_party/googletest/custom -I../../third_party/googletest/src/googlemock/include -fno-strict-aliasing -fstack-protector -Wno-builtin-macro-redefined -D__DATE__= -D__TIME__= -D__TIMESTAMP__= -fcolor-diagnostics -Xclang -mllvm -Xclang -instcombine-lower-dbg-declare=0 -no-canonical-prefixes -arch x86_64 -Wall -Werror -Wextra -Wunguarded-availability -Wno-missing-field-initializers -Wno-unused-parameter -Wno-c++11-narrowing -Wno-covered-switch-default -Wno-unneeded-internal-declaration -Wno-inconsistent-missing-override -Wno-undefined-var-template -Wno-nonportable-include-path -Wno-address-of-packed-member -Wno-unused-lambda-capture -Wno-user-defined-warnings -Wno-enum-compare-switch -Wno-tautological-unsigned-zero-compare -Wno-null-pointer-arithmetic -Wno-tautological-constant-compare -Wtautological-constant-out-of-range-compare -O2 -fno-omit-frame-pointer -gdwarf-2 -isysroot ../../build/mac_files/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.12.sdk -mmacosx-version-min=10.9.0 -fvisibility=hidden -Xclang -load -Xclang ../../third_party/llvm-build/Release+Asserts/lib/libFindBadConstructs.dylib -Xclang -add-plugin -Xclang find-bad-constructs -Wheader-hygiene -Wstring-conversion -Wtautological-overlap-compare -std=c++14 -stdlib=libc++ -fno-exceptions -fno-rtti -fvisibility-inlines-hidden -c ../../media/mojo/services/media_service_unittest.cc -o obj/media/mojo/services/media_service_unittests/media_service_unittest.o
../../media/mojo/services/media_service_unittest.cc:36:10: fatal error: 'media/mojo/interfaces/media_service_mac.mojom.h' file not found
#include "media/mojo/interfaces/media_service_mac.mojom.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.


Original change's description:
> media: Split CdmService from MediaService
> 
> Current CdmService is just an alias of MediaService because the latter
> can also support hosting a CDM. This help prevent duplication of code.
> However, there are a few issues with this approach:
> 1. It makes the code harder to understand.
> 2. CdmService doesn't need all the functionality provided by the
> MediaService.
> 3. CdmService needs some customization (e.g. LoadCdm) which introduces
> some complexity to MediaService.
> 
> This CL split CdmService from MediaService so that we can do some clean
> up. More simplification will follow in future CLs.
> 
> BUG= 771791 
> TEST=No functionality change.
> 
> Change-Id: I0ee627f2d01a77b33610956482197fbdc5425dcd
> Reviewed-on: https://chromium-review.googlesource.com/833369
> Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
> Reviewed-by: Greg Kerr <kerrnel@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: John Abd-El-Malek <jam@chromium.org>
> Reviewed-by: John Rummell <jrummell@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#525875}

TBR=dcheng@chromium.org,xhwang@chromium.org,jam@chromium.org,jrummell@chromium.org,kerrnel@chromium.org

Change-Id: I575ed4ffde40e6bec7ce8960e0e5eaa80c1ee0c8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  771791 
Reviewed-on: https://chromium-review.googlesource.com/841882
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525885}

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 22 2017

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

commit b183e18c137bb7131252e4acdea6074ecff0cd8d
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Fri Dec 22 04:27:53 2017

(reland) media: Split CdmService from MediaService

This reverts commit e4a2ef5ec9862f8edb27ecffdeb6c4129aca4960 and relands
the original CL with fix.

Original CL description:

Current CdmService is just an alias of MediaService because the latter
can also support hosting a CDM. This help prevent duplication of code.
However, there are a few issues with this approach:
1. It makes the code harder to understand.
2. CdmService doesn't need all the functionality provided by the
MediaService.
3. CdmService needs some customization (e.g. LoadCdm) which introduces
some complexity to MediaService.

This CL split CdmService from MediaService so that we can do some clean
up. More simplification will follow in future CLs.

TBR=jrummell@chromium.org,dcheng@chromium.org,jam@chromium.org
BUG= 771791 
TEST=No functionality change.

Change-Id: I3e040bb2395fb4aaddba8644ba4632b2ea22283e
Reviewed-on: https://chromium-review.googlesource.com/841754
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#525928}
[modify] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/content/browser/media/media_interface_proxy.cc
[modify] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/content/utility/utility_service_factory.cc
[modify] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/interfaces/BUILD.gn
[add] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/interfaces/cdm_service.mojom
[rename] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/interfaces/cdm_service_mac.mojom
[modify] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/interfaces/media_service.mojom
[modify] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/services/BUILD.gn
[modify] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/services/cdm_manifest.json
[add] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/services/cdm_service.cc
[add] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/services/cdm_service.h
[modify] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/services/media_service.cc
[modify] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/services/media_service.h
[modify] https://crrev.com/b183e18c137bb7131252e4acdea6074ecff0cd8d/media/mojo/services/media_service_unittest.cc

Blocking: 768926
Summary: Split off mojo CdmService from mojo MediaService (was: Consider splitting off mojo CdmService from mojo MediaService)
Status: Started (was: Assigned)
Cc: julien.isorce@chromium.org
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 23 2018

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

commit a3cb5eee99a8d6327fcedc517f903c8c53a92286
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Tue Jan 23 20:23:56 2018

media: Simplify CdmService

- Restrict CdmService to only host library CDM. Previously CdmService
  is generic, but it's never used anywhere else, and that assumption
  has caused some unnecessary code complexity.
- As such, also remove ENABLE_STANDALONE_CDM_SERVICE and replace it
  with ENABLE_LIBRARY_CDMS where applicable.
- Remove the use of media::mojom::InterfaceFactory for the CdmService,
  and replace it with a dedicated media::mojom::CdmFactory.
- Remove the use of MojoMediaClient in CdmService, and replace it with
  CdmService::Client.

BUG= 771791 

Change-Id: I49e0fc4958b18e1b619e20ab4772245b256ab454
Reviewed-on: https://chromium-review.googlesource.com/852556
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531329}
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/content/browser/media/media_interface_proxy.cc
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/content/browser/media/media_interface_proxy.h
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/content/utility/utility_service_factory.cc
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/media_options.gni
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/BUILD.gn
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/interfaces/cdm_service.mojom
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/interfaces/cdm_service_mac.mojom
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/interfaces/content_decryption_module.mojom
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/services/BUILD.gn
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/services/cdm_service.cc
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/services/cdm_service.h
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/services/interface_factory_impl.cc
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/services/interface_factory_impl.h
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/services/mojo_media_client.cc
[modify] https://crrev.com/a3cb5eee99a8d6327fcedc517f903c8c53a92286/media/mojo/services/mojo_media_client.h

Project Member

Comment 12 by bugdroid1@chromium.org, Jan 23 2018

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

commit 7ad8abbb833ae7a2da936e2d58995c0d2ffac922
Author: Joe Downing <joedow@chromium.org>
Date: Tue Jan 23 21:06:31 2018

Revert "media: Simplify CdmService"

This reverts commit a3cb5eee99a8d6327fcedc517f903c8c53a92286.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> media: Simplify CdmService
> 
> - Restrict CdmService to only host library CDM. Previously CdmService
>   is generic, but it's never used anywhere else, and that assumption
>   has caused some unnecessary code complexity.
> - As such, also remove ENABLE_STANDALONE_CDM_SERVICE and replace it
>   with ENABLE_LIBRARY_CDMS where applicable.
> - Remove the use of media::mojom::InterfaceFactory for the CdmService,
>   and replace it with a dedicated media::mojom::CdmFactory.
> - Remove the use of MojoMediaClient in CdmService, and replace it with
>   CdmService::Client.
> 
> BUG= 771791 
> 
> Change-Id: I49e0fc4958b18e1b619e20ab4772245b256ab454
> Reviewed-on: https://chromium-review.googlesource.com/852556
> Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
> Reviewed-by: John Rummell <jrummell@chromium.org>
> Reviewed-by: Nasko Oskov <nasko@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#531329}

TBR=nasko@chromium.org,xhwang@chromium.org,jrummell@chromium.org

Change-Id: I330da3e6cd95e801d1def4abe4f9b8ec5c8fff1b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  771791 
Reviewed-on: https://chromium-review.googlesource.com/881362
Reviewed-by: Joe Downing <joedow@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531343}
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/content/browser/media/media_interface_proxy.cc
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/content/browser/media/media_interface_proxy.h
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/content/utility/utility_service_factory.cc
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/media_options.gni
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/BUILD.gn
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/interfaces/cdm_service.mojom
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/interfaces/cdm_service_mac.mojom
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/interfaces/content_decryption_module.mojom
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/services/BUILD.gn
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/services/cdm_service.cc
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/services/cdm_service.h
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/services/interface_factory_impl.cc
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/services/interface_factory_impl.h
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/services/mojo_media_client.cc
[modify] https://crrev.com/7ad8abbb833ae7a2da936e2d58995c0d2ffac922/media/mojo/services/mojo_media_client.h

Project Member

Comment 13 by bugdroid1@chromium.org, Jan 24 2018

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

commit a0689979d60ae9b2501cd0eac1cf361eb37a6df5
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Wed Jan 24 19:08:49 2018

(reland) media: Simplify CdmService

This reverts commit 7ad8abbb833ae7a2da936e2d58995c0d2ffac922 and
 reland commit a3cb5eee99a8d6327fcedc517f903c8c53a92286 with fix.

Original CL description:

- Restrict CdmService to only host library CDM. Previously CdmService
  is generic, but it's never used anywhere else, and that assumption
  has caused some unnecessary code complexity.
- As such, also remove ENABLE_STANDALONE_CDM_SERVICE and replace it
  with ENABLE_LIBRARY_CDMS where applicable.
- Remove the use of media::mojom::InterfaceFactory for the CdmService,
  and replace it with a dedicated media::mojom::CdmFactory.
- Remove the use of MojoMediaClient in CdmService, and replace it with
  CdmService::Client.

TBR=jrummell@chromium.org,nasko@chromium.org
BUG= 771791 

Change-Id: Id247bfba64a920b714ae2b080815dc3cb56e7d31
Reviewed-on: https://chromium-review.googlesource.com/882307
Reviewed-by: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531637}
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/content/browser/media/media_interface_proxy.cc
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/content/browser/media/media_interface_proxy.h
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/content/browser/service_manager/service_manager_context.cc
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/content/utility/utility_service_factory.cc
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/media_options.gni
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/BUILD.gn
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/interfaces/cdm_service.mojom
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/interfaces/cdm_service_mac.mojom
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/interfaces/content_decryption_module.mojom
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/services/BUILD.gn
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/services/cdm_service.cc
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/services/cdm_service.h
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/services/interface_factory_impl.cc
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/services/interface_factory_impl.h
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/services/mojo_media_client.cc
[modify] https://crrev.com/a0689979d60ae9b2501cd0eac1cf361eb37a6df5/media/mojo/services/mojo_media_client.h

Status: Fixed (was: Started)

Sign in to add a comment