Commit ef07a0a9 authored by Findit's avatar Findit

Revert "Migrate CdmFactory to new Mojo type"

This reverts commit 126c42bd.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 700895 as the
culprit for failures in the build cycles as shown on:
https://analysis.chromium.org/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtLzEyNmM0MmJkMGU0ODE1Zjg5ODVkOTEyNWZhNGVlNzA3YjJjYjM0ZDgM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.memory/android-asan/2330

Sample Failed Step: compile

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}


Change-Id: Ic41a2dcdf08887757f21f2af9d762b0f2f970618
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 955171
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1829253
Cr-Commit-Position: refs/heads/master@{#700939}
parent adbd27b2
......@@ -146,5 +146,5 @@ interface CdmFactory {
// 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
// implementation must fully validate |key_system| before creating the CDM.
CreateCdm(string key_system, pending_receiver<ContentDecryptionModule> cdm);
CreateCdm(string key_system, ContentDecryptionModule& cdm);
};
......@@ -11,8 +11,6 @@
#include "media/media_buildflags.h"
#include "media/mojo/services/mojo_cdm_service.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"
#if defined(OS_MACOSX)
......@@ -43,7 +41,7 @@ constexpr base::TimeDelta kKeepaliveIdleTimeout =
// during browser shutdown, when the Cdservice could be destroyed directly,
// ignoring any outstanding ServiceKeepaliveRefs.
// - mojo::CdmFactory connection error happens, AND CdmFactoryImpl doesn't own
// any CDMs (|cdm_receivers_| is empty). This is to prevent destroying the
// any CDMs (|cdm_bindings_| is empty). This is to prevent destroying the
// CDMs too early (e.g. during page navigation) which could cause errors
// (session closed) on the client side. See https://crbug.com/821171 for
// details.
......@@ -57,36 +55,35 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> {
keepalive_ref_(std::move(keepalive_ref)) {
DVLOG(1) << __func__;
// base::Unretained is safe because |cdm_receivers_| is owned by |this|. If
// |this| is destructed, |cdm_receivers_| will be destructed as well and the
// base::Unretained is safe because |cdm_bindings_| is owned by |this|. If
// |this| is destructed, |cdm_bindings_| will be destructed as well and the
// error handler should never be called.
cdm_receivers_.set_disconnect_handler(base::BindRepeating(
cdm_bindings_.set_connection_error_handler(base::BindRepeating(
&CdmFactoryImpl::OnBindingConnectionError, base::Unretained(this)));
}
~CdmFactoryImpl() final { DVLOG(1) << __func__; }
// mojom::CdmFactory implementation.
void CreateCdm(
const std::string& key_system,
mojo::PendingReceiver<mojom::ContentDecryptionModule> receiver) final {
void CreateCdm(const std::string& key_system,
mojom::ContentDecryptionModuleRequest request) final {
DVLOG(2) << __func__;
auto* cdm_factory = GetCdmFactory();
if (!cdm_factory)
return;
cdm_receivers_.Add(
cdm_bindings_.AddBinding(
std::make_unique<MojoCdmService>(cdm_factory, &cdm_service_context_),
std::move(receiver));
std::move(request));
}
// DeferredDestroy<mojom::CdmFactory> implemenation.
void OnDestroyPending(base::OnceClosure destroy_cb) final {
destroy_cb_ = std::move(destroy_cb);
if (cdm_receivers_.empty())
if (cdm_bindings_.empty())
std::move(destroy_cb_).Run();
// else the callback will be called when |cdm_receivers_| become empty.
// else the callback will be called when |cdm_bindings_| become empty.
}
private:
......@@ -99,7 +96,7 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> {
}
void OnBindingConnectionError() {
if (destroy_cb_ && cdm_receivers_.empty())
if (destroy_cb_ && cdm_bindings_.empty())
std::move(destroy_cb_).Run();
}
......@@ -110,7 +107,7 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> {
CdmService::Client* client_;
service_manager::mojom::InterfaceProviderPtr interfaces_;
mojo::UniqueReceiverSet<mojom::ContentDecryptionModule> cdm_receivers_;
mojo::StrongBindingSet<mojom::ContentDecryptionModule> cdm_bindings_;
std::unique_ptr<ServiceKeepaliveRef> keepalive_ref_;
std::unique_ptr<media::CdmFactory> cdm_factory_;
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