Commit a91fe279 authored by Julie Jeongeun Kim's avatar Julie Jeongeun Kim Committed by Commit Bot

Convert DeferredDestroyStrongBindingSet to new Mojo types

This CL converts DeferredDestroyStrongBindingSet to new
Mojo types using UniqueReceiverSet.
It renames DeferredDestroyStrongBindingSet to
DeferredDestroyUniqueReceiverSet which holds UniqueReceiverSet
instead of StrongBindingSet.

Bug: 955171
Change-Id: Iaf5655eb7050d5c7db49745fec60a6e9a504a300
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1930286Reviewed-by: default avatarKen Rockot <rockot@google.com>
Reviewed-by: default avatarXiaohan Wang <xhwang@chromium.org>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#718336}
parent 360fdc91
......@@ -11,7 +11,7 @@ import("//testing/test.gni")
jumbo_component("services") {
output_name = "media_mojo_services"
sources = [
"deferred_destroy_strong_binding_set.h",
"deferred_destroy_unique_receiver_set.h",
"gpu_mojo_media_client.cc",
"gpu_mojo_media_client.h",
"interface_factory_impl.cc",
......@@ -188,7 +188,7 @@ source_set("unit_tests") {
testonly = true
sources = [
"deferred_destroy_strong_binding_set_unittest.cc",
"deferred_destroy_unique_receiver_set_unittest.cc",
"media_metrics_provider_unittest.cc",
"mojo_audio_input_stream_unittest.cc",
"mojo_audio_output_stream_provider_unittest.cc",
......
......@@ -34,7 +34,7 @@ constexpr base::TimeDelta kKeepaliveIdleTimeout =
// CdmService::Client.
//
// Lifetime Note:
// 1. CdmFactoryImpl instances are owned by a DeferredDestroyStrongBindingSet
// 1. CdmFactoryImpl instances are owned by a DeferredDestroyUniqueReceiverSet
// directly, which is owned by CdmService.
// 2. Note that CdmFactoryImpl also holds a ServiceKeepaliveRef tied to the
// CdmService.
......@@ -42,7 +42,7 @@ constexpr base::TimeDelta kKeepaliveIdleTimeout =
// - CdmService is destroyed. Because of (2) this should not happen except for
// during browser shutdown, when the Cdservice could be destroyed directly,
// ignoring any outstanding ServiceKeepaliveRefs.
// - mojo::CdmFactory connection error happens, AND CdmFactoryImpl doesn't own
// - mojo::CdmFactory disconnection happens, AND CdmFactoryImpl doesn't own
// any CDMs (|cdm_receivers_| 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
......@@ -62,7 +62,7 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> {
// |this| is destructed, |cdm_receivers_| will be destructed as well and the
// error handler should never be called.
cdm_receivers_.set_disconnect_handler(base::BindRepeating(
&CdmFactoryImpl::OnBindingConnectionError, base::Unretained(this)));
&CdmFactoryImpl::OnReceiverDisconnect, base::Unretained(this)));
}
~CdmFactoryImpl() final { DVLOG(1) << __func__; }
......@@ -99,7 +99,7 @@ class CdmFactoryImpl : public DeferredDestroy<mojom::CdmFactory> {
return cdm_factory_.get();
}
void OnBindingConnectionError() {
void OnReceiverDisconnect() {
if (destroy_cb_ && cdm_receivers_.empty())
std::move(destroy_cb_).Run();
}
......@@ -159,7 +159,7 @@ void CdmService::OnBindInterface(
}
void CdmService::OnDisconnected() {
cdm_factory_bindings_.CloseAllBindings();
cdm_factory_receivers_.CloseAllReceivers();
client_.reset();
Terminate();
}
......@@ -237,7 +237,7 @@ void CdmService::CreateCdmFactory(
if (!client_)
return;
cdm_factory_bindings_.AddBinding(
cdm_factory_receivers_.AddReceiver(
std::make_unique<CdmFactoryImpl>(
client_.get(), std::move(host_interfaces), keepalive_->CreateRef()),
std::move(receiver));
......
......@@ -12,7 +12,7 @@
#include "media/media_buildflags.h"
#include "media/mojo/mojom/cdm_service.mojom.h"
#include "media/mojo/mojom/content_decryption_module.mojom.h"
#include "media/mojo/services/deferred_destroy_strong_binding_set.h"
#include "media/mojo/services/deferred_destroy_unique_receiver_set.h"
#include "media/mojo/services/media_mojo_export.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
......@@ -64,11 +64,11 @@ class MEDIA_MOJO_EXPORT CdmService : public service_manager::Service,
void SetServiceReleaseDelayForTesting(base::TimeDelta delay);
size_t BoundCdmFactorySizeForTesting() const {
return cdm_factory_bindings_.size();
return cdm_factory_receivers_.size();
}
size_t UnboundCdmFactorySizeForTesting() const {
return cdm_factory_bindings_.unbound_size();
return cdm_factory_receivers_.unbound_size();
}
private:
......@@ -98,7 +98,7 @@ class MEDIA_MOJO_EXPORT CdmService : public service_manager::Service,
std::unique_ptr<service_manager::ServiceKeepalive> keepalive_;
std::unique_ptr<Client> client_;
std::unique_ptr<CdmFactory> cdm_factory_;
DeferredDestroyStrongBindingSet<mojom::CdmFactory> cdm_factory_bindings_;
DeferredDestroyUniqueReceiverSet<mojom::CdmFactory> cdm_factory_receivers_;
service_manager::BinderRegistry registry_;
mojo::ReceiverSet<mojom::CdmService> receivers_;
......
......@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef MEDIA_MOJO_SERVICES_DEFERRED_DESTROY_STRONG_BINDING_SET_H_
#define MEDIA_MOJO_SERVICES_DEFERRED_DESTROY_STRONG_BINDING_SET_H_
#ifndef MEDIA_MOJO_SERVICES_DEFERRED_DESTROY_UNIQUE_RECEIVER_SET_H_
#define MEDIA_MOJO_SERVICES_DEFERRED_DESTROY_UNIQUE_RECEIVER_SET_H_
#include <stdint.h>
......@@ -15,12 +15,12 @@
#include "base/memory/ptr_util.h"
#include "base/memory/weak_ptr.h"
#include "media/base/bind_to_current_loop.h"
#include "mojo/public/cpp/bindings/strong_binding_set.h"
#include "mojo/public/cpp/bindings/unique_receiver_set.h"
namespace media {
// A class that can be deferred destroyed by its owner. For example, when used
// in DeferredDestroyStrongBindingSet.
// in DeferredDestroyUniqueReceiverSet.
template <typename Interface>
class DeferredDestroy : public Interface {
public:
......@@ -30,16 +30,16 @@ class DeferredDestroy : public Interface {
virtual void OnDestroyPending(base::OnceClosure destroy_cb) = 0;
};
// Similar to mojo::StrongBindingSet, but provide a way to defer the destruction
// of the interface implementation:
// - When connection error happended on a binding, the binding is immediately
// Similar to mojo::UniqueReceiverSet, but provide a way to defer the
// destruction of the interface implementation:
// - When disconnection happened on a receiver, the receiver is immediately
// destroyed and removed from the set. The interface implementation will be
// destroyed when the DestroyCallback is called.
// - When the DeferredDestroyStrongBindingSet is destructed, all outstanding
// bindings and interface implementations in the set are destroyed immediately
// - When the DeferredDestroyUniqueReceiverSet is destructed, all outstanding
// receivers and interface implementations in the set are destroyed immediately
// without any deferral.
template <typename Interface>
class DeferredDestroyStrongBindingSet {
class DeferredDestroyUniqueReceiverSet {
public:
// Converts a delete callback to a deleter. If the callback is null or has
// been cancelled, callback bound with invalidated weak pointer, the pointer
......@@ -57,7 +57,7 @@ class DeferredDestroyStrongBindingSet {
// Immediately wrap |p| into a unique_ptr to avoid any potential leak.
auto ptr = base::WrapUnique<Interface>(p);
// Can be cancelled during DeferredDestroyStrongBindingSet destruction.
// Can be cancelled during DeferredDestroyUniqueReceiverSet destruction.
if (delete_cb_ && !delete_cb_.IsCancelled())
delete_cb_.Run(std::move(ptr));
else
......@@ -68,41 +68,41 @@ class DeferredDestroyStrongBindingSet {
DeleteCallback delete_cb_;
};
DeferredDestroyStrongBindingSet() {}
DeferredDestroyUniqueReceiverSet() {}
void AddBinding(std::unique_ptr<DeferredDestroy<Interface>> impl,
mojo::InterfaceRequest<Interface> request) {
void AddReceiver(std::unique_ptr<DeferredDestroy<Interface>> impl,
mojo::PendingReceiver<Interface> receiver) {
// Wrap the pointer into a unique_ptr with a deleter.
Deleter deleter(
base::BindRepeating(&DeferredDestroyStrongBindingSet::OnBindingRemoved,
weak_factory_.GetWeakPtr()));
Deleter deleter(base::BindRepeating(
&DeferredDestroyUniqueReceiverSet::OnReceiverRemoved,
weak_factory_.GetWeakPtr()));
std::unique_ptr<Interface, Deleter> impl_with_deleter(impl.release(),
deleter);
bindings_.AddBinding(std::move(impl_with_deleter), std::move(request));
receivers_.Add(std::move(impl_with_deleter), std::move(receiver));
}
// TODO(xhwang): Add RemoveBinding() if needed.
// TODO(xhwang): Add RemoveReceiver() if needed.
void CloseAllBindings() {
void CloseAllReceivers() {
weak_factory_.InvalidateWeakPtrs();
bindings_.CloseAllBindings();
receivers_.Clear();
unbound_impls_.clear();
}
bool empty() const { return bindings_.empty(); }
bool empty() const { return receivers_.empty(); }
size_t size() const { return bindings_.size(); }
size_t size() const { return receivers_.size(); }
size_t unbound_size() const { return unbound_impls_.size(); }
private:
void OnBindingRemoved(std::unique_ptr<Interface> ptr) {
void OnReceiverRemoved(std::unique_ptr<Interface> ptr) {
DVLOG(1) << __func__;
id_++;
// The cast is safe since AddBinding() takes DeferredDestroy<Interface>.
// The cast is safe since AddReceiver() takes DeferredDestroy<Interface>.
auto* impl_ptr = static_cast<DeferredDestroy<Interface>*>(ptr.get());
// Put the |ptr| in the map before calling OnDestroyPending() because the
......@@ -113,7 +113,7 @@ class DeferredDestroyStrongBindingSet {
// needed because the callback may be called directly in the same stack
// where the implemenation is being destroyed.
impl_ptr->OnDestroyPending(BindToCurrentLoop(
base::BindOnce(&DeferredDestroyStrongBindingSet::OnDestroyable,
base::BindOnce(&DeferredDestroyUniqueReceiverSet::OnDestroyable,
weak_factory_.GetWeakPtr(), id_)));
}
......@@ -124,15 +124,15 @@ class DeferredDestroyStrongBindingSet {
uint32_t id_ = 0;
std::map<uint32_t, std::unique_ptr<Interface>> unbound_impls_;
mojo::StrongBindingSet<Interface, void, Deleter> bindings_;
mojo::UniqueReceiverSet<Interface, void, Deleter> receivers_;
// Note: This should remain the last member so it'll be destroyed and
// invalidate its weak pointers before any other members are destroyed.
base::WeakPtrFactory<DeferredDestroyStrongBindingSet> weak_factory_{this};
base::WeakPtrFactory<DeferredDestroyUniqueReceiverSet> weak_factory_{this};
DISALLOW_COPY_AND_ASSIGN(DeferredDestroyStrongBindingSet);
DISALLOW_COPY_AND_ASSIGN(DeferredDestroyUniqueReceiverSet);
};
} // namespace media
#endif // MEDIA_MOJO_SERVICES_DEFERRED_DESTROY_STRONG_BINDING_SET_H_
#endif // MEDIA_MOJO_SERVICES_DEFERRED_DESTROY_UNIQUE_RECEIVER_SET_H_
......@@ -7,7 +7,7 @@
#include "base/run_loop.h"
#include "base/test/task_environment.h"
#include "media/mojo/services/deferred_destroy_strong_binding_set.h"
#include "media/mojo/services/deferred_destroy_unique_receiver_set.h"
#include "mojo/public/interfaces/bindings/tests/ping_service.mojom.h"
#include "testing/gtest/include/gtest/gtest.h"
......@@ -43,48 +43,49 @@ class DeferredDestroyPingImpl : public DeferredDestroy<PingService> {
};
int DeferredDestroyPingImpl::instance_count = 0;
DeferredDestroyPingImpl* AddDeferredDestroyBinding(
DeferredDestroyStrongBindingSet<PingService>* bindings,
DeferredDestroyPingImpl* AddDeferredDestroyReceiver(
DeferredDestroyUniqueReceiverSet<PingService>* receivers,
mojo::PendingRemote<PingService>* ptr) {
auto impl = std::make_unique<DeferredDestroyPingImpl>();
DeferredDestroyPingImpl* impl_ptr = impl.get();
bindings->AddBinding(std::move(impl), ptr->InitWithNewPipeAndPassReceiver());
receivers->AddReceiver(std::move(impl),
ptr->InitWithNewPipeAndPassReceiver());
return impl_ptr;
}
class DeferredDestroyStrongBindingSetTest : public testing::Test {
class DeferredDestroyUniqueReceiverSetTest : public testing::Test {
public:
DeferredDestroyStrongBindingSetTest() = default;
~DeferredDestroyStrongBindingSetTest() override = default;
DeferredDestroyUniqueReceiverSetTest() = default;
~DeferredDestroyUniqueReceiverSetTest() override = default;
protected:
base::test::TaskEnvironment task_environment_;
};
TEST_F(DeferredDestroyStrongBindingSetTest, Destructor) {
TEST_F(DeferredDestroyUniqueReceiverSetTest, Destructor) {
mojo::PendingRemote<PingService> ping[2];
auto bindings =
std::make_unique<DeferredDestroyStrongBindingSet<PingService>>();
auto receivers =
std::make_unique<DeferredDestroyUniqueReceiverSet<PingService>>();
for (int i = 0; i < 2; ++i)
AddDeferredDestroyBinding(bindings.get(), ping + i);
AddDeferredDestroyReceiver(receivers.get(), ping + i);
EXPECT_EQ(2, DeferredDestroyPingImpl::instance_count);
bindings.reset();
receivers.reset();
EXPECT_EQ(0, DeferredDestroyPingImpl::instance_count);
}
TEST_F(DeferredDestroyStrongBindingSetTest, ConnectionError) {
TEST_F(DeferredDestroyUniqueReceiverSetTest, ConnectionError) {
mojo::PendingRemote<PingService> ping[4];
DeferredDestroyPingImpl* impl[4];
auto bindings =
std::make_unique<DeferredDestroyStrongBindingSet<PingService>>();
auto receivers =
std::make_unique<DeferredDestroyUniqueReceiverSet<PingService>>();
for (int i = 0; i < 4; ++i)
impl[i] = AddDeferredDestroyBinding(bindings.get(), ping + i);
impl[i] = AddDeferredDestroyReceiver(receivers.get(), ping + i);
EXPECT_EQ(4, DeferredDestroyPingImpl::instance_count);
// Destroy deferred after connection error until set_can_destroy()..
// Destroy deferred after disconnection until set_can_destroy()..
ping[0].reset();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(4, DeferredDestroyPingImpl::instance_count);
......@@ -92,40 +93,41 @@ TEST_F(DeferredDestroyStrongBindingSetTest, ConnectionError) {
base::RunLoop().RunUntilIdle();
EXPECT_EQ(3, DeferredDestroyPingImpl::instance_count);
// Destroyed immediately after connection error if set_can_destroy() in
// Destroyed immediately after disconnection if set_can_destroy() in
// advance.
impl[1]->set_can_destroy();
ping[1].reset();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(2, DeferredDestroyPingImpl::instance_count);
// Deferred after connection until binding set destruction.
// Deferred after connection until receiver set destruction.
ping[2].reset();
base::RunLoop().RunUntilIdle();
EXPECT_EQ(2, DeferredDestroyPingImpl::instance_count);
// Destructing the binding set will destruct all impls, including deferred
// Destructing the receiver set will destruct all impls, including deferred
// destroy impls.
bindings.reset();
receivers.reset();
EXPECT_EQ(0, DeferredDestroyPingImpl::instance_count);
}
TEST_F(DeferredDestroyStrongBindingSetTest, CloseAllBindings) {
TEST_F(DeferredDestroyUniqueReceiverSetTest, CloseAllReceivers) {
mojo::PendingRemote<PingService> ping[3];
DeferredDestroyPingImpl* impl[3];
DeferredDestroyStrongBindingSet<PingService> bindings;
DeferredDestroyUniqueReceiverSet<PingService> receivers;
for (int i = 0; i < 2; ++i)
impl[i] = AddDeferredDestroyBinding(&bindings, ping + i);
impl[i] = AddDeferredDestroyReceiver(&receivers, ping + i);
EXPECT_EQ(2, DeferredDestroyPingImpl::instance_count);
EXPECT_FALSE(bindings.empty());
EXPECT_FALSE(receivers.empty());
bindings.CloseAllBindings();
receivers.CloseAllReceivers();
EXPECT_EQ(0, DeferredDestroyPingImpl::instance_count);
EXPECT_TRUE(bindings.empty());
EXPECT_TRUE(receivers.empty());
// After CloseAllBindings, new added bindings can still be deferred destroyed.
impl[2] = AddDeferredDestroyBinding(&bindings, ping + 2);
// After CloseAllReceivers, new added receivers can still be deferred
// destroyed.
impl[2] = AddDeferredDestroyReceiver(&receivers, ping + 2);
EXPECT_EQ(1, DeferredDestroyPingImpl::instance_count);
ping[2].reset();
......
......@@ -14,7 +14,6 @@
#include "media/mojo/mojom/renderer_extensions.mojom.h"
#include "media/mojo/services/mojo_decryptor_service.h"
#include "media/mojo/services/mojo_media_client.h"
#include "mojo/public/cpp/bindings/strong_binding.h"
#include "services/service_manager/public/mojom/interface_provider.mojom.h"
#if BUILDFLAG(ENABLE_MOJO_AUDIO_DECODER)
......@@ -53,7 +52,7 @@ InterfaceFactoryImpl::InterfaceFactoryImpl(
DVLOG(1) << __func__;
DCHECK(mojo_media_client_);
SetBindingConnectionErrorHandler();
SetReceiverDisconnectHandler();
}
InterfaceFactoryImpl::~InterfaceFactoryImpl() {
......@@ -258,37 +257,37 @@ bool InterfaceFactoryImpl::IsEmpty() {
return true;
}
void InterfaceFactoryImpl::SetBindingConnectionErrorHandler() {
void InterfaceFactoryImpl::SetReceiverDisconnectHandler() {
// base::Unretained is safe because all receivers are owned by |this|. If
// |this| is destructed, the receivers will be destructed as well and the
// connection error handler should never be called.
auto connection_error_cb = base::BindRepeating(
&InterfaceFactoryImpl::OnBindingConnectionError, base::Unretained(this));
// disconnect handler should never be called.
auto disconnect_cb = base::BindRepeating(
&InterfaceFactoryImpl::OnReceiverDisconnect, base::Unretained(this));
#if BUILDFLAG(ENABLE_MOJO_AUDIO_DECODER)
audio_decoder_receivers_.set_disconnect_handler(connection_error_cb);
audio_decoder_receivers_.set_disconnect_handler(disconnect_cb);
#endif // BUILDFLAG(ENABLE_MOJO_AUDIO_DECODER)
#if BUILDFLAG(ENABLE_MOJO_VIDEO_DECODER)
video_decoder_receivers_.set_disconnect_handler(connection_error_cb);
video_decoder_receivers_.set_disconnect_handler(disconnect_cb);
#endif // BUILDFLAG(ENABLE_MOJO_VIDEO_DECODER)
#if BUILDFLAG(ENABLE_MOJO_RENDERER)
renderer_receivers_.set_disconnect_handler(connection_error_cb);
renderer_receivers_.set_disconnect_handler(disconnect_cb);
#endif // BUILDFLAG(ENABLE_MOJO_RENDERER)
#if BUILDFLAG(ENABLE_MOJO_CDM)
cdm_receivers_.set_disconnect_handler(connection_error_cb);
cdm_receivers_.set_disconnect_handler(disconnect_cb);
#endif // BUILDFLAG(ENABLE_MOJO_CDM)
#if BUILDFLAG(ENABLE_CDM_PROXY)
cdm_proxy_receivers_.set_disconnect_handler(connection_error_cb);
cdm_proxy_receivers_.set_disconnect_handler(disconnect_cb);
#endif // BUILDFLAG(ENABLE_CDM_PROXY)
decryptor_receivers_.set_disconnect_handler(connection_error_cb);
decryptor_receivers_.set_disconnect_handler(disconnect_cb);
}
void InterfaceFactoryImpl::OnBindingConnectionError() {
void InterfaceFactoryImpl::OnReceiverDisconnect() {
DVLOG(2) << __func__;
if (destroy_cb_ && IsEmpty())
std::move(destroy_cb_).Run();
......
......@@ -19,7 +19,7 @@
#include "media/mojo/mojom/interface_factory.mojom.h"
#include "media/mojo/mojom/renderer.mojom.h"
#include "media/mojo/mojom/video_decoder.mojom.h"
#include "media/mojo/services/deferred_destroy_strong_binding_set.h"
#include "media/mojo/services/deferred_destroy_unique_receiver_set.h"
#include "media/mojo/services/mojo_cdm_service_context.h"
#include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
......@@ -88,17 +88,17 @@ class InterfaceFactoryImpl : public DeferredDestroy<mojom::InterfaceFactory> {
private:
// Returns true when there is no media component (audio/video decoder,
// renderer, cdm and cdm proxy) bindings exist.
// renderer, cdm and cdm proxy) receivers exist.
bool IsEmpty();
void SetBindingConnectionErrorHandler();
void OnBindingConnectionError();
void SetReceiverDisconnectHandler();
void OnReceiverDisconnect();
#if BUILDFLAG(ENABLE_MOJO_CDM)
CdmFactory* GetCdmFactory();
#endif // BUILDFLAG(ENABLE_MOJO_CDM)
// Must be declared before the bindings below because the bound objects might
// Must be declared before the receivers below because the bound objects might
// take a raw pointer of |cdm_service_context_| and assume it's always
// available.
MojoCdmServiceContext cdm_service_context_;
......
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