[webkit-reviews] review denied: [Bug 54330] [chromium] Cannot use multiple accelerated 2D canvas' in the same render process : [Attachment 86815] Capitalize comments, use static consts, remove LF

[bugzilla-daemon at webkit.org]

James Robinson <jamesr at chromium.org> has denied Mike Reed <reed at google.com>'s
request for review:
Bug 54330: [chromium] Cannot use multiple accelerated 2D canvas' in the same
render process
https://bugs.webkit.org/show_bug.cgi?id=54330

Attachment 86815: Capitalize comments, use static consts, remove LF
https://bugs.webkit.org/attachment.cgi?id=86815&action=review

------- Additional Comments from James Robinson <jamesr at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=86815&action=review

Style still isn't quite right, and there are still two unanswered questions:
1.) Given that the lifetime of the GraphicsContext3D is >= the DrawingBuffer,
do we need the refcounting on the GrContext here?
2.) Does this code assume that ganesh is the exclusive user of
SharedGraphicsContext3D?  That's not really consistent with the intended design
of SGC3D (after all it's supposed to be shared by multiple cooperative users).

> WebCore/platform/graphics/gpu/SharedGraphicsContext3D.cpp:54
> +// Limit the number of textures we hold in the bitmap->texture cache
> +static const int cMaxTextureCacheCount = 512;
> +// Limit the bytes allocated toward textures in the bitmap->texture cache
> +static const size_t cMaxTextureCacheBytes = 50 * 1024 * 1024;

missing trailing periods on the comments.

we don't use the "c" or any other prefix for static constants in WebKit.

> WebCore/platform/graphics/gpu/SharedGraphicsContext3D.h:143
> +    GrContext* getGrContext();

Accessors in WebKit do not use a 'get' prefix - this should be 'grContext()'. 
See http://www.webkit.org/coding/coding-style.html.