ConversionContext::SwitchToClip DCHECK |
|||
Issue description
Hit this DCHECK with a clusterfuzz repro of something else:
#if DCHECK_IS_ON()
DCHECK(state_stack_.size() && state_stack_.back().type == StateEntry::kClip)
<< "Error: Chunk has a clip that escaped its layer's or effect's clip."
<< "\ntarget_clip:\n"
<< target_clip->ToTreeString().Utf8().data() << "current_clip_:\n"
<< current_clip_->ToTreeString().Utf8().data();
#endif
Attached.
,
Jun 15 2018
The output was
[1:1:0615/172138.664299:FATAL:paint_chunks_to_cc_layer.cc(322)] Check failed: state_stack_.size() && state_stack_.back().type == StateEntry::kClip. Error: Chunk has a clip that escaped its layer's or effect's clip.
target_clip:
root 0x611000028400 {"localTransformSpace":"0x612000009c40","rect":"InfiniteIntRect"}
OverflowClip (LayoutView #document) 0x611000068280 {"parent":"0x611000028400","localTransformSpace":"0x612000020740","rect":"0,0 785x600","rectExcludingOverlayScrollbars":"0,0 785x600"}
MaskClip (LayoutBlockFlow HTML) 0x611000068500 {"parent":"0x611000068280","localTransformSpace":"0x61200003b140","rect":"0,0 3x3.35544e+07"}
FragmentClip (LayoutMultiColumnFlowThread (anonymous)) 0x61100007c780 {"parent":"0x611000068500","localTransformSpace":"0x61200004c240","rect":"-999999,-1e+06 2e+06x1.00003e+06"}
current_clip_:
root 0x611000028400 {"localTransformSpace":"0x612000009c40","rect":"InfiniteIntRect"}
OverflowClip (LayoutView #document) 0x611000068280 {"parent":"0x611000028400","localTransformSpace":"0x612000020740","rect":"0,0 785x600","rectExcludingOverlayScrollbars":"0,0 785x600"}
MaskClip (LayoutBlockFlow HTML) 0x611000068500 {"parent":"0x611000068280","localTransformSpace":"0x61200003b140","rect":"0,0 3x3.35544e+07"}
FragmentClip (LayoutMultiColumnFlowThread (anonymous)) 0x611000069540 {"parent":"0x611000068500","localTransformSpace":"0x61200003b140","rect":"-999990,3.35544e+07 2e+06x0"}
#0 0x00000084cc11 (/usr/local/google/home/danakj/s/c/src/out_desktop/Release/content_shell+0x84cc10)
,
Jun 18 2018
,
Sep 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/4009d1128e1cf626145afe071d90f12b7ec30867 commit 4009d1128e1cf626145afe071d90f12b7ec30867 Author: Vladimir Levin <vmpstr@chromium.org> Date: Fri Sep 07 21:13:49 2018 Fix a null-dereference in PaintChunksToCc conversion. The patch to call Unalias caused a nullptr dereference, because of another bug that previously only triggered a DCHECK. This patch ensures to do a "safe unalias" where we first check for null. This check can be reverted once the underlying issues that trigger the DCHECK are fixed (see the two referenced bugs for the nullptr dereference and for the DCHECK failure). R=pdr@chromium.org, trchen@chromium.org Bug: 881788, 853357 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I412d60581466a40e4543b0ce78b18fb1a739f02b Reviewed-on: https://chromium-review.googlesource.com/1213869 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#589651} [modify] https://crrev.com/4009d1128e1cf626145afe071d90f12b7ec30867/third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc
,
Sep 7
I investigated the issue today and it brings back some forgotten memory about that DCHECK...
That "chunk clip escaped layer clip" DCHECK is not supposed to ever happen in SPv2, because the layerization algorithm should never assign a chunk to a layer that has excessive clip.
However in SPv175 the DCHECK can actually happen due to some core compositing bug in PLC/CLM that are related to clip escaping. Therefore it was intended to be a "soft" DCHECK that it should fail gracefully in release build. With SPv175 release in production, it really should have been a DLOG if it can be triggered by a known bug that is intended to left unfixed.
I think it should be changed to this instead:
#if DCHECK_IS_ON()
if (impossible_condition) {
DLOG(ERROR) << "chunk clip escape layer clip";
if (RuntimeEnabledFeatures::SlimmingPaintV2Enabled())
NOTREACHED();
}
#endif
,
Sep 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bb7972c1f8d03235a76020e91fecd52a5463de0f commit bb7972c1f8d03235a76020e91fecd52a5463de0f Author: Tien-Ren Chen <trchen@chromium.org> Date: Mon Sep 10 20:51:38 2018 [Blink/SPv175+] Change DCHECK(chunk clip escaped layer clip) to a DLOG The PaintChunkToCcLayer algorithm was originally designed for SPv2 compositor, and it was expected the layerization algorithm should never assign a chunk to a excessively clipped layer, thus the DCHECK. Later this algorithm was adopted in SPv175 to be used with the SPv1 compositor. There is a known bug that in certain corner case we can fail to escape clip, and the bug is difficult to fix in the legacy architecture. The DCHECK is expected to be a "soft" one that we have a fail-safe path to recover in a sane way. This CL converts the DCHECK to a DLOG in SPv1 mode, and it should still trap in SPv2 mode. In addition, it reverts a workaround to a nullptr bug in the fail-safe path, and add a test for the fail-safe. BUG=881788,853357 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I4606acf4885f3344bb45a901bb2e8e46dbcda49a Reviewed-on: https://chromium-review.googlesource.com/1213952 Reviewed-by: vmpstr <vmpstr@chromium.org> Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Tien-Ren Chen <trchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#590052} [modify] https://crrev.com/bb7972c1f8d03235a76020e91fecd52a5463de0f/third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc [modify] https://crrev.com/bb7972c1f8d03235a76020e91fecd52a5463de0f/third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer_test.cc
,
Sep 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3197afbf61dabb197256230fa979998ace7726fd commit 3197afbf61dabb197256230fa979998ace7726fd Author: Tien-Ren Chen <trchen@chromium.org> Date: Mon Sep 10 23:21:51 2018 Revert "[Blink/SPv175+] Change DCHECK(chunk clip escaped layer clip) to a DLOG" This reverts commit bb7972c1f8d03235a76020e91fecd52a5463de0f. Reason for revert: crbug.com/882642 Original change's description: > [Blink/SPv175+] Change DCHECK(chunk clip escaped layer clip) to a DLOG > > The PaintChunkToCcLayer algorithm was originally designed for > SPv2 compositor, and it was expected the layerization algorithm should > never assign a chunk to a excessively clipped layer, thus the DCHECK. > > Later this algorithm was adopted in SPv175 to be used with the > SPv1 compositor. There is a known bug that in certain corner case we > can fail to escape clip, and the bug is difficult to fix in the > legacy architecture. The DCHECK is expected to be a "soft" one that > we have a fail-safe path to recover in a sane way. > > This CL converts the DCHECK to a DLOG in SPv1 mode, and it should still > trap in SPv2 mode. In addition, it reverts a workaround to a nullptr > bug in the fail-safe path, and add a test for the fail-safe. > > BUG=881788,853357 > > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I4606acf4885f3344bb45a901bb2e8e46dbcda49a > Reviewed-on: https://chromium-review.googlesource.com/1213952 > Reviewed-by: vmpstr <vmpstr@chromium.org> > Reviewed-by: Philip Rogers <pdr@chromium.org> > Commit-Queue: Tien-Ren Chen <trchen@chromium.org> > Cr-Commit-Position: refs/heads/master@{#590052} TBR=vmpstr@chromium.org,trchen@chromium.org,pdr@chromium.org Change-Id: I8f669d48422eb2cc3bb21db2d4866857710c3fbb No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 881788, 853357 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1217712 Reviewed-by: Tien-Ren Chen <trchen@chromium.org> Commit-Queue: Tien-Ren Chen <trchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#590114} [modify] https://crrev.com/3197afbf61dabb197256230fa979998ace7726fd/third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc [modify] https://crrev.com/3197afbf61dabb197256230fa979998ace7726fd/third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer_test.cc
,
Sep 14
,
Sep 14
I'm leaving the team, thus re-assigning. This DCHECK is expected to happen on SPv1 (though the corner cases should be rare), so it is converted to a DLOG. If we ever hit this in SPv2, PAC must be very broken.
,
Sep 15
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/545de80a86c0f1b94c0e354c0bf2c09b91fe69af commit 545de80a86c0f1b94c0e354c0bf2c09b91fe69af Author: Tien-Ren Chen <trchen@chromium.org> Date: Sat Sep 15 14:25:04 2018 [Blink/SPv175+] Change DCHECK(chunk clip escaped layer clip) to a DLOG This is a scaled-back version of a same-named previous CL. This version only converts the DCHECK into a DLOG, but keep the not-so-robust error recovery algorithm as-is. The PaintChunkToCcLayer algorithm was originally designed for SPv2 compositor, and it was expected the layerization algorithm should never assign a chunk to a excessively clipped layer, thus the DCHECK. Later this algorithm was adopted in SPv175 to be used with the SPv1 compositor. There is a known bug that in certain corner case we can fail to escape clip, and the bug is difficult to fix in the legacy architecture. The DCHECK is expected to be a "soft" one that we have a fail-safe path to recover in a sane way. BUG=881788,853357 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ic534ea754ac392e839534e3bfbeb1fbccc64d120 Reviewed-on: https://chromium-review.googlesource.com/1227062 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#591587} [modify] https://crrev.com/545de80a86c0f1b94c0e354c0bf2c09b91fe69af/third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc [modify] https://crrev.com/545de80a86c0f1b94c0e354c0bf2c09b91fe69af/third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer_test.cc |
|||
►
Sign in to add a comment |
|||
Comment 1 by danakj@chromium.org
, Jun 15 2018#0 0x00000084cc11 in __interceptor_backtrace /b/build/slave/linux_upload_clang/build/src/third_party/llvm/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:3980:13 #1 0x7f64c06b80ac in base::debug::StackTrace::StackTrace(unsigned long) ./../../base/debug/stack_trace_posix.cc:808:41 #2 0x7f64c04361b3 in logging::LogMessage::~LogMessage() ./../../base/logging.cc:592:29 #3 0x7f649e51c7c7 in blink::(anonymous namespace)::ConversionContext::SwitchToClip(blink::ClipPaintPropertyNode const*) ./../../third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc:322:5 #4 0x7f649e517a05 in blink::(anonymous namespace)::ConversionContext::SwitchToChunkState(blink::PaintChunk const&) ./../../third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc:268:3 #5 0x7f649e517a05 in blink::(anonymous namespace)::ConversionContext::Convert(blink::PaintChunkSubset const&, blink::DisplayItemList const&) ./../../third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc:663:0 #6 0x7f649e517a05 in blink::PaintChunksToCcLayer::ConvertInto(blink::PaintChunkSubset const&, blink::PropertyTreeState const&, gfx::Vector2dF const&, blink::FloatSize const&, blink::DisplayItemList const&, cc::DisplayItemList&) ./../../third_party/blink/renderer/platform/graphics/compositing/paint_chunks_to_cc_layer.cc:688:0 #7 0x7f649e6f4a09 in blink::GraphicsLayer::PaintContentsToDisplayList(cc::ContentLayerClient::PaintingControlSetting) ./../../third_party/blink/renderer/platform/graphics/graphics_layer.cc:1431:5 #8 0x7f649e6f5202 in non-virtual thunk to blink::GraphicsLayer::PaintContentsToDisplayList(cc::ContentLayerClient::PaintingControlSetting) ./../../third_party/blink/renderer/platform/graphics/graphics_layer.cc:0:0 #9 0x7f64b73b348f in cc::PictureLayer::Update() ./../../cc/layers/picture_layer.cc:125:39 #10 0x7f64b75cfbde in cc::LayerTreeHost::PaintContent(std::__1::vector<scoped_refptr<cc::Layer>, std::__1::allocator<scoped_refptr<cc::Layer> > > const&, bool*, bool*) ./../../cc/trees/layer_tree_host.cc:1237:33 #11 0x7f64b75cd7bf in cc::LayerTreeHost::DoUpdateLayers(cc::Layer*) ./../../cc/trees/layer_tree_host.cc:800:7 #12 0x7f64b75cbb68 in cc::LayerTreeHost::UpdateLayers() ./../../cc/trees/layer_tree_host.cc:665:17 #13 0x7f64b77287cc in DoPainting ./../../cc/trees/single_thread_proxy.cc:773:21 #14 0x7f64b77287cc in cc::SingleThreadProxy::CompositeImmediately(base::TimeTicks, bool) ./../../cc/trees/single_thread_proxy.cc:524:0 #15 0x7f64b75cc108 in cc::LayerTreeHost::Composite(base::TimeTicks, bool) ./../../cc/trees/layer_tree_host.cc:640:10 #16 0x7f64bd504286 in content::RenderWidgetCompositor::SynchronouslyComposite(bool, std::__1::unique_ptr<cc::SwapPromise, std::__1::default_delete<cc::SwapPromise> >) ./../../content/renderer/gpu/render_widget_compositor.cc:1090:21 #17 0x7f64bd505496 in content::RenderWidgetCompositor::SynchronouslyCompositeNoRasterForTesting() ./../../content/renderer/gpu/render_widget_compositor.cc:1056:3 Running with is_asan=true fwiw, but shouldn't matter I'd think