[webkit-dev] Crash in RenderLayer related to setStyle() - Questions
I've identified a crash with the MathML implementation related to use of
CSS style rules that cause a RenderLayer instance to be created. In the
MathML code's various createRenderer() methods, they always call
RenderObject::setStyle() on the object they've just created.
When the setStyle() method is called, a style difference is calculated
and propagated. If you call setStyle() inside your createRenderer()
method, you're calling it on an unattached RenderObject instance.
Further, if there happens to be a rule like:
math {
overflow: auto;
}
you'll get a layer created by RenderBoxModelObject. That layer
will get the style change.
Right now, as the code stands, you'll get an exception and crash
due to the fact that the RenderLayer code makes some assumptions
about the RenderObject instance being attached to the tree.
The fix is trivial but my question is basically: what's the right thing
to do here? Should the setStyle() be called in createRenderer() ? It
seems like the answer is no because the style gets associated
somewhere else. If I remove the call, the crash is also fixed (without
any change to RenderLayer) and the tests still all pass for MathML.
Further, should RenderLayer be made more safe? Should it check
for zero pointer values in a couple places and avoid this?
If we shouldn't be calling setStyle() in createRenderer(), then fixing
RenderLayer would just hide that fact. While an ASSERT wouldn't
hide things, it would still only arise when a layer is created.
I know how to fix the MathML code and I just want to make
sure I understand why calling setStyle() is "bad".
I'm not sure what should be done about RenderLayer or otherwise.
Suggestions?
--
--Alex Milowski
"The excellence of grammar as a guide is proportional to the paucity of the
inflexions, i.e. to the degree of analysis effected by the language
considered."
Bertrand Russell in a footnote of Principles of Mathematics
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Crash in RenderLayer related to setStyle() - Questions
On Jul 23, 2010, at 3:48 PM, Alex Milowski wrote:
> I've identified a crash with the MathML implementation related to use of
> CSS style rules that cause a RenderLayer instance to be created. In the
> MathML code's various createRenderer() methods, they always call
> RenderObject::setStyle() on the object they've just created.
>
> When the setStyle() method is called, a style difference is calculated
> and propagated. If you call setStyle() inside your createRenderer()
> method, you're calling it on an unattached RenderObject instance.
>
> Further, if there happens to be a rule like:
>
> math {
> overflow: auto;
> }
>
> you'll get a layer created by RenderBoxModelObject. That layer
> will get the style change.
>
> Right now, as the code stands, you'll get an exception and crash
> due to the fact that the RenderLayer code makes some assumptions
> about the RenderObject instance being attached to the tree.
>
> The fix is trivial but my question is basically: what's the right thing
> to do here? Should the setStyle() be called in createRenderer() ? It
> seems like the answer is no because the style gets associated
> somewhere else. If I remove the call, the crash is also fixed (without
> any change to RenderLayer) and the tests still all pass for MathML.
>
> Further, should RenderLayer be made more safe? Should it check
> for zero pointer values in a couple places and avoid this?
>
> If we shouldn't be calling setStyle() in createRenderer(), then fixing
> RenderLayer would just hide that fact. While an ASSERT wouldn't
> hide things, it would still only arise when a layer is created.
>
> I know how to fix the MathML code and I just want to make
> sure I understand why calling setStyle() is "bad".
>
> I'm not sure what should be done about RenderLayer or otherwise.
>
> Suggestions?
Do not call setStyle() from createRenderer(). Node::createRendererIfNeeded()
calls setAnimatableStyle() on the renderer after setting it as the node’s
renderer, so there is no need for createRenderer() to call setStyle(); and as
you saw, calling it before setting the node’s renderer violates the assumption
that the pointers between node and renderer are in a consistent state (which is
not to say that renderer->node->renderer is always the original renderer).
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Crash in RenderLayer related to setStyle() - Questions
Coincidentally, http://webkit.org/coding/dom-element-attach.html
:DG<
On Sat, Jul 24, 2010 at 9:53 AM, Dan Bernstein wrote:
>
> On Jul 23, 2010, at 3:48 PM, Alex Milowski wrote:
>
>> I've identified a crash with the MathML implementation related to use of
>> CSS style rules that cause a RenderLayer instance to be created. In the
>> MathML code's various createRenderer() methods, they always call
>> RenderObject::setStyle() on the object they've just created.
>>
>> When the setStyle() method is called, a style difference is calculated
>> and propagated. If you call setStyle() inside your createRenderer()
>> method, you're calling it on an unattached RenderObject instance.
>>
>> Further, if there happens to be a rule like:
>>
>> math {
>> overflow: auto;
>> }
>>
>> you'll get a layer created by RenderBoxModelObject. That layer
>> will get the style change.
>>
>> Right now, as the code stands, you'll get an exception and crash
>> due to the fact that the RenderLayer code makes some assumptions
>> about the RenderObject instance being attached to the tree.
>>
>> The fix is trivial but my question is basically: what's the right thing
>> to do here? Should the setStyle() be called in createRenderer() ? It
>> seems like the answer is no because the style gets associated
>> somewhere else. If I remove the call, the crash is also fixed (without
>> any change to RenderLayer) and the tests still all pass for MathML.
>>
>> Further, should RenderLayer be made more safe? Should it check
>> for zero pointer values in a couple places and avoid this?
>>
>> If we shouldn't be calling setStyle() in createRenderer(), then fixing
>> RenderLayer would just hide that fact. While an ASSERT wouldn't
>> hide things, it would still only arise when a layer is created.
>>
>> I know how to fix the MathML code and I just want to make
>> sure I understand why calling setStyle() is "bad".
>>
>> I'm not sure what should be done about RenderLayer or otherwise.
>>
>> Suggestions?
>
> Do not call setStyle() from createRenderer(). Node::createRendererIfNeeded()
> calls setAnimatableStyle() on the renderer after setting it as the node’s
> renderer, so there is no need for createRenderer() to call setStyle(); and as
> you saw, calling it before setting the node’s renderer violates the
> assumption that the pointers between node and renderer are in a consistent
> state (which is not to say that renderer->node->renderer is always the
> original renderer).
> ___
> webkit-dev mailing list
> webkit-dev@lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>
___
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
