Bug 80856 – Split the extra logic out of RenderObjectChildList::updateBeforeAfterContent

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Igor Trindade Oliveira]

Currently, updateBeforeAfterContent has a complex logic, it creates and updates the before and after elements. 
Splitting the extra logic out of  RenderObjectChildList::updateBeforeAfterContent, makes it easier to understand.

[Igor Trindade Oliveira]

Patch coming.

[Igor Trindade Oliveira]

Created attachment 131396 [details]
Patch.

Proposed patch.

[WebKit Review Bot]

Comment on attachment 131396 [details]
Patch.

Attachment 131396 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11943161

New failing tests:
svg/text/text-block-child-crash.xhtml

[Julien Chaffraix]

Comment on attachment 131396 [details]
Patch.

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

The rest of the change looks fine but I would like a green EWS!

> Source/WebCore/rendering/RenderObjectChildList.cpp:540
> -                    return;
> -                }
> +                generatedContentContainer = createBeforeAfterGeneratedContentContainer(owner, renderer, styledObject, pseudoElementStyle);

I don't think this change is right. We used to bail out if |generatedContentContainer| was not allowed by |owner| and now we don't - because it's missing a NULL-check. I guess that's why the EWS is unhappy.

Arguably this doesn't add much so I would advise just dropping this part of the change unless you need that for your refactoring.

[Igor Trindade Oliveira]

Created attachment 131455 [details]
Patch

Proposed patch v2.

[Julien Chaffraix]

Comment on attachment 131455 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Split the extra logic out of RenderObjectChildList::updateBeforeAfterContent

Btw, I don't know what the "extra logic" is. I guess it should be either
* Split some logic out of RenderObjectChildList::updateBeforeAfterContent
* Extra part of the generated content handling logic out of RenderObjectChildList::updateBeforeAfterContent

> Source/WebCore/ChangeLog:12
> +        (WebCore):

Useless line (not your fault as prepare-ChangeLog has been sick for some time).

> Source/WebCore/ChangeLog:14
> +        (WebCore::createBeforeAfterGeneratedContentContainer):

Stale entry.

> Source/WebCore/rendering/RenderObjectChildList.cpp:350
> +    if (child && child->style()->styleType() == type) {

Nit: This could use an early return.

[Igor Trindade Oliveira]

Committed r110656: http://trac.webkit.org/changeset/110656