Bug 171643 – Should never hit layout while updating the render tree.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[alan]

Running layout on a half-backed tree is not the greatest idea. We'd better ASSERT on it.

[alan]

Created attachment 309007 [details]
Patch

[Brent Fulgham]

Comment on attachment 309007 [details]
Patch

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

> Source/WebCore/page/FrameView.cpp:1291
> +    ASSERT(!frame().document()->inRenderTreeUpdate());

Should this be a release assert so we can't continue if this happens?

[Simon Fraser (smfr)]

Comment on attachment 309007 [details]
Patch

What about asserting about layouts in render tree update, and about style recall in render tree update?

Seems like we really want to have more stateful asserting.

[alan]

Comment on attachment 309007 [details]
Patch

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

>> Source/WebCore/page/FrameView.cpp:1291
>> +    ASSERT(!frame().document()->inRenderTreeUpdate());
> 
> Should this be a release assert so we can't continue if this happens?

That might work too (though we could actually finish the layout just fine), but it should be at least assertion with security implication.

[alan]

Created attachment 309009 [details]
Patch

[alan]

(In reply to zalan from comment #4)
> Comment on attachment 309007 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=309007&action=review
> 
> >> Source/WebCore/page/FrameView.cpp:1291
> >> +    ASSERT(!frame().document()->inRenderTreeUpdate());
> > 
> > Should this be a release assert so we can't continue if this happens?
> 
> That might work too (though we could actually finish the layout just fine),
> but it should be at least assertion with security implication.
Actually I think this should be part of the "why don't we turn all assert_with_security_implication into release asserts" conversation.

[WebKit Commit Bot]

Comment on attachment 309009 [details]
Patch

Clearing flags on attachment: 309009

Committed r216196: <http://trac.webkit.org/changeset/216196>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.