New issue
Advanced search Search tips

Issue 693813 link

Starred by 1 user
Status: Fixed
Owner:
xhw...@chromium.org
Closed: Feb 2017
Cc:
ddorwin@chromium.org
jrumm...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Race condition between setMediaKeys and load()

Project Member Reported by xhw...@chromium.org, Feb 18 2017 
Chrome Version: M58
OS: ALL

Steps to repro:
0) video.load()
1) video.setMediaKeys(a)
2) without waiting for setMediaKeys, call video.load()

Expected results:
video.load() could destroy the WebMediaPlayer, where (1) is still pending. So the promise of (1) should be rejected.

Actual result:
The promise of (1) is never fulfilled.

Comment 1 by xhw...@chromium.org, Feb 18 2017 

Thought about this more. I think this case should be the same as setMediaKeys() is called before load(), where we don't have a webMediaKeys(). So the promise of (1) should simply be resolved.

Comment 2 by ddorwin@chromium.org, Feb 24 2017 

I think there is a missing step 0 that is video.load().

I believe comment 1 is referring to the case that is just steps 1 and 2 (and there is no WebMediaPlayer to destroy.

While I think the spec might allow options, preferring the successful path and matching the scenario in comment 1 makes sense.

Comment 3 by xhw...@chromium.org, Feb 24 2017

Description: Show this description
Project Member

Comment 4 by bugdroid1@chromium.org, Feb 24 2017 

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

commit 5113973bc4260b94300d81502935d5ebab57a1d0
Author: xhwang <xhwang@chromium.org>
Date: Fri Feb 24 19:36:08 2017

media: Resolve pending SetCdm() when WebMediaPlayerImpl is destructed

When WebMediaPlayerImpl is destructed, if there is a pending SetCdm()
call, we should resolve the promise so that the CDM can still be set on
the HTMLMediaElement. If we load() again, that CDM will be used as the
initial CDM for the new WebMediaPlayerImpl created.

BUG= 693813 
TEST=Added a new layout test.

Review-Url: https://codereview.chromium.org/2703993002
Cr-Commit-Position: refs/heads/master@{#452905}

[modify] https://crrev.com/5113973bc4260b94300d81502935d5ebab57a1d0/media/blink/webmediaplayer_impl.cc
[modify] https://crrev.com/5113973bc4260b94300d81502935d5ebab57a1d0/media/renderers/renderer_impl.cc
[add] https://crrev.com/5113973bc4260b94300d81502935d5ebab57a1d0/third_party/WebKit/LayoutTests/media/encrypted-media/encrypted-media-reset-src-during-setmediakeys.html
[modify] https://crrev.com/5113973bc4260b94300d81502935d5ebab57a1d0/third_party/WebKit/Source/modules/encryptedmedia/HTMLMediaElementEncryptedMedia.cpp

Comment 5 by xhw...@chromium.org, Feb 24 2017

Status: Fixed (was: Started)
►

Sign in to add a comment