Commit 06347df2 authored by Gyuyoung Kim's avatar Gyuyoung Kim Committed by Commit Bot

Reland "Migrate CdmFactory to new Mojo type"

This is a reland of 126c42bd

There was no build error on Android with is_asan=true option.
So, the original CL was not the culprit of the build error.
Merge the original CL again.

Original change's description:
> Migrate CdmFactory to new Mojo type
>
> This CL applies pending_receiver to CreateCdm function
> in CdmFactory interface. Additionally, this CL converts
> mojo::StrongBindingSet to mojo::UniqueReceiverSet.
>
> Bug: 955171
> Change-Id: I4bc983708e394800c3a31d3d6a598a39381fa3b5
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1826755
> Reviewed-by: Dominick Ng <dominickn@chromium.org>
> Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
> Reviewed-by: Chrome Cunningham <chcunningham@chromium.org>
> Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
> Cr-Commit-Position: refs/heads/master@{#700895}

Bug: 955171
Change-Id: I05175a44c4543cf6e2f99a710480a05377a5db50
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1830477Reviewed-by: default avatarChrome Cunningham <chcunningham@chromium.org>
Reviewed-by: default avatarOksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: Gyuyoung Kim <gyuyoung@igalia.com>
Cr-Commit-Position: refs/heads/master@{#704119}
parent 02f429d4
...@@ -146,5 +146,5 @@ interface CdmFactory { ...@@ -146,5 +146,5 @@ interface CdmFactory {
// It should be a reverse domain name, e.g. "com.example.somesystem". However, // It should be a reverse domain name, e.g. "com.example.somesystem". However,
// this call may be initiated by an untrusted process (e.g. renderer), so the // this call may be initiated by an untrusted process (e.g. renderer), so the
// implementation must fully validate |key_system| before creating the CDM. // implementation must fully validate |key_system| before creating the CDM.
CreateCdm(string key_system, ContentDecryptionModule& cdm); CreateCdm(string key_system, pending_receiver<ContentDecryptionModule> cdm);
}; };
...@@ -11,6 +11,8 @@ ...@@ -11,6 +11,8 @@
#include "media/media_buildflags.h" #include "media/media_buildflags.h"
#include "media/mojo/services/mojo_cdm_service.h" #include "media/mojo/services/mojo_cdm_service.h"
#include "media/mojo/services/mojo_cdm_service_context.h" #include "media/mojo/services/mojo_cdm_service_context.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/unique_receiver_set.h"
#include "services/service_manager/public/cpp/connector.h" #include "services/service_manager/public/cpp/connector.h"
#if defined(OS_MACOSX) #if defined(OS_MACOSX)
...@@ -41,7 +43,7 @@ constexpr base::TimeDelta kKeepaliveIdleTimeout = ...@@ -41,7 +43,7 @@ constexpr base::TimeDelta kKeepaliveIdleTimeout =
// during browser shutdown, when the Cdservice could be destroyed directly, // during browser shutdown, when the Cdservice could be destroyed directly,
// ignoring any outstanding ServiceKeepaliveRefs. // ignoring any outstanding ServiceKeepaliveRefs.
// - mojo::CdmFactory connection error happens, AND CdmFactoryImpl doesn't own // - mojo::CdmFactory connection error happens, AND CdmFactoryImpl doesn't own
// any CDMs (|cdm_bindings_| is empty). This is to prevent destroying the // any CDMs (|cdm_receivers_| is empty). This is to prevent destroying the
// CDMs too early (e.g. during page navigation) which could cause errors // CDMs too early (e.g. during page navigation) which could cause errors
// (session closed) on the client side. See https://crbug.com/821171 for // (session closed) on the client side. See https://crbug.com/821171 for
// details. // details.
...@@ -55,35 +57,36 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> { ...@@ -55,35 +57,36 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> {
keepalive_ref_(std::move(keepalive_ref)) { keepalive_ref_(std::move(keepalive_ref)) {
DVLOG(1) << __func__; DVLOG(1) << __func__;
// base::Unretained is safe because |cdm_bindings_| is owned by |this|. If // base::Unretained is safe because |cdm_receivers_| is owned by |this|. If
// |this| is destructed, |cdm_bindings_| will be destructed as well and the // |this| is destructed, |cdm_receivers_| will be destructed as well and the
// error handler should never be called. // error handler should never be called.
cdm_bindings_.set_connection_error_handler(base::BindRepeating( cdm_receivers_.set_disconnect_handler(base::BindRepeating(
&CdmFactoryImpl::OnBindingConnectionError, base::Unretained(this))); &CdmFactoryImpl::OnBindingConnectionError, base::Unretained(this)));
} }
~CdmFactoryImpl() final { DVLOG(1) << __func__; } ~CdmFactoryImpl() final { DVLOG(1) << __func__; }
// mojom::CdmFactory implementation. // mojom::CdmFactory implementation.
void CreateCdm(const std::string& key_system, void CreateCdm(
mojom::ContentDecryptionModuleRequest request) final { const std::string& key_system,
mojo::PendingReceiver<mojom::ContentDecryptionModule> receiver) final {
DVLOG(2) << __func__; DVLOG(2) << __func__;
auto* cdm_factory = GetCdmFactory(); auto* cdm_factory = GetCdmFactory();
if (!cdm_factory) if (!cdm_factory)
return; return;
cdm_bindings_.AddBinding( cdm_receivers_.Add(
std::make_unique<MojoCdmService>(cdm_factory, &cdm_service_context_), std::make_unique<MojoCdmService>(cdm_factory, &cdm_service_context_),
std::move(request)); std::move(receiver));
} }
// DeferredDestroy<mojom::CdmFactory> implemenation. // DeferredDestroy<mojom::CdmFactory> implemenation.
void OnDestroyPending(base::OnceClosure destroy_cb) final { void OnDestroyPending(base::OnceClosure destroy_cb) final {
destroy_cb_ = std::move(destroy_cb); destroy_cb_ = std::move(destroy_cb);
if (cdm_bindings_.empty()) if (cdm_receivers_.empty())
std::move(destroy_cb_).Run(); std::move(destroy_cb_).Run();
// else the callback will be called when |cdm_bindings_| become empty. // else the callback will be called when |cdm_receivers_| become empty.
} }
private: private:
...@@ -96,7 +99,7 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> { ...@@ -96,7 +99,7 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> {
} }
void OnBindingConnectionError() { void OnBindingConnectionError() {
if (destroy_cb_ && cdm_bindings_.empty()) if (destroy_cb_ && cdm_receivers_.empty())
std::move(destroy_cb_).Run(); std::move(destroy_cb_).Run();
} }
...@@ -107,7 +110,7 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> { ...@@ -107,7 +110,7 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> {
CdmService::Client* client_; CdmService::Client* client_;
service_manager::mojom::InterfaceProviderPtr interfaces_; service_manager::mojom::InterfaceProviderPtr interfaces_;
mojo::StrongBindingSet<mojom::ContentDecryptionModule> cdm_bindings_; mojo::UniqueReceiverSet<mojom::ContentDecryptionModule> cdm_receivers_;
std::unique_ptr<ServiceKeepaliveRef> keepalive_ref_; std::unique_ptr<ServiceKeepaliveRef> keepalive_ref_;
std::unique_ptr<media::CdmFactory> cdm_factory_; std::unique_ptr<media::CdmFactory> cdm_factory_;
base::OnceClosure destroy_cb_; base::OnceClosure destroy_cb_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment