[webkit-reviews] review denied: [Bug 26457] Build DumpRenderTree under Cairo : [Attachment 31577] Proposed patch for adding this feature

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Brent Fulgham <bfulgham at webkit.org>'s
request for review:
Bug 26457: Build DumpRenderTree under Cairo
https://bugs.webkit.org/show_bug.cgi?id=26457

Attachment 31577: Proposed patch for adding this feature
https://bugs.webkit.org/attachment.cgi?id=31577&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
I would think this would be abstracted into a shared call:
68     printf("Content-Type: %s\n", "image/png");
 69	printf("Content-Length: %lu\n", static_cast<unsigned
long>(dataLength));
 70 
 71	const size_t bytesToWriteInOneChunk = 1 << 15;
 72	size_t dataRemainingToWrite = dataLength;
 73	while (dataRemainingToWrite) {
 74	    size_t bytesToWriteInThisChunk = min(dataRemainingToWrite,
bytesToWriteInOneChunk);
 75	    size_t bytesWritten = fwrite(data, 1, bytesToWriteInThisChunk,
stdout);
 76	    if (bytesWritten != bytesToWriteInThisChunk)
 77		break;
 78	    dataRemainingToWrite -= bytesWritten;
 79	    data += bytesWritten;
 80	}

Style:
 65	const size_t dataLength = pixeldata.size ();

This part should all be shared:
93     // We need to swap the bytes to ensure consistent hashes independently
of endianness
 94	MD5_CTX md5Context;
 95	MD5_Init(&md5Context);
 96	unsigned char* bitmapData = static_cast<unsigned
char*>(cairo_image_surface_get_data(surface));
 97	for (unsigned row = 0; row < pixelsHigh; row++) {
 98	    MD5_Update(&md5Context, bitmapData, 4 * pixelsWide);
 99	    bitmapData += bytesPerRow;
 100	 }
 101	 unsigned char hash[16];
 102	 MD5_Final(hash, &md5Context);
 103 
 104	 hashString[0] = '\0';
 105	 for (int i = 0; i < 16; i++)
 106	     snprintf(hashString, 33, "%s%02x", hashString, hash[i]);
 107 }

There seems to be almost nothing cairo-specific about:
 109 void dumpWebViewAsPixelsAndCompareWithExpected(const std::string&
expectedHash)


Can we share more code here?  At least can we abstract things into methods
which we could share later?