CHECK failure: !column_sets_invalidated_ |
||||||||||
Issue descriptionDetailed report: https://clusterfuzz.com/testcase?key=5594729006497792 Fuzzer: bj_broddelwerk Job Type: linux_debug_chrome Platform Id: linux Crash Type: CHECK failure Crash Address: Crash State: !RuntimeEnabledFeatures::SlimmingPaintV175Enabled() || !column_sets_invalidated_ blink::LayoutFlowThread::FragmentsBoundingBox blink::LayoutBlockFlow::AddOverflowFromInlineChildren Sanitizer: address (ASAN) Regressed: https://clusterfuzz.com/revisions?job=linux_debug_chrome&range=537453:537466 Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5594729006497792 Issue filed automatically. See https://github.com/google/clusterfuzz-tools for more information.
,
Mar 18 2018
Automatically adding ccs based on suspected regression changelists: Reland "[jumbo] avoid helper function collisions in VisibleUnits*Test.cpp" by brucedawson@chromium.org - https://chromium.googlesource.com/chromium/src/+/0fbd51f0347223bf09ae85d28fa34536c1dbb2db Correctly serialize empty content_type for blobs. by mek@chromium.org - https://chromium.googlesource.com/chromium/src/+/bb84d169288460af40536ebbe0303cb35f11a5a1 [SPv175] Enable SlimmingPaintV175 by default by wangxianzhu@chromium.org - https://chromium.googlesource.com/chromium/src/+/0a9a5c311a1d3a298f952e495510bd6fe3faa2f6 If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label.
,
Mar 19 2018
,
Mar 20 2018
,
Mar 20 2018
,
Mar 22 2018
Confirmed this repros with SPV175 but not with --disable-blink-features=SlimmingPaintV175. I will take a look.
,
Mar 22 2018
Here's a minimized testcase: <!doctype html> <object style="outline-style: auto;"> <div style="column-count: 1;">A</div> </object>
,
Mar 23 2018
The core issue turned out to be unrelated to SPV175 and is just a bug in outline rects with continuations and multicolumn. To compute the outline rects for a block, we compute the outlines of all children which can include children that have continuations. When computing the outline rect of a continuation, we can jump to a different subtree which has not yet entered layout. Without entering layout, multicolumn has dirty state and will hit a DCHECK because LayoutFlowThread::AddOutlineRects calls LayoutFlowThread::FragmentsBoundingBox. This bug was introduced in http://crrev.com/10b7639 but it was masked by an incorrect SPV175 DCHECK. This incorrect check was removed in http://crrev.com/fed1028c3 which exposes this bug with and without SPV175. Here's a minimal testcase: <!doctype html> <span style="outline-style: auto;"> <div style="column-count: 1;">A</div> </span> @mstensho, would you be able to look at this?
,
Mar 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/fed1028c3d76098c8d47f8ee977dac3db6e11167 commit fed1028c3d76098c8d47f8ee977dac3db6e11167 Author: Philip Rogers <pdr@chromium.org> Date: Fri Mar 23 15:38:49 2018 [CI] Remove SPV175 check in LayoutFlowThread::FragmentsBoundingBox This patch removes a check for SPV175 that was added in [1]. It is not safe to call LayoutFlowThread::FragmentsBoundingBox if |column_sets_invalidated_|, regardless of SPV175/SPV2. column_sets_invalidated_ is a bit that tracks whether multi_column_set_list_ is up-to-date. In LayoutMultiColumnFlowThread::ColumnSetAtBlockOffset, multi_column_set_list_ is used (and there's a DCHECK that it's valid). FragmentsBoundingBox can call ColumnSetAtBlockOffset through the following: LayoutFlowThread::FragmentsBoundingBox -> LayoutMultiColumnSet::FragmentsBoundingBox -> MultiColumnFragmentainerGroup::FragmentsBoundingBox -> MultiColumnFragmentainerGroup::FlowThreadTranslationAtOffset -> LayoutMultiColumnFlowThread::FlowThreadTranslationAtOffset -> LayoutMultiColumnFlowThread::ColumnSetAtBlockOffset [1] https://codereview.chromium.org/1337233002 Bug: 823127 Change-Id: I2a6e848f4f8b449b2bf4f40cd5d70c4f801f10f6 Reviewed-on: https://chromium-review.googlesource.com/977024 Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Commit-Queue: Philip Rogers <pdr@chromium.org> Cr-Commit-Position: refs/heads/master@{#545459} [modify] https://crrev.com/fed1028c3d76098c8d47f8ee977dac3db6e11167/third_party/WebKit/Source/core/layout/LayoutFlowThread.cpp
,
Mar 25 2018
Thanks for the analysis. Yeah, in order to read out correct layout data, we first need to lay out. :) I don't understand how this is supposed to work if we calculate outline rectangles before laying out (which happens when there are continuations). Isn't that approach flawed? We wouldn't be able to calculate anything useful for non-multicol blocks either, I guess, although the DCHECK failure is multicol-specific. So are you asking me to just add some code to evade the DCHECK failure?
,
Mar 25 2018
Move AddOutlineRects (even visual overflow calculation) out of Layout into PrePaint?
,
Mar 26 2018
@mstensho, I incorrectly thought that only multicolumn needed layout, but I think you're right that this affects all outline code. I think Xianzhu's solution is a good one, and could even have a perf benefit by moving work to a later stage. There is a correctness bug hiding in here. I think we need to focus on SPV175 bugs over this existing architectural bug though, so lowering to P3.
,
Jul 10
ClusterFuzz testcase 5594729006497792 appears to be flaky, updating reproducibility label.
,
Jul 10
ClusterFuzz testcase 5594729006497792 is flaky and no longer crashes, so closing issue. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue. |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by ClusterFuzz
, Mar 18 2018Labels: Test-Predator-Auto-Components