[webkit-reviews] review canceled: [Bug 123759] ASSERTION FAILED: frame().view() == this in WebCore::FrameView::layout : [Attachment 215960] proposed patch

[bugzilla-daemon at webkit.org]

Sergio Correia <sergio.correia at openbossa.org> has canceled Sergio Correia
<sergio.correia at openbossa.org>'s request for review:
Bug 123759: ASSERTION FAILED: frame().view() == this in
WebCore::FrameView::layout
https://bugs.webkit.org/show_bug.cgi?id=123759

Attachment 215960: proposed patch
https://bugs.webkit.org/attachment.cgi?id=215960&action=review

------- Additional Comments from Sergio Correia <sergio.correia at openbossa.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=215960&action=review


>> Source/WebCore/rendering/RenderFrameBase.cpp:62
>> +	    if (childFrameView && childFrameView->frame().view())
> 
> By looking at this check, what probably happens here is that childFrameView
gets wiped out in updateWidgetPosition(), but since someone, somewhere down in
the callstack is retaining it, it is only detached at this point (-missing it
from Changelog)
> While layouting a detached FrameView is absolutely unnecessary, I am
wondering where we actually segfault on this. Knowing that, we might find a
better place/way to fix it. 
> This check itself is just odd and confuses me when I read the code. if you
can't find a better way to fix it, I'd rather not cache the view instead.

Actually I uploaded an earlier version of the patch. I am using
childFrameView->needsLayout() to check if we should call layout(), as we do in
other places. Tomorrow I can look again to see if we can find a better place to
fix it.
Thanks for the review.