Mark scoped_refptr move and swap operations as noexcept
to align with chromium scoped_refptr implementation
and prefer move over copy in some cases.
Bug: webrtc:11078
Change-Id: I3178e74e611e4b23435668878e6bcc98bc2ce77d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/159541
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29768}
diff --git a/api/BUILD.gn b/api/BUILD.gn
index 2ebf6e6..1c6c952 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -954,6 +954,7 @@
"rtp_packet_info_unittest.cc",
"rtp_packet_infos_unittest.cc",
"rtp_parameters_unittest.cc",
+ "scoped_refptr_unittest.cc",
"test/loopback_media_transport_unittest.cc",
]
@@ -966,6 +967,7 @@
":rtc_event_log_output_file",
":rtp_packet_info",
":rtp_parameters",
+ ":scoped_refptr",
"../rtc_base:checks",
"../rtc_base:gunit_helpers",
"../rtc_base:rtc_base_approved",
diff --git a/api/scoped_refptr.h b/api/scoped_refptr.h
index 67d179f..fa4e83d 100644
--- a/api/scoped_refptr.h
+++ b/api/scoped_refptr.h
@@ -92,10 +92,10 @@
}
// Move constructors.
- scoped_refptr(scoped_refptr<T>&& r) : ptr_(r.release()) {}
+ scoped_refptr(scoped_refptr<T>&& r) noexcept : ptr_(r.release()) {}
template <typename U>
- scoped_refptr(scoped_refptr<U>&& r) : ptr_(r.release()) {}
+ scoped_refptr(scoped_refptr<U>&& r) noexcept : ptr_(r.release()) {}
~scoped_refptr() {
if (ptr_)
@@ -136,24 +136,24 @@
return *this = r.get();
}
- scoped_refptr<T>& operator=(scoped_refptr<T>&& r) {
+ scoped_refptr<T>& operator=(scoped_refptr<T>&& r) noexcept {
scoped_refptr<T>(std::move(r)).swap(*this);
return *this;
}
template <typename U>
- scoped_refptr<T>& operator=(scoped_refptr<U>&& r) {
+ scoped_refptr<T>& operator=(scoped_refptr<U>&& r) noexcept {
scoped_refptr<T>(std::move(r)).swap(*this);
return *this;
}
- void swap(T** pp) {
+ void swap(T** pp) noexcept {
T* p = ptr_;
ptr_ = *pp;
*pp = p;
}
- void swap(scoped_refptr<T>& r) { swap(&r.ptr_); }
+ void swap(scoped_refptr<T>& r) noexcept { swap(&r.ptr_); }
protected:
T* ptr_;
diff --git a/api/scoped_refptr_unittest.cc b/api/scoped_refptr_unittest.cc
new file mode 100644
index 0000000..75a202b
--- /dev/null
+++ b/api/scoped_refptr_unittest.cc
@@ -0,0 +1,111 @@
+/*
+ * Copyright 2019 The WebRTC Project Authors. All rights reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "api/scoped_refptr.h"
+
+#include <utility>
+#include <vector>
+
+#include "test/gtest.h"
+
+namespace rtc {
+namespace {
+
+struct FunctionsCalled {
+ int addref = 0;
+ int release = 0;
+};
+
+class ScopedRefCounted {
+ public:
+ explicit ScopedRefCounted(FunctionsCalled* called) : called_(*called) {}
+ ScopedRefCounted(const ScopedRefCounted&) = delete;
+ ScopedRefCounted& operator=(const ScopedRefCounted&) = delete;
+
+ void AddRef() {
+ ++called_.addref;
+ ++ref_count_;
+ }
+ void Release() {
+ ++called_.release;
+ if (0 == --ref_count_)
+ delete this;
+ }
+
+ private:
+ ~ScopedRefCounted() = default;
+
+ FunctionsCalled& called_;
+ int ref_count_ = 0;
+};
+
+TEST(ScopedRefptrTest, IsCopyConstructable) {
+ FunctionsCalled called;
+ scoped_refptr<ScopedRefCounted> ptr = new ScopedRefCounted(&called);
+ scoped_refptr<ScopedRefCounted> another_ptr = ptr;
+
+ EXPECT_TRUE(ptr);
+ EXPECT_TRUE(another_ptr);
+ EXPECT_EQ(called.addref, 2);
+}
+
+TEST(ScopedRefptrTest, IsCopyAssignable) {
+ FunctionsCalled called;
+ scoped_refptr<ScopedRefCounted> another_ptr;
+ scoped_refptr<ScopedRefCounted> ptr = new ScopedRefCounted(&called);
+ another_ptr = ptr;
+
+ EXPECT_TRUE(ptr);
+ EXPECT_TRUE(another_ptr);
+ EXPECT_EQ(called.addref, 2);
+}
+
+TEST(ScopedRefptrTest, IsMoveConstructableWithoutExtraAddRefRelease) {
+ FunctionsCalled called;
+ scoped_refptr<ScopedRefCounted> ptr = new ScopedRefCounted(&called);
+ scoped_refptr<ScopedRefCounted> another_ptr = std::move(ptr);
+
+ EXPECT_FALSE(ptr);
+ EXPECT_TRUE(another_ptr);
+ EXPECT_EQ(called.addref, 1);
+ EXPECT_EQ(called.release, 0);
+}
+
+TEST(ScopedRefptrTest, IsMoveAssignableWithoutExtraAddRefRelease) {
+ FunctionsCalled called;
+ scoped_refptr<ScopedRefCounted> another_ptr;
+ scoped_refptr<ScopedRefCounted> ptr = new ScopedRefCounted(&called);
+ another_ptr = std::move(ptr);
+
+ EXPECT_FALSE(ptr);
+ EXPECT_TRUE(another_ptr);
+ EXPECT_EQ(called.addref, 1);
+ EXPECT_EQ(called.release, 0);
+}
+
+TEST(ScopedRefptrTest, MovableDuringVectorReallocation) {
+ static_assert(
+ std::is_nothrow_move_constructible<scoped_refptr<ScopedRefCounted>>(),
+ "");
+ // Test below describes a scenario where it is helpful for move constructor
+ // to be noexcept.
+ FunctionsCalled called;
+ std::vector<scoped_refptr<ScopedRefCounted>> ptrs;
+ ptrs.reserve(1);
+ // Insert more elements than reserved to provoke reallocation.
+ ptrs.push_back(new ScopedRefCounted(&called));
+ ptrs.push_back(new ScopedRefCounted(&called));
+
+ EXPECT_EQ(called.addref, 2);
+ EXPECT_EQ(called.release, 0);
+}
+
+} // namespace
+} // namespace rtc