[webkit-reviews] review denied: [Bug 126133] Make CachedSVGDocument independent of CSS Filters : [Attachment 219870] Patch

[bugzilla-daemon at webkit.org]

Antti Koivisto <koivisto at iki.fi> has denied Dirk Schulze <krit at webkit.org>'s
request for review:
Bug 126133: Make CachedSVGDocument independent of CSS Filters
https://bugs.webkit.org/show_bug.cgi?id=126133

Attachment 219870: Patch
https://bugs.webkit.org/attachment.cgi?id=219870&action=review

------- Additional Comments from Antti Koivisto <koivisto at iki.fi>
View in context: https://bugs.webkit.org/attachment.cgi?id=219870&action=review


> Source/WebCore/loader/SVGDocumentConsumer.h:46
> +class SVGDocumentConsumer {
> +public:
> +    CachedSVGDocumentReference* cachedSVGDocumentReference() const { return
m_cachedSVGDocumentReference.get(); }
> +    void
setCachedSVGDocumentReference(PassOwnPtr<CachedSVGDocumentReference>
cachedSVGDocumentReference) { m_cachedSVGDocumentReference =
cachedSVGDocumentReference; }
> +
> +private:
> +    OwnPtr<CachedSVGDocumentReference> m_cachedSVGDocumentReference;
> +};	 

This class is just a pointer. I don't see why we are introducing it and making
code more abstract. Also name seem off. It doesn't 'consume' anything.

We use unique_ptr not OwnPtr.

> Source/WebCore/platform/graphics/filters/FilterOperation.h:160
> +class ReferenceFilterOperation : public FilterOperation, public
SVGDocumentConsumer {

Why is ReferenceFilterOperation inheriting SVGDocumentConsumer instead of
having one as a member?