[webkit-reviews] review denied: [Bug 43779] RenderImage::imageChanged invalidates wrong area : [Attachment 64272] Patch #1h (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 64272: Patch #1h (add RenderImageResource)
https://bugs.webkit.org/attachment.cgi?id=64272&action=review

------- Additional Comments from Nikolas Zimmermann <zimmermann at kde.org>
Interessting, didn't think about the possibility to remove
RenderImageGeneratedContent, with that removal your two new
RenderImageResources classes make a lot more sense.
Though I still have a suggestion. I'd love to have an abstract
RenderImageResource (renamed to RenderImageSource) class, and two other classes
inheriting from it.

RenderImageSource (contains pure-virtual image/errorOccurred, etc.. functions)
RenderCachedImageSource (inherits from RenderImageSource)
RenderStyleImageSource (ditto)

The problem is that you're still storing the CachedResourceHandle<CachedImage>
for RenderImageResourceStyleImage through inheritance, which wastes memory, as
it's unused.
Your approach is fine in general, I'm just asking for one more class, to save
memory, and some renames.

Do you agree with the idea?
I have another idea to avoid the toRenderFoo(renderer())->imageResource()
dance, and would like to discuss that on IRC.

P.S. ClipboardGtk has problems, see gtk build output.