New issue
Advanced search Search tips

Issue 893830 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Switching from non-MS PiP'ed Video to a MS Video causes crash

Project Member Reported by lethalantidote@chromium.org, Oct 9

Issue description

Chrome Version: 71.0.3576.0 (Developer Build) (64-bit)
OS: Linux

What steps will reproduce the problem?
(1) Go to https://beaufortfrancois.github.io/sandbox/media/picture-in-picture-playground with SurfaceLayerMode = kAlways, or kOnDemand
(2) Toggle PiP on given video (should be non-MediaStream)
(3) Toggle MediaStream

What is the expected result?
We switch over to the MediaStream video

What happens instead?
Crash

[58234:58234:1009/155329.969994:FATAL:picture_in_picture_window_controller_impl.cc(160)] Check failed: media_player_id_.has_value(). 
#0 0x7f3d140cad6d base::debug::StackTrace::StackTrace()
#1 0x7f3d13dd913a base::debug::StackTrace::StackTrace()
#2 0x7f3d13e436ce logging::LogMessage::~LogMessage()
#3 0x7f3d0cf6f8b1 content::PictureInPictureWindowControllerImpl::UpdatePlaybackState()
#4 0x7f3d0ce7aa5a content::MediaWebContentsObserver::OnMediaPaused()
#5 0x7f3d0ce7ee5c _ZN3IPC20DispatchToMethodImplIN7content24MediaWebContentsObserverEMS2_FvPNS1_15RenderFrameHostEibES3_NSt3__15tupleIJibEEEJLm0ELm1EEEEvPT_T0_PT1_OT2_NS7_16integer_sequenceImJXspT3_EEEE
#6 0x7f3d0ce7ed60 _ZN3IPC16DispatchToMethodIN7content24MediaWebContentsObserverENS1_15RenderFrameHostEJibENSt3__15tupleIJibEEEEENS4_9enable_ifIXeqsZT1_sr3std10tuple_sizeINS4_5decayIT2_E4typeEEE5valueEvE4typeEPT_MSE_FvPT0_DpT1_ESH_OS9_
#7 0x7f3d0ce7cd00 _ZN3IPC8MessageTI45MediaPlayerDelegateHostMsg_OnMediaPaused_MetaNSt3__15tupleIJibEEEvE8DispatchIN7content24MediaWebContentsObserverES8_NS7_15RenderFrameHostEMS8_FvPS9_ibEEEbPKNS_7MessageEPT_PT0_PT1_T2_
#8 0x7f3d0ce7a635 content::MediaWebContentsObserver::OnMessageReceived()
#9 0x7f3d0d53272c content::WebContentsImpl::OnMessageReceived()
#10 0x7f3d0cacb7fc content::RenderFrameHostImpl::OnMessageReceived()
#11 0x7f3d0d1439a3 content::RenderProcessHostImpl::OnMessageReceived()
#12 0x7f3d107fcd25 IPC::ChannelProxy::Context::OnDispatchMessage()
#13 0x7f3d10802acf _ZN4base8internal13FunctorTraitsIMN3IPC12ChannelProxy7ContextEFvRKNS2_7MessageEEvE6InvokeIS9_RK13scoped_refptrIS4_EJS7_EEEvT_OT0_DpOT1_
#14 0x7f3d10802a2f _ZN4base8internal12InvokeHelperILb0EvE8MakeItSoIRKMN3IPC12ChannelProxy7ContextEFvRKNS4_7MessageEEJRK13scoped_refptrIS6_ES9_EEEvOT_DpOT0_
#15 0x7f3d108029bd _ZN4base8internal7InvokerINS0_9BindStateIMN3IPC12ChannelProxy7ContextEFvRKNS3_7MessageEEJ13scoped_refptrIS5_ES6_EEEFvvEE7RunImplIRKSA_RKNSt3__15tupleIJSC_S6_EEEJLm0ELm1EEEEvOT_OT0_NSJ_16integer_sequenceImJXspT1_EEEE
#16 0x7f3d108028bc _ZN4base8internal7InvokerINS0_9BindStateIMN3IPC12ChannelProxy7ContextEFvRKNS3_7MessageEEJ13scoped_refptrIS5_ES6_EEEFvvEE3RunEPNS0_13BindStateBaseE
#17 0x7f3d13d8a21e _ZNO4base12OnceCallbackIFvvEE3RunEv
#18 0x7f3d13dda66a base::debug::TaskAnnotator::RunTask()
#19 0x7f3d13e69918 base::MessageLoop::RunTask()
#20 0x7f3d13e69c1b base::MessageLoop::DeferOrRunPendingTask()
#21 0x7f3d13e6a064 base::MessageLoop::DoWork()
#22 0x7f3d1411a5c9 base::MessagePumpLibevent::Run()
#23 0x7f3d13e690ee base::MessageLoop::Run()
#24 0x7f3d13f11bd2 base::RunLoop::Run()
#25 0x55916015f4be ChromeBrowserMainParts::MainMessageLoopRun()
#26 0x7f3d0c55046a content::BrowserMainLoop::RunMainMessageLoopParts()
#27 0x7f3d0c558cc0 content::BrowserMainRunnerImpl::Run()
#28 0x7f3d0c54356e content::BrowserMain()
#29 0x7f3d0e772a90 content::RunBrowserProcessMain()
#30 0x7f3d0e77575a content::ContentMainRunnerImpl::Run()
#31 0x7f3d0e76a38c content::ContentServiceManagerMainDelegate::RunEmbedderProcess()
#32 0x7f3d143f07d1 service_manager::Main()
#33 0x7f3d0e770505 content::ContentMain()
#34 0x55915caa5c26 ChromeMain
#35 0x55915caa5b32 main
#36 0x7f3ce57b62b1 __libc_start_main
#37 0x55915caa5a0a _start





 
I can reproduce.
I've applied WIP CL at https://chromium-review.googlesource.com/c/chromium/src/+/1244361 and it is fixed.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 10

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

