Change NeedKeyCB to use std::vector.

Change NeedKeyCB to use std::vector.

Changes various NeedKeyCB instances to use std::vector<uint8> instead
of a scoped_ptr<uint8[]> and a size parameter.

TEST=All tests still pass.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=219575

[xhwang, 2013-08-23 17:24:36]

Thanks for the patch! It's long been in my TODO list...

Most comments are about the use of &vector[0]. This will crash if vector is
empty. I wish we could use vector::data() but it's a C++11 feature and is not
available yet. So we need to either DCHECK the vector is not empty or check the
size explicitly.

https://codereview.chromium.org/23072043/diff/2001/content/renderer/media/and...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/23072043/diff/2001/content/renderer/media/and...
content/renderer/media/android/webmediaplayer_android.cc:1149: &init_data[0],
Is it possible that init_data.empty()? It'll crash if yes. We wither need to 

DCHECK(!init_data.empty())

 or use 

init_data.empty() ? NULL : &init_data[0]

here

https://codereview.chromium.org/23072043/diff/2001/content/renderer/media/web...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/23072043/diff/2001/content/renderer/media/web...
content/renderer/media/webmediaplayer_impl.cc:941: &init_data[0],
ditto

https://codereview.chromium.org/23072043/diff/2001/media/filters/chunk_demuxe...
File media/filters/chunk_demuxer_unittest.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/filters/chunk_demuxe...
media/filters/chunk_demuxer_unittest.cc:860: NeedKeyMock(type, &init_data[0],
init_data.size());
ditto

https://codereview.chromium.org/23072043/diff/2001/media/filters/ffmpeg_demux...
File media/filters/ffmpeg_demuxer.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer.cc:818: memcpy(&key_id_local[0],
encryption_key_id.data(), key_id_size);
How about:

std::vector<uint8> key_id_local(encryption_key_id.begin(),
encryption_key_id.end());

It's probably not as efficient as mempcy, but seems cleaner, and key_id is
usually short anyways.

https://codereview.chromium.org/23072043/diff/2001/media/filters/ffmpeg_demux...
File media/filters/ffmpeg_demuxer_unittest.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer_unittest.cc:84: media::Demuxer::NeedKeyCB
need_key_cb =
s/media:://

