Make CopyOnWriteBuffer keep capacity
for SetData and Clear functions too.
This way result of all functions is same
for shared and non-shared buffer cases
R=kwiberg@webrtc.org
Review URL: https://codereview.webrtc.org/2328553002 .
Cr-Commit-Position: refs/heads/master@{#14196}
diff --git a/webrtc/base/copyonwritebuffer.h b/webrtc/base/copyonwritebuffer.h
index 108aaa1..70833d6 100644
--- a/webrtc/base/copyonwritebuffer.h
+++ b/webrtc/base/copyonwritebuffer.h
@@ -153,9 +153,10 @@
internal::BufferCompat<uint8_t, T>::value>::type* = nullptr>
void SetData(const T* data, size_t size) {
RTC_DCHECK(IsConsistent());
- if (!buffer_ || !buffer_->HasOneRef()) {
- buffer_ = size > 0 ? new RefCountedObject<Buffer>(data, size)
- : nullptr;
+ if (!buffer_) {
+ buffer_ = size > 0 ? new RefCountedObject<Buffer>(data, size) : nullptr;
+ } else if (!buffer_->HasOneRef()) {
+ buffer_ = new RefCountedObject<Buffer>(data, size, buffer_->capacity());
} else {
buffer_->SetData(data, size);
}
@@ -253,13 +254,16 @@
RTC_DCHECK(IsConsistent());
}
- // Resets the buffer to zero size and capacity.
+ // Resets the buffer to zero size without altering capacity. Works even if the
+ // buffer has been moved from.
void Clear() {
- RTC_DCHECK(IsConsistent());
- if (!buffer_ || !buffer_->HasOneRef()) {
- buffer_ = nullptr;
- } else {
+ if (!buffer_)
+ return;
+
+ if (buffer_->HasOneRef()) {
buffer_->Clear();
+ } else {
+ buffer_ = new RefCountedObject<Buffer>(0, buffer_->capacity());
}
RTC_DCHECK(IsConsistent());
}
diff --git a/webrtc/base/copyonwritebuffer_unittest.cc b/webrtc/base/copyonwritebuffer_unittest.cc
index dbf59f7..fefd0c7 100644
--- a/webrtc/base/copyonwritebuffer_unittest.cc
+++ b/webrtc/base/copyonwritebuffer_unittest.cc
@@ -116,37 +116,57 @@
EXPECT_EQ(buf2, CopyOnWriteBuffer(exp));
}
-TEST(CopyOnWriteBufferTest, TestSetData) {
+TEST(CopyOnWriteBufferTest, SetEmptyData) {
+ CopyOnWriteBuffer buf(10);
+
+ buf.SetData<uint8_t>(nullptr, 0);
+
+ EXPECT_EQ(0u, buf.size());
+}
+
+TEST(CopyOnWriteBufferTest, SetDataNoMoreThanCapacityDoesntCauseReallocation) {
+ CopyOnWriteBuffer buf1(3, 10);
+ const uint8_t* const original_allocation = buf1.cdata();
+
+ buf1.SetData(kTestData, 10);
+
+ EXPECT_EQ(original_allocation, buf1.cdata());
+ EXPECT_EQ(buf1, CopyOnWriteBuffer(kTestData, 10));
+}
+
+TEST(CopyOnWriteBufferTest, SetDataMakeReferenceCopy) {
CopyOnWriteBuffer buf1(kTestData, 3, 10);
CopyOnWriteBuffer buf2;
buf2.SetData(buf1);
- // buf2 shares the same data as buf1 now.
- EnsureBuffersShareData(buf1, buf2);
- CopyOnWriteBuffer buf3(buf1);
- // buf3 is re-allocated with new data, existing buffers are not modified.
- buf3.SetData("foo");
- EXPECT_EQ(buf1, CopyOnWriteBuffer(kTestData, 3));
EnsureBuffersShareData(buf1, buf2);
- EnsureBuffersDontShareData(buf1, buf3);
- const int8_t exp[] = {'f', 'o', 'o', 0x0};
- EXPECT_EQ(buf3, CopyOnWriteBuffer(exp));
-
- buf2.SetData(static_cast<const uint8_t*>(nullptr), 0u);
- EnsureBuffersDontShareData(buf1, buf2);
- EXPECT_EQ(buf1.size(), 3u);
- EXPECT_EQ(buf1.capacity(), 10u);
- EXPECT_EQ(buf2.size(), 0u);
- EXPECT_EQ(buf2.capacity(), 0u);
}
-TEST(CopyOnWriteBufferTest, TestSetDataEmpty) {
- CopyOnWriteBuffer buf;
- buf.SetData(static_cast<const uint8_t*>(nullptr), 0u);
- EXPECT_EQ(buf.size(), 0u);
- EXPECT_EQ(buf.capacity(), 0u);
- EXPECT_EQ(buf.data(), nullptr);
+TEST(CopyOnWriteBufferTest, SetDataOnSharedKeepsOriginal) {
+ const uint8_t data[] = "foo";
+ CopyOnWriteBuffer buf1(kTestData, 3, 10);
+ const uint8_t* const original_allocation = buf1.cdata();
+ CopyOnWriteBuffer buf2(buf1);
+
+ buf2.SetData(data);
+
+ EnsureBuffersDontShareData(buf1, buf2);
+ EXPECT_EQ(original_allocation, buf1.cdata());
+ EXPECT_EQ(buf1, CopyOnWriteBuffer(kTestData, 3));
+ EXPECT_EQ(buf2, CopyOnWriteBuffer(data));
+}
+
+TEST(CopyOnWriteBufferTest, SetDataOnSharedKeepsCapacity) {
+ CopyOnWriteBuffer buf1(kTestData, 3, 10);
+ CopyOnWriteBuffer buf2(buf1);
+ EnsureBuffersShareData(buf1, buf2);
+
+ buf2.SetData(kTestData, 2);
+
+ EnsureBuffersDontShareData(buf1, buf2);
+ EXPECT_EQ(2u, buf2.size());
+ EXPECT_EQ(10u, buf2.capacity());
}
TEST(CopyOnWriteBufferTest, TestEnsureCapacity) {
@@ -172,31 +192,70 @@
EXPECT_EQ(buf1, buf2);
}
-TEST(CopyOnWriteBufferTest, TestSetSize) {
+TEST(CopyOnWriteBufferTest, SetSizeDoesntChangeOriginal) {
+ CopyOnWriteBuffer buf1(kTestData, 3, 10);
+ const uint8_t* const original_allocation = buf1.cdata();
+ CopyOnWriteBuffer buf2(buf1);
+
+ buf2.SetSize(16);
+
+ EnsureBuffersDontShareData(buf1, buf2);
+ EXPECT_EQ(original_allocation, buf1.cdata());
+ EXPECT_EQ(3u, buf1.size());
+ EXPECT_EQ(10u, buf1.capacity());
+}
+
+TEST(CopyOnWriteBufferTest, SetSizeCloneContent) {
CopyOnWriteBuffer buf1(kTestData, 3, 10);
CopyOnWriteBuffer buf2(buf1);
buf2.SetSize(16);
- EnsureBuffersDontShareData(buf1, buf2);
- EXPECT_EQ(buf1.size(), 3u);
- EXPECT_EQ(buf1.capacity(), 10u);
+
EXPECT_EQ(buf2.size(), 16u);
- EXPECT_EQ(buf2.capacity(), 16u);
- // The contents got cloned.
EXPECT_EQ(0, memcmp(buf2.data(), kTestData, 3));
}
-TEST(CopyOnWriteBufferTest, TestClear) {
+TEST(CopyOnWriteBufferTest, SetSizeMayIncreaseCapacity) {
+ CopyOnWriteBuffer buf(kTestData, 3, 10);
+
+ buf.SetSize(16);
+
+ EXPECT_EQ(16u, buf.size());
+ EXPECT_EQ(16u, buf.capacity());
+}
+
+TEST(CopyOnWriteBufferTest, SetSizeDoesntDecreaseCapacity) {
+ CopyOnWriteBuffer buf1(kTestData, 5, 10);
+ CopyOnWriteBuffer buf2(buf1);
+
+ buf2.SetSize(2);
+
+ EXPECT_EQ(2u, buf2.size());
+ EXPECT_EQ(10u, buf2.capacity());
+}
+
+TEST(CopyOnWriteBufferTest, ClearDoesntChangeOriginal) {
+ CopyOnWriteBuffer buf1(kTestData, 3, 10);
+ const uint8_t* const original_allocation = buf1.cdata();
+ CopyOnWriteBuffer buf2(buf1);
+
+ buf2.Clear();
+
+ EnsureBuffersDontShareData(buf1, buf2);
+ EXPECT_EQ(3u, buf1.size());
+ EXPECT_EQ(10u, buf1.capacity());
+ EXPECT_EQ(original_allocation, buf1.cdata());
+ EXPECT_EQ(0u, buf2.size());
+}
+
+TEST(CopyOnWriteBufferTest, ClearDoesntChangeCapacity) {
CopyOnWriteBuffer buf1(kTestData, 3, 10);
CopyOnWriteBuffer buf2(buf1);
buf2.Clear();
- EnsureBuffersDontShareData(buf1, buf2);
- EXPECT_EQ(buf1.size(), 3u);
- EXPECT_EQ(buf1.capacity(), 10u);
- EXPECT_EQ(0, memcmp(buf1.data(), kTestData, 3));
- EXPECT_EQ(buf2.size(), 0u);
- EXPECT_EQ(buf2.capacity(), 0u);
+
+ EXPECT_EQ(0u, buf2.size());
+ EXPECT_EQ(10u, buf2.capacity());
}
TEST(CopyOnWriteBufferTest, TestConstDataAccessor) {
