Buffer interface is not uint8*. Slight change to way data buffers are create...

Buffer interface is now uint8*.  Slight change to way data buffers are created.  Caller passes no parameters to the 
constructor of a data buffer now.  GetWritableData() method is responsible for allocating memory.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=11760

[scherkus (not reviewing), 2009-03-13 17:47:44]

LGTM!  Just a few nits, nothing serious.

http://codereview.chromium.org/46015/diff/16/1009
File media/base/data_buffer.cc (right):

http://codereview.chromium.org/46015/diff/16/1009#newcode13
Line 13: data_size_(0) {
I assume we're just letting the base class initialize timestamp/duration to 0,
right?

http://codereview.chromium.org/46015/diff/16/1009#newcode30
Line 30: delete data_;
should be delete[], I think...

and I can never remember if C++ cries or not if data_ is NULL and you attempt to
delete it.  I'm pretty sure its ok.

[scherkus (not reviewing), 2009-03-13 17:49:36]

just a comment on your tests

http://codereview.chromium.org/46015/diff/16/1008
File media/base/data_buffer_unittest.cc (right):

http://codereview.chromium.org/46015/diff/16/1008#newcode48
Line 48: uint8* data = buffer->GetWritableData(kDataSize);
can we also test that GetData() returns NULL initially?

http://codereview.chromium.org/46015/diff/16/1008#newcode56
Line 56: data = buffer->GetWritableData(kNewDataSize + 10);
might be nice to test that the pointers are the same for GetWriteableData(n),
where n <= PreviousN

Also pointers are different where n > PreviousN

Technically white-box testing, but nothing wrong with that :)

[ralphl, 2009-03-16 17:20:26]

Fixed tests and incorporated Andrew's comments.  When I get a try success I'll
check this in.

http://codereview.chromium.org/46015/diff/16/1009
File media/base/data_buffer.cc (right):

http://codereview.chromium.org/46015/diff/16/1009#newcode13
Line 13: data_size_(0) {
On 2009/03/13 17:47:45, scherkus wrote:
> I assume we're just letting the base class initialize timestamp/duration to 0,
> right?

Yes.  Base class initializes it.

http://codereview.chromium.org/46015/diff/16/1009#newcode30
Line 30: delete data_;
On 2009/03/13 17:47:45, scherkus wrote:
> should be delete[], I think...
> 
> and I can never remember if C++ cries or not if data_ is NULL and you attempt
to
> delete it.  I'm pretty sure its ok.
 
It is OK for sure to delete NULL.  You are right about [] -- Fixed.

http://codereview.chromium.org/46015/diff/16/1008
File media/base/data_buffer_unittest.cc (right):

http://codereview.chromium.org/46015/diff/16/1008#newcode48
Line 48: uint8* data = buffer->GetWritableData(kDataSize);
On 2009/03/13 17:49:36, scherkus wrote:
> can we also test that GetData() returns NULL initially?

Done.

http://codereview.chromium.org/46015/diff/16/1008#newcode56
Line 56: data = buffer->GetWritableData(kNewDataSize + 10);
On 2009/03/13 17:49:36, scherkus wrote:
> might be nice to test that the pointers are the same for GetWriteableData(n),
> where n <= PreviousN
> 
> Also pointers are different where n > PreviousN
> 
> Technically white-box testing, but nothing wrong with that :) 

I check for pointer not changing after a SetDataSize, but it's not safe to
assume that the pointer will change even if you grow a buffer since a delete
followed by a new might just re-allocate the same heap block, so the pointers
would be equal, but the heap blocks are different sizes (which I can't test).