Clean up decryption tests in FFmpegVideoDecoderTest.

Clean up decryption tests in FFmpegVideoDecoderTest.

BUG=132801
TEST=media_unittest


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

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

[xhwang, 2012-07-20 16:47:54]

Hello scherkus and ddorwin,

I cleaned up FFmpegVideoDecoderTest a little bit after fgalligan's CL is landed.

I am debating about whether we want to keep the test file
"vp8-encrypted-I-frame-320x240" and encrypted_i_frame_buffer_ member at all. 

Reason to keep: They are used to feed the FFVD a legitimate encrypted buffer as
input, which sounds to me the correct thing to do. 

Reason not to keep: Since we use MockDecryptor, the decryptor can return
whatever we want regardless what the input is. Therefore, the input
(encrypted_i_frame_buffer_) actually doesn't matter in these tests.

Any thoughts?

[fgalligan, 2012-07-20 17:29:53]

Keeping the "vp8-encrypted-I-frame-**320x240" and the other values
associated with it in sync, kKeyID, KHmac and kIV extra work.

But feeding the decryptor wrong values seems bad.

Also what is DecodeEncryptedFrame_Normal testing now? I think it is pretty
much testing DecodeFrame_Normal. Should we
remove DecodeEncryptedFrame_Normal?

Frank


On Fri, Jul 20, 2012 at 9:47 AM, <xhwang@chromium.org> wrote:

