[webkit-reviews] review denied: [Bug 43779] RenderImage::imageChanged invalidates wrong area : [Attachment 65547] Patch #1m (add RenderImageResource)

[bugzilla-daemon at webkit.org]

Nikolas Zimmermann <zimmermann at kde.org> has denied Patrick R. Gansterer
<paroga at paroga.com>'s request for review:
Bug 43779: RenderImage::imageChanged invalidates wrong area
https://bugs.webkit.org/show_bug.cgi?id=43779

Attachment 65547: Patch #1m (add RenderImageResource)
https://bugs.webkit.org/attachment.cgi?id=65547&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
WebCore/rendering/RenderImageResource.cpp:31
 +  #include "RenderImageResourceStyleImage.h"
This include is unncessary.

WebCore/rendering/RenderImageResource.cpp:44
 +	if (m_cachedImage)
You should also ASSERT(m_renderer) here.

WebCore/rendering/RenderImageResource.cpp:87
 +	if (!m_cachedImage)
And here (before the if).

WebCore/rendering/RenderImageResource.h:31
 +  #include "StyleImage.h"
Superfluous.

WebCore/rendering/RenderImageResource.h:37
 +  class StyleImage;
Superflous.


WebCore/rendering/RenderImageResource.h:69
 +	RenderObject* renderer() { return m_renderer; }
I'd remove these accesor, and rather make m_renderer protected.

WebCore/rendering/RenderImageResourceStyleImage.cpp:47
 +	m_styleImage->removeClient(renderer());
Just use m_renderer here, once it's protected.

WebCore/rendering/RenderImageResourceStyleImage.cpp:53
 +	m_styleImage->addClient(renderer());
Just use the passed newRenderer here.

It's a pity you can't land on your own yet, otherwhise I'd give r+.
Please fix and upload a new version again, that I can r+/cq+.