Bug 106132 - Remove RenderObjectChildList::beforePseudoElementRenderer and afterPseudoElementRenderer
Summary: Remove RenderObjectChildList::beforePseudoElementRenderer and afterPseudoElem...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Elliott Sprehn
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-01-04 14:05 PST by Elliott Sprehn
Modified: 2013-01-05 15:08 PST (History)
9 users (show)

See Also:


Attachments
Patch (14.44 KB, patch)
2013-01-04 14:19 PST, Elliott Sprehn
no flags Details | Formatted Diff | Diff
Patch (14.48 KB, patch)
2013-01-04 14:40 PST, Elliott Sprehn
no flags Details | Formatted Diff | Diff
Patch (15.64 KB, patch)
2013-01-05 13:43 PST, Elliott Sprehn
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Elliott Sprehn 2013-01-04 14:05:56 PST
Remove RenderObjectChildList::beforePseudoElementRenderer and afterPseudoElementRenderer
Comment 1 Elliott Sprehn 2013-01-04 14:19:35 PST
Created attachment 181374 [details]
Patch
Comment 2 Early Warning System Bot 2013-01-04 14:31:47 PST
Comment on attachment 181374 [details]
Patch

Attachment 181374 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/15697303
Comment 3 Abhishek Arya 2013-01-04 14:32:17 PST
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 ?
Comment 4 Early Warning System Bot 2013-01-04 14:34:01 PST
Comment on attachment 181374 [details]
Patch

Attachment 181374 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/15686298
Comment 5 Abhishek Arya 2013-01-04 14:38:11 PST
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
Comment 6 Elliott Sprehn 2013-01-04 14:40:16 PST
Created attachment 181376 [details]
Patch
Comment 7 Elliott Sprehn 2013-01-04 14:42:03 PST
(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.
Comment 8 Elliott Sprehn 2013-01-05 13:43:55 PST
Created attachment 181444 [details]
Patch

Made variable name usage consistent.
Comment 9 WebKit Review Bot 2013-01-05 15:08:25 PST
Comment on attachment 181444 [details]
Patch

Clearing flags on attachment: 181444

Committed r138909: <http://trac.webkit.org/changeset/138909>
Comment 10 WebKit Review Bot 2013-01-05 15:08:30 PST
All reviewed patches have been landed.  Closing bug.