> Reviewers: ddorwin, scherkus,
>
> Message:
> Hello scherkus and ddorwin,
>
> I cleaned up FFmpegVideoDecoderTest a little bit after fgalligan's CL is
> landed.
>
> I am debating about whether we want to keep the test file
> "vp8-encrypted-I-frame-**320x240" and encrypted_i_frame_buffer_ member at
> all.
>
> Reason to keep: They are used to feed the FFVD a legitimate encrypted
> buffer as
> input, which sounds to me the correct thing to do.
>
> Reason not to keep: Since we use MockDecryptor, the decryptor can return
> whatever we want regardless what the input is. Therefore, the input
> (encrypted_i_frame_buffer_) actually doesn't matter in these tests.
>
> Any thoughts?
>
> Description:
> Clean up decryption tests in FFmpegVideoDecoderTest.
>
> BUG=132801
> TEST=media_unittest
>
>
> Please review this at
http://codereview.chromium.**org/10800057/<http://codereview.chromium.org/108...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>   M media/filters/ffmpeg_video_**decoder_unittest.cc
>
>
> Index: media/filters/ffmpeg_video_**decoder_unittest.cc
> diff --git a/media/filters/ffmpeg_video_**decoder_unittest.cc
> b/media/filters/ffmpeg_video_**decoder_unittest.cc
> index a21c3c84a6d63a92302e6f7b43be7c**1f3073a01e..**
> bab1273d0452ffb5af07142b973d16**c72452af33 100644
> --- a/media/filters/ffmpeg_video_**decoder_unittest.cc
> +++ b/media/filters/ffmpeg_video_**decoder_unittest.cc
> @@ -50,6 +50,38 @@ static const uint8 kHmac[] = {
>    0xb8, 0x32, 0x35, 0x56
>  };
>
> +// Generates a 16 byte CTR counter block. The CTR counter block format is
> a
> +// CTR IV appended with a CTR block counter. |iv| is an 8 byte CTR IV.
> +static scoped_array<uint8> GenerateCounterBlock(uint64 iv) {
> +  scoped_array<uint8> counter_block_data(
> +      new uint8[DecryptConfig::**kDecryptionKeySize]);
> +
> +  // Set the IV.
> +  memcpy(counter_block_data.get(**), &iv, sizeof(iv));
> +
> +  // Set block counter to all 0's.
> +  memset(counter_block_data.get(**) + sizeof(iv),
> +         0,
> +         DecryptConfig::**kDecryptionKeySize - sizeof(iv));
> +
> +  return counter_block_data.Pass();
> +}
> +
> +static scoped_refptr<DecoderBuffer> ReadEncryptedFrameBuffer() {
> +  scoped_refptr<DecoderBuffer> temp_buffer = ReadTestDataFile(
> +      "vp8-encrypted-I-frame-**320x240");
> +  scoped_refptr<DecoderBuffer> encrypted_i_frame_buffer =
> +      DecoderBuffer::CopyFrom(temp_**buffer->GetData() +
> arraysize(kHmac),
> +                              temp_buffer->GetDataSize() -
> arraysize(kHmac));
> +  scoped_array<uint8> counter_block(**GenerateCounterBlock(kIv));
> +  encrypted_i_frame_buffer->**SetDecryptConfig(scoped_ptr<**
> DecryptConfig>(
> +      new DecryptConfig(kKeyId, arraysize(kKeyId),
> +                        counter_block.get(), DecryptConfig::**
> kDecryptionKeySize,
> +                        kHmac, arraysize(kHmac),
> +                        sizeof(kIv))));
> +  return encrypted_i_frame_buffer;
> +}
> +
>  ACTION_P(ReturnBuffer, buffer) {
>    arg0.Run(buffer ? DemuxerStream::kOk : DemuxerStream::kAborted, buffer);
>  }
> @@ -76,13 +108,7 @@ class FFmpegVideoDecoderTest : public testing::Test {
>      end_of_stream_buffer_ = DecoderBuffer::**CreateEOSBuffer();
>      i_frame_buffer_ = ReadTestDataFile("vp8-I-frame-**320x240");
>      corrupt_i_frame_buffer_ = ReadTestDataFile("vp8-corrupt-**I-frame");
> -    {
> -      scoped_refptr<DecoderBuffer> temp_buffer = ReadTestDataFile(
> -          "vp8-encrypted-I-frame-**320x240");
> -      encrypted_i_frame_buffer_ = DecoderBuffer::CopyFrom(
> -          temp_buffer->GetData() + arraysize(kHmac),
> -          temp_buffer->GetDataSize() - arraysize(kHmac));
> -    }
> +    encrypted_i_frame_buffer_ = ReadEncryptedFrameBuffer();
>
>      config_.Initialize(kCodecVP8, VIDEO_CODEC_PROFILE_UNKNOWN,
>                         kVideoFormat, kCodedSize, kVisibleRect,
> @@ -211,24 +237,6 @@ class FFmpegVideoDecoderTest : public testing::Test {
>      message_loop_.RunAllPending();
>    }
>
> -  // Generates a 16 byte CTR counter block. The CTR counter block format
> is a
> -  // CTR IV appended with a CTR block counter. |iv| is an 8 byte CTR IV.
> -  static scoped_array<uint8> GenerateCounterBlock(uint64 iv) {
> -    scoped_array<uint8> counter_block_data(
> -        new uint8[DecryptConfig::**kDecryptionKeySize]);
> -
> -    // Set the IV.
> -    memcpy(counter_block_data.get(**), &iv, sizeof(iv));
> -
> -    // Set block counter to all 0's.
> -    memset(counter_block_data.get(**) + sizeof(iv),
> -           0,
> -           DecryptConfig::**kDecryptionKeySize - sizeof(iv));
> -
> -    return counter_block_data.Pass();
> -  }
> -
> -
>    MOCK_METHOD2(FrameReady, void(VideoDecoder::**DecoderStatus,
>                                  const scoped_refptr<VideoFrame>&));
>
> @@ -409,20 +417,10 @@ TEST_F(FFmpegVideoDecoderTest,
> DecodeFrame_SmallerHeight) {
>    DecodeIFrameThenTestFile("vp8-**I-frame-320x120", 320, 120);
>  }
>
> -// TODO(fgalligan): Enable test when encrypted test data is updated and
> new
> -// decryption code is landed. http://crbug.com/132801
> -TEST_F(**FFmpegVideoDecoderTest, DISABLED_DecodeEncryptedFrame_**Normal)
> {
> +TEST_F(**FFmpegVideoDecoderTest, DecodeEncryptedFrame_Normal) {
>    Initialize();
>
>    // Simulate decoding a single encrypted frame.
> -  scoped_array<uint8> counter_block(**GenerateCounterBlock(kIv));
> -  encrypted_i_frame_buffer_->**SetDecryptConfig(scoped_ptr<**
> DecryptConfig>(
> -      new DecryptConfig(
> -          kKeyId, arraysize(kKeyId),
> -          counter_block.get(), DecryptConfig::**kDecryptionKeySize,
> -          kHmac, arraysize(kHmac),
> -          sizeof(kIv))));
> -
>    EXPECT_CALL(*decryptor_, Decrypt(encrypted_i_frame_**buffer_, _))
>        .WillRepeatedly(RunDecryptCB(**Decryptor::kSuccess,
> i_frame_buffer_));
>
> @@ -440,14 +438,6 @@ TEST_F(FFmpegVideoDecoderTest, DecodeEncryptedFrame_*
> *DecryptError) {
>    Initialize();
>
>    // Simulate decoding a single encrypted frame.
> -  scoped_array<uint8> counter_block(**GenerateCounterBlock(kIv));
> -  encrypted_i_frame_buffer_->**SetDecryptConfig(scoped_ptr<**
> DecryptConfig>(
> -      new DecryptConfig(
> -          kKeyId, arraysize(kKeyId),
> -          counter_block.get(), DecryptConfig::**kDecryptionKeySize,
> -          kHmac, arraysize(kHmac),
> -          sizeof(kIv))));
> -
>    EXPECT_CALL(*demuxer_, Read(_))
>        .WillRepeatedly(ReturnBuffer(**encrypted_i_frame_buffer_));
>    EXPECT_CALL(*decryptor_, Decrypt(encrypted_i_frame_**buffer_, _))
> @@ -471,14 +461,6 @@ TEST_F(FFmpegVideoDecoderTest, DecodeEncryptedFrame_*
> *CorruptedBufferReturned) {
>    Initialize();
>
>    // Simulate decoding a single encrypted frame.
> -  scoped_array<uint8> counter_block(**GenerateCounterBlock(kIv));
> -  encrypted_i_frame_buffer_->**SetDecryptConfig(scoped_ptr<**
> DecryptConfig>(
> -      new DecryptConfig(
> -          kKeyId, arraysize(kKeyId),
> -          counter_block.get(), DecryptConfig::**kDecryptionKeySize,
> -          kHmac, arraysize(kHmac),
> -          sizeof(kIv))));
> -
>    EXPECT_CALL(*demuxer_, Read(_))
>        .WillRepeatedly(ReturnBuffer(**encrypted_i_frame_buffer_));
>    EXPECT_CALL(*decryptor_, Decrypt(encrypted_i_frame_**buffer_, _))
>
>
>

[xhwang, 2012-07-20 17:58:12]

On 2012/07/20 17:29:53, fgalligan wrote:
> Keeping the "vp8-encrypted-I-frame-**320x240" and the other values
> associated with it in sync, kKeyID, KHmac and kIV extra work.
> 
> But feeding the decryptor wrong values seems bad.
> 
> Also what is DecodeEncryptedFrame_Normal testing now? I think it is pretty
> much testing DecodeFrame_Normal. Should we
> remove DecodeEncryptedFrame_Normal?

DecodeEncryptedFrame_Normal tests the code path where the Decrypt() is called
and a correct result is returned:
http://code.google.com/searchframe#OAMlx_jo-ck/src/media/filters/ffmpeg_video...

DecodeFrame_Normal doesn't test this code path.
> 
> Frank
> 
> 
> On Fri, Jul 20, 2012 at 9:47 AM, <mailto:xhwang@chromium.org> wrote:
> 
> > Reviewers: ddorwin, scherkus,
> >
> > Message:
> > Hello scherkus and ddorwin,
> >
> > I cleaned up FFmpegVideoDecoderTest a little bit after fgalligan's CL is
> > landed.
> >
> > I am debating about whether we want to keep the test file
> > "vp8-encrypted-I-frame-**320x240" and encrypted_i_frame_buffer_ member at
> > all.
> >
> > Reason to keep: They are used to feed the FFVD a legitimate encrypted
> > buffer as
> > input, which sounds to me the correct thing to do.
> >
> > Reason not to keep: Since we use MockDecryptor, the decryptor can return
> > whatever we want regardless what the input is. Therefore, the input
> > (encrypted_i_frame_buffer_) actually doesn't matter in these tests.
> >
> > Any thoughts?
> >
> > Description:
> > Clean up decryption tests in FFmpegVideoDecoderTest.
> >
> > BUG=132801
> > TEST=media_unittest
> >
> >
> > Please review this at
>
http://codereview.chromium.**org/10800057/%3Chttp://codereview.chromium.org/1...>
> >
> > SVN Base:
>
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
> >
> > Affected files:
> >   M media/filters/ffmpeg_video_**decoder_unittest.cc
> >
> >
> > Index: media/filters/ffmpeg_video_**decoder_unittest.cc
> > diff --git a/media/filters/ffmpeg_video_**decoder_unittest.cc
> > b/media/filters/ffmpeg_video_**decoder_unittest.cc
> > index a21c3c84a6d63a92302e6f7b43be7c**1f3073a01e..**
> > bab1273d0452ffb5af07142b973d16**c72452af33 100644
> > --- a/media/filters/ffmpeg_video_**decoder_unittest.cc
> > +++ b/media/filters/ffmpeg_video_**decoder_unittest.cc
> > @@ -50,6 +50,38 @@ static const uint8 kHmac[] = {
> >    0xb8, 0x32, 0x35, 0x56
> >  };
> >
> > +// Generates a 16 byte CTR counter block. The CTR counter block format is
> > a
> > +// CTR IV appended with a CTR block counter. |iv| is an 8 byte CTR IV.
> > +static scoped_array<uint8> GenerateCounterBlock(uint64 iv) {
> > +  scoped_array<uint8> counter_block_data(
> > +      new uint8[DecryptConfig::**kDecryptionKeySize]);
> > +
> > +  // Set the IV.
> > +  memcpy(counter_block_data.get(**), &iv, sizeof(iv));
> > +
> > +  // Set block counter to all 0's.
> > +  memset(counter_block_data.get(**) + sizeof(iv),
> > +         0,
> > +         DecryptConfig::**kDecryptionKeySize - sizeof(iv));
> > +
> > +  return counter_block_data.Pass();
> > +}
> > +
> > +static scoped_refptr<DecoderBuffer> ReadEncryptedFrameBuffer() {
> > +  scoped_refptr<DecoderBuffer> temp_buffer = ReadTestDataFile(
> > +      "vp8-encrypted-I-frame-**320x240");
> > +  scoped_refptr<DecoderBuffer> encrypted_i_frame_buffer =
> > +      DecoderBuffer::CopyFrom(temp_**buffer->GetData() +
> > arraysize(kHmac),
> > +                              temp_buffer->GetDataSize() -
> > arraysize(kHmac));
> > +  scoped_array<uint8> counter_block(**GenerateCounterBlock(kIv));
> > +  encrypted_i_frame_buffer->**SetDecryptConfig(scoped_ptr<**
> > DecryptConfig>(
> > +      new DecryptConfig(kKeyId, arraysize(kKeyId),
> > +                        counter_block.get(), DecryptConfig::**
> > kDecryptionKeySize,
> > +                        kHmac, arraysize(kHmac),
> > +                        sizeof(kIv))));
> > +  return encrypted_i_frame_buffer;
> > +}
> > +
> >  ACTION_P(ReturnBuffer, buffer) {
> >    arg0.Run(buffer ? DemuxerStream::kOk : DemuxerStream::kAborted, buffer);
> >  }
> > @@ -76,13 +108,7 @@ class FFmpegVideoDecoderTest : public testing::Test {
> >      end_of_stream_buffer_ = DecoderBuffer::**CreateEOSBuffer();
> >      i_frame_buffer_ = ReadTestDataFile("vp8-I-frame-**320x240");
> >      corrupt_i_frame_buffer_ = ReadTestDataFile("vp8-corrupt-**I-frame");
> > -    {
> > -      scoped_refptr<DecoderBuffer> temp_buffer = ReadTestDataFile(
> > -          "vp8-encrypted-I-frame-**320x240");
> > -      encrypted_i_frame_buffer_ = DecoderBuffer::CopyFrom(
> > -          temp_buffer->GetData() + arraysize(kHmac),
> > -          temp_buffer->GetDataSize() - arraysize(kHmac));
> > -    }
> > +    encrypted_i_frame_buffer_ = ReadEncryptedFrameBuffer();
> >
> >      config_.Initialize(kCodecVP8, VIDEO_CODEC_PROFILE_UNKNOWN,
> >                         kVideoFormat, kCodedSize, kVisibleRect,
> > @@ -211,24 +237,6 @@ class FFmpegVideoDecoderTest : public testing::Test {
> >      message_loop_.RunAllPending();
> >    }
> >
> > -  // Generates a 16 byte CTR counter block. The CTR counter block format
> > is a
> > -  // CTR IV appended with a CTR block counter. |iv| is an 8 byte CTR IV.
> > -  static scoped_array<uint8> GenerateCounterBlock(uint64 iv) {
> > -    scoped_array<uint8> counter_block_data(
> > -        new uint8[DecryptConfig::**kDecryptionKeySize]);
> > -
> > -    // Set the IV.
> > -    memcpy(counter_block_data.get(**), &iv, sizeof(iv));
> > -
> > -    // Set block counter to all 0's.
> > -    memset(counter_block_data.get(**) + sizeof(iv),
> > -           0,
> > -           DecryptConfig::**kDecryptionKeySize - sizeof(iv));
> > -
> > -    return counter_block_data.Pass();
> > -  }
> > -
> > -
> >    MOCK_METHOD2(FrameReady, void(VideoDecoder::**DecoderStatus,
> >                                  const scoped_refptr<VideoFrame>&));
> >
> > @@ -409,20 +417,10 @@ TEST_F(FFmpegVideoDecoderTest,
> > DecodeFrame_SmallerHeight) {
> >    DecodeIFrameThenTestFile("vp8-**I-frame-320x120", 320, 120);
> >  }
> >
> > -// TODO(fgalligan): Enable test when encrypted test data is updated and
> > new
> > -// decryption code is landed. http://crbug.com/132801
> > -TEST_F(**FFmpegVideoDecoderTest, DISABLED_DecodeEncryptedFrame_**Normal)
> > {
> > +TEST_F(**FFmpegVideoDecoderTest, DecodeEncryptedFrame_Normal) {
> >    Initialize();
> >
> >    // Simulate decoding a single encrypted frame.
> > -  scoped_array<uint8> counter_block(**GenerateCounterBlock(kIv));
> > -  encrypted_i_frame_buffer_->**SetDecryptConfig(scoped_ptr<**
> > DecryptConfig>(
> > -      new DecryptConfig(
> > -          kKeyId, arraysize(kKeyId),
> > -          counter_block.get(), DecryptConfig::**kDecryptionKeySize,
> > -          kHmac, arraysize(kHmac),
> > -          sizeof(kIv))));
> > -
> >    EXPECT_CALL(*decryptor_, Decrypt(encrypted_i_frame_**buffer_, _))
> >        .WillRepeatedly(RunDecryptCB(**Decryptor::kSuccess,
> > i_frame_buffer_));
> >
> > @@ -440,14 +438,6 @@ TEST_F(FFmpegVideoDecoderTest, DecodeEncryptedFrame_*
> > *DecryptError) {
> >    Initialize();
> >
> >    // Simulate decoding a single encrypted frame.
> > -  scoped_array<uint8> counter_block(**GenerateCounterBlock(kIv));
> > -  encrypted_i_frame_buffer_->**SetDecryptConfig(scoped_ptr<**
> > DecryptConfig>(
> > -      new DecryptConfig(
> > -          kKeyId, arraysize(kKeyId),
> > -          counter_block.get(), DecryptConfig::**kDecryptionKeySize,
> > -          kHmac, arraysize(kHmac),
> > -          sizeof(kIv))));
> > -
> >    EXPECT_CALL(*demuxer_, Read(_))
> >        .WillRepeatedly(ReturnBuffer(**encrypted_i_frame_buffer_));
> >    EXPECT_CALL(*decryptor_, Decrypt(encrypted_i_frame_**buffer_, _))
> > @@ -471,14 +461,6 @@ TEST_F(FFmpegVideoDecoderTest, DecodeEncryptedFrame_*
> > *CorruptedBufferReturned) {
> >    Initialize();
> >
> >    // Simulate decoding a single encrypted frame.
> > -  scoped_array<uint8> counter_block(**GenerateCounterBlock(kIv));
> > -  encrypted_i_frame_buffer_->**SetDecryptConfig(scoped_ptr<**
> > DecryptConfig>(
> > -      new DecryptConfig(
> > -          kKeyId, arraysize(kKeyId),
> > -          counter_block.get(), DecryptConfig::**kDecryptionKeySize,
> > -          kHmac, arraysize(kHmac),
> > -          sizeof(kIv))));
> > -
> >    EXPECT_CALL(*demuxer_, Read(_))
> >        .WillRepeatedly(ReturnBuffer(**encrypted_i_frame_buffer_));
> >    EXPECT_CALL(*decryptor_, Decrypt(encrypted_i_frame_**buffer_, _))
> >
> >
> >

[scherkus (not reviewing), 2012-07-20 18:10:48]

if we have an integration test w/ a real encrypted web clip + FFVD +
AesDecryptor we can probably ditch the single frame version

[xhwang, 2012-07-20 19:38:14]

On 2012/07/20 18:10:48, scherkus wrote:
> if we have an integration test w/ a real encrypted web clip + FFVD +
> AesDecryptor we can probably ditch the single frame version

I feel these tests still have value for now. It covers the code path
(decryptor_->Decrypt()) with good granularity that can't be achieved with the
integration test. I am just not super happy that we have so many encryption
related code in this test just to create a encrypted frame buffer that we don't
actually use. Fortunately when we separate decryptor from the decoder in the
future these tests will be removed from FFVDTest.

Could you please review this CL as is?

[scherkus (not reviewing), 2012-07-20 19:39:27]

On 2012/07/20 19:38:14, xhwang wrote:
> On 2012/07/20 18:10:48, scherkus wrote:
> > if we have an integration test w/ a real encrypted web clip + FFVD +
> > AesDecryptor we can probably ditch the single frame version
> 
> I feel these tests still have value for now. It covers the code path
> (decryptor_->Decrypt()) with good granularity that can't be achieved with the
> integration test. I am just not super happy that we have so many encryption
> related code in this test just to create a encrypted frame buffer that we
don't
> actually use. Fortunately when we separate decryptor from the decoder in the
> future these tests will be removed from FFVDTest.
> 
> Could you please review this CL as is?

I meant getting rid of the vp8 file and encrypted_i_frame_buffer_ not the tests!

[xhwang, 2012-07-20 19:47:47]

On 2012/07/20 19:39:27, scherkus wrote:
> On 2012/07/20 19:38:14, xhwang wrote:
> > On 2012/07/20 18:10:48, scherkus wrote:
> > > if we have an integration test w/ a real encrypted web clip + FFVD +
> > > AesDecryptor we can probably ditch the single frame version
> > 
> > I feel these tests still have value for now. It covers the code path
> > (decryptor_->Decrypt()) with good granularity that can't be achieved with
the
> > integration test. I am just not super happy that we have so many encryption
> > related code in this test just to create a encrypted frame buffer that we
> don't
> > actually use. Fortunately when we separate decryptor from the decoder in the
> > future these tests will be removed from FFVDTest.
> > 
> > Could you please review this CL as is?
> 
> I meant getting rid of the vp8 file and encrypted_i_frame_buffer_ not the
tests!

That SGTM. But we still need a fake encrypted_i_frame_buffer_ that has a
DecryptConfig to trigger the decrypt_->Decrypt() code path. I'll update the
patch for review later. Thanks!

[xhwang, 2012-07-21 00:07:33]

I introduced a CreateDummyEncryptedBuffer() function to create a dummy encrypted
DecoderBuffer to trigger the decryption code path for testing. PTAL!

[fgalligan1, 2012-07-21 00:30:00]

On 2012/07/21 00:07:33, xhwang wrote:
> I introduced a CreateDummyEncryptedBuffer() function to create a dummy
encrypted
> DecoderBuffer to trigger the decryption code path for testing. PTAL!

LGTM

If this lands then I will delete http://codereview.chromium.org/10797048/

[scherkus (not reviewing), 2012-07-21 01:11:35]

lgtm w/ naming nit

http://codereview.chromium.org/10800057/diff/7001/media/filters/ffmpeg_video_...
File media/filters/ffmpeg_video_decoder_unittest.cc (right):

http://codereview.chromium.org/10800057/diff/7001/media/filters/ffmpeg_video_...
media/filters/ffmpeg_video_decoder_unittest.cc:40: static const uint8
kDummyKeyId[] = { 0x4b, 0x65, 0x79, 0x20, 0x49, 0x44 };
nit: use Fake instead -- we don't use Dummy very much in our test code

here + below

[commit-bot: I haz the power, 2012-07-21 01:20:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/10800057/5

[commit-bot: I haz the power, 2012-07-21 01:20:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/10800057/5

[commit-bot: I haz the power, 2012-07-21 01:40:03]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is android, revision is 147754, job name
was 10800057-5 (previous was lost) (previous was lost) (previous was lost)
(previous was lost).

[commit-bot: I haz the power, 2012-07-21 05:01:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/10800057/5

[commit-bot: I haz the power, 2012-07-21 05:01:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/10800057/5

[commit-bot: I haz the power, 2012-07-21 05:20:58]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is android, revision is 147776, job name
was 10800057-5 (previous was lost) (previous was lost) (previous was lost)
(previous was lost).

[commit-bot: I haz the power, 2012-07-21 16:57:14]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/10800057/5

[commit-bot: I haz the power, 2012-07-21 16:57:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/10800057/5

[commit-bot: I haz the power, 2012-07-21 17:16:47]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is android, revision is 147799, job name
was 10800057-5 (previous was lost) (previous was lost) (previous was lost)
(previous was lost).

[commit-bot: I haz the power, 2012-07-23 15:36:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/10800057/12002

[commit-bot: I haz the power, 2012-07-23 15:36:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/10800057/12002

[commit-bot: I haz the power, 2012-07-23 15:55:06]

The commit queue went berserk retrying too often for a
seemingly flaky test. Builder is android, revision is 147854, job name
was 10800057-12002 (previous was lost) (previous was lost) (previous was lost)
(previous was lost).

[commit-bot: I haz the power, 2012-07-23 16:07:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/xhwang@chromium.org/10800057/4005

[commit-bot: I haz the power, 2012-07-23 17:14:08]

Change committed as 147868