https://codereview.chromium.org/23072043/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer_unittest.cc:140: NeedKeyCBMock(type, &init_data[0],
init_data.size());
ditto

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
File media/filters/pipeline_integration_test.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:167: const uint8* key_id =
&init_data[0];
ditto

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:301: DCHECK_GT(init_data.size(), 0u);
DCHECK(!init_data.empty());

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:302: DCHECK(&init_data[0]);
drop this?

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
File media/filters/pipeline_integration_test_base.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
media/filters/pipeline_integration_test_base.cc:70: DCHECK(&init_data[0]);
ditto

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
media/filters/pipeline_integration_test_base.cc:214: media::Demuxer::NeedKeyCB
need_key_cb = base::Bind(
drop "media::"

https://codereview.chromium.org/23072043/diff/2001/media/mp4/mp4_stream_parse...
File media/mp4/mp4_stream_parser_unittest.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/mp4/mp4_stream_parse...
media/mp4/mp4_stream_parser_unittest.cc:101: EXPECT_GT(init_data.size(), 0u);
ditto

https://codereview.chromium.org/23072043/diff/2001/media/webm/webm_stream_par...
File media/webm/webm_stream_parser.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/webm/webm_stream_par...
media/webm/webm_stream_parser.cc:326: memcpy(&key_id_array[0], key_id.data(),
key_id.size());
ditto about using vector::vector(firstIter, lastIter).

[scherkus (not reviewing), 2013-08-23 18:41:08]

On 2013/08/23 17:24:36, xhwang wrote:
> Thanks for the patch! It's long been in my TODO list...
> 
> Most comments are about the use of &vector[0]. This will crash if vector is
> empty. I wish we could use vector::data() but it's a C++11 feature and is not
> available yet. So we need to either DCHECK the vector is not empty or check
the
> size explicitly.

Short version: you should always check .size() / .empty()

Long version: I ran into this recently and ajwong@ and looked up the C++11 ...
while my local test build with gcc returns 0 when accessing v.data(),
&v.front(), and &v[0] ... the spec says all of those are undefined operations on
an empty std::vector

[xhwang, 2013-08-23 19:10:55]

On 2013/08/23 18:41:08, scherkus wrote:
> On 2013/08/23 17:24:36, xhwang wrote:
> > Thanks for the patch! It's long been in my TODO list...
> > 
> > Most comments are about the use of &vector[0]. This will crash if vector is
> > empty. I wish we could use vector::data() but it's a C++11 feature and is
not
> > available yet. So we need to either DCHECK the vector is not empty or check
> the
> > size explicitly.
> 
> Short version: you should always check .size() / .empty()
> 
> Long version: I ran into this recently and ajwong@ and looked up the C++11 ...
> while my local test build with gcc returns 0 when accessing v.data(),
> &v.front(), and &v[0] ... the spec says all of those are undefined operations
on
> an empty std::vector

Really! Good to know! Though I don't know why the spec does it this way :(

[acolwell GONE FROM CHROMIUM, 2013-08-23 19:55:28]

https://codereview.chromium.org/23072043/diff/2001/content/renderer/media/and...
File content/renderer/media/android/webmediaplayer_android.cc (right):

https://codereview.chromium.org/23072043/diff/2001/content/renderer/media/and...
content/renderer/media/android/webmediaplayer_android.cc:1149: &init_data[0],
On 2013/08/23 17:24:37, xhwang wrote:
> Is it possible that init_data.empty()? It'll crash if yes. We wither need to 
> 
> DCHECK(!init_data.empty())
> 
>  or use 
> 
> init_data.empty() ? NULL : &init_data[0]
> 
> here
> 

Done.

https://codereview.chromium.org/23072043/diff/2001/content/renderer/media/web...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/23072043/diff/2001/content/renderer/media/web...
content/renderer/media/webmediaplayer_impl.cc:941: &init_data[0],
On 2013/08/23 17:24:37, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/23072043/diff/2001/media/filters/chunk_demuxe...
File media/filters/chunk_demuxer_unittest.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/filters/chunk_demuxe...
media/filters/chunk_demuxer_unittest.cc:860: NeedKeyMock(type, &init_data[0],
init_data.size());
On 2013/08/23 17:24:37, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/23072043/diff/2001/media/filters/ffmpeg_demux...
File media/filters/ffmpeg_demuxer.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer.cc:818: memcpy(&key_id_local[0],
encryption_key_id.data(), key_id_size);
On 2013/08/23 17:24:37, xhwang wrote:
> How about:
> 
> std::vector<uint8> key_id_local(encryption_key_id.begin(),
> encryption_key_id.end());
> 
> It's probably not as efficient as mempcy, but seems cleaner, and key_id is
> usually short anyways.
Done. I didn't realize this would work. :)

https://codereview.chromium.org/23072043/diff/2001/media/filters/ffmpeg_demux...
File media/filters/ffmpeg_demuxer_unittest.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer_unittest.cc:84: media::Demuxer::NeedKeyCB
need_key_cb =
On 2013/08/23 17:24:37, xhwang wrote:
> s/media:://

Done.

https://codereview.chromium.org/23072043/diff/2001/media/filters/ffmpeg_demux...
media/filters/ffmpeg_demuxer_unittest.cc:140: NeedKeyCBMock(type, &init_data[0],
init_data.size());
On 2013/08/23 17:24:37, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
File media/filters/pipeline_integration_test.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:167: const uint8* key_id =
&init_data[0];
On 2013/08/23 17:24:37, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:301: DCHECK_GT(init_data.size(), 0u);
On 2013/08/23 17:24:37, xhwang wrote:
> DCHECK(!init_data.empty());

Done.

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:302: DCHECK(&init_data[0]);
On 2013/08/23 17:24:37, xhwang wrote:
> drop this?

Done.

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
File media/filters/pipeline_integration_test_base.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
media/filters/pipeline_integration_test_base.cc:70: DCHECK(&init_data[0]);
On 2013/08/23 17:24:37, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/23072043/diff/2001/media/filters/pipeline_int...
media/filters/pipeline_integration_test_base.cc:214: media::Demuxer::NeedKeyCB
need_key_cb = base::Bind(
On 2013/08/23 17:24:37, xhwang wrote:
> drop "media::"

Done.

https://codereview.chromium.org/23072043/diff/2001/media/mp4/mp4_stream_parse...
File media/mp4/mp4_stream_parser_unittest.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/mp4/mp4_stream_parse...
media/mp4/mp4_stream_parser_unittest.cc:101: EXPECT_GT(init_data.size(), 0u);
On 2013/08/23 17:24:37, xhwang wrote:
> ditto

Done.

https://codereview.chromium.org/23072043/diff/2001/media/webm/webm_stream_par...
File media/webm/webm_stream_parser.cc (right):

https://codereview.chromium.org/23072043/diff/2001/media/webm/webm_stream_par...
media/webm/webm_stream_parser.cc:326: memcpy(&key_id_array[0], key_id.data(),
key_id.size());
On 2013/08/23 17:24:37, xhwang wrote:
> ditto about using vector::vector(firstIter, lastIter).

Done.

[xhwang, 2013-08-26 16:09:54]

Thanks! LGTM!

[commit-bot: I haz the power, 2013-08-26 16:15:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/acolwell@chromium.org/23072043/22001

[commit-bot: I haz the power, 2013-08-26 18:50:46]

Change committed as 219575