Hello noel,
I'd like you to do a code review. Please execute
g4 diff -c 10530005
or point your web browser to
http://mondrian/10530005
to review the following code:
Change 10530005 by nigel...@nigeltao-srcgears2 on 2009/03/19 10:00:24 *pending*
Rename Canvas' load and toBlob to decode and encode.
PRESUBMIT=passed
R=noel
[email protected]
DELTA=74 (9 added, 2 deleted, 63 changed)
OCL=10530005
Affected files ...
... //depot/googleclient/gears/opensource/gears/canvas/canvas.cc#25 edit
... //depot/googleclient/gears/opensource/gears/canvas/canvas.h#20 edit
... //depot/googleclient/gears/opensource/gears/test/manual/canvas.html#4 edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/canvas_tests.js#13
edit
74 delta lines: 9 added, 2 deleted, 63 changed
Also consider running:
g4 lint -c 10530005
which verifies that the changelist doesn't introduce new style violations.
If you can't do the review, please let me know as soon as possible. During
your review, please ensure that all new code has corresponding unit tests and
that existing unit tests are updated appropriately. Visit
http://www/eng/code_review.html for more information.
This is a semiautomated message from "g4 mail". Complaints or suggestions?
Mail [email protected].
Change 10530005 by nigel...@nigeltao-srcgears2 on 2009/03/19 10:00:24 *pending*
Rename Canvas' load and toBlob to decode and encode.
Affected files ...
... //depot/googleclient/gears/opensource/gears/canvas/canvas.cc#25 edit
... //depot/googleclient/gears/opensource/gears/canvas/canvas.h#20 edit
... //depot/googleclient/gears/opensource/gears/test/manual/canvas.html#4 edit
...
//depot/googleclient/gears/opensource/gears/test/testcases/canvas_tests.js#13
edit
==== //depot/googleclient/gears/opensource/gears/canvas/canvas.cc#25 -
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/canvas/canvas.cc
====
# action=edit type=text
--- googleclient/gears/opensource/gears/canvas/canvas.cc 2009-03-12
22:22:05.000000000 +1100
+++ googleclient/gears/opensource/gears/canvas/canvas.cc 2009-03-19
10:03:24.000000000 +1100
@@ -64,13 +64,18 @@
template<>
void Dispatcher<GearsCanvas>::Init() {
- RegisterMethod("load", &GearsCanvas::Load);
- RegisterMethod("toBlob", &GearsCanvas::ToBlob);
RegisterMethod("crop", &GearsCanvas::Crop);
+ RegisterMethod("decode", &GearsCanvas::Decode);
+ RegisterMethod("encode", &GearsCanvas::Encode);
+ RegisterMethod("getContext", &GearsCanvas::GetContext);
RegisterMethod("resize", &GearsCanvas::Resize);
+ RegisterProperty("height", &GearsCanvas::GetHeight, &GearsCanvas::SetHeight);
RegisterProperty("width", &GearsCanvas::GetWidth, &GearsCanvas::SetWidth);
- RegisterProperty("height", &GearsCanvas::GetHeight, &GearsCanvas::SetHeight);
- RegisterMethod("getContext", &GearsCanvas::GetContext);
+
+ // TODO(nigeltao): Remove these methods whose sole reason for existance is
+ // backwards compatability with privately built Gears binaries.
+ RegisterMethod("load", &GearsCanvas::Decode);
+ RegisterMethod("toBlob", &GearsCanvas::Encode);
}
void GearsCanvas::EnsureBitmapPixelsAreAllocated() {
@@ -80,7 +85,7 @@
}
}
-void GearsCanvas::Load(JsCallContext *context) {
+void GearsCanvas::Decode(JsCallContext *context) {
ModuleImplBaseClass *other_module;
JsArgument args[] = {
{ JSPARAM_REQUIRED, JSPARAM_MODULE, &other_module },
@@ -106,7 +111,7 @@
}
}
-void GearsCanvas::ToBlob(JsCallContext *context) {
+void GearsCanvas::Encode(JsCallContext *context) {
std::string16 mime_type;
scoped_ptr<JsObject> attributes;
JsArgument args[] = {
@@ -137,7 +142,7 @@
// pixels. If this flag is set, the encoder creates a file without alpha
// channel. We don't want this to happen, for several reasons:
// 1. Consider:
- // canvas.load(jpegBlob);
+ // canvas.decode(jpegBlob);
// canvas.crop(0, 0, canvas.width, canvas.height);
// The original SkBitmap has isOpaque set to true (jpegs have no
// transparent pixels), but the new SkBitmap created during crop() has
@@ -145,10 +150,10 @@
// 2. A similar situation occurs with clone()...
// 3. ... and with drawImage().
// 4. Consider:
- // canvas.load(blob);
+ // canvas.decode(blob);
// canvas.width = 100;
// do some drawing on the canvas
- // var outputBlob = canvas.toBlob();
+ // var outputBlob = canvas.encode();
//
// When the image is loaded, the isOpaque flag is updated, and is not
// modified again anywhere. As a result, exported blobs will have an
==== //depot/googleclient/gears/opensource/gears/canvas/canvas.h#20 -
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/canvas/canvas.h
====
# action=edit type=text
--- googleclient/gears/opensource/gears/canvas/canvas.h 2009-03-12
22:22:06.000000000 +1100
+++ googleclient/gears/opensource/gears/canvas/canvas.h 2009-03-19
09:55:50.000000000 +1100
@@ -54,16 +54,18 @@
// dangling pointer. The rendering context will be recreated when needed.
void ClearRenderingContextReference();
- // Loads an image from a supplied blob.
- // IN: Blob blob.
+ // Loads an image (given as a blob) into this canvas, overwriting any
+ // existing canvas contents. The canvas' width and height will be set to
+ // the natural width and height of the image.
+ // IN: Blob blob
// OUT: -
- void Load(JsCallContext *context);
+ void Decode(JsCallContext *context);
- // Exports the contents of this canvas into a blob. This is a one-time
+ // Exports the contents of this canvas to a blob. This is a one-time
// operation; updates to the canvas don't reflect in the blob.
// IN: optional String mimeType, optional Object attributes
// OUT: Blob
- void ToBlob(JsCallContext *context);
+ void Encode(JsCallContext *context);
// Crops the canvas to the specified rectangle, in-place.
// IN: int x, int y, int width, int height
==== //depot/googleclient/gears/opensource/gears/test/manual/canvas.html#4 -
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/test/manual/canvas.html
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/manual/canvas.html 2009-03-16
13:58:14.000000000 +1100
+++ googleclient/gears/opensource/gears/test/manual/canvas.html 2009-03-19
09:58:47.000000000 +1100
@@ -62,7 +62,7 @@
localServer.removeStore("test_manual_canvas");
store = localServer.createStore("test_manual_canvas");
var capturedUrl = 'g' + parseInt(Math.random() * 1000) + '.png';
- store.captureBlob(gearsCanvas.toBlob(), capturedUrl, 'image/png');
+ store.captureBlob(gearsCanvas.encode(), capturedUrl, 'image/png');
document.getElementById('gc').src = capturedUrl;
}
@@ -122,7 +122,7 @@
diffContext.putImageData(diffImageData, 0, 0);
var capturedUrl = 'd' + parseInt(Math.random() * 1000) + '.png';
- store.captureBlob(diffCanvas.toBlob(), capturedUrl, 'image/png');
+ store.captureBlob(diffCanvas.encode(), capturedUrl, 'image/png');
document.getElementById('dc').src = capturedUrl;
}
====
//depot/googleclient/gears/opensource/gears/test/testcases/canvas_tests.js#13 -
/home/nigeltao/srcgears2/googleclient/gears/opensource/gears/test/testcases/canvas_tests.js
====
# action=edit type=text
--- googleclient/gears/opensource/gears/test/testcases/canvas_tests.js
2009-03-19 10:00:32.000000000 +1100
+++ googleclient/gears/opensource/gears/test/testcases/canvas_tests.js
2009-03-19 10:00:20.000000000 +1100
@@ -107,7 +107,7 @@
loadBlob('blank-300x150.png', function(blankBlob) {
var canvas = google.gears.factory.create('beta.canvas');
verifyCanvasState(canvas);
- assertBlobProbablyEqual(blankBlob, canvas.toBlob());
+ assertBlobProbablyEqual(blankBlob, canvas.encode());
completeAsync();
});
}
@@ -130,7 +130,7 @@
loadBlobs(filenames, function(blobs) {
var canvas = google.gears.factory.create('beta.canvas');
var originalBlob = blobs[originalFilename];
- canvas.load(originalBlob);
+ canvas.decode(originalBlob);
assertEqual(313, canvas.width);
assertEqual(234, canvas.height);
@@ -138,14 +138,14 @@
setContextProperties(canvas);
canvas.height = 120;
verifyCanvasState(canvas);
- assertBlobProbablyEqual(blobs['blank-313x120.png'], canvas.toBlob());
+ assertBlobProbablyEqual(blobs['blank-313x120.png'], canvas.encode());
// Test that changing the width resets the canvas.
- canvas.load(originalBlob);
+ canvas.decode(originalBlob);
setContextProperties(canvas);
canvas.width = 240;
verifyCanvasState(canvas);
- assertBlobProbablyEqual(blobs['blank-240x234.png'], canvas.toBlob());
+ assertBlobProbablyEqual(blobs['blank-240x234.png'], canvas.encode());
completeAsync();
});
@@ -161,27 +161,27 @@
loadBlobs(filenames, function(blobs) {
var canvas = google.gears.factory.create('beta.canvas');
var originalBlob = blobs[originalFilename];
- canvas.load(originalBlob);
+ canvas.decode(originalBlob);
// Make sure the original file has the right dimensions.
assertEqual(313, canvas.width);
assertEqual(234, canvas.height);
// The canvas is not blank.
- assertNotEqual(canvas.toBlob().length, blobs['blank-313x234.png'].length);
+ assertNotEqual(canvas.encode().length, blobs['blank-313x234.png'].length);
// Setting width to current width must blank the canvas:
setContextProperties(canvas);
canvas.width = canvas.width;
verifyCanvasState(canvas);
- assertBlobProbablyEqual(canvas.toBlob(), blobs['blank-313x234.png']);
+ assertBlobProbablyEqual(canvas.encode(), blobs['blank-313x234.png']);
// Test the same for height:
- canvas.load(originalBlob);
+ canvas.decode(originalBlob);
setContextProperties(canvas);
canvas.height = canvas.height;
verifyCanvasState(canvas);
- assertBlobProbablyEqual(canvas.toBlob(), blobs['blank-313x234.png']);
+ assertBlobProbablyEqual(canvas.encode(), blobs['blank-313x234.png']);
completeAsync();
});
@@ -192,16 +192,16 @@
for (var i = 0; i < formats.length; ++i) {
var format = formats[i];
var canvas = google.gears.factory.create('beta.canvas');
- canvas.load(blobs['sample-original.' + format]);
-
- var pngBlob = canvas.toBlob('image/png');
+ canvas.decode(blobs['sample-original.' + format]);
+
+ var pngBlob = canvas.encode('image/png');
// The default format is png.
- assertBlobProbablyEqual(canvas.toBlob(), pngBlob,
+ assertBlobProbablyEqual(canvas.encode(), pngBlob,
'The default export format is not png.');
- var jpegBlob = canvas.toBlob('image/jpeg');
- var jpegLowBlob = canvas.toBlob('image/jpeg', { quality: 0.02 });
+ var jpegBlob = canvas.encode('image/jpeg');
+ var jpegLowBlob = canvas.encode('image/jpeg', { quality: 0.02 });
// Now compare the exported versions with golden files.
// The golden files have been manually checked for format and size.
@@ -238,7 +238,7 @@
startAsync();
loadBlob('sample.txt', function(blob) {
assertError(function() {
- canvas.load(blob);
+ canvas.decode(blob);
});
completeAsync();
});
@@ -247,7 +247,7 @@
function testExportToUnsupportedFormat() {
var canvas = google.gears.factory.create('beta.canvas');
assertError(function() {
- canvas.toBlob('image/gif');
+ canvas.encode('image/gif');
});
}
@@ -255,11 +255,11 @@
// Create a canvas and set various properties on it and on its context.
var originalCanvas = google.gears.factory.create('beta.canvas');
var originalCtx = originalCanvas.getContext('gears-2d');
- originalCanvas.load(blob);
+ originalCanvas.decode(blob);
var originalWidth = originalCanvas.width;
var originalHeight = originalCanvas.height;
- var originalBlob = originalCanvas.toBlob();
+ var originalBlob = originalCanvas.encode();
var alpha = 0.5;
var compositeOperation = 'copy';
@@ -276,7 +276,7 @@
assertEqual(originalWidth, clonedCanvas.width);
assertEqual(originalHeight, clonedCanvas.height);
- assertBlobProbablyEqual(originalBlob, clonedCanvas.toBlob());
+ assertBlobProbablyEqual(originalBlob, clonedCanvas.encode());
assertEqual(originalCtx.globalAlpha, cloneCtx.globalAlpha);
assertEqual(originalCtx.globalCompositeOperation,
@@ -290,7 +290,7 @@
function checkOriginal() {
assertEqual(originalWidth, originalCanvas.width);
assertEqual(originalHeight, originalCanvas.height);
- assertBlobProbablyEqual(originalBlob, originalCanvas.toBlob());
+ assertBlobProbablyEqual(originalBlob, originalCanvas.encode());
assertEqual(alpha, originalCtx.globalAlpha);
assertEqual(compositeOperation, originalCtx.globalCompositeOperation);
assertEqual(fillStyle, originalCtx.fillStyle);
@@ -308,7 +308,7 @@
// Load a random image into the clone and check that the original isn't
// affected. We don't care what the dummy is, just that it's a valid image.
- clonedCanvas.load(dummyBlob);
+ clonedCanvas.decode(dummyBlob);
checkOriginal();
}
@@ -328,14 +328,14 @@
var goldenFilename = 'sample-jpeg-cropped-40-40-100-100.png';
loadBlobs(['sample-original.jpeg', goldenFilename], function(blobs) {
var canvas = google.gears.factory.create('beta.canvas');
- canvas.load(blobs['sample-original.jpeg']);
+ canvas.decode(blobs['sample-original.jpeg']);
canvas.crop(40, 40, 100, 100);
- var exportedBlob = canvas.toBlob();
+ var exportedBlob = canvas.encode();
var goldenBlob = blobs[goldenFilename];
// Ensure that the golden file is what we think it is.
var goldenCanvas = google.gears.factory.create('beta.canvas');
- goldenCanvas.load(goldenBlob);
+ goldenCanvas.decode(goldenBlob);
assertEqual(100, goldenCanvas.width);
assertEqual(100, goldenCanvas.height);
@@ -348,7 +348,7 @@
startAsync();
loadBlob('sample-original.png', function(blob) {
var canvas = google.gears.factory.create('beta.canvas');
- canvas.load(blob);
+ canvas.decode(blob);
canvas.crop(40, 40, 0, 0);
assertEqual(0, canvas.width);
assertEqual(0, canvas.height);
@@ -360,12 +360,12 @@
startAsync();
loadBlob('sample-original.jpeg', function(blob) {
var canvas = google.gears.factory.create('beta.canvas');
- canvas.load(blob);
- var originalBlob = canvas.toBlob();
+ canvas.decode(blob);
+ var originalBlob = canvas.encode();
var originalWidth = canvas.width;
var originalHeight = canvas.height;
canvas.crop(0, 0, canvas.width, canvas.height);
- assertBlobProbablyEqual(originalBlob, canvas.toBlob());
+ assertBlobProbablyEqual(originalBlob, canvas.encode());
assertEqual(originalWidth, canvas.width);
assertEqual(originalHeight, canvas.height);
completeAsync();
@@ -378,14 +378,14 @@
startAsync();
loadBlobs([originalFilename , resizedFilename], function(blobs) {
var canvas = google.gears.factory.create('beta.canvas');
- canvas.load(blobs[originalFilename]);
+ canvas.decode(blobs[originalFilename]);
var newWidth = 400;
var newHeight = 40;
canvas.resize(newWidth, newHeight);
assertEqual(newWidth, canvas.width);
assertEqual(newHeight, canvas.height);
- assertBlobProbablyEqual(blobs[resizedFilename], canvas.toBlob());
+ assertBlobProbablyEqual(blobs[resizedFilename], canvas.encode());
completeAsync();
});
}
@@ -415,8 +415,8 @@
var dest = 'sample-jpeg-cropped-40-40-100-100.png';
loadBlobs([src, dest], function(blobs) {
var srcCanvas = google.gears.factory.create('beta.canvas');
- srcCanvas.load(blobs[src]);
- var srcCanvasBeforeDrawBlob = srcCanvas.toBlob();
+ srcCanvas.decode(blobs[src]);
+ var srcCanvasBeforeDrawBlob = srcCanvas.encode();
var destCanvas = google.gears.factory.create('beta.canvas');
destCanvas.width = srcCanvas.width;
@@ -425,10 +425,10 @@
ctx.drawImage(srcCanvas, 0, 0);
// The source should not have been affected.
- assertBlobProbablyEqual(srcCanvasBeforeDrawBlob, srcCanvas.toBlob());
+ assertBlobProbablyEqual(srcCanvasBeforeDrawBlob, srcCanvas.encode());
// Both canvii must be the same.
- assertBlobProbablyEqual(srcCanvas.toBlob(), destCanvas.toBlob());
+ assertBlobProbablyEqual(srcCanvas.encode(), destCanvas.encode());
completeAsync();
});
@@ -437,8 +437,8 @@
function runDrawImage2SmallerAndBiggerTest(srcFilename, goldenFilename, blobs,
isSrcBigger, offsetX, offsetY) {
var srcCanvas = google.gears.factory.create('beta.canvas');
- srcCanvas.load(blobs[srcFilename]);
- var srcCanvasBlobBeforeDraw = srcCanvas.toBlob();
+ srcCanvas.decode(blobs[srcFilename]);
+ var srcCanvasBlobBeforeDraw = srcCanvas.encode();
var destCanvas = google.gears.factory.create('beta.canvas');
if (isSrcBigger) {
@@ -456,13 +456,13 @@
destCtx.drawImage(srcCanvas, offsetX, offsetY);
// The source should not have been affected.
- assertBlobProbablyEqual(srcCanvasBlobBeforeDraw, srcCanvas.toBlob());
+ assertBlobProbablyEqual(srcCanvasBlobBeforeDraw, srcCanvas.encode());
// The destination's dimensions should remain untouched.
assertEqual(destCanvasOriginalWidth, destCanvas.width);
assertEqual(destCanvasOriginalHeight, destCanvas.height);
- assertBlobProbablyEqual(blobs[goldenFilename], destCanvas.toBlob());
+ assertBlobProbablyEqual(blobs[goldenFilename], destCanvas.encode());
}
// Use the two-argument drawImage() (which does not resize the image while
@@ -497,7 +497,7 @@
startAsync();
loadBlob('sample-original.png', function(inputBlob) {
var srcCanvas = google.gears.factory.create('beta.canvas');
- srcCanvas.load(inputBlob);
+ srcCanvas.decode(inputBlob);
var destCanvas = google.gears.factory.create('beta.canvas');
var ctx = destCanvas.getContext('gears-2d');
@@ -539,10 +539,10 @@
var goldenFile = 'sample-drawImageWithAlpha-result.png';
loadBlobs([srcFile, destFile, goldenFile], function(blobs) {
var srcCanvas = google.gears.factory.create('beta.canvas');
- srcCanvas.load(blobs[srcFile]);
+ srcCanvas.decode(blobs[srcFile]);
var destCanvas = google.gears.factory.create('beta.canvas');
- destCanvas.load(blobs[destFile]);
+ destCanvas.decode(blobs[destFile]);
destCanvas.resize(srcCanvas.width, srcCanvas.height);
var ctx = destCanvas.getContext('gears-2d');
@@ -550,7 +550,7 @@
ctx.drawImage(srcCanvas, 0, 0);
- assertBlobProbablyEqual(blobs[goldenFile], destCanvas.toBlob());
+ assertBlobProbablyEqual(blobs[goldenFile], destCanvas.encode());
completeAsync();
});
}
@@ -560,21 +560,21 @@
startAsync();
loadBlob('sample-original.png', function(inputBlob) {
var srcCanvas = google.gears.factory.create('beta.canvas');
- srcCanvas.load(inputBlob);
+ srcCanvas.decode(inputBlob);
var destCanvas = google.gears.factory.create('beta.canvas');
// Make the destCanvas big enough to hold the image in the srcCanvas.
destCanvas.width = 400;
destCanvas.height = 400;
var ctx = destCanvas.getContext('gears-2d');
- var destSnapshot = destCanvas.toBlob();
+ var destSnapshot = destCanvas.encode();
ctx.globalAlpha = 0;
// Should be a noop.
ctx.drawImage(srcCanvas, 3, 9);
- assertBlobProbablyEqual(destSnapshot, destCanvas.toBlob());
+ assertBlobProbablyEqual(destSnapshot, destCanvas.encode());
completeAsync();
});
}
@@ -587,7 +587,7 @@
startAsync();
loadBlob('sample-original.png', function(blob) {
var srcCanvas = google.gears.factory.create('beta.canvas');
- srcCanvas.load(blob);
+ srcCanvas.decode(blob);
var destCanvas = google.gears.factory.create('beta.canvas');
destCanvas.resize(srcCanvas.width, destCanvas.height);
@@ -596,7 +596,7 @@
ctx.rotate(0);
ctx.drawImage(srcCanvas, 0, 0);
- assertBlobProbablyEqual(srcCanvas.toBlob(), destCanvas.toBlob());
+ assertBlobProbablyEqual(srcCanvas.encode(), destCanvas.encode());
});
}
