[Webkit-unassigned] [Bug 44127] [chromium] Thumbnails not generated for GPU Rendered Pages

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=44127


David Levin <levin at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #64628|review?, commit-queue?      |review-
               Flag|                            |




--- Comment #6 from David Levin <levin at chromium.org>  2010-08-17 17:47:26 PST ---
(From update of attachment 64628)
r- see Vangelis' comment (and a minor style issue in the bug too) plus the style issue below.

WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:509
 +  void LayerRendererChromium::readPixels(skia::PlatformCanvas* canvasPtr)
Just canvas should suffice.

WebKit avoids abbreviates and in general adding type information to variable names (so Ptr is out on both accounts).

WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:526
 +      // Flip pixels vertically ...
"// Flip pixels vertically."
works too. 

This comes from a person who uses "..." to often in the past....


WebCore/platform/graphics/chromium/LayerRendererChromium.cpp:529
 +          unsigned char *ptr1 = static_cast<unsigned char *>(pixels) + row1 * bitmap.rowBytes();
The * (when used as a pointer as opposed to multiplication) should be near the type.

There are lots of instances of this problem in this patch.

WebKit/chromium/public/WebView.h:215
 +      virtual void readPixels(WebCanvas *canvas) {}
The parameter name |canvas| adds no information in the function definition, so it should be omitted. (There are other instances of this.)

Also WebKit tends to put a space inside of braces like this, so s/{}/{ }/

(And the * is in the wrong place.)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.