New issue
Advanced search Search tips

Issue 730766 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug

Blocking:
issue 403462



Sign in to add a comment

ECKEncryptedMediaTest.CDMCrashDuringDecode test fails with mojo CDM

Project Member Reported by xhw...@chromium.org, Jun 7 2017

Issue description

[ RUN      ] Mojo/ECKEncryptedMediaTest.CDMCrashDuringDecode/0
...

../../chrome/browser/media/media_browsertest.cc:57: Failure
      Expected: expected_title
      Which is: "EME_SESSION_CLOSED_AND_ERROR"
To be equal to: final_title
      Which is: "ENDED"
Still waiting for the following processes to finish:
        out/GN/browser_tests --gtest_also_run_disabled_tests --gtest_filter=Mojo/ECKEncryptedMediaTest.CDMCrashDuringDecode/0 --single_process --user-data-dir=/tmp/.com.google.Chrome.Rj69Yf/dDKIkqM
[  FAILED  ] Mojo/ECKEncryptedMediaTest.CDMCrashDuringDecode/0, where GetParam() = 4-byte object <01-00 00-00> (13860 ms)
[----------] 1 test from Mojo/ECKEncryptedMediaTest (13860 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test case ran. (13863 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 1 test, listed below:
[  FAILED  ] Mojo/ECKEncryptedMediaTest.CDMCrashDuringDecode/0, where GetParam() = 4-byte object <01-00 00-00>

 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/689cd428394fe4065d040bb56e24df681f634cd5

commit 689cd428394fe4065d040bb56e24df681f634cd5
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Fri Aug 04 07:54:55 2017

media: Add kError to DemuxerStream::Status

Some DemuxerStream could hit non-recoverable fatal errors. For example,
DecryptingDemuxerStream cannot decrypt a buffer because the CDM was
crashed.

This CL adds a new kError status to DemuxerStream::Status to indicate
such conditions.

BUG= 730766 
TEST=New unittests added.

Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ia07e14662d7da852f712076dcc94aed1900aaba3
Reviewed-on: https://chromium-review.googlesource.com/598702
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org>
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491960}
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/base/demuxer_stream.h
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/base/fake_demuxer_stream.cc
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/base/fake_demuxer_stream.h
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/base/fake_demuxer_stream_unittest.cc
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/filters/decoder_stream.cc
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/filters/decrypting_audio_decoder_unittest.cc
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/filters/decrypting_demuxer_stream.cc
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/filters/decrypting_demuxer_stream_unittest.cc
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/filters/video_frame_stream_unittest.cc
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/remoting/demuxer_stream_adapter.cc
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/remoting/proto_enum_utils.cc
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/remoting/rpc.proto
[modify] https://crrev.com/689cd428394fe4065d040bb56e24df681f634cd5/media/remoting/stream_provider.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f492a4e2b9bdb627ee9921a1f54fcf0eb31260aa

commit f492a4e2b9bdb627ee9921a1f54fcf0eb31260aa
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Fri Aug 04 19:40:17 2017

media: Enable CDMCrashDuringDecode test using Mojo CDM

The test was failing for multiple reasons.

First, previously InitializeCdmModule() was not called, so that FFmpeg
wasn't properly initialized. As a result, FFmpegCdmAudioDecoder could
not be initialized to do decrypt-and-decode and hence we fallback
to do decrypt-only. However, only when we do decrypt-and-decode will the
expected crash happen. This was fixed by adding CdmModule in a previous
CL.

Second, the test expects a media element decode error, and the session
to be closed. However, there could be a race condition between session
creation and the DecryptAndDecodeAudio() call which causes the crash.
If the crash happens before the session is created, the test will fail
because no session will be closed. This is fixed by delaying the crash
until a session has been created.

Third, MojoDecryptor binds callbacks directly into the mojo calls. When
crash happens, mojo connection will be lost and those callbacks will be
dropped silently and never run. This is fixed using ScopedCallbackRunner
which will run bound callbacks when they are dropped.

Fourth, after the crash DecryptingAudioDecoder will report a decode
error. Then DecoderStream could try to fallback to decrypt-only using
DecryptingDemuxerStream and FFmpegAudioDecoder. Since the CDM process
has crashed, the Decrypt() call will immediately hit a decrypt error.
However, DecryptingDemuxerStream translates this into a kAborted status
because DemuxerStream::Status didn't have a way to indicate error. This
has been fixed in a separate CL which will be landed before this CL.

With all the fixes, this CL enables the test using mojo CDM.

BUG= 730766 
TEST=Enable a disabled test.

Change-Id: Idafc7cfc25de1228b78cd17ca6c30ec710cc4cb3
Reviewed-on: https://chromium-review.googlesource.com/598706
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Reviewed-by: Frank Liberato <liberato@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492103}
[modify] https://crrev.com/f492a4e2b9bdb627ee9921a1f54fcf0eb31260aa/chrome/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/f492a4e2b9bdb627ee9921a1f54fcf0eb31260aa/media/base/scoped_callback_runner.h
[modify] https://crrev.com/f492a4e2b9bdb627ee9921a1f54fcf0eb31260aa/media/cdm/ppapi/external_clear_key/clear_key_cdm.cc
[modify] https://crrev.com/f492a4e2b9bdb627ee9921a1f54fcf0eb31260aa/media/mojo/clients/mojo_cdm.cc
[modify] https://crrev.com/f492a4e2b9bdb627ee9921a1f54fcf0eb31260aa/media/mojo/clients/mojo_decryptor.cc
[modify] https://crrev.com/f492a4e2b9bdb627ee9921a1f54fcf0eb31260aa/media/mojo/clients/mojo_decryptor.h

Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6ed7cf171a940bbe589e27f540340dbe812d769e

