Bug 106132 – Remove RenderObjectChildList::beforePseudoElementRenderer and afterPseudoElementRenderer

• Component: New Bugs
• Status: RESOLVED FIXED

[Elliott Sprehn]

Remove RenderObjectChildList::beforePseudoElementRenderer and afterPseudoElementRenderer

[Elliott Sprehn]

Created attachment 181374 [details]
Patch

[Early Warning System Bot]

Comment on attachment 181374 [details]
Patch

Attachment 181374 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15697303

[Abhishek Arya]

Comment on attachment 181374 [details]
Patch

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

> Source/WebCore/rendering/RenderCounter.cpp:87
> +                if (RenderObject* before = toElement(renderer->node())->pseudoElementRenderer(BEFORE))

What ensures that this cast to toElement is safe ?

[Early Warning System Bot]

Comment on attachment 181374 [details]
Patch

Attachment 181374 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15686298

[Abhishek Arya]

Comment on attachment 181374 [details]
Patch

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

> Source/WebCore/rendering/RenderCounter.cpp:172
>              return result;

minor issue s/result/before.

Also, there is lot of different names used in the code like result, renderer, before, etc. I would prefer a consistent naming like beforePseudoElementRenderer and afterPseudoElementRenderer

[Elliott Sprehn]

Created attachment 181376 [details]
Patch

[Elliott Sprehn]

(In reply to comment #3)
> (From update of attachment 181374 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=181374&action=review
> 
> > Source/WebCore/rendering/RenderCounter.cpp:87
> > +                if (RenderObject* before = toElement(renderer->node())->pseudoElementRenderer(BEFORE))
> 
> What ensures that this cast to toElement is safe ?

If you look at the rest of that function you can see that renderer is the result of sibling->renderer() and sibling is an Element. The way that function is written is bad because it overwrites sibling right before this check so we need to do renderer->node() to get a reference to the original sibling.

[Elliott Sprehn]

Created attachment 181444 [details]
Patch

Made variable name usage consistent.

[WebKit Review Bot]

Comment on attachment 181444 [details]
Patch

Clearing flags on attachment: 181444

Committed r138909: <http://trac.webkit.org/changeset/138909>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.