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

Issue 871450 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows , Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Crash in cc::mojo_embedder::AsyncLayerTreeFrameSink::SubmitCompositorFrame(viz::CompositorFrame)

Project Member Reported by sky@chromium.org, Aug 6

Issue description

Sample report: https://crash.corp.google.com/browse?q=product_name%3D%27Chrome_ChromeOS%27+AND+expanded_custom_data.ChromeCrashProto.ptype%3D%27shortcut_viewer_app%27+AND+expanded_custom_data.ChromeCrashProto.magic_signature_1.name%3D%27cc%3A%3Amojo_embedder%3A%3AAsyncLayerTreeFrameSink%3A%3ASubmitCompositorFrame%27&stbtiq=&reportid=3ac5b0c3a1168f2f&index=0 . Here's the stack:
Thread 0 (id: 0x40d8) CRASHED [SIGTRAP @ 0x00000000 ] MAGIC SIGNATURE THREAD
Stack Quality99%Show frame trust levels
0x00005c91632f1e12	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/cc/mojo_embedder/async_layer_tree_frame_sink.cc:140 )	cc::mojo_embedder::AsyncLayerTreeFrameSink::SubmitCompositorFrame(viz::CompositorFrame)
0x00005c9162f52025	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/cc/trees/layer_tree_host_impl.cc:2038 )	cc::LayerTreeHostImpl::DrawLayers(cc::LayerTreeHostImpl::FrameData*)
0x00005c9162fe1cd3	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/cc/trees/single_thread_proxy.cc:619 )	cc::SingleThreadProxy::DoComposite(cc::LayerTreeHostImpl::FrameData*)
0x00005c9162fe308f	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/cc/trees/single_thread_proxy.cc:808 )	non-virtual thunk to cc::SingleThreadProxy::ScheduledActionDrawIfPossible()
0x00005c915ef715ad	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/cc/scheduler/scheduler.cc:703 )	<name omitted>
0x00005c9162ff497f	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/cc/scheduler/scheduler.cc:691 )	cc::Scheduler::OnBeginImplFrameDeadline()
0x00005c915fd9e832	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/callback.h:99 )	void base::internal::CancelableCallbackImpl<base::OnceCallback<void ()> >::ForwardOnce<>()
0x00005c915ef21b2d	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/callback.h:99 )	base::MessageLoop::DoWork()
0x00005c9161b738b8	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/message_loop/message_pump_default.cc:37 )	base::MessagePumpDefault::Run(base::MessagePump::Delegate*)
0x00005c9161b95903	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/base/run_loop.cc:102 )	<name omitted>
0x00005c91617456df	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/utility/utility_main.cc:121 )	content::UtilityMain(content::MainFunctionParams const&)
0x00005c9161764dc3	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/app/content_main_runner_impl.cc:595 )	content::ContentMainRunnerImpl::Run(bool)
0x00005c916176d182	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/services/service_manager/embedder/main.cc:472 )	service_manager::Main(service_manager::MainParams const&)
0x00005c915eff7834	(chrome -./../../../../../../../home/chrome-bot/chrome_root/src/content/app/content_main.cc:19 )	ChromeMain
0x00007fa24b47f735	(libc-2.23.so -libc-start.c:289 )	__libc_start_main
0x00005c915efe57f8	(chrome + 0x002ab7f8 )	_start

This comes from the keyboard-shortcut-viewer, which is running out of process of the browser.

Fady, I'm passing your way, but please reassign if you aren't a good owner.
 
Owner: fsam...@chromium.org
Cc: kylec...@chromium.org moh...@chromium.org samans@chromium.org
My best guess at the moment is there's a surface invariants violation in WindowPortMus somewhere. cc'ing some folks. I'll dig more into this next week (I'm OOO for a week).
Owner: moh...@chromium.org
This isn't really actionable and really turns a momentary glitch into a crash. I think we should convert these into UMA. Passing along to mohsen@ to convert this into a UMA. 

