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

Issue 845747 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
no longer active
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

DCHECK for "invalid" LocalSurfaceId when resizing PIP window

Project Member Reported by mlamouri@chromium.org, May 23 2018

Issue description

STR:
 1. Enable picture-in-picture and video surface layer in chrome://flags
( chrome://flags/#enable-surfaces-for-videos chrome://flags/#enable-picture-in-picture )
 2. Go to https://mounirlamouri.github.io/sandbox/media/dynamic-controls.html
 3. In video menu, select "picture-in-picture"
 4. Resize the picture-in-picture window

Expected result: window resizes
Actual result: window freezes for a short time and the browser crashes

Stack:

[59598:59598:0522/200813.563243:FATAL:window_tree_host.cc(65)] Check failed: local_surface_id_ == window_->GetLocalSurfaceId() (LocalSurfaceId(2, 1, 26DD...) vs. LocalSurfaceId(3, 1, 26DD...))
#0 0x7eff7bdd23dd base::debug::StackTrace::StackTrace()
#1 0x7eff7bafb62c base::debug::StackTrace::StackTrace()
#2 0x7eff7bb6a6fa logging::LogMessage::~LogMessage()
#3 0x7eff7047f78f aura::(anonymous namespace)::ScopedLocalSurfaceIdValidator::~ScopedLocalSurfaceIdValidator()
#4 0x7eff70482031 aura::WindowTreeHost::OnHostResizedInPixels()
#5 0x7eff6c12fb38 views::DesktopWindowTreeHostX11::DelayedResize()
#6 0x7eff6c13550f _ZN4base8internal13FunctorTraitsIMN5views24DesktopWindowTreeHostX11EFvRKN3gfx4SizeEEvE6InvokeIS9_RKNS_7WeakPtrIS3_EEJS7_EEEvT_OT0_DpOT1_
#7 0x7eff6c135475 _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIRKMN5views24DesktopWindowTreeHostX11EFvRKN3gfx4SizeEERKNS_7WeakPtrIS5_EEJS9_EEEvOT_OT0_DpOT1_
#8 0x7eff6c1353ed _ZN4base8internal7InvokerINS0_9BindStateIMN5views24DesktopWindowTreeHostX11EFvRKN3gfx4SizeEEJNS_7WeakPtrIS4_EES6_EEEFvvEE7RunImplIRKSA_RKNSt3__15tupleIJSC_S6_EEEJLm0ELm1EEEEvOT_OT0_NSJ_16integer_sequenceImJXspT1_EEEE
#9 0x7eff6c1352fc _ZN4base8internal7InvokerINS0_9BindStateIMN5views24DesktopWindowTreeHostX11EFvRKN3gfx4SizeEEJNS_7WeakPtrIS4_EES6_EEEFvvEE3RunEPNS0_13BindStateBaseE
#10 0x7eff6bff688d _ZNKR4base17RepeatingCallbackIFvvEE3RunEv
#11 0x7eff6c11f295 _ZN4base8internal22CancelableCallbackImplINS_17RepeatingCallbackIFvvEEEE16ForwardRepeatingIJEEEvDpT_
#12 0x7eff6c094e0f _ZN4base8internal13FunctorTraitsIMN5views12MouseWatcher8ObserverEFvvEvE6InvokeIS6_NS_7WeakPtrIS4_EEJEEEvT_OT0_DpOT1_
#13 0x7eff6c094d8a _ZN4base8internal12InvokeHelperILb1EvE8MakeItSoIMN5views12MouseWatcher8ObserverEFvvENS_7WeakPtrIS6_EEJEEEvOT_OT0_DpOT1_
#14 0x7eff6c094d20 _ZN4base8internal7InvokerINS0_9BindStateIMN5views12MouseWatcher8ObserverEFvvEJNS_7WeakPtrIS5_EEEEEFvvEE7RunImplIS7_NSt3__15tupleIJS9_EEEJLm0EEEEvOT_OT0_NSE_16integer_sequenceImJXspT1_EEEE
#15 0x7eff6c11f1cc _ZN4base8internal7InvokerINS0_9BindStateIMN5views16DesktopScreenX11EFvvEJNS_7WeakPtrIS4_EEEEEFvvEE3RunEPNS0_13BindStateBaseE
#16 0x7eff7baaa1fe _ZNO4base12OnceCallbackIFvvEE3RunEv
#17 0x7eff7bafcaf2 base::debug::TaskAnnotator::RunTask()
#18 0x7eff7bb89cf9 base::internal::IncomingTaskQueue::RunTask()
#19 0x7eff7bb92de7 base::MessageLoop::RunTask()


Assigning to sadrul@ for advice as they added the DCHECK.
 

Comment 1 by sadrul@chromium.org, May 23 2018

I get the following error when I visit the page in site 2:

[10437:10487:0522/211740.896913:ERROR:render_media_log.cc(30)] MediaEvent: MEDIA_ERROR_LOG_ENTRY {"error":"FFmpegDemuxer: no supported streams"}
[10437:10487:0522/211740.897419:ERROR:render_media_log.cc(30)] MediaEvent: MEDIA_ERROR_LOG_ENTRY {"error":"FFmpegDemuxer: no supported streams"}
[10437:10437:0522/211740.897974:ERROR:render_media_log.cc(30)] MediaEvent: PIPELINE_ERROR DEMUXER_ERROR_NO_SUPPORTED_STREAMS
[10437:10437:0522/211740.899176:ERROR:render_media_log.cc(30)] MediaEvent: PIPELINE_ERROR DEMUXER_ERROR_NO_SUPPORTED_STREAMS

Am I holding it wrong?
Yes :) You did not enable proprietary codecs and that's an mp4 video.

