Make CopyOnWriteBuffer keep capacity

webrtc/base/copyonwritebuffer.h

Index: webrtc/base/copyonwritebuffer.h
diff --git a/webrtc/base/copyonwritebuffer.h b/webrtc/base/copyonwritebuffer.h
index 72fb4eea53495a2575ee435efade9d182afd4a69..41d4090a869b20f35b3ca01eaffb8f795ff87869 100644
--- a/webrtc/base/copyonwritebuffer.h
+++ b/webrtc/base/copyonwritebuffer.h
@@ -162,6 +162,23 @@ class CopyOnWriteBuffer {
     RTC_DCHECK(IsConsistent());
   }
 
+  // Replace the contents of the buffer. Ensure capacity is at least |capacity|.
+  template <typename T,
+            typename std::enable_if<
+                internal::BufferCompat<uint8_t, T>::value>::type* = nullptr>
+  void SetData(const T* data, size_t size, size_t capacity) {

[kwiberg-webrtc, 2016/09/09 08:55:13]

Isn't this equivalent to calling

  x.EnsureCapacity(foo);
  x.SetData(bar, baz);

Oh, no, because that'll copy the old data if .SetCapacity() needs to
reallocate. Got it.

However,

  1. All the SetData functions should get optional capacity arguments,
     right? (But the AppendData functions don't need them, because
     there it *is* OK to do x.EnsureCapacity(); x.AppendData();.)

  2. Buffer needs the extra arguments just as much as
     CopyOnWriteBuffer, right?

[danilchap, 2016/09/09 11:21:17]

1. SetData signatures mirror constructor signatures. There is only one
constructor that take capacity argument explicitly.
So other SetData probably do no need version with capacity.

2. Buffer probably doesn't need extra argument because Buffer doesn't reduce
capacity (except for move assign operator).
Buffer b; b.EnsureCapacity(capacity); b.SetData(data, size);
would do same as b.SetData(data, size, capacity);

consistency with CopyOnWriteBuffer is the only reason to add it there too. You
think it is strong enough reason?

[kwiberg-webrtc, 2016/09/09 12:59:13]

I'm not sure. You can always replace

  Buffer buf(...);

with

  Buffer buf;
  buf.SetData(...);

with essentially zero performance penalty, so you could argue that the
constructors don't need to be as general as SetData. But you could also argue
that the constructors also ought to have those extra capacity arguments...
No, "b.EnsureCapacity(capacity); b.SetData(data, size);" would copy all the old
data. You'd need "b.Clear(); b.EnsureCapacity(capacity); b.SetData(data,
size);", and no callers are going to get that right.

[danilchap, 2016/09/09 16:00:56]

Yes, I'm wrong. it is not same. Buffer may benefit from SetData(data, size,
capacity) too.

+    RTC_DCHECK(IsConsistent());
+    capacity = std::max(size, capacity);
+    if (!buffer_ || !buffer_->HasOneRef() || buffer_->capacity() < capacity) {
+      buffer_ = capacity > 0
+                    ? new RefCountedObject<Buffer>(data, size, capacity)
+                    : nullptr;
+    } else {
+      buffer_->SetData(data, size);
+    }
+    RTC_DCHECK(IsConsistent());
+  }
+
   template <typename T,
             size_t N,
             typename std::enable_if<