ASSERTION FAILED: interval.low() == m_layoutObject->logicalTopForFloat(floatingO |
||||||||||||
Issue descriptionDetailed report: https://cluster-fuzz.appspot.com/testcase?key=4943328809844736 Fuzzer: mbarbella_js_mutation_layout Job Type: linux_debug_content_shell_drt Platform Id: linux Crash Type: ASSERT Crash Address: Crash State: ASSERTION FAILED: interval.low() == m_layoutObject->logicalTopForFloat(floatingO blink::ComputeFloatOffsetAdapter< void blink::PODIntervalTree<blink::LayoutUnit, blink::FloatingObject*>::searchFo Minimized Testcase (0.33 Kb): Download: https://cluster-fuzz.appspot.com/download/AMIfv96HpS36A1V1KMEesnxJ9AxTsMcrD23dwDi1bXDkPOxOHeCr17QVPxO0-0O5w4dlXlLs6b1iYWQRZjfWTufb6daCKnpkyCjX_pPcPPtpSqvTNm1xoykj-WwPlLqFDj501GSYJ6KStW18GNhn5ioP6GaQauEwnw <style> #header ul { } #header li { padding: 0 0 0 9px; } #header a { float: left; </style> <div id="header"> <li> <a> This is link two and it shouldn't overlap link one </a> </li> </script> <style> html { writing-mode:vertical-rl; } #h { writing-mode: horizontal-tb; </style> <div id="h"> Filer: mummareddy See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
,
Apr 18 2016
What's happening here is: - we have a float already laid out in a container; - we change the writing mode of the container and its child and make them both orthogonal to each other; - when we come to layout next we call layoutOrthogonalWritingModeRoots() on the orthogonal child; - when we attempt to shrinkLogicalWidthToAvoidFloats() on the orthogonal child we find that its container's float list has not been updated to reflect its new writing mode (because the container hasn't been laid out yet). - we ASSERT because the interval tree's high/low values don't match those of the container in its new writing mode. This is very specific to when the writing mode changes on *both* container and child making both orthogonal to each other and *only then* do we get to layout on them. I don't think there's anything in this scenario that allows us to safely skip laying out the orthogonal root and just lay it out as normal. I'm hoping kojii might have an idea on how to resolve this circular-looking dependency - the orthogonal root seems to need its container to be laid out first and I think the same could be said for the container and its orthoganal child!
,
Apr 19 2016
@kojii - assigning to you so you can take a look? This is a regression introduced by https://codereview.chromium.org/1549153002. Happy to do up a CL if you can give some feedback on #2.
,
Apr 22 2016
Apologies for a late response, I was swamped by a few RBs. I'm not really familiar with how floats work, so please consider I understand only from what you wrote in #2.
From this bug IIUC, I think there are both cases where:
A. container depends on orthogonal child's layout, and
B. orthogonal child depends on parent's layout
so blindly doing one before the other can't serve all cases.
Examples of case A is where parent has "(logical-)width: fit-content" and that it needs to know the logical-height of orthogonal children to compute preferred width. This is where layoutOrthogonalWritingModeRoots() should help.
layoutOrthogonalWritingModeRoots() has some exceptions already. Can we identify this case and add to the exceptions?
Maybe this is more complicated if we add:
body { height: fit-content } /* logical-width of body */
We will then need to know the logical-height of #h to layout body, but the position of #h depends on floats in body to be laid out? My lack of knowledge on floats prevents me whether this can be supported nicely or not. WDYT?
,
Apr 24 2016
The layout in layoutOrthogonalWritingModeRoots() for #h is going to be wrong in this test case. #h gets corrected in normal layout. So I think we want to skip orthoganal root layout for #h here and the class of object it represents. Here are some of the reasons orthoganal layout will always be wrong for #h: - its container has an orthogonal writing mode to it (obviously) - its container hasn't laid out yet in its orthogonal writing mode yet - its current stale 'layout state' reflects one that isn't orthogonal - its container contains floats that affect #h's width, so when it lays out it needs to avoid them We don't have a way of detecting that the orthogonal writing mode on the container hasn't been applied to the container yet. Do we? If we do, we could use that and the fact that container has floats to skip layoutOrthogonalWritingModeRoots() for #h in this instance.
,
Apr 24 2016
ClusterFuzz has detected this testcase as flaky and is unable to reproduce it in the original crash revision. Skipping fixed testing check and marking it as potentially fixed. Detailed report: https://cluster-fuzz.appspot.com/testcase?key=4943328809844736 Fuzzer: mbarbella_js_mutation_layout Job Type: linux_debug_content_shell_drt Platform Id: linux Crash Type: ASSERT Crash Address: Crash State: ASSERTION FAILED: interval.low() == m_layoutObject->logicalTopForFloat(floatingO blink::ComputeFloatOffsetAdapter< void blink::PODIntervalTree<blink::LayoutUnit, blink::FloatingObject*>::searchFo Minimized Testcase (0.33 Kb): Download: https://cluster-fuzz.appspot.com/download/AMIfv96HpS36A1V1KMEesnxJ9AxTsMcrD23dwDi1bXDkPOxOHeCr17QVPxO0-0O5w4dlXlLs6b1iYWQRZjfWTufb6daCKnpkyCjX_pPcPPtpSqvTNm1xoykj-WwPlLqFDj501GSYJ6KStW18GNhn5ioP6GaQauEwnw <style> #header ul { } #header li { padding: 0 0 0 9px; } #header a { float: left; </style> <div id="header"> <li> <a> This is link two and it shouldn't overlap link one </a> </li> </script> <style> html { writing-mode:vertical-rl; } #h { writing-mode: horizontal-tb; </style> <div id="h"> See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information. If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
,
Apr 25 2016
#5: If we skip orthogonal root layout, out layout will be incorrect when the container is fit-content (such as display: inline-block), correct? http://jsbin.com/zahepiw/edit?html,output i.e., if we layout #c before #h, the width of #c will not count #h. Maybe what's wrong is here: - when we attempt to shrinkLogicalWidthToAvoidFloats() on the orthogonal child we find that its container's float list has not been updated to reflect its new writing mode (because the container hasn't been laid out yet). If the child is orthogonal, parent's floats should not affect logical width, do they?
,
Apr 27 2016
@kojii: Yes, I think it does - at least the orthogonal child should shrink to avoid it overlapping with it. I think the test case in #2 shows this in action. Wouldn't the child overlap with the float if it didn't have to shrink to avoid it? It overlaps with it when the float and the div have the same writing mode (and thus the same formatting context, so they don't repel each other). That said I'm not very certain that I'm correct - so am hoping you can point to another, better reason why the orthoganal flow div doesn't need to worry about avoiding floats from other formatting contexts.
,
May 28 2016
,
May 29 2016
From #6, it's no longer reproducible. I'm guessing this was caused by a side effect fixed in r389691. Shall we close this now?
,
May 29 2016
No, it still asserts for me at r395275.
,
May 29 2016
Thanks, I'll check your case.
,
May 29 2016
Confirmed that the assert fires with the case in #12, thanks robhogan@. Unlike issue 613869 , this is not use-after-free. I guess issue 613869 is easier to track to see where we fail to clear m_floatingObjects.m_lowestFloatBottomCache.
,
May 31 2016
From your comment #8, it looks like we have different views but I can't figure it out exactly what it is, so let me try a bit more. When shrinking logical width to avoid floats, and when it's orthogonal, it should try to shrink logical height instead. However, height is not shrinkable. But your comment #8 reads to me that you want to shrink logical height. I also don't understand what you mean when you say "overlaps". In case this helps, I'm attaching screenshot of test case in #2 with the WIP patch. From this screenshot: * It no longer asserts. * The height of "Text" is no shrinkable. * The "Text" does not overlap with anything else. Sorry that I just tried to explain my understanding, but from this, can you figure out what I miss?
,
Jun 6 2016
,
Jun 6 2016
"When shrinking logical width to avoid floats, and when it's orthogonal, it should try to shrink logical height instead. However, height is not shrinkable." Not with you here - the 'Text' child needs to shrinkLogicalWidthToAvoidFloats(), i.e. shrink it's width to avoid the block of vertical floating text to it's own logical right. When it tries to do this it gets into trouble because the co-ordinates of the float in its container's float list does not reflect the container's current writing mode. We get a correct result because the orthogonal child relayouts during the 'main' layout. Rather than removing floats in this situation I think it would be better to skip laying out the orthogonal child and let it relayout later.
,
Jun 6 2016
Assigning to kojii@chromium.org as this is an issue with orthogonal layout.
,
Jun 10 2016
Let me lower the priority, because this is assertion-failure-only, no crashing bug, and engineers are having hard time to come up with good resolution. WIP here https://codereview.chromium.org/2025543002 but I won't have time to work on it in near term.
,
Jun 10 2016
,
Jun 13 2016
ClusterFuzz testcase is verified as fixed, closing issue. If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
,
Jun 14 2016
,
Sep 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5c43382692e6b687373576984181e52c30ccd142 commit 5c43382692e6b687373576984181e52c30ccd142 Author: kojii <kojii@chromium.org> Date: Sat Sep 17 19:39:59 2016 Fix when orthogonal writing mode roots have floating siblings When orthogonal writing mode roots have floating siblings, its containing block may still have old or even deleted LayoutObjects. This occurs when LayoutMultiColumnFlowThread::populate(), LayoutBoxModelObject::moveChildrenTo() with !fullRemoveInsert, or more, for the optimization purposes. This patch clears such objects to be re-created when the containing block is laid out. BUG= 604095 , 633409 , 646178 Review-Url: https://codereview.chromium.org/2025543002 Cr-Commit-Position: refs/heads/master@{#419376} [add] https://crrev.com/5c43382692e6b687373576984181e52c30ccd142/third_party/WebKit/LayoutTests/fast/writing-mode/orthogonal-writing-modes-floats-crash-expected.txt [add] https://crrev.com/5c43382692e6b687373576984181e52c30ccd142/third_party/WebKit/LayoutTests/fast/writing-mode/orthogonal-writing-modes-floats-crash.html [modify] https://crrev.com/5c43382692e6b687373576984181e52c30ccd142/third_party/WebKit/Source/core/frame/FrameView.cpp
,
Sep 20 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0a010e317a1043e7faf7160f6d2afb760d6f1f5 commit f0a010e317a1043e7faf7160f6d2afb760d6f1f5 Author: Koji Ishii <kojii@chromium.org> Date: Tue Sep 20 13:11:28 2016 Merge 2840: Fix when orthogonal writing mode roots have floating siblings When orthogonal writing mode roots have floating siblings, its containing block may still have old or even deleted LayoutObjects. This occurs when LayoutMultiColumnFlowThread::populate(), LayoutBoxModelObject::moveChildrenTo() with !fullRemoveInsert, or more, for the optimization purposes. This patch clears such objects to be re-created when the containing block is laid out. BUG= 604095 , 633409 , 646178 Review-Url: https://codereview.chromium.org/2025543002 Cr-Commit-Position: refs/heads/master@{#419376} (cherry picked from commit 5c43382692e6b687373576984181e52c30ccd142) Review URL: https://codereview.chromium.org/2355793002 . Cr-Commit-Position: refs/branch-heads/2840@{#436} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [add] https://crrev.com/f0a010e317a1043e7faf7160f6d2afb760d6f1f5/third_party/WebKit/LayoutTests/fast/writing-mode/orthogonal-writing-modes-floats-crash-expected.txt [add] https://crrev.com/f0a010e317a1043e7faf7160f6d2afb760d6f1f5/third_party/WebKit/LayoutTests/fast/writing-mode/orthogonal-writing-modes-floats-crash.html [modify] https://crrev.com/f0a010e317a1043e7faf7160f6d2afb760d6f1f5/third_party/WebKit/Source/core/frame/FrameView.cpp
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f0a010e317a1043e7faf7160f6d2afb760d6f1f5 commit f0a010e317a1043e7faf7160f6d2afb760d6f1f5 Author: Koji Ishii <kojii@chromium.org> Date: Tue Sep 20 13:11:28 2016 Merge 2840: Fix when orthogonal writing mode roots have floating siblings When orthogonal writing mode roots have floating siblings, its containing block may still have old or even deleted LayoutObjects. This occurs when LayoutMultiColumnFlowThread::populate(), LayoutBoxModelObject::moveChildrenTo() with !fullRemoveInsert, or more, for the optimization purposes. This patch clears such objects to be re-created when the containing block is laid out. BUG= 604095 , 633409 , 646178 Review-Url: https://codereview.chromium.org/2025543002 Cr-Commit-Position: refs/heads/master@{#419376} (cherry picked from commit 5c43382692e6b687373576984181e52c30ccd142) Review URL: https://codereview.chromium.org/2355793002 . Cr-Commit-Position: refs/branch-heads/2840@{#436} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [add] https://crrev.com/f0a010e317a1043e7faf7160f6d2afb760d6f1f5/third_party/WebKit/LayoutTests/fast/writing-mode/orthogonal-writing-modes-floats-crash-expected.txt [add] https://crrev.com/f0a010e317a1043e7faf7160f6d2afb760d6f1f5/third_party/WebKit/LayoutTests/fast/writing-mode/orthogonal-writing-modes-floats-crash.html [modify] https://crrev.com/f0a010e317a1043e7faf7160f6d2afb760d6f1f5/third_party/WebKit/Source/core/frame/FrameView.cpp
,
Nov 22 2016
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
||||||||||||
►
Sign in to add a comment |
||||||||||||
Comment 1 by mummare...@chromium.org
, Apr 16 2016Owner: robhogan@chromium.org
Status: Assigned (was: Available)