[webkit-reviews] review granted: [Bug 175444] Followup (r220289): RenderImageResourceStyleImage code clean up : [Attachment 318418] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Said Abou-Hallawa
<sabouhallawa at apple.com>'s request for review:
Bug 175444: Followup (r220289): RenderImageResourceStyleImage code clean up
https://bugs.webkit.org/show_bug.cgi?id=175444

Attachment 318418: Patch

https://bugs.webkit.org/attachment.cgi?id=318418&action=review




--- Comment #13 from Darin Adler <darin at apple.com> ---
Comment on attachment 318418
  --> https://bugs.webkit.org/attachment.cgi?id=318418
Patch

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

OK, thanks. That fixes the "setCachedImage is dangerous" issue. I’m not sure I
love the name of “m_cachedImageClientAddedExternally” but I don’t have a better
name to suggest at the moment. I also would have put the inline function bodies
outside the class definitions to keep them cleaner, but that’s a style question
upon which it seems various reasonable people differ.

Side question about the original code (not the new code). What guarantees that
m_cachedImage->removeClient is called if a RenderImageResource object is
destroyed without first calling setCachedImage(nullptr)?

> Source/WebCore/rendering/RenderImageResource.h:46
> +    RenderElement* renderer() const { return m_renderer; }

Can this be protected instead of public, please?