From c24178863301cd9b2295a5a0e8dd94715dbe7f7a Mon Sep 17 00:00:00 2001 From: Ryan Keane Date: Fri, 8 May 2020 09:41:21 -0700 Subject: Move SerialDeletePtr to platform/api util/ isn't exposed to clients, so this file should be moved to platform/api which is. NOTE: platform/base cannot be used because platform/base cannot depend on platform/api. Bug: b/155511578 Change-Id: Ib27bf33809c431b844e4443601813758c81d75e7 Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2189915 Reviewed-by: mark a. foltz Commit-Queue: Ryan Keane --- cast/sender/public/sender_socket_factory.h | 2 +- cast/standalone_receiver/cast_agent.h | 2 +- cast/test/cast_socket_e2e_test.cc | 2 +- discovery/dnssd/impl/service_dispatcher.cc | 1 + discovery/dnssd/public/dns_sd_service.h | 1 - discovery/public/dns_sd_service_factory.h | 2 +- platform/BUILD.gn | 14 +-- platform/api/serial_delete_ptr.h | 113 +++++++++++++++++++ platform/api/serial_delete_ptr_unittest.cc | 172 +++++++++++++++++++++++++++++ util/BUILD.gn | 6 +- util/serial_delete_ptr.h | 113 ------------------- util/serial_delete_ptr_unittest.cc | 172 ----------------------------- 12 files changed, 296 insertions(+), 304 deletions(-) create mode 100644 platform/api/serial_delete_ptr.h create mode 100644 platform/api/serial_delete_ptr_unittest.cc delete mode 100644 util/serial_delete_ptr.h delete mode 100644 util/serial_delete_ptr_unittest.cc diff --git a/cast/sender/public/sender_socket_factory.h b/cast/sender/public/sender_socket_factory.h index 67d8c901..f0247a28 100644 --- a/cast/sender/public/sender_socket_factory.h +++ b/cast/sender/public/sender_socket_factory.h @@ -12,10 +12,10 @@ #include #include "cast/common/public/cast_socket.h" +#include "platform/api/serial_delete_ptr.h" #include "platform/api/task_runner.h" #include "platform/api/tls_connection_factory.h" #include "platform/base/ip_address.h" -#include "util/serial_delete_ptr.h" namespace openscreen { namespace cast { diff --git a/cast/standalone_receiver/cast_agent.h b/cast/standalone_receiver/cast_agent.h index 74b2f892..d9932b9d 100644 --- a/cast/standalone_receiver/cast_agent.h +++ b/cast/standalone_receiver/cast_agent.h @@ -16,10 +16,10 @@ #include "cast/streaming/environment.h" #include "cast/streaming/receiver_session.h" #include "platform/api/scoped_wake_lock.h" +#include "platform/api/serial_delete_ptr.h" #include "platform/base/error.h" #include "platform/base/interface_info.h" #include "platform/impl/task_runner.h" -#include "util/serial_delete_ptr.h" namespace openscreen { namespace cast { diff --git a/cast/test/cast_socket_e2e_test.cc b/cast/test/cast_socket_e2e_test.cc index 522a0209..edffe0bd 100644 --- a/cast/test/cast_socket_e2e_test.cc +++ b/cast/test/cast_socket_e2e_test.cc @@ -20,6 +20,7 @@ #include "cast/receiver/public/receiver_socket_factory.h" #include "cast/sender/public/sender_socket_factory.h" #include "gtest/gtest.h" +#include "platform/api/serial_delete_ptr.h" #include "platform/api/tls_connection_factory.h" #include "platform/base/tls_connect_options.h" #include "platform/base/tls_credentials.h" @@ -29,7 +30,6 @@ #include "platform/impl/platform_client_posix.h" #include "util/crypto/certificate_utils.h" #include "util/osp_logging.h" -#include "util/serial_delete_ptr.h" namespace openscreen { namespace cast { diff --git a/discovery/dnssd/impl/service_dispatcher.cc b/discovery/dnssd/impl/service_dispatcher.cc index 794511a8..ce350ca6 100644 --- a/discovery/dnssd/impl/service_dispatcher.cc +++ b/discovery/dnssd/impl/service_dispatcher.cc @@ -10,6 +10,7 @@ #include "discovery/dnssd/impl/service_instance.h" #include "discovery/dnssd/public/dns_sd_instance.h" #include "discovery/mdns/public/mdns_service.h" +#include "platform/api/serial_delete_ptr.h" #include "platform/api/task_runner.h" #include "util/trace_logging.h" diff --git a/discovery/dnssd/public/dns_sd_service.h b/discovery/dnssd/public/dns_sd_service.h index f0fe130b..c5e84384 100644 --- a/discovery/dnssd/public/dns_sd_service.h +++ b/discovery/dnssd/public/dns_sd_service.h @@ -11,7 +11,6 @@ #include "platform/base/error.h" #include "platform/base/interface_info.h" #include "platform/base/ip_address.h" -#include "util/serial_delete_ptr.h" namespace openscreen { diff --git a/discovery/public/dns_sd_service_factory.h b/discovery/public/dns_sd_service_factory.h index e2dcb836..3e973541 100644 --- a/discovery/public/dns_sd_service_factory.h +++ b/discovery/public/dns_sd_service_factory.h @@ -6,7 +6,7 @@ #define DISCOVERY_PUBLIC_DNS_SD_SERVICE_FACTORY_H_ #include "discovery/dnssd/public/dns_sd_service.h" -#include "util/serial_delete_ptr.h" +#include "platform/api/serial_delete_ptr.h" namespace openscreen { diff --git a/platform/BUILD.gn b/platform/BUILD.gn index 296f71f8..375ba41a 100644 --- a/platform/BUILD.gn +++ b/platform/BUILD.gn @@ -44,6 +44,7 @@ source_set("api") { "api/network_interface.h", "api/scoped_wake_lock.cc", "api/scoped_wake_lock.h", + "api/serial_delete_ptr.h", "api/task_runner.h", "api/time.h", "api/tls_connection.cc", @@ -56,9 +57,7 @@ source_set("api") { "api/udp_socket.h", ] - public_deps = [ - ":base", - ] + public_deps = [ ":base" ] } # The following target is only activated in standalone builds (see :platform). @@ -150,13 +149,9 @@ if (!build_with_chromium) { # The main target, which either assumes an embedder will link-in the platform # API implementation elsewhere, or links-in the :standalone_impl in the build. source_set("platform") { - public_deps = [ - ":api", - ] + public_deps = [ ":api" ] if (!build_with_chromium) { - deps = [ - ":standalone_impl", - ] + deps = [ ":standalone_impl" ] } } @@ -202,6 +197,7 @@ source_set("unittests") { testonly = true sources = [ + "api/serial_delete_ptr_unittest.cc", "api/time_unittest.cc", "base/error_unittest.cc", "base/ip_address_unittest.cc", diff --git a/platform/api/serial_delete_ptr.h b/platform/api/serial_delete_ptr.h new file mode 100644 index 00000000..a3bdf412 --- /dev/null +++ b/platform/api/serial_delete_ptr.h @@ -0,0 +1,113 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef PLATFORM_API_SERIAL_DELETE_PTR_H_ +#define PLATFORM_API_SERIAL_DELETE_PTR_H_ + +#include +#include +#include + +#include "platform/api/task_runner.h" + +namespace openscreen { + +// A functor that deletes an object via a task on a specific TaskRunner. This is +// used for ensuring an object is deleted after any tasks that reference it have +// completed. +template +class SerialDelete { + public: + SerialDelete() : deleter_() {} + + explicit SerialDelete(TaskRunner* task_runner) + : task_runner_(task_runner), deleter_() { + assert(task_runner); + } + + template + SerialDelete(TaskRunner* task_runner, DT&& deleter) + : task_runner_(task_runner), deleter_(std::forward
(deleter)) { + assert(task_runner); + } + + void operator()(Type* pointer) const { + if (task_runner_) { + // Deletion of the object depends on the task being run by the task + // runner. + task_runner_->PostTask( + [pointer, deleter = std::move(deleter_)] { deleter(pointer); }); + } + } + + private: + TaskRunner* task_runner_; + DeleterType deleter_; +}; + +// A wrapper around std::unique_ptr<> that uses SerialDelete<> to schedule the +// object's deletion. +template > +class SerialDeletePtr + : public std::unique_ptr> { + public: + SerialDeletePtr() noexcept + : std::unique_ptr>( + nullptr, + SerialDelete()) {} + + explicit SerialDeletePtr(TaskRunner* task_runner) noexcept + : std::unique_ptr>( + nullptr, + SerialDelete(task_runner)) { + assert(task_runner); + } + + SerialDeletePtr(TaskRunner* task_runner, std::nullptr_t) noexcept + : std::unique_ptr>( + nullptr, + SerialDelete(task_runner)) { + assert(task_runner); + } + + SerialDeletePtr(TaskRunner* task_runner, Type* pointer) noexcept + : std::unique_ptr>( + pointer, + SerialDelete(task_runner)) { + assert(task_runner); + } + + SerialDeletePtr( + TaskRunner* task_runner, + Type* pointer, + typename std::conditional::value, + DeleterType, + const DeleterType&>::type deleter) noexcept + : std::unique_ptr>( + pointer, + SerialDelete(task_runner, deleter)) { + assert(task_runner); + } + + SerialDeletePtr( + TaskRunner* task_runner, + Type* pointer, + typename std::remove_reference::type&& deleter) noexcept + : std::unique_ptr>( + pointer, + SerialDelete(task_runner, std::move(deleter))) { + assert(task_runner); + } +}; + +template +SerialDeletePtr MakeSerialDelete(TaskRunner* task_runner, + Args&&... args) { + return SerialDeletePtr(task_runner, + new Type(std::forward(args)...)); +} + +} // namespace openscreen + +#endif // PLATFORM_API_SERIAL_DELETE_PTR_H_ diff --git a/platform/api/serial_delete_ptr_unittest.cc b/platform/api/serial_delete_ptr_unittest.cc new file mode 100644 index 00000000..5a37ad33 --- /dev/null +++ b/platform/api/serial_delete_ptr_unittest.cc @@ -0,0 +1,172 @@ +// Copyright 2019 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "platform/api/serial_delete_ptr.h" + +#include "gtest/gtest.h" +#include "platform/test/fake_clock.h" +#include "platform/test/fake_task_runner.h" + +namespace openscreen { + +class SerialDeletePtrTest : public ::testing::Test { + public: + SerialDeletePtrTest() : clock_(Clock::now()), task_runner_(&clock_) {} + + protected: + static void FunctionPointerDeleter(int* pointer) { + deleter_called_ = true; + delete pointer; + } + + FakeClock clock_; + FakeTaskRunner task_runner_; + + static bool deleter_called_; +}; + +// static +bool SerialDeletePtrTest::deleter_called_ = false; + +TEST_F(SerialDeletePtrTest, Empty) { + SerialDeletePtr pointer(&task_runner_); +} + +TEST_F(SerialDeletePtrTest, Nullptr) { + SerialDeletePtr pointer(&task_runner_, nullptr); +} + +TEST_F(SerialDeletePtrTest, DefaultDeleter) { + { SerialDeletePtr pointer(&task_runner_, new int); } + task_runner_.RunTasksUntilIdle(); +} + +TEST_F(SerialDeletePtrTest, FuncitonPointerDeleter) { + deleter_called_ = false; + { + SerialDeletePtr pointer( + &task_runner_, new int, FunctionPointerDeleter); + } + task_runner_.RunTasksUntilIdle(); + EXPECT_TRUE(deleter_called_); +} + +TEST_F(SerialDeletePtrTest, LambdaDeleter) { + deleter_called_ = false; + auto deleter = [](int* pointer) { + deleter_called_ = true; + delete pointer; + }; + + { + SerialDeletePtr pointer(&task_runner_, new int, + deleter); + } + + task_runner_.RunTasksUntilIdle(); + EXPECT_TRUE(deleter_called_); +} + +TEST_F(SerialDeletePtrTest, BindDeleter) { + deleter_called_ = false; + auto deleter = std::bind(FunctionPointerDeleter, std::placeholders::_1); + + { + SerialDeletePtr pointer(&task_runner_, new int, + deleter); + } + + task_runner_.RunTasksUntilIdle(); + EXPECT_TRUE(deleter_called_); +} + +TEST_F(SerialDeletePtrTest, FunctionDeleter) { + deleter_called_ = false; + std::function deleter = FunctionPointerDeleter; + + { + SerialDeletePtr> pointer(&task_runner_, + new int, deleter); + } + + task_runner_.RunTasksUntilIdle(); + EXPECT_TRUE(deleter_called_); +} + +TEST_F(SerialDeletePtrTest, FunctionDeleterWithFunctionPointer) { + deleter_called_ = false; + + { + SerialDeletePtr> pointer( + &task_runner_, new int, FunctionPointerDeleter); + } + + task_runner_.RunTasksUntilIdle(); + EXPECT_TRUE(deleter_called_); +} + +TEST_F(SerialDeletePtrTest, FunctionDeleterWithLambda) { + deleter_called_ = false; + auto deleter = [](int* pointer) { + deleter_called_ = true; + delete pointer; + }; + + { + SerialDeletePtr> pointer(&task_runner_, + new int, deleter); + } + + task_runner_.RunTasksUntilIdle(); + EXPECT_TRUE(deleter_called_); +} + +TEST_F(SerialDeletePtrTest, FunctionDeleterWithBind) { + deleter_called_ = false; + auto deleter = std::bind(FunctionPointerDeleter, std::placeholders::_1); + + { + SerialDeletePtr> pointer(&task_runner_, + new int, deleter); + } + + task_runner_.RunTasksUntilIdle(); + EXPECT_TRUE(deleter_called_); +} + +TEST_F(SerialDeletePtrTest, StructDeleter) { + struct TestDeleter { + void operator()(int* pointer) const { delete pointer; } + }; + + TestDeleter deleter; + { + SerialDeletePtr pointer(&task_runner_, new int, deleter); + } + + task_runner_.RunTasksUntilIdle(); +} + +TEST_F(SerialDeletePtrTest, Move) { + deleter_called_ = false; + + { + SerialDeletePtr pointer( + &task_runner_); + { + SerialDeletePtr temp_pointer( + &task_runner_, new int, FunctionPointerDeleter); + pointer = std::move(temp_pointer); + + // No deletion should happen on move + task_runner_.RunTasksUntilIdle(); + EXPECT_FALSE(deleter_called_); + } + } + + task_runner_.RunTasksUntilIdle(); + EXPECT_TRUE(deleter_called_); +} + +} // namespace openscreen diff --git a/util/BUILD.gn b/util/BUILD.gn index 991b5188..b2100346 100644 --- a/util/BUILD.gn +++ b/util/BUILD.gn @@ -45,7 +45,6 @@ source_set("util") { "operation_loop.h", "osp_logging.h", "saturate_cast.h", - "serial_delete_ptr.h", "simple_fraction.cc", "simple_fraction.h", "std_util.h", @@ -62,9 +61,7 @@ source_set("util") { "yet_another_bit_vector.h", ] - public_deps = [ - "../third_party/jsoncpp", - ] + public_deps = [ "../third_party/jsoncpp" ] deps = [ "../third_party/abseil", @@ -93,7 +90,6 @@ source_set("unittests") { "json/json_value_unittest.cc", "operation_loop_unittest.cc", "saturate_cast_unittest.cc", - "serial_delete_ptr_unittest.cc", "simple_fraction_unittest.cc", "stringprintf_unittest.cc", "trace_logging/scoped_trace_operations_unittest.cc", diff --git a/util/serial_delete_ptr.h b/util/serial_delete_ptr.h deleted file mode 100644 index 1aff2495..00000000 --- a/util/serial_delete_ptr.h +++ /dev/null @@ -1,113 +0,0 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef UTIL_SERIAL_DELETE_PTR_H_ -#define UTIL_SERIAL_DELETE_PTR_H_ - -#include -#include -#include - -#include "platform/api/task_runner.h" - -namespace openscreen { - -// A functor that deletes an object via a task on a specific TaskRunner. This is -// used for ensuring an object is deleted after any tasks that reference it have -// completed. -template -class SerialDelete { - public: - SerialDelete() : deleter_() {} - - explicit SerialDelete(TaskRunner* task_runner) - : task_runner_(task_runner), deleter_() { - assert(task_runner); - } - - template - SerialDelete(TaskRunner* task_runner, DT&& deleter) - : task_runner_(task_runner), deleter_(std::forward
(deleter)) { - assert(task_runner); - } - - void operator()(Type* pointer) const { - if (task_runner_) { - // Deletion of the object depends on the task being run by the task - // runner. - task_runner_->PostTask( - [pointer, deleter = std::move(deleter_)] { deleter(pointer); }); - } - } - - private: - TaskRunner* task_runner_; - DeleterType deleter_; -}; - -// A wrapper around std::unique_ptr<> that uses SerialDelete<> to schedule the -// object's deletion. -template > -class SerialDeletePtr - : public std::unique_ptr> { - public: - SerialDeletePtr() noexcept - : std::unique_ptr>( - nullptr, - SerialDelete()) {} - - explicit SerialDeletePtr(TaskRunner* task_runner) noexcept - : std::unique_ptr>( - nullptr, - SerialDelete(task_runner)) { - assert(task_runner); - } - - SerialDeletePtr(TaskRunner* task_runner, std::nullptr_t) noexcept - : std::unique_ptr>( - nullptr, - SerialDelete(task_runner)) { - assert(task_runner); - } - - SerialDeletePtr(TaskRunner* task_runner, Type* pointer) noexcept - : std::unique_ptr>( - pointer, - SerialDelete(task_runner)) { - assert(task_runner); - } - - SerialDeletePtr( - TaskRunner* task_runner, - Type* pointer, - typename std::conditional::value, - DeleterType, - const DeleterType&>::type deleter) noexcept - : std::unique_ptr>( - pointer, - SerialDelete(task_runner, deleter)) { - assert(task_runner); - } - - SerialDeletePtr( - TaskRunner* task_runner, - Type* pointer, - typename std::remove_reference::type&& deleter) noexcept - : std::unique_ptr>( - pointer, - SerialDelete(task_runner, std::move(deleter))) { - assert(task_runner); - } -}; - -template -SerialDeletePtr MakeSerialDelete(TaskRunner* task_runner, - Args&&... args) { - return SerialDeletePtr(task_runner, - new Type(std::forward(args)...)); -} - -} // namespace openscreen - -#endif // UTIL_SERIAL_DELETE_PTR_H_ diff --git a/util/serial_delete_ptr_unittest.cc b/util/serial_delete_ptr_unittest.cc deleted file mode 100644 index c8b899c5..00000000 --- a/util/serial_delete_ptr_unittest.cc +++ /dev/null @@ -1,172 +0,0 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "util/serial_delete_ptr.h" - -#include "gtest/gtest.h" -#include "platform/test/fake_clock.h" -#include "platform/test/fake_task_runner.h" - -namespace openscreen { - -class SerialDeletePtrTest : public ::testing::Test { - public: - SerialDeletePtrTest() : clock_(Clock::now()), task_runner_(&clock_) {} - - protected: - static void FunctionPointerDeleter(int* pointer) { - deleter_called_ = true; - delete pointer; - } - - FakeClock clock_; - FakeTaskRunner task_runner_; - - static bool deleter_called_; -}; - -// static -bool SerialDeletePtrTest::deleter_called_ = false; - -TEST_F(SerialDeletePtrTest, Empty) { - SerialDeletePtr pointer(&task_runner_); -} - -TEST_F(SerialDeletePtrTest, Nullptr) { - SerialDeletePtr pointer(&task_runner_, nullptr); -} - -TEST_F(SerialDeletePtrTest, DefaultDeleter) { - { SerialDeletePtr pointer(&task_runner_, new int); } - task_runner_.RunTasksUntilIdle(); -} - -TEST_F(SerialDeletePtrTest, FuncitonPointerDeleter) { - deleter_called_ = false; - { - SerialDeletePtr pointer( - &task_runner_, new int, FunctionPointerDeleter); - } - task_runner_.RunTasksUntilIdle(); - EXPECT_TRUE(deleter_called_); -} - -TEST_F(SerialDeletePtrTest, LambdaDeleter) { - deleter_called_ = false; - auto deleter = [](int* pointer) { - deleter_called_ = true; - delete pointer; - }; - - { - SerialDeletePtr pointer(&task_runner_, new int, - deleter); - } - - task_runner_.RunTasksUntilIdle(); - EXPECT_TRUE(deleter_called_); -} - -TEST_F(SerialDeletePtrTest, BindDeleter) { - deleter_called_ = false; - auto deleter = std::bind(FunctionPointerDeleter, std::placeholders::_1); - - { - SerialDeletePtr pointer(&task_runner_, new int, - deleter); - } - - task_runner_.RunTasksUntilIdle(); - EXPECT_TRUE(deleter_called_); -} - -TEST_F(SerialDeletePtrTest, FunctionDeleter) { - deleter_called_ = false; - std::function deleter = FunctionPointerDeleter; - - { - SerialDeletePtr> pointer(&task_runner_, - new int, deleter); - } - - task_runner_.RunTasksUntilIdle(); - EXPECT_TRUE(deleter_called_); -} - -TEST_F(SerialDeletePtrTest, FunctionDeleterWithFunctionPointer) { - deleter_called_ = false; - - { - SerialDeletePtr> pointer( - &task_runner_, new int, FunctionPointerDeleter); - } - - task_runner_.RunTasksUntilIdle(); - EXPECT_TRUE(deleter_called_); -} - -TEST_F(SerialDeletePtrTest, FunctionDeleterWithLambda) { - deleter_called_ = false; - auto deleter = [](int* pointer) { - deleter_called_ = true; - delete pointer; - }; - - { - SerialDeletePtr> pointer(&task_runner_, - new int, deleter); - } - - task_runner_.RunTasksUntilIdle(); - EXPECT_TRUE(deleter_called_); -} - -TEST_F(SerialDeletePtrTest, FunctionDeleterWithBind) { - deleter_called_ = false; - auto deleter = std::bind(FunctionPointerDeleter, std::placeholders::_1); - - { - SerialDeletePtr> pointer(&task_runner_, - new int, deleter); - } - - task_runner_.RunTasksUntilIdle(); - EXPECT_TRUE(deleter_called_); -} - -TEST_F(SerialDeletePtrTest, StructDeleter) { - struct TestDeleter { - void operator()(int* pointer) const { delete pointer; } - }; - - TestDeleter deleter; - { - SerialDeletePtr pointer(&task_runner_, new int, deleter); - } - - task_runner_.RunTasksUntilIdle(); -} - -TEST_F(SerialDeletePtrTest, Move) { - deleter_called_ = false; - - { - SerialDeletePtr pointer( - &task_runner_); - { - SerialDeletePtr temp_pointer( - &task_runner_, new int, FunctionPointerDeleter); - pointer = std::move(temp_pointer); - - // No deletion should happen on move - task_runner_.RunTasksUntilIdle(); - EXPECT_FALSE(deleter_called_); - } - } - - task_runner_.RunTasksUntilIdle(); - EXPECT_TRUE(deleter_called_); -} - -} // namespace openscreen -- cgit v1.2.3