Bug 123329 - [CSS Regions] Fix Assert SHOULD NEVER BE REACHED in RenderLayer::enclosingElement()
Summary: [CSS Regions] Fix Assert SHOULD NEVER BE REACHED in RenderLayer::enclosingEle...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mihai Maerean
URL:
Keywords: AdobeTracked
Depends on:
Blocks: 57312
  Show dependency treegraph
 
Reported: 2013-10-25 02:22 PDT by Mihai Maerean
Modified: 2014-02-04 06:56 PST (History)
7 users (show)

See Also:


Attachments
layout test causing an assert (968 bytes, text/html)
2013-10-25 02:22 PDT, Mihai Maerean
no flags Details
layout test expected result (999 bytes, text/plain)
2013-10-25 02:23 PDT, Mihai Maerean
no flags Details
Another test (639 bytes, text/html)
2014-01-27 02:26 PST, Mihnea Ovidenie
no flags Details
Another repro (425 bytes, text/html)
2014-01-28 04:57 PST, Mihnea Ovidenie
no flags Details
patch (6.51 KB, patch)
2014-01-30 09:19 PST, Mihai Maerean
mihnea: review+
mihnea: commit-queue-
Details | Formatted Diff | Diff
patch for landing (6.82 KB, patch)
2014-02-04 01:27 PST, Mihai Maerean
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>