[webkit-reviews] review granted: [Bug 44719] Fix RenderStyle::addCursor to use a StyleImage, not a CachedImage : [Attachment 65786] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 44719: Fix RenderStyle::addCursor to use a StyleImage, not a CachedImage
https://bugs.webkit.org/show_bug.cgi?id=44719

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

------- Additional Comments from Darin Adler <darin at apple.com>
>	   case CSSPropertyBackgroundImage:
> +	       // FIXME: broken for multiple backgrounds.

It would be better if this was capitalized sentence-style like our comments
usually are, and if there was a corresponding bug umber.

> +	       case CSSPropertyCursor: {
> +		   if (CursorList* cursorList = m_style->cursors()) {
> +		       for (size_t i = 0; i < cursorList->size(); ++i) {
> +			   CursorData& currentCursor = (*cursorList)[i];
> +			   if (currentCursor.image()->isPendingImage()) {
> +			       CSSImageValue* imageValue =
static_cast<StylePendingImage*>(currentCursor.image())->cssImageValue();
> +			      
currentCursor.setImage(imageValue->cachedImage(docLoader));
> +			   }
> +		       }
> +		   }
>		   break;
> +	       }

Is it correct to do nothing if there are not cursors?

> +    void setImage(StyleImage* image) { m_image = image; }	

This function should take a PassRefPtr, not a raw pointer.

> +    void addCursor(StyleImage*, const IntPoint& = IntPoint());

Should this take a PassRefPtr instead of a raw pointer? The IntPoint argument
should have a name, hotSpot, instead of being unnamed. I don't think it's
obvious what it is without a name.