Bug 67103 – Implement list style properties in CSSStyleApplyProperty.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Luke Macpherson]

Implement list style properties in CSSStyleApplyProperty.

[Luke Macpherson]

Created attachment 105457 [details]
Patch

[Darin Adler]

Comment on attachment 105457 [details]
Patch

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

> Source/WebCore/ChangeLog:22
> +        Added version to take a raw pointer. The existing code relies on an implicit cast to PassRefPtr,
> +        but because of the templates used an exactly matching type signature is required.

I think a better fix to that issue would be to use create functions that return PassRefPtr. It seems this code is making too much use of raw pointers for newly-created RefCounted objects that are known to have a reference count of 1. That’s a confusing non-standard coding style in WebKit, more like how we did things in the distant past. Could we use create functions instead? Maybe the create functions would bulk the source code up too much?

[Luke Macpherson]

Created attachment 105573 [details]
Patch

[Darin Adler]

Comment on attachment 105573 [details]
Patch

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

> Source/WebCore/rendering/style/RenderStyle.h:607
> -    StyleImage* listStyleImage() const { return inherited->list_style_image.get(); }
> +    PassRefPtr<StyleImage> listStyleImage() const { return inherited->list_style_image; }

This change is not a good idea. While it’s convenient to have a PassRefPtr in CSSStyleApplyProperty.cpp, in other call sites using this type will result in reference count churn.

[Luke Macpherson]

Created attachment 106300 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 106300 [details]
Patch

Seems reasonable.

[WebKit Review Bot]

Comment on attachment 106300 [details]
Patch

Clearing flags on attachment: 106300

Committed r94625: <http://trac.webkit.org/changeset/94625>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.