Check failed: !NeedsSectionRecalc() |
||||
Issue descriptionGot this on my local ToT build on macOS: [74328:775:0719/132633.066598:FATAL:LayoutTable.h(607)] Check failed: !NeedsSectionRecalc(). 0 Chromium Framework 0x0000000112a004cc base::debug::StackTrace::StackTrace(unsigned long) + 28 1 Chromium Framework 0x0000000112a24c50 logging::LogMessage::~LogMessage() + 224 2 Chromium Framework 0x00000001173b4f7d blink::LayoutTable::TopNonEmptySection() const + 93 3 Chromium Framework 0x00000001173b7d69 blink::LayoutTable::ComputeCollapsedOuterBorderBefore() const + 105 4 Chromium Framework 0x00000001173b1bc6 blink::LayoutTable::BorderStart() const + 358 5 Chromium Framework 0x000000011731d589 blink::LayoutBox::NoOverflowRect() const + 233 6 Chromium Framework 0x000000011740bf5d blink::RelativeBounds(blink::LayoutObject const*, blink::ScrollableArea const*) + 605 7 Chromium Framework 0x000000011740bb3d blink::ScrollAnchor::Examine(blink::LayoutObject const*) const + 493 8 Chromium Framework 0x000000011740c45f blink::ScrollAnchor::FindAnchorRecursive(blink::LayoutObject*) + 31 9 Chromium Framework 0x000000011740c606 blink::ScrollAnchor::FindAnchorRecursive(blink::LayoutObject*) + 454 10 Chromium Framework 0x000000011740c606 blink::ScrollAnchor::FindAnchorRecursive(blink::LayoutObject*) + 454 11 Chromium Framework 0x000000011740c606 blink::ScrollAnchor::FindAnchorRecursive(blink::LayoutObject*) + 454 12 Chromium Framework 0x000000011740c346 blink::ScrollAnchor::FindAnchor() + 342 13 Chromium Framework 0x000000011740c868 blink::ScrollAnchor::NotifyBeforeLayout() + 312 14 Chromium Framework 0x0000000116ebdd54 blink::LocalFrameView::PerformPreLayoutTasks() + 564 15 Chromium Framework 0x0000000116ebb49c blink::LocalFrameView::UpdateLayout() + 716 16 Chromium Framework 0x0000000116b72a8d blink::Document::UpdateStyleAndLayout() + 365 17 Chromium Framework 0x0000000116b728ea blink::Document::UpdateStyleAndLayoutIgnorePendingStylesheets(blink::Document::RunPostLayoutTasks) + 26 18 Chromium Framework 0x0000000116e77fd9 blink::DOMVisualViewport::pageTop() const + 57 19 Chromium Framework 0x0000000116e9653f blink::LocalDOMWindow::scrollY() const + 95 20 Chromium Framework 0x0000000116556429 blink::V8Window::pageYOffsetAttributeGetterCallback(v8::FunctionCallbackInfo<v8::Value> const&) + 137 21 Chromium Framework 0x00000001117238a9 v8::internal::FunctionCallbackArguments::Call(void (*)(v8::FunctionCallbackInfo<v8::Value> const&)) + 441 22 Chromium Framework 0x000000011181c436 v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<false>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) + 918 23 Chromium Framework 0x000000011181b387 v8::internal::Builtins::InvokeApiFunction(v8::internal::Isolate*, bool, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, v8::internal::Handle<v8::internal::HeapObject>) + 1095 24 Chromium Framework 0x0000000111d71c96 v8::internal::Object::GetPropertyWithAccessor(v8::internal::LookupIterator*) + 742 25 Chromium Framework 0x0000000111f50285 v8::internal::__RT_impl_Runtime_GetProperty(v8::internal::Arguments, v8::internal::Isolate*) + 133 26 ??? 0x00001901915844c4 0x0 + 27494524142788 27 ??? 0x0000190191718197 0x0 + 27494525796759 28 ??? 0x00001901916a42aa 0x0 + 27494525321898 29 ??? 0x000019019158691b 0x0 + 27494524152091 30 ??? 0x0000190191725478 0x0 + 27494525850744 31 ??? 0x00001901916a42aa 0x0 + 27494525321898 32 ??? 0x0000190191725478 0x0 + 27494525850744 33 ??? 0x00001901916a42aa 0x0 + 27494525321898 34 ??? 0x00001901916a0859 0x0 + 27494525306969 The site is https://habrahabr.ru, and it was restored from previous run (where I killed Chrome with Ctrl-C).
,
Jul 27 2017
+wangxianzhu, who refactored some of this code recently
,
Sep 1 2017
,
Sep 2 2017
Assuming "section recalc" happens during layout, we may want to simply disable the check during FindAnchor. FindAnchor intentionally reads layout state before layout, so that it can compare before vs. after.
,
Sep 5 2017
Section recalc happens during layout, among other places. So 2 questions. How should LayoutTable check to see if FindAnchor is running? How do I write a test for this code change?
,
Sep 5 2017
I'd suggest creating a class similar to DisableCompositingQueryAsserts, that disables the DCHECK in LayoutTable if it exists on the stack. Then add an instance of it in FindAnchor. To write a test you could try to minimize the web content from the original repro, or just construct some content that causes needs_section_recalc_ to become true while scrolled down, and forces a layout.
,
Sep 5 2017
Do you have pointers to tests that do similar? I assume that, because of the need to simulate scrolling, we're talking about unit tests or sim tests, not layout?
,
Sep 5 2017
I think either a unit test or a layout test would work. In a layout test, you can scroll with window.scrollBy. You probably want the table overlapping the top of the viewport, so that FindAnchor will look at it. ScrollAnchorTest.cpp has a bunch of test cases for scroll anchoring.
,
Sep 22 2017
,
Sep 26 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/89aa7bd6d74c9aedbdb8a61048911eafed1474e1 commit 89aa7bd6d74c9aedbdb8a61048911eafed1474e1 Author: David Grogan <dgrogan@chromium.org> Date: Tue Sep 26 03:58:21 2017 [css] Fix scroll anchor X dirty table layout crash When an element is selected as scroll anchor, we need to get its bounding box. But when a table has unlaid out table parts, we DCHECK that its borders aren't being queried. When a table with unlaid out table parts is the scroll anchor, the DCHECK was triggered. The "fix" in this CL is to use the old border widths when scroll anchoring code is looking for an anchor. Bug: 746570 Change-Id: I424e3807c2ed7a7a20f02617e2e0586dba83e708 Reviewed-on: https://chromium-review.googlesource.com/679994 Reviewed-by: Steve Kobes <skobes@chromium.org> Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Commit-Queue: David Grogan <dgrogan@chromium.org> Cr-Commit-Position: refs/heads/master@{#504279} [add] https://crrev.com/89aa7bd6d74c9aedbdb8a61048911eafed1474e1/third_party/WebKit/LayoutTests/fast/layout/scroll-anchoring/table-collapsed-borders-crash.html [modify] https://crrev.com/89aa7bd6d74c9aedbdb8a61048911eafed1474e1/third_party/WebKit/Source/core/layout/LayoutTable.cpp [modify] https://crrev.com/89aa7bd6d74c9aedbdb8a61048911eafed1474e1/third_party/WebKit/Source/core/layout/LayoutTable.h [modify] https://crrev.com/89aa7bd6d74c9aedbdb8a61048911eafed1474e1/third_party/WebKit/Source/core/layout/ScrollAnchor.cpp
,
Sep 26 2017
,
Oct 4 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/41ad015abed2dbe9e0fbe64fecfc73c94afda343 commit 41ad015abed2dbe9e0fbe64fecfc73c94afda343 Author: David Grogan <dgrogan@chromium.org> Date: Wed Oct 04 01:35:48 2017 [css-tables] Remove DCHECK around dirty borders If something wants a table's collapsed borders when they might have changed but before we've recalculated them, just return the old ones. There used to be an explicit exception allowing this for scroll anchoring, but other things need the borders too. The scroll anchoring specific code is removed in this patch. Bug: 746570 , 770348 Change-Id: I1b7ceb607cf27ff04eb941ee9aabdadb2e5bfe2e Reviewed-on: https://chromium-review.googlesource.com/699735 Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org> Commit-Queue: David Grogan <dgrogan@chromium.org> Cr-Commit-Position: refs/heads/master@{#506269} [add] https://crrev.com/41ad015abed2dbe9e0fbe64fecfc73c94afda343/third_party/WebKit/LayoutTests/fast/table/flexbox-collapsed-borders-crash.html [modify] https://crrev.com/41ad015abed2dbe9e0fbe64fecfc73c94afda343/third_party/WebKit/Source/core/layout/LayoutTable.cpp [modify] https://crrev.com/41ad015abed2dbe9e0fbe64fecfc73c94afda343/third_party/WebKit/Source/core/layout/LayoutTable.h [modify] https://crrev.com/41ad015abed2dbe9e0fbe64fecfc73c94afda343/third_party/WebKit/Source/core/layout/ScrollAnchor.cpp |
||||
►
Sign in to add a comment |
||||
Comment 1 by e...@chromium.org
, Jul 27 2017Components: -Blink>Layout Blink>Layout>Table
Status: Available (was: Untriaged)