New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 684256 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

WMPI: Check failed: IsHidden() in PauseVideoIfNeeded()

Project Member Reported by w...@chromium.org, Jan 24 2017

Issue description

I saw this when closing a tab.

[FATAL:webmediaplayer_impl.cc(2192)] Check failed: IsHidden().

Stack Trace:
RELADDR   FUNCTION                                                                                                                                                                                                                             0009d637  logging::LogMessage::~LogMessage()                                                                                                                                                                                                   00039cd1  media::WebMediaPlayerImpl::PauseVideoIfNeeded()                                                                                                                 
0003cb8b  media::WebMediaPlayerImpl::OnFrameHidden()                                                                                                                                                                                           00accbab  media::RendererWebMediaPlayerDelegate::WasHidden()                                                                                                                                                                                   00ae7fd7  content::RenderFrameImpl::WasHidden()                                                                                                                                              
00b12659  content::RenderWidget::OnWasHidden()                                                                                                                                                                                                 v------>  bool IPC::MessageT<InputMsg_RequestTextInputStateUpdate_Meta, std::__ndk1::tuple<>, void>::Dispatch<content::RenderWidget, content::RenderWidget, void, void (content::RenderWidget::*)()>(IPC::Message const*, content::RenderWidg  00b158e1  content::RenderWidget::OnMessageReceived(IPC::Message const&)                                                                                                                                          
00b0c0b3  content::RenderViewImpl::OnMessageReceived(IPC::Message const&)                                                                                                                                                                      004df48f  content::ChildThreadImpl::ChildThreadMessageRouter::RouteMessage(IPC::Message const&)                                                                                                                                                004dc6fb  content::ChildThreadImpl::OnMessageReceived(IPC::Message const&)                                                                                                                                       



 
Cc: sande...@chromium.org
Owner: avayvod@chromium.org
Seems like Dan's suspicion was correct and we can get frame hidden callback when the frame is being closed.
Labels: M-57
Dan, do you think we should just exit early in OnFrameHidden() if delegate_->IsFrameClosed()?
No, we still want to run UpdatePlayState() so that the results are correct no matter which order the two methods are called. We should just explicitly check IsHidden() before calling PauseVideoIfNeeded().
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 25 2017

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

commit 5f91d1754a7ffa39cff9619db090b91b8d1b4a98
Author: avayvod <avayvod@chromium.org>
Date: Wed Jan 25 00:11:03 2017

[Video] Fix DCHECK crash in PauseVideoIfHidden

WMPI::OnFrameHidden() can get called when the frame is closed.
WMPI::IsHidden() will return false in this case, however it's assumed to
return true in OnFrameHidden and the methods called from there.

BUG= 684256 
TEST=manual

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

[modify] https://crrev.com/5f91d1754a7ffa39cff9619db090b91b8d1b4a98/media/blink/webmediaplayer_impl.cc

Labels: Merge-Request-57
Status: Started (was: Assigned)

Comment 6 by gov...@chromium.org, Jan 25 2017

Is this change applicable to all OSes or any specific OS?
The feature affected is enabled by default on Android and is behind the flag on other platforms (iOS excluded).
I don't think there's anything to verify here though.

Comment 8 by gov...@chromium.org, Jan 25 2017

Labels: OS-Android
Applying OS=Android as feature is enabled by default on Android.
Project Member

Comment 9 by sheriffbot@chromium.org, Jan 25 2017

Labels: -Merge-Request-57 Hotlist-Merge-Approved Merge-Approved-57
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions.
Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by sheriffbot@chromium.org, Jan 30 2017

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 11 by bugdroid1@chromium.org, Jan 30 2017

Labels: -merge-approved-57 merge-merged-2987
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/276c7a24aa74d92c9b0139e11850dd128f121528

commit 276c7a24aa74d92c9b0139e11850dd128f121528
Author: Anton Vayvod <avayvod@chromium.org>
Date: Mon Jan 30 17:53:25 2017

[Video] Fix DCHECK crash in PauseVideoIfHidden

WMPI::OnFrameHidden() can get called when the frame is closed.
WMPI::IsHidden() will return false in this case, however it's assumed to
return true in OnFrameHidden and the methods called from there.

BUG= 684256 
TEST=manual

Review-Url: https://codereview.chromium.org/2653553005
Cr-Commit-Position: refs/heads/master@{#445869}
(cherry picked from commit 5f91d1754a7ffa39cff9619db090b91b8d1b4a98)

Review-Url: https://codereview.chromium.org/2666543003 .
Cr-Commit-Position: refs/branch-heads/2987@{#168}
Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943}

[modify] https://crrev.com/276c7a24aa74d92c9b0139e11850dd128f121528/media/blink/webmediaplayer_impl.cc

Status: Fixed (was: Started)

Sign in to add a comment