[webkit-reviews] review denied: [Bug 24947] Special-case chromium drawing text-shadow when opaque color & no shadow blur : [Attachment 29092] Spaces removed, variables now reflect webkit style guide

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Rafael Weinstein
<rafaelw at chromium.org>'s request for review:
Bug 24947: Special-case chromium drawing text-shadow when opaque color & no
shadow blur
https://bugs.webkit.org/show_bug.cgi?id=24947

Attachment 29092: Spaces removed, variables now reflect webkit style guide
https://bugs.webkit.org/attachment.cgi?id=29092&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
The bug url should be in the ChangeLog.

Much stronger would be to ASSERT these conditions instead of just makign a
comment:

+    // Note: If the fillColor or shadowColor are ever non-opaque, we should 
+    // never reach this code. SkiaFontWin::windowsCanHandleTextDrawing() will
+    // have set m_useGDI == false and the above check will have passed and 
+    // the this function returned.

tempColor isn't a very descriptiveName.

textColor would probably be better.

savedColor or savedTextColor would be better than "saveColor".

+bool windowsCanHandleDrawTextShadow(WebCore::GraphicsContext *context) {

{ should be on its own line.

"context" is not needed here (nor on the line after) as it provides no
additional description to purpose of the argument (See style guide).

+bool windowsCanHandleDrawTextShadow(GraphicsContext *context);