DCHECK in composited_layer_mapping.cc |
|||||
Issue descriptionChrome Version: 69. OS: Linux Debian 4.9.82 What steps will reproduce the problem? (1) Build and run Chrome with "dcheck_always_on" (alternatively a debug build). (2) Copy and Paste "https://play.google.com/store/apps/details?id=zw.co.mobility.drive&hl=en" in omnibox. What is the expected result? Page loads. What happens instead? DCHECK fires. (!paint_root_background_onto_scrolling_contents_layer || !foreground_layer)
,
Jul 9
Based on https://chromium-review.googlesource.com/c/chromium/src/+/1067076, which has changed the signature of DCEHCK, assigning to pdr@ to further triage. The CL mentioned is not a culprit and DCHECK existed before that.
,
Jul 10
,
Aug 13
Here's a minimized repro. A foreground layer is created when: ------------------8<------------------ // If an element has composited negative z-index children, those children paint // in front of the layer background, so we need an extra 'contents' layer for // the foreground of the layer object. bool PaintLayerCompositor::NeedsContentsCompositingLayer( ... ------------------8<------------------ The DCHECK is ensuring that we do not paint into the scrolling contents layer if there is a foreground layer. I think this is because we need to draw the negative z children between the background and the scrolling contents.
,
Aug 14
Work in progress: https://chromium-review.googlesource.com/c/chromium/src/+/1173532
,
Aug 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7834ee2169007c67e67feec762999a9d85c200f6 commit 7834ee2169007c67e67feec762999a9d85c200f6 Author: Philip Rogers <pdr@chromium.org> Date: Tue Aug 21 17:14:39 2018 Fix background painting on scrolling contents layer with foregrounds This patch removes a negative z-index children check from PaintLayer::GetBackgroundPaintLocation. This lets backgrounds paint in the scrolling contents layer in the presence of negative z-index children, fixing the painting of background-attachment: local. This patch also removes a DCHECK that was hit when a foreground layer and kBackgroundPaintInScrollingContents coexisted. This DCHECK[1] could be hit for root layers without the functional change in this patch. As the test in this patch shows, kBackgroundPaintInScrollingContents and foreground layers work together, so this patch makes non-root-layers use both, and removes the DCHECK. What are foreground layers? A foreground layer is created if a stacked PaintLayer has composited negative z-index children that should wedge between the layer's background and the layer's contents. The foreground layer will have the normal-flow contents in this case. See the comment above foreground_layer_ in composited_layer_mapping.h. [1] DCHECK originally added in: https://crrev.com/9a2cdc8d96d. Reading the comments, I think the DCHECK was intended to ensure the background would not be painted into the foreground layer. Bug: 861948 Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I7409cbe9ac61fed444856df342d0707d6babab09 Reviewed-on: https://chromium-review.googlesource.com/1173532 Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#584802} [add] https://crrev.com/7834ee2169007c67e67feec762999a9d85c200f6/third_party/WebKit/LayoutTests/paint/background/scrolling-background-with-negative-z-child-expected.html [add] https://crrev.com/7834ee2169007c67e67feec762999a9d85c200f6/third_party/WebKit/LayoutTests/paint/background/scrolling-background-with-negative-z-child.html [modify] https://crrev.com/7834ee2169007c67e67feec762999a9d85c200f6/third_party/WebKit/LayoutTests/paint/invalidation/invalidation-on-foreground-graphics-layer-expected.txt [add] https://crrev.com/7834ee2169007c67e67feec762999a9d85c200f6/third_party/WebKit/LayoutTests/paint/stacking/html-with-negative-z-index-expected.html [add] https://crrev.com/7834ee2169007c67e67feec762999a9d85c200f6/third_party/WebKit/LayoutTests/paint/stacking/html-with-negative-z-index.html [modify] https://crrev.com/7834ee2169007c67e67feec762999a9d85c200f6/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc [modify] https://crrev.com/7834ee2169007c67e67feec762999a9d85c200f6/third_party/blink/renderer/core/paint/paint_layer.cc [modify] https://crrev.com/7834ee2169007c67e67feec762999a9d85c200f6/third_party/blink/renderer/core/paint/paint_layer_scrollable_area_test.cc
,
Aug 21
,
Aug 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e06da142bd6f0c6857318a598423914ff029b55e commit e06da142bd6f0c6857318a598423914ff029b55e Author: Philip Rogers <pdr@chromium.org> Date: Tue Aug 21 20:53:46 2018 Revert "Fix background painting on scrolling contents layer with foregrounds" This reverts commit 7834ee2169007c67e67feec762999a9d85c200f6. Reason for revert: Suspected as causing failing tests in 874162 Original change's description: > Fix background painting on scrolling contents layer with foregrounds > > This patch removes a negative z-index children check from > PaintLayer::GetBackgroundPaintLocation. This lets backgrounds paint in > the scrolling contents layer in the presence of negative z-index > children, fixing the painting of background-attachment: local. This > patch also removes a DCHECK that was hit when a foreground layer and > kBackgroundPaintInScrollingContents coexisted. This DCHECK[1] could be > hit for root layers without the functional change in this patch. As the > test in this patch shows, kBackgroundPaintInScrollingContents and > foreground layers work together, so this patch makes non-root-layers use > both, and removes the DCHECK. > > What are foreground layers? A foreground layer is created if a stacked > PaintLayer has composited negative z-index children that should wedge > between the layer's background and the layer's contents. The foreground > layer will have the normal-flow contents in this case. See the comment > above foreground_layer_ in composited_layer_mapping.h. > > [1] DCHECK originally added in: https://crrev.com/9a2cdc8d96d. Reading > the comments, I think the DCHECK was intended to ensure the background > would not be painted into the foreground layer. > > Bug: 861948 > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I7409cbe9ac61fed444856df342d0707d6babab09 > Reviewed-on: https://chromium-review.googlesource.com/1173532 > Reviewed-by: Chris Harrelson <chrishtr@chromium.org> > Commit-Queue: Philip Rogers <pdr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#584802} TBR=pdr@chromium.org,chrishtr@chromium.org Change-Id: Ifd22a963dbd5ce288dfb31ab1e8ac41ebe63062d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 861948 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/1184109 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#584895} [delete] https://crrev.com/92b6a49660ca370c2354b910a29149b27e6503de/third_party/WebKit/LayoutTests/paint/background/scrolling-background-with-negative-z-child-expected.html [delete] https://crrev.com/92b6a49660ca370c2354b910a29149b27e6503de/third_party/WebKit/LayoutTests/paint/background/scrolling-background-with-negative-z-child.html [modify] https://crrev.com/e06da142bd6f0c6857318a598423914ff029b55e/third_party/WebKit/LayoutTests/paint/invalidation/invalidation-on-foreground-graphics-layer-expected.txt [delete] https://crrev.com/92b6a49660ca370c2354b910a29149b27e6503de/third_party/WebKit/LayoutTests/paint/stacking/html-with-negative-z-index-expected.html [delete] https://crrev.com/92b6a49660ca370c2354b910a29149b27e6503de/third_party/WebKit/LayoutTests/paint/stacking/html-with-negative-z-index.html [modify] https://crrev.com/e06da142bd6f0c6857318a598423914ff029b55e/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc [modify] https://crrev.com/e06da142bd6f0c6857318a598423914ff029b55e/third_party/blink/renderer/core/paint/paint_layer.cc [modify] https://crrev.com/e06da142bd6f0c6857318a598423914ff029b55e/third_party/blink/renderer/core/paint/paint_layer_scrollable_area_test.cc
,
Aug 21
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bea4d986d6fb6aa030542a1057901981af39c0ff commit bea4d986d6fb6aa030542a1057901981af39c0ff Author: Philip Rogers <pdr@chromium.org> Date: Tue Aug 21 22:22:22 2018 Reland "Fix background painting on scrolling contents layer with foregrounds" This reverts commit e06da142bd6f0c6857318a598423914ff029b55e. Reason for revert: Was not the root cause of test failures. Original change's description: > Revert "Fix background painting on scrolling contents layer with foregrounds" > > This reverts commit 7834ee2169007c67e67feec762999a9d85c200f6. > > Reason for revert: Suspected as causing failing tests in 874162 > > Original change's description: > > Fix background painting on scrolling contents layer with foregrounds > > > > This patch removes a negative z-index children check from > > PaintLayer::GetBackgroundPaintLocation. This lets backgrounds paint in > > the scrolling contents layer in the presence of negative z-index > > children, fixing the painting of background-attachment: local. This > > patch also removes a DCHECK that was hit when a foreground layer and > > kBackgroundPaintInScrollingContents coexisted. This DCHECK[1] could be > > hit for root layers without the functional change in this patch. As the > > test in this patch shows, kBackgroundPaintInScrollingContents and > > foreground layers work together, so this patch makes non-root-layers use > > both, and removes the DCHECK. > > > > What are foreground layers? A foreground layer is created if a stacked > > PaintLayer has composited negative z-index children that should wedge > > between the layer's background and the layer's contents. The foreground > > layer will have the normal-flow contents in this case. See the comment > > above foreground_layer_ in composited_layer_mapping.h. > > > > [1] DCHECK originally added in: https://crrev.com/9a2cdc8d96d. Reading > > the comments, I think the DCHECK was intended to ensure the background > > would not be painted into the foreground layer. > > > > Bug: 861948 > > Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel > > Change-Id: I7409cbe9ac61fed444856df342d0707d6babab09 > > Reviewed-on: https://chromium-review.googlesource.com/1173532 > > Reviewed-by: Chris Harrelson <chrishtr@chromium.org> > > Commit-Queue: Philip Rogers <pdr@chromium.org> > > Cr-Commit-Position: refs/heads/master@{#584802} > > TBR=pdr@chromium.org,chrishtr@chromium.org > > Change-Id: Ifd22a963dbd5ce288dfb31ab1e8ac41ebe63062d > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: 861948 > 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/1184109 > Reviewed-by: Philip Rogers <pdr@chromium.org> > Commit-Queue: Philip Rogers <pdr@chromium.org> > Cr-Commit-Position: refs/heads/master@{#584895} TBR=pdr@chromium.org,chrishtr@chromium.org Change-Id: I569313a9d5e1dd0b79e541cc3e3ae8816c4c3e05 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 861948 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/1184181 Reviewed-by: Philip Rogers <pdr@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#584911} [add] https://crrev.com/bea4d986d6fb6aa030542a1057901981af39c0ff/third_party/WebKit/LayoutTests/paint/background/scrolling-background-with-negative-z-child-expected.html [add] https://crrev.com/bea4d986d6fb6aa030542a1057901981af39c0ff/third_party/WebKit/LayoutTests/paint/background/scrolling-background-with-negative-z-child.html [modify] https://crrev.com/bea4d986d6fb6aa030542a1057901981af39c0ff/third_party/WebKit/LayoutTests/paint/invalidation/invalidation-on-foreground-graphics-layer-expected.txt [add] https://crrev.com/bea4d986d6fb6aa030542a1057901981af39c0ff/third_party/WebKit/LayoutTests/paint/stacking/html-with-negative-z-index-expected.html [add] https://crrev.com/bea4d986d6fb6aa030542a1057901981af39c0ff/third_party/WebKit/LayoutTests/paint/stacking/html-with-negative-z-index.html [modify] https://crrev.com/bea4d986d6fb6aa030542a1057901981af39c0ff/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc [modify] https://crrev.com/bea4d986d6fb6aa030542a1057901981af39c0ff/third_party/blink/renderer/core/paint/paint_layer.cc [modify] https://crrev.com/bea4d986d6fb6aa030542a1057901981af39c0ff/third_party/blink/renderer/core/paint/paint_layer_scrollable_area_test.cc |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by ekaramad@chromium.org
, Jul 9