Bug 123329

Summary: [CSS Regions] Fix Assert SHOULD NEVER BE REACHED in RenderLayer::enclosingElement()
Product: WebKit Reporter: Mihai Maerean <mmaerean>
Component: WebCore Misc.Assignee: Mihai Maerean <mmaerean>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, esprehn+autocc, glenn, kondapallykalyan, mihnea, simon.fraser, WebkitBugTracker
Priority: P2 Keywords: AdobeTracked
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 57312    
Attachments:
Description Flags
layout test causing an assert
none
layout test expected result
none
Another test
none
Another repro
none
patch
mihnea: review+, mihnea: commit-queue-
patch for landing none

Description Mihai Maerean 2013-10-25 02:22:58 PDT
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
Comment 1 Mihai Maerean 2013-10-25 02:23:57 PDT
Created attachment 215155 [details]
layout test expected result
Comment 2 Mihnea Ovidenie 2014-01-27 02:26:41 PST
Created attachment 222313 [details]
Another test

Another test. Move mouse at the top of "AAAAA" text inside the region to repro the assert.
Comment 3 Mihnea Ovidenie 2014-01-28 04:57:27 PST
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.
Comment 4 Mihai Maerean 2014-01-30 09:19:06 PST
Created attachment 222673 [details]
patch
Comment 5 Andrei Bucur 2014-02-03 01:06:11 PST
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?
Comment 6 Mihai Maerean 2014-02-03 05:02:55 PST
(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.
Comment 7 Mihnea Ovidenie 2014-02-04 00:49:57 PST
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.
Comment 8 Mihai Maerean 2014-02-04 01:27:36 PST
Created attachment 223085 [details]
patch for landing
Comment 9 WebKit Commit Bot 2014-02-04 02:05:54 PST
Comment on attachment 223085 [details]
patch for landing

Clearing flags on attachment: 223085

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