commit 6ed7cf171a940bbe589e27f540340dbe812d769e
Author: Takashi Sakamoto <tasak@google.com>
Date: Mon Aug 07 11:03:51 2017

Revert "media: Enable CDMCrashDuringDecode test using Mojo CDM"

This reverts commit f492a4e2b9bdb627ee9921a1f54fcf0eb31260aa.

Reason for revert:
browser_tests browser_tests
Run on OS: 'Windows-7-SP1'
failures:
Mojo/ECKEncryptedMediaTest.CDMCrashDuringDecode/0 (because of timeout)


Original change's description:
> media: Enable CDMCrashDuringDecode test using Mojo CDM
> 
> The test was failing for multiple reasons.
> 
> First, previously InitializeCdmModule() was not called, so that FFmpeg
> wasn't properly initialized. As a result, FFmpegCdmAudioDecoder could
> not be initialized to do decrypt-and-decode and hence we fallback
> to do decrypt-only. However, only when we do decrypt-and-decode will the
> expected crash happen. This was fixed by adding CdmModule in a previous
> CL.
> 
> Second, the test expects a media element decode error, and the session
> to be closed. However, there could be a race condition between session
> creation and the DecryptAndDecodeAudio() call which causes the crash.
> If the crash happens before the session is created, the test will fail
> because no session will be closed. This is fixed by delaying the crash
> until a session has been created.
> 
> Third, MojoDecryptor binds callbacks directly into the mojo calls. When
> crash happens, mojo connection will be lost and those callbacks will be
> dropped silently and never run. This is fixed using ScopedCallbackRunner
> which will run bound callbacks when they are dropped.
> 
> Fourth, after the crash DecryptingAudioDecoder will report a decode
> error. Then DecoderStream could try to fallback to decrypt-only using
> DecryptingDemuxerStream and FFmpegAudioDecoder. Since the CDM process
> has crashed, the Decrypt() call will immediately hit a decrypt error.
> However, DecryptingDemuxerStream translates this into a kAborted status
> because DemuxerStream::Status didn't have a way to indicate error. This
> has been fixed in a separate CL which will be landed before this CL.
> 
> With all the fixes, this CL enables the test using mojo CDM.
> 
> BUG= 730766 
> TEST=Enable a disabled test.
> 
> Change-Id: Idafc7cfc25de1228b78cd17ca6c30ec710cc4cb3
> Reviewed-on: https://chromium-review.googlesource.com/598706
> Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#492103}

TBR=xhwang@chromium.org,liberato@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  730766 
Change-Id: Icf1936983d2a5a9147a11e40dfea928a4a3c0b6c
Reviewed-on: https://chromium-review.googlesource.com/602846
Reviewed-by: Takashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#492294}
[modify] https://crrev.com/6ed7cf171a940bbe589e27f540340dbe812d769e/chrome/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/6ed7cf171a940bbe589e27f540340dbe812d769e/media/base/scoped_callback_runner.h
[modify] https://crrev.com/6ed7cf171a940bbe589e27f540340dbe812d769e/media/cdm/ppapi/external_clear_key/clear_key_cdm.cc
[modify] https://crrev.com/6ed7cf171a940bbe589e27f540340dbe812d769e/media/mojo/clients/mojo_cdm.cc
[modify] https://crrev.com/6ed7cf171a940bbe589e27f540340dbe812d769e/media/mojo/clients/mojo_decryptor.cc
[modify] https://crrev.com/6ed7cf171a940bbe589e27f540340dbe812d769e/media/mojo/clients/mojo_decryptor.h

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ba41c7f701550f29486bf793fd59dde77128fee9

commit ba41c7f701550f29486bf793fd59dde77128fee9
Author: Vasilii Sukhanov <vasilii@chromium.org>
Date: Mon Aug 07 11:04:25 2017

Revert "media: Enable CDMCrashDuringDecode test using Mojo CDM"

This reverts commit f492a4e2b9bdb627ee9921a1f54fcf0eb31260aa.

