Add unit tests for config changes involving encrypted streams.

Add unit tests for config changes involving encrypted streams.

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

[ddorwin, 2013-01-07 18:54:39]

WebM only for now. Will add others when we have test files.

https://codereview.chromium.org/11785023/diff/4001/media/filters/pipeline_int...
File media/filters/pipeline_integration_test.cc (left):

https://codereview.chromium.org/11785023/diff/4001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:312: ASSERT_EQ(pipeline_status_,
PIPELINE_OK);
This check was meaningless since any error would posted and only picked up after
the message loop runs.

[ddorwin, 2013-01-07 18:59:48]

I added these while debugging config change issues and tests. I did not see any
existing logging of when new configs were detected or when an actual change
occurred (other than in one of the decoders).

[ddorwin, 2013-01-07 19:03:55]

Oops, that previous comment was for a different CL. Please ignore.

[acolwell GONE FROM CHROMIUM, 2013-01-07 21:17:50]

LGTM % nits

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

https://codereview.chromium.org/11785023/diff/4001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:36: // They do not exercise the
Decrypting{Audio|Video}Decoder path.
nit: Add a TODO for exercising the decrypting decoder paths?

https://codereview.chromium.org/11785023/diff/4001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:377:
MediaSource_ConfigChange_EncryptedThenClear_WebM) {
nit: Isn't this testing "clear then encrypted" and not "encrypted then clear"?

https://codereview.chromium.org/11785023/diff/4001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:409:
MediaSource_ConfigChange_ClearThenEncrypted_WebM) {
ditto. Isn't this name backwards?

[ddorwin, 2013-01-07 22:19:29]

Fixed naming and converted times to constants to make tests easier to
understand.

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

https://codereview.chromium.org/11785023/diff/4001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:36: // They do not exercise the
Decrypting{Audio|Video}Decoder path.
On 2013/01/07 21:17:50, acolwell wrote:
> nit: Add a TODO for exercising the decrypting decoder paths?

shadi@ is working on browser tests that will do this. Testing those paths here
would require a lot of work to use essentially External Clear Key.

https://codereview.chromium.org/11785023/diff/4001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:377:
MediaSource_ConfigChange_EncryptedThenClear_WebM) {
On 2013/01/07 21:17:50, acolwell wrote:
> nit: Isn't this testing "clear then encrypted" and not "encrypted then clear"?

Done. Copy-paste-edit mistake. Thanks.

https://codereview.chromium.org/11785023/diff/4001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:409:
MediaSource_ConfigChange_ClearThenEncrypted_WebM) {
On 2013/01/07 21:17:50, acolwell wrote:
> ditto. Isn't this name backwards?

Done.

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

https://codereview.chromium.org/11785023/diff/9001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:35: static const int AppendTimeSec =
2;
acolwell: Any problems dropping this to 1 to speed up the tests?

[acolwell GONE FROM CHROMIUM, 2013-01-07 22:21:57]

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

https://codereview.chromium.org/11785023/diff/9001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:35: static const int AppendTimeSec =
2;
On 2013/01/07 22:19:29, ddorwin wrote:
> acolwell: Any problems dropping this to 1 to speed up the tests?
No. I have no problems w/ that.

[ddorwin, 2013-01-07 23:34:53]

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

https://codereview.chromium.org/11785023/diff/9001/media/filters/pipeline_int...
media/filters/pipeline_integration_test.cc:35: static const int AppendTimeSec =
2;
On 2013/01/07 22:21:57, acolwell wrote:
> On 2013/01/07 22:19:29, ddorwin wrote:
> > acolwell: Any problems dropping this to 1 to speed up the tests?
> No. I have no problems w/ that.

Done.

[commit-bot: I haz the power, 2013-01-07 23:46:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11785023/10001

[xhwang, 2013-01-08 00:11:05]

lgtm

[commit-bot: I haz the power, 2013-01-08 18:09:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11785023/10001

[commit-bot: I haz the power, 2013-01-08 18:09:43]

Change committed as 175552