[webkit-reviews] review granted: [Bug 36328] Missing plug-ins should be represented by text only, instead of lego block : [Attachment 51326] Add code to draw a subtle rounded rectangle containing the text "Missing Plug-in"

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Kevin Decker <kdecker at apple.com>'s
request for review:
Bug 36328: Missing plug-ins should be represented by text only, instead of lego
block
https://bugs.webkit.org/show_bug.cgi?id=36328

Attachment 51326: Add code to draw a subtle rounded rectangle containing the
text "Missing Plug-in"
https://bugs.webkit.org/attachment.cgi?id=51326&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    virtual void paint(PaintInfo& paintInfo, int tx, int ty);

Should omit the argument name paintInfo here.

> +    tx += borderLeft() + paddingLeft();
> +    ty += borderTop() + paddingTop();
> +    FloatRect pluginRect = FloatRect(tx, ty, contentWidth(),
contentHeight());

It'd be cleaner not to modify the passed-in tx and ty here. I suggest writing
this like this:

    FloatRect pluginRect = contentBoxRect();
    pluginRect.move(tx, ty);

This accomplishes the same thing and reads a bit cleaner to me.

> +    fontDescription.setFontSmoothing(Antialiased);

I don't understand why the setFontSmoothing call is needed here.

> +    FloatRect missingPluginRect = FloatRect();

The "= FloatRect()" is not needed. That's the same thing you'd get if you
didn't explicitly initialize it.

> +    FloatPoint labelPoint = FloatPoint();

Same here.

> +    labelPoint.setX(roundf(missingPluginRect.location().x() +
(missingPluginRect.size().width() - textWidth) / 2));
> +    labelPoint.setY(floorf(missingPluginRect.location().y()+
(missingPluginRect.size().height() - font.height()) / 2 + font.ascent()));

I think this would read better if you just use the constructor:

    FloatPoint labelPoint(roundf(missingPluginRect.location().x() +
(missingPluginRect.size().width() - textWidth) / 2)),
	floorf(missingPluginRect.location().y()+
(missingPluginRect.size().height() - font.height()) / 2 + font.ascent()));

I don't understand the use of floorf instead of roundf for the Y coordinate.

r=me