Reason for revert: The test fails reliably by timeout on Win (https://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20(dbg)(1))

Original change's description:
> media: Enable CDMCrashDuringDecode test using Mojo CDM
> 
> The test was failing for multiple reasons.
> 
> First, previously InitializeCdmModule() was not called, so that FFmpeg
> wasn't properly initialized. As a result, FFmpegCdmAudioDecoder could
> not be initialized to do decrypt-and-decode and hence we fallback
> to do decrypt-only. However, only when we do decrypt-and-decode will the
> expected crash happen. This was fixed by adding CdmModule in a previous
> CL.
> 
> Second, the test expects a media element decode error, and the session
> to be closed. However, there could be a race condition between session
> creation and the DecryptAndDecodeAudio() call which causes the crash.
> If the crash happens before the session is created, the test will fail
> because no session will be closed. This is fixed by delaying the crash
> until a session has been created.
> 
> Third, MojoDecryptor binds callbacks directly into the mojo calls. When
> crash happens, mojo connection will be lost and those callbacks will be
> dropped silently and never run. This is fixed using ScopedCallbackRunner
> which will run bound callbacks when they are dropped.
> 
> Fourth, after the crash DecryptingAudioDecoder will report a decode
> error. Then DecoderStream could try to fallback to decrypt-only using
> DecryptingDemuxerStream and FFmpegAudioDecoder. Since the CDM process
> has crashed, the Decrypt() call will immediately hit a decrypt error.
> However, DecryptingDemuxerStream translates this into a kAborted status
> because DemuxerStream::Status didn't have a way to indicate error. This
> has been fixed in a separate CL which will be landed before this CL.
> 
> With all the fixes, this CL enables the test using mojo CDM.
> 
> BUG= 730766 
> TEST=Enable a disabled test.
> 
> Change-Id: Idafc7cfc25de1228b78cd17ca6c30ec710cc4cb3
> Reviewed-on: https://chromium-review.googlesource.com/598706
> Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
> Reviewed-by: Frank Liberato <liberato@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#492103}

TBR=xhwang@chromium.org,liberato@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  730766 
Change-Id: I90bee67719ea9f6cc10de2cdec48cb40eb8a48fe
Reviewed-on: https://chromium-review.googlesource.com/603287
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Commit-Queue: Vasilii Sukhanov <vasilii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492295}

Status: Available (was: Fixed)
I can repro the timeout locally. When the crash happens, a popup dialog containing the stacktrace is shown. If I close the dialog by clicking the button, the test passes. Otherwise, the test will timeout. This might explain why the test is failing on the bot.

However, the pepper version of the same test passes on the bot, but timeouts for the same reason on my local build. I don't know why that is the case.

For now, I'll just disable the test for mojo+Win, and will revisit this when I have time.

Mark this issue as available to keep track of this.
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 8 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/415e4627a277230b5b0264fe14c843e16156c41f

commit 415e4627a277230b5b0264fe14c843e16156c41f
Author: Xiaohan Wang <xhwang@chromium.org>
Date: Tue Aug 08 19:26:04 2017

(reland) media: Enable CDMCrashDuringDecode test using Mojo CDM

This reverts commit 6ed7cf171a940bbe589e27f540340dbe812d769e which
reverts the original CL.

The test was failing on Windows because when the CDM crashes
(intentionally), the OS shows a popup dialog showing the callstack. If
no action was made to dismiss the dialog, errors will not be forwarded
to the render process and the test will timeout. If I dismiss the dialog
the test passes immediately.

I can reproduce the same issue on the Pepper version of the test, but
that test is passing on the bot which I do not know why. One possibility
is that the pepper CDM is running in a sandboxed PPAPI process while the
mojo CDM is running in an unsandboxed utility process.

At this moment I'll just disable the mojo version of the test on Windows
bots. When we run the mojo CDM in a sandboxed utility process I'll
revisit this issue.

BUG= 730766 
TEST=Disables the test on Windows.

Change-Id: Ib6d60836e7b0695c98aef7eb1c0d12faca7704ad
Reviewed-on: https://chromium-review.googlesource.com/604694
Reviewed-by: John Rummell <jrummell@chromium.org>
Commit-Queue: Xiaohan Wang <xhwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492725}
[modify] https://crrev.com/415e4627a277230b5b0264fe14c843e16156c41f/chrome/browser/media/encrypted_media_browsertest.cc
[modify] https://crrev.com/415e4627a277230b5b0264fe14c843e16156c41f/media/base/scoped_callback_runner.h
[modify] https://crrev.com/415e4627a277230b5b0264fe14c843e16156c41f/media/cdm/ppapi/external_clear_key/clear_key_cdm.cc
[modify] https://crrev.com/415e4627a277230b5b0264fe14c843e16156c41f/media/mojo/clients/mojo_cdm.cc
[modify] https://crrev.com/415e4627a277230b5b0264fe14c843e16156c41f/media/mojo/clients/mojo_decryptor.cc
[modify] https://crrev.com/415e4627a277230b5b0264fe14c843e16156c41f/media/mojo/clients/mojo_decryptor.h

Status: Fixed (was: Available)
I filed  issue 770748  to track the timeout on Windows.

Sign in to add a comment