The ideal situation is a user files a repro-able bug with the glitch.
Labels: -Pri-2 ReleaseBlock-Stable M-69 Pri-1
Owner: fsam...@chromium.org
This is a release blocker. I'll assign to myself.
We already report surface invariants violations service-side, so I'm just going to convert the checks into DCHECKs.

I'd also like to merge this back into M69 for reporting purposes:

https://chromium-review.googlesource.com/c/chromium/src/+/1155471
Fady, if this is only for the ksv no need to backport as we're turning it off for 69. Plan is to enable everywhere in 70.
I think killing the client if it abuses surfaces is a bit draconian. I'm going to go ahead with the backport in case there are bugs in other clients.
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 20

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

commit 96dfe14093fb89c65d68e78b66ea381398592199
Author: Fady Samuel <fsamuel@chromium.org>
Date: Mon Aug 20 21:11:15 2018

Surface Synchronization: Convert invariants violations to DCHECKs

This CL converts some not so useful CHECKs in AsyncLayerTreeFrameSink
into DCHECKs that can be caught in debug builds by developers.

We would like to merge this CL into M69 as well to eliminate a source
of renderer crashes.

Bug:  871450 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I235d068ea72774b020ba7a241507d0f4201d8dc0
Reviewed-on: https://chromium-review.googlesource.com/1181766
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#584560}
[modify] https://crrev.com/96dfe14093fb89c65d68e78b66ea381398592199/cc/mojo_embedder/async_layer_tree_frame_sink.cc

Cc: pbomm...@chromium.org gov...@chromium.org
Labels: M-70 FoundIn-70 Target-69 FoundIn-69 OS-Windows
So far there aren't any crashes on 70.0.3529.0 which is been released 10 hours back. 

Please find the crash impact on Chrome version here : https://goto.google.com/dbtvq
Labels: Merge-Request-69
Project Member

Comment 12 by sheriffbot@chromium.org, Aug 21

Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: We are only 13 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
This is a very very low risk change. This is changing some CHECKs to DCHECKs.
Labels: -Merge-Review-69 Merge-Approved-69
Approving merge to M69 branch 3497 based on comment #10, #13 and per offline char with fseb@. Pls merge now. Thank you.
Project Member

Comment 15 by bugdroid1@chromium.org, Aug 21

Labels: -merge-approved-69 merge-merged-3497
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9994bdf8ae85684694c72f3462c6436237fe452

commit c9994bdf8ae85684694c72f3462c6436237fe452
Author: Fady Samuel <fsamuel@chromium.org>
Date: Tue Aug 21 17:38:23 2018

Surface Synchronization: Convert invariants violations to DCHECKs

This CL converts some not so useful CHECKs in AsyncLayerTreeFrameSink
into DCHECKs that can be caught in debug builds by developers.

We would like to merge this CL into M69 as well to eliminate a source
of renderer crashes.

Bug:  871450 
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: I235d068ea72774b020ba7a241507d0f4201d8dc0
Reviewed-on: https://chromium-review.googlesource.com/1181766
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Commit-Queue: Fady Samuel <fsamuel@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#584560}(cherry picked from commit 96dfe14093fb89c65d68e78b66ea381398592199)
Reviewed-on: https://chromium-review.googlesource.com/1183921
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Cr-Commit-Position: refs/branch-heads/3497@{#742}
Cr-Branched-From: 271eaf50594eb818c9295dc78d364aea18c82ea8-refs/heads/master@{#576753}
[modify] https://crrev.com/c9994bdf8ae85684694c72f3462c6436237fe452/cc/mojo_embedder/async_layer_tree_frame_sink.cc

M69 Stable promotion is coming VERY soon. Your bug is labelled as Stable ReleaseBlock, pls make sure to land the fix and request a merge into the release branch ASAP. Merge has to happen latest by 4:00 PM PT Friday (08/24/18) in order to make it to next week stable cut. Thank you.
Status: Fixed (was: Assigned)

Sign in to add a comment