commit c0dc584775e65fc731a62ccbbb1dceb09fc4a86e
Author: François Beaufort <beaufort.francois@gmail.com>
Date: Wed Oct 10 18:58:12 2018

Picture-in-Picture: check that pip_player matches player_id.

This makes sure we don't reset media_player_id_ when switching
non-Mediastream Picture-in-Picture video to a Mediastream video.

Bug:  872066 ,  893830 
Change-Id: Ie84b51ae1f7e0ac822743dfeb948bea6afd076ba
Reviewed-on: https://chromium-review.googlesource.com/c/1273515
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#598428}
[modify] https://crrev.com/c0dc584775e65fc731a62ccbbb1dceb09fc4a86e/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc
[modify] https://crrev.com/c0dc584775e65fc731a62ccbbb1dceb09fc4a86e/chrome/test/data/media/picture-in-picture/window-size.html
[modify] https://crrev.com/c0dc584775e65fc731a62ccbbb1dceb09fc4a86e/content/browser/media/media_web_contents_observer.cc

Cc: -fbeaufort@chromium.org mlamouri@chromium.org
Owner: fbeaufort@chromium.org
Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Oct 11

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ebacf403c9b00332746b18d085a25061078a29ad

commit ebacf403c9b00332746b18d085a25061078a29ad
Author: François Beaufort <beaufort.francois@gmail.com>
Date: Thu Oct 11 20:16:03 2018

Picture-in-Picture: check that pip_player matches player_id.

This makes sure we don't reset media_player_id_ when switching
non-Mediastream Picture-in-Picture video to a Mediastream video.

Bug:  872066 ,  893830 
Change-Id: Ie84b51ae1f7e0ac822743dfeb948bea6afd076ba
Reviewed-on: https://chromium-review.googlesource.com/c/1273515
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Original-Commit-Position: refs/heads/master@{#598428}(cherry picked from commit c0dc584775e65fc731a62ccbbb1dceb09fc4a86e)
Reviewed-on: https://chromium-review.googlesource.com/c/1277985
Reviewed-by: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/branch-heads/3538@{#968}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/ebacf403c9b00332746b18d085a25061078a29ad/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc
[modify] https://crrev.com/ebacf403c9b00332746b18d085a25061078a29ad/chrome/test/data/media/picture-in-picture/window-size.html
[modify] https://crrev.com/ebacf403c9b00332746b18d085a25061078a29ad/content/browser/media/media_web_contents_observer.cc

Labels: Merge-Merged-70-3538
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/ebacf403c9b00332746b18d085a25061078a29ad

Commit: ebacf403c9b00332746b18d085a25061078a29ad
Author: beaufort.francois@gmail.com
Commiter: beaufort.francois@gmail.com
Date: 2018-10-11 20:16:03 +0000 UTC

Picture-in-Picture: check that pip_player matches player_id.

This makes sure we don't reset media_player_id_ when switching
non-Mediastream Picture-in-Picture video to a Mediastream video.

Bug:  872066 ,  893830 
Change-Id: Ie84b51ae1f7e0ac822743dfeb948bea6afd076ba
Reviewed-on: https://chromium-review.googlesource.com/c/1273515
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Original-Commit-Position: refs/heads/master@{#598428}(cherry picked from commit c0dc584775e65fc731a62ccbbb1dceb09fc4a86e)
Reviewed-on: https://chromium-review.googlesource.com/c/1277985
Reviewed-by: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/branch-heads/3538@{#968}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}

Sign in to add a comment