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

Issue 785558 link

Starred by 1 user
Status: Fixed
Owner:
kylec...@chromium.org
Closed: Nov 2017
Cc:
danakj@chromium.org
█ fsam...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug



Sign in to add a comment

DCHECK hit in FrameSinkManagerImpl after context loss

Project Member Reported by emir...@chromium.org, Nov 16 2017 
We came across to this issue while adding a context loss recover test in [0]. Here[1] is a minimized test case that showcases the behavior, see WebRtcCaptureFromElementBrowserTest.CaptureFromCanvas2DHandlesContextLoss failures on Android bots. 

Below are the error logs. There are some related changes[2][3] which seems to be missing UnregisterBeginFrameSource() calls. Please note that you should re-enable WebRtcCaptureFromElementBrowserTest disabled cases when this is fixed. 

[FATAL:frame_sink_manager_impl.cc(251)] Check failed: registered_sources_.count(source) == 0u (1 vs. 0)
[ERROR:test_suite.cc(296)] Currently running: WebRtcCaptureFromElementBrowserTest.CaptureFromCanvas2DHandlesContextLoss
  00675fc1  logging::LogMessage::~LogMessage()+112                                                                                                                                                                                                                                                                                                                                                                                                                                                                             /b/c/b/linux_android_rel_ng/src/base/logging.cc:581
  010c0e0f  viz::FrameSinkManagerImpl::RegisterBeginFrameSource(viz::BeginFrameSource*, viz::FrameSinkId const&)+242                                                                                                                                                                                                                                                                                                                                                                                                           /b/c/b/linux_android_rel_ng/src/components/viz/service/frame_sinks/frame_sink_manager_impl.cc:251
  0146adf1  content::CompositorImpl::InitializeDisplay(std::__ndk1::unique_ptr<viz::OutputSurface, std::__ndk1::default_delete<viz::OutputSurface> >, scoped_refptr<viz::VulkanContextProvider>, scoped_refptr<viz::ContextProvider>)+536                                                                                                                                                                                                                                                                                      /b/c/b/linux_android_rel_ng/src/content/browser/renderer_host/compositor_impl_android.cc:832
  0146aa93  content::CompositorImpl::OnGpuChannelEstablished(scoped_refptr<gpu::GpuChannelHost>)+922                                                                                                                                                                                                                                                                                                                                                                                                                           /b/c/b/linux_android_rel_ng/src/content/browser/renderer_host/compositor_impl_android.cc:781
  0146b36b  void base::internal::FunctorTraits<void (content::CompositorImpl::*)(scoped_refptr<gpu::GpuChannelHost>), void>::Invoke<base::WeakPtr<content::CompositorImpl> const&, scoped_refptr<gpu::GpuChannelHost> >(void (content::CompositorImpl::*)(scoped_refptr<gpu::GpuChannelHost>), base::WeakPtr<content::CompositorImpl> const&, scoped_refptr<gpu::GpuChannelHost>&&)+40                                                                                                                                         /b/c/b/linux_android_rel_ng/src/base/bind_internal.h:194
  012bccb3  content::BrowserGpuChannelHostFactory::GpuChannelEstablished()+298                                                                                                                                                                                                                                                                                                                                                                                                                                                 /b/c/b/linux_android_rel_ng/src/content/browser/gpu/browser_gpu_channel_host_factory.cc:344
  012bcb79  content::BrowserGpuChannelHostFactory::EstablishRequest::FinishOnMain()+20    


[0] https://chromium-review.googlesource.com/c/chromium/src/+/756474
[1] https://chromium-review.googlesource.com/c/chromium/src/+/773107
[2] https://chromium.googlesource.com/chromium/src/+/d3b9d651b0b0ea57990ecf8e0991f39f2870c4fa
[3] https://chromium.googlesource.com/chromium/src/+/96cb9e71e6049ebab48ae85f3e405366c3e21e35

Comment 1 by kylec...@chromium.org, Nov 16 2017 

Thanks for the simple example to reproduce. CompositorImpl is missing some logic when recreating a viz::Display.

Comment 2 by emir...@chromium.org, Nov 16 2017 

Additionally, this test hit NOTREACHED on linux-chromeos-rel bots, see [0]. It is added from [1]. Is that expected?
[0] https://chromium-review.googlesource.com/c/chromium/src/+/775098
[1] https://chromium.googlesource.com/chromium/src/+/883e99d9896c0dbcaf1ce4da26a1a3aa2fe2cdbe

Comment 3 by kylec...@chromium.org, Nov 17 2017 

What test did it hit the NOTREACHED() in: content_browsertests or viz_content_browsertests? In viz_content_browsertests then yes it's expected.

Comment 4 by kylec...@chromium.org, Nov 17 2017 

Oh wait, did you mean NOTREACHED() or NOTIMPLEMENTED()? I don't think [1] added any NOTREACHED() statements.
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 21 2017 

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

commit a60c41ce6430e5b646af58471f84e4f6b71713fe
Author: kylechar <kylechar@chromium.org>
Date: Tue Nov 21 01:25:50 2017

Fix DCHECK on Android context loss.

On Android when CompositorImpl loses it's context it doesn't unregister
it's BeginFrameSource. CompositorImpl ended up re-registering the
BeginFrameSource and triggering a DCHECK. Check if we are recreating the
display and don't register BeginFrameSource.

For Android enable CaptureFromCanvas2DHandlesContextLoss and
CaptureFromOpaqueCanvas2DHandlesContextLoss again. Also enable the tests
under Chrome OS. It's only with --mus where capture is broken that the
tests fail, so add tests content_browsertests --mus filter file.

Bug:  785558 
Change-Id: I6bf6a09ecbd83d82cab3141838b465dd5479dd17
Reviewed-on: https://chromium-review.googlesource.com/777685
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Bo <boliu@chromium.org>
Reviewed-by: Emircan Uysaler <emircan@chromium.org>
Commit-Queue: Emircan Uysaler <emircan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518022}
[modify] https://crrev.com/a60c41ce6430e5b646af58471f84e4f6b71713fe/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/a60c41ce6430e5b646af58471f84e4f6b71713fe/content/browser/webrtc/webrtc_capture_from_element_browsertest.cc
[modify] https://crrev.com/a60c41ce6430e5b646af58471f84e4f6b71713fe/testing/buildbot/filters/mus.content_browsertests.filter

Comment 7 by kylec...@chromium.org, Nov 21 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment