Bug 123329 – [CSS Regions] Fix Assert SHOULD NEVER BE REACHED in RenderLayer::enclosingElement()

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Mihai Maerean]

Created attachment 215154 [details]
layout test causing an assert

SHOULD NEVER BE REACHED
/Volumes/.../WebKit/Source/WebCore/rendering/RenderLayer.cpp(4445) : WebCore::Element *WebCore::RenderLayer::enclosingElement() const
1   0x10da273f0 WTFCrash
2   0x10fda0585 WebCore::RenderLayer::enclosingElement() const
3   0x10fda20bb WebCore::RenderLayer::hitTestContents
4   0x10fda1eb1 WebCore::RenderLayer::hitTestContentsForFragments
5   0x10fda0082 WebCore::RenderLayer::hitTestLayer
6   0x10fd9f285 WebCore::RenderLayer::hitTest
7   0x10fd41086 WebCore::RenderFlowThread::hitTestFlowThreadPortionInRegion
8   0x10fe3be64 WebCore::RenderRegion::hitTestContents
9   0x10fc84a90 WebCore::RenderBlock::nodeAtPoint
10  0x10fc85451 WebCore::RenderBlock::hitTestContents
11  0x10fc84a90 WebCore::RenderBlock::nodeAtPoint
12  0x10fe31639 WebCore::RenderObject::hitTest
13  0x10fda2007 WebCore::RenderLayer::hitTestContents
...
20  0x10fd9f285 WebCore::RenderLayer::hitTest
21  0x10ff2fb2f WebCore::RenderView::hitTest
22  0x10ff2fae4 WebCore::RenderView::hitTest
23  0x10ee2a4cb WebCore::Document::prepareMouseEvent
24  0x10efcbbba WebCore::EventHandler::prepareMouseEvent
25  0x10efcc0cf WebCore::EventHandler::handleMouseMoveEvent
26  0x10efcbc56 WebCore::EventHandler::mouseMoved
27  0x10be63708 WebKit::handleMouseEvent
28  0x10be63507 WebKit::WebPage::mouseEvent

[Mihai Maerean]

Created attachment 215155 [details]
layout test expected result

[Mihnea Ovidenie]

Created attachment 222313 [details]
Another test

Another test. Move mouse at the top of "AAAAA" text inside the region to repro the assert.

[Mihnea Ovidenie]

Created attachment 222431 [details]
Another repro

Found another repro (you need to manually create a green.png with 100x100). After you load the test file, move the mouse below the green rectangle.

[Mihai Maerean]

Created attachment 222673 [details]
patch

[Andrei Bucur]

Comment on attachment 222673 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=222673&action=review

> Source/WebCore/rendering/RenderLayer.cpp:5023
> +        if (isOutOfFlowRenderFlowThread()) {

Can you explain why this is not happening for the new multi-column implementation? The multi-column flow thread is anonymous as well if I'm not mistaken so this should be an issue in that case also, right?

[Mihai Maerean]

(In reply to comment #5)
> (From update of attachment 222673 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=222673&action=review
> 
> > Source/WebCore/rendering/RenderLayer.cpp:5023
> > +        if (isOutOfFlowRenderFlowThread()) {
> 
> Can you explain why this is not happening for the new multi-column implementation? The multi-column flow thread is anonymous as well if I'm not mistaken so this should be an issue in that case also, right?

The RenderMultiColumnFlowThread is the child of the RenderBlock that has the multi-col CSS property and, because of this, the enclosingElement of the RenderMultiColumnFlowThread is the enclosingElement of the RenderBlock.
In the case of the RenderNamedFlowThread that is the child of the RenderView, there is no enclosingElement.

[Mihnea Ovidenie]

Comment on attachment 222673 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=222673&action=review

r=me with small tweaks for the tests.

> LayoutTests/fast/regions/assert-hit-test-image.html:19
> +    <p class="result">

Please write the result in a different place. Right now, the description is overwritten by PASS/FAIL.

> LayoutTests/fast/regions/auto-size/region-same-height-as-div-with-inline-child.html:26
> +    <div class="result">

Same as above, lets have the description of the test separate than the result of running the test.

[Mihai Maerean]

Created attachment 223085 [details]
patch for landing

[WebKit Commit Bot]

Comment on attachment 223085 [details]
patch for landing

Clearing flags on attachment: 223085

Committed r163369: <http://trac.webkit.org/changeset/163369>