You should add these two lines to your args.gn:
```
proprietary_codecs = true
ffmpeg_branding = "Chrome"
```

Comment 3 by sadrul@chromium.org, May 25 2018

Cc: -apaci...@chromium.org sadrul@chromium.org
Owner: apaci...@chromium.org
I have commented on the CL that I think is causing this: https://chromium-review.googlesource.com/c/chromium/src/+/1043546#message-988169fcf940c0069e698d76925f9c6470fcc01c
Thank you for taking a look! We'll have to rethink how to resize with aspect ratio as SetBounds() needs to be async.
Status: Assigned (was: Untriaged)
Status: Started (was: Assigned)
Labels: -Pri-2 Pri-1
Project Member

Comment 8 by bugdroid1@chromium.org, Jun 1 2018

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

commit 17982700ad2b8ed7b1c139ab80afdad17d15dd40
Author: Jennifer Apacible <apacible@chromium.org>
Date: Fri Jun 01 23:10:16 2018

[Picture in Picture] Letterbox video during window resizing.

This change updates the way Picture-in-Picture window resizing is
handled.

Previously, there was an issue where OverlayWindowViews attempts to
resize itself in the middle of already changing its size, which puts
aura into a conflicted state.

This is an initial step towards allowing the window to adhere to the
video aspect ratio by just letterboxing the video. The window can be
of any dimension as long as it is constrained by the already given
min and max values of the window.

Bug:  845747   829677 
Change-Id: I6a120f3d346f7aa64909b06d5f48068497d8d7d7
Reviewed-on: https://chromium-review.googlesource.com/1080129
Commit-Queue: apacible <apacible@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563851}
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/chrome/browser/ui/views/overlay/overlay_window_views.cc
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/chrome/browser/ui/views/overlay/overlay_window_views.h
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/content/browser/picture_in_picture/overlay_surface_embedder.cc
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/content/public/browser/overlay_window.h
[modify] https://crrev.com/17982700ad2b8ed7b1c139ab80afdad17d15dd40/content/shell/browser/layout_test/layout_test_content_browser_client.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 5 2018

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

commit 38a175872c91adb9eeedce02ca22d89563a4c9be
Author: François Beaufort <beaufort.francois@gmail.com>
Date: Tue Jun 05 07:03:51 2018

Run Picture-in-Picture browser_tests on Windows.

Bug:  845747 
Change-Id: I46bae3d46de5478916719793b895d66aad40b36d
Reviewed-on: https://chromium-review.googlesource.com/1084212
Reviewed-by: apacible <apacible@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Commit-Queue: François Beaufort <beaufort.francois@gmail.com>
Cr-Commit-Position: refs/heads/master@{#564382}
[modify] https://crrev.com/38a175872c91adb9eeedce02ca22d89563a4c9be/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc

mlamouri Can you make sure Picture-in-Picture browser_tests run on Linux as well?
fwiw I made a change that fails on one of these tests on windows bots only. because there's no linux coverage it's hard to me to test and iterate.
Cc: mlamouri@chromium.org
Labels: -M-68 M-69 Target-69
Sorry about that. It seems that the issue is specific to mash because we do not run VideoSurfaceLayer there on purpose. I will update the tests to only be skipped when Mash is enabled.
Project Member

Comment 13 by bugdroid1@chromium.org, Jun 20 2018

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

commit fe3bdcc0104f0530dcecc5c0f5054aac54a3dec4
Author: François Beaufort <beaufort.francois@gmail.com>
Date: Wed Jun 20 00:21:26 2018

[Picture-in-Picture] enable browser tests on OS_LINUX.

This makes sures Picture-in-Picture browser_tests run on Linux but not
on Chrome OS yet.

Bug:  845747 
Change-Id: Ie22f3b5a3d3cebe9f2298b70882aae5e3679897b
Reviewed-on: https://chromium-review.googlesource.com/1105048
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568660}
[modify] https://crrev.com/fe3bdcc0104f0530dcecc5c0f5054aac54a3dec4/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc

Project Member

Comment 14 by bugdroid1@chromium.org, Jun 20 2018

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

commit d9fda94cd4a965385cc8089aaf798ffff7fa841e
Author: Mounir Lamouri <mlamouri@chromium.org>
Date: Wed Jun 20 13:22:49 2018

Picture-in-Picture: disable browser tests on mash.

This will allow the browser tests to run on all platforms.

Bug:  845747 
Change-Id: I07eb6fbd8240c5f559ef078538ffc047e7ab784c
Reviewed-on: https://chromium-review.googlesource.com/1105073
Reviewed-by: apacible <apacible@chromium.org>
Commit-Queue: Mounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568815}
[modify] https://crrev.com/d9fda94cd4a965385cc8089aaf798ffff7fa841e/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc
[modify] https://crrev.com/d9fda94cd4a965385cc8089aaf798ffff7fa841e/testing/buildbot/filters/mash.browser_tests.filter

Status: Fixed (was: Started)
The tests are now fully disabled on mash because video surface layer are not enabled there so the tests were always failing.

Sign in to add a comment