rtc::Buffer: Let SetData and AppendData accept anything with .data() and .size()

rtc::Buffer: Let SetData and AppendData accept anything with .data() and .size()

In addition to setting or appending from another Buffer, which was
already possible, this allows for e.g. std::vector and rtc::ArrayView
arguments.

Committed: https://crrev.com/d31340356428af861c12f9de87f298cbbd985843
Cr-Commit-Position: refs/heads/master@{#14073}

[kwiberg-webrtc, 2016-08-30 18:08:06]

kwiberg@webrtc.org changed reviewers:
+ ossu@webrtc.org

[ossu, 2016-08-31 09:31:03]

https://codereview.webrtc.org/2293983002/diff/1/webrtc/base/buffer.h
File webrtc/base/buffer.h (right):

https://codereview.webrtc.org/2293983002/diff/1/webrtc/base/buffer.h#newcode195
webrtc/base/buffer.h:195: template <typename W>
"W" as in Whatever? :)

https://codereview.webrtc.org/2293983002/diff/1/webrtc/base/buffer_unittest.cc
File webrtc/base/buffer_unittest.cc (right):

https://codereview.webrtc.org/2293983002/diff/1/webrtc/base/buffer_unittest.c...
webrtc/base/buffer_unittest.cc:75: Buffer buf2;
Do you want to test this with an unrelated class as well? Just a test-local one
with data() and size() and basically nothing else? (well, except for some data,
I guess).

[kwiberg-webrtc, 2016-09-01 09:23:53]

New patch set posted.

https://codereview.webrtc.org/2293983002/diff/1/webrtc/base/buffer.h
File webrtc/base/buffer.h (right):

https://codereview.webrtc.org/2293983002/diff/1/webrtc/base/buffer.h#newcode195
webrtc/base/buffer.h:195: template <typename W>
On 2016/08/31 09:31:03, ossu wrote:
> "W" as in Whatever? :)

Yeah, whatever.

https://codereview.webrtc.org/2293983002/diff/1/webrtc/base/buffer_unittest.cc
File webrtc/base/buffer_unittest.cc (right):

https://codereview.webrtc.org/2293983002/diff/1/webrtc/base/buffer_unittest.c...
webrtc/base/buffer_unittest.cc:75: Buffer buf2;
On 2016/08/31 09:31:03, ossu wrote:
> Do you want to test this with an unrelated class as well? Just a test-local
one
> with data() and size() and basically nothing else? (well, except for some
data,
> I guess).

Done.

[kwiberg-webrtc, 2016-09-05 07:21:29]

Ping on this one.

Oh, and do you suppose I should use the template magic from
https://codereview.webrtc.org/2312473002/ here as well?

[ossu, 2016-09-05 09:24:00]

On 2016/09/05 07:21:29, kwiberg-webrtc wrote:
> Ping on this one.
> 
> Oh, and do you suppose I should use the template magic from
> https://codereview.webrtc.org/2312473002/ here as well?

Yeah, I think that would be great! Oh, and as I mentioned on
https://codereview.webrtc.org/2289383002/ (about the stuff in internal), the
HasDataAndSize would need to move to a common header, or duplicated within each
class.

[kwiberg-webrtc, 2016-09-05 13:24:00]

On 2016/09/05 09:24:00, ossu wrote:
> On 2016/09/05 07:21:29, kwiberg-webrtc wrote:
> > Ping on this one.
> > 
> > Oh, and do you suppose I should use the template magic from
> > https://codereview.webrtc.org/2312473002/ here as well?
> 
> Yeah, I think that would be great! Oh, and as I mentioned on
> https://codereview.webrtc.org/2289383002/ (about the stuff in internal), the
> HasDataAndSize would need to move to a common header, or duplicated within
each
> class.

New patch set uploaded.

[ossu, 2016-09-05 13:33:49]

lgtm with a comment

https://codereview.webrtc.org/2293983002/diff/60001/webrtc/base/type_traits.h
File webrtc/base/type_traits.h (right):

https://codereview.webrtc.org/2293983002/diff/60001/webrtc/base/type_traits.h...
webrtc/base/type_traits.h:39: namespace test_has_data_and_size {
Should this be hidden away in a .cc file somewhere? On the one hand it means you
don't have to clutter the header file (and the compiler) with stuff that's only
used as the test for HasDataAndSize. On the other hand, it means you could miss
compiling the tests (or including them when steal... borrowing our code) when
(for some reason) changing the implementation of HasDataAndSize.

I'll leave it up to you. I think I prefer it separate, even though that adds
another file to the build.

[kwiberg-webrtc, 2016-09-05 14:04:11]

https://codereview.webrtc.org/2293983002/diff/60001/webrtc/base/type_traits.h
File webrtc/base/type_traits.h (right):

https://codereview.webrtc.org/2293983002/diff/60001/webrtc/base/type_traits.h...
webrtc/base/type_traits.h:39: namespace test_has_data_and_size {
On 2016/09/05 13:33:48, ossu wrote:
> Should this be hidden away in a .cc file somewhere? On the one hand it means
you
> don't have to clutter the header file (and the compiler) with stuff that's
only
> used as the test for HasDataAndSize. On the other hand, it means you could
miss
> compiling the tests (or including them when steal... borrowing our code) when
> (for some reason) changing the implementation of HasDataAndSize.
> 
> I'll leave it up to you. I think I prefer it separate, even though that adds
> another file to the build.

It used to be in a separate file (array_view_unittests.cc), but I switched to
just using a header file in this CL after evaluating precisely the arguments you
just listed. The deciding factors were simplicity, and that the tests are at
present quite small. If they grow large enough to obscure the useful part of the
file, we can move them then.

[kwiberg-webrtc, 2016-09-05 14:04:35]

The CQ bit was checked by kwiberg@webrtc.org

[commit-bot: I haz the power, 2016-09-05 14:04:43]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-05 14:46:23]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-05 14:46:30]

Description was changed from

==========
rtc::Buffer: Let SetData and AppendData accept anything with .data() and .size()

In addition to setting or appending from another Buffer, which was
already possible, this allows for e.g. std::vector and rtc::ArrayView
arguments.
==========

to

==========
rtc::Buffer: Let SetData and AppendData accept anything with .data() and .size()

In addition to setting or appending from another Buffer, which was
already possible, this allows for e.g. std::vector and rtc::ArrayView
arguments.

Committed: https://crrev.com/d31340356428af861c12f9de87f298cbbd985843
Cr-Commit-Position: refs/heads/master@{#14073}
==========

[commit-bot: I haz the power, 2016-09-05 14:46:31]

Patchset 4 (id:??) landed as
https://crrev.com/d31340356428af861c12f9de87f298cbbd985843
Cr-Commit-Position: refs/heads/master@{#14073}