Added type annotations to galler/js/image_editor/image_encoder.js. (issue 716093003 by yawano@chromium.org)

0 views
Skip to first unread message

yaw...@chromium.org

unread,
Nov 17, 2014, 7:39:55 PM11/17/14
to hir...@chromium.org, chromium...@chromium.org, rginda...@chromium.org, mtomas...@chromium.org, yoshik...@chromium.org
Reviewers: hirono,

Message:
@hirono: PTAL. Thanks!

Description:
Added type annotations to galler/js/image_editor/image_encoder.js.

BUG=433728
TEST=GYP_GENERATORS=ninja tools/gyp/gyp --depth .
ui/file_manager/gallery/js/compiled_resources.gyp && ninja -C out/Default |
grep
"image_encoder.js"

Please review this at https://codereview.chromium.org/716093003/

Base URL: https://chromium.googlesource.com/chromium/src.git@master

Affected files (+34, -29 lines):
M ui/file_manager/gallery/js/image_editor/image_encoder.js


Index: ui/file_manager/gallery/js/image_editor/image_encoder.js
diff --git a/ui/file_manager/gallery/js/image_editor/image_encoder.js
b/ui/file_manager/gallery/js/image_editor/image_encoder.js
index
0cc5e654fc8e85d655ce261639a8aaa29ac525bd..45f03e8372c4e7ad9857cbf7eff7d07ec10598ff
100644
--- a/ui/file_manager/gallery/js/image_editor/image_encoder.js
+++ b/ui/file_manager/gallery/js/image_editor/image_encoder.js
@@ -8,14 +8,17 @@
function ImageEncoder() {}

/**
- * @type {Array.<Object>}
+ * Metadata encoders.
+ * @type
{!Object.<string,function(new:ImageEncoder.MetadataEncoder,!Object)>}
+ * @const
*/
ImageEncoder.metadataEncoders = {};

/**
- * @param {function(new:ImageEncoder.MetadataEncoder)} constructor
- * // TODO(JSDOC).
- * @param {string} mimeType // TODO(JSDOC).
+ * Registers metadata encoder.
+ * @param {function(new:ImageEncoder.MetadataEncoder,!Object)} constructor
+ * Constructor of a metadata encoder.
+ * @param {string} mimeType Mime type of the metadata encoder.
*/
ImageEncoder.registerMetadataEncoder = function(constructor, mimeType) {
ImageEncoder.metadataEncoders[mimeType] = constructor;
@@ -26,8 +29,8 @@ ImageEncoder.registerMetadataEncoder =
function(constructor, mimeType) {
*
* The encoder will own and modify a copy of the original metadata.
*
- * @param {Object} metadata Original metadata.
- * @return {ImageEncoder.MetadataEncoder} Created metadata encoder.
+ * @param {!Object} metadata Original metadata.
+ * @return {!ImageEncoder.MetadataEncoder} Created metadata encoder.
*/
ImageEncoder.createMetadataEncoder = function(metadata) {
var constructor =
@@ -41,10 +44,10 @@ ImageEncoder.createMetadataEncoder = function(metadata)
{
* Create a metadata encoder object holding a copy of metadata
* modified according to the properties of the supplied image.
*
- * @param {Object} metadata Original metadata.
- * @param {HTMLCanvasElement} canvas Canvas to use for metadata.
+ * @param {!Object} metadata Original metadata.
+ * @param {!HTMLCanvasElement} canvas Canvas to use for metadata.
* @param {number} quality Encoding quality (defaults to 1).
- * @return {ImageEncoder.MetadataEncoder} Encoder with encoded metadata.
+ * @return {!ImageEncoder.MetadataEncoder} Encoder with encoded metadata.
*/
ImageEncoder.encodeMetadata = function(metadata, canvas, quality) {
var encoder = ImageEncoder.createMetadataEncoder(metadata);
@@ -56,10 +59,10 @@ ImageEncoder.encodeMetadata = function(metadata,
canvas, quality) {

/**
* Return a blob with the encoded image with metadata inserted.
- * @param {HTMLCanvasElement} canvas The canvas with the image to be
encoded.
- * @param {ImageEncoder.MetadataEncoder} metadataEncoder Encoder to use.
+ * @param {!HTMLCanvasElement} canvas The canvas with the image to be
encoded.
+ * @param {!ImageEncoder.MetadataEncoder} metadataEncoder Encoder to use.
* @param {number} quality (0..1], Encoding quality, defaults to 0.9.
- * @return {Blob} encoded data.
+ * @return {!Blob} encoded data.
*/
ImageEncoder.getBlob = function(canvas, metadataEncoder, quality) {
// Contrary to what one might think 1.0 is not a good default. Opening
and
@@ -119,15 +122,15 @@ ImageEncoder.getBlob = function(canvas,
metadataEncoder, quality) {
ImageEncoder.decodeDataURL = function(dataURL) {
// Skip the prefix ('data:image/<type>;base64,')
var base64string = dataURL.substring(dataURL.indexOf(',') + 1);
- return atob(base64string);
+ return window.atob(base64string);
};

/**
* Return a thumbnail for an image.
- * @param {HTMLCanvasElement} canvas Original image.
+ * @param {!HTMLCanvasElement} canvas Original image.
* @param {number=} opt_shrinkage Thumbnail should be at least this much
smaller
* than the original image (in each dimension).
- * @return {HTMLCanvasElement} Thumbnail canvas.
+ * @return {!HTMLCanvasElement} Thumbnail canvas.
*/
ImageEncoder.createThumbnail = function(canvas, opt_shrinkage) {
var MAX_THUMBNAIL_DIMENSION = 320;
@@ -136,7 +139,8 @@ ImageEncoder.createThumbnail = function(canvas,
opt_shrinkage) {
canvas.width / MAX_THUMBNAIL_DIMENSION,
canvas.height / MAX_THUMBNAIL_DIMENSION);

- var thumbnailCanvas = canvas.ownerDocument.createElement('canvas');
+ var thumbnailCanvas = assertInstanceof(
+ canvas.ownerDocument.createElement('canvas'), HTMLCanvasElement);
thumbnailCanvas.width = Math.round(canvas.width / opt_shrinkage);
thumbnailCanvas.height = Math.round(canvas.height / opt_shrinkage);

@@ -149,11 +153,11 @@ ImageEncoder.createThumbnail = function(canvas,
opt_shrinkage) {
};

/**
- * TODO(JSDOC)
- * @param {string} string // TODO(JSDOC).
- * @param {number} from // TODO(JSDOC).
- * @param {number} to // TODO(JSDOC).
- * @return {ArrayBuffer} // TODO(JSDOC).
+ * Convert string to an array buffer.
+ * @param {string} string A string.
+ * @param {number} from Strt index.
+ * @param {number} to End index.
+ * @return {!ArrayBuffer} A created array buffer is returned.
*/
ImageEncoder.stringToArrayBuffer = function(string, from, to) {
var size = to - from;
@@ -170,8 +174,9 @@ ImageEncoder.stringToArrayBuffer = function(string,
from, to) {
* Serves as a default metadata encoder for images that none of the
metadata
* parsers recognized.
*
- * @param {Object} original_metadata Starting metadata.
+ * @param {!Object} original_metadata Starting metadata.
* @constructor
+ * @struct
*/
ImageEncoder.MetadataEncoder = function(original_metadata) {
this.metadata_ = MetadataCache.cloneMetadata(original_metadata) || {};
@@ -183,15 +188,15 @@ ImageEncoder.MetadataEncoder =
function(original_metadata) {
};

/**
- * TODO(JSDOC)
- * @return {Object} // TODO(JSDOC).
+ * Returns metadata.
+ * @return {!Object} A metadata.
*/
ImageEncoder.MetadataEncoder.prototype.getMetadata = function() {
return this.metadata_;
};

/**
- * @param {HTMLCanvasElement|Object} canvas Canvas or or anything with
+ * @param {!HTMLCanvasElement} canvas Canvas or or anything with
* width and height properties.
*/
ImageEncoder.MetadataEncoder.prototype.setImageData = function(canvas) {
@@ -200,7 +205,7 @@ ImageEncoder.MetadataEncoder.prototype.setImageData =
function(canvas) {
};

/**
- * @param {HTMLCanvasElement} canvas Canvas to use as thumbnail.
+ * @param {!HTMLCanvasElement} canvas Canvas to use as thumbnail.
* @param {number} quality Thumbnail quality.
*/
ImageEncoder.MetadataEncoder.prototype.setThumbnailData =
@@ -212,8 +217,8 @@ ImageEncoder.MetadataEncoder.prototype.setThumbnailData
=

/**
* Return a range where the metadata is (or should be) located.
- * @param {string} encodedImage // TODO(JSDOC).
- * @return {Object} An object with from and to properties.
+ * @param {string} encodedImage An encoded image.
+ * @return {!{from:number, to:number}} An object with from and to
properties.
*/
ImageEncoder.MetadataEncoder.prototype.
findInsertionRange = function(encodedImage) { return {from: 0, to: 0};
};
@@ -221,7 +226,7 @@ ImageEncoder.MetadataEncoder.prototype.
/**
* Return serialized metadata ready to write to an image file.
* The return type is optimized for passing to Blob.append.
- * @return {ArrayBuffer} // TODO(JSDOC).
+ * @return {ArrayBuffer} Serialized metadata.
*/
ImageEncoder.MetadataEncoder.prototype.encode = function() {
return new Uint8Array(0).buffer;


hir...@chromium.org

unread,
Nov 17, 2014, 8:43:56 PM11/17/14
to yaw...@chromium.org, chromium...@chromium.org, rginda...@chromium.org, mtomas...@chromium.org, yoshik...@chromium.org
Thank you!


https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js
File ui/file_manager/gallery/js/image_editor/image_encoder.js (right):

https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode156
ui/file_manager/gallery/js/image_editor/image_encoder.js:156: * Convert
string to an array buffer.
nit: Converts

https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode219
ui/file_manager/gallery/js/image_editor/image_encoder.js:219: * Return a
range where the metadata is (or should be) located.
Please fix it with 'Returns'.

https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode221
ui/file_manager/gallery/js/image_editor/image_encoder.js:221: * @return
{!{from:number, to:number}} An object with from and to properties.
I think we don't need to add '!'. Record type (such as {from:number,
to:number}) is non-nullable.

https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode227
ui/file_manager/gallery/js/image_editor/image_encoder.js:227: * Return
serialized metadata ready to write to an image file.
ditto.

https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode229
ui/file_manager/gallery/js/image_editor/image_encoder.js:229: * @return
{ArrayBuffer} Serialized metadata.
!ArrayBuffer ?

https://codereview.chromium.org/716093003/

yaw...@chromium.org

unread,
Nov 17, 2014, 11:26:54 PM11/17/14
to hir...@chromium.org, chromium...@chromium.org, rginda...@chromium.org, mtomas...@chromium.org, yoshik...@chromium.org
Thank you for the review! PTAL.


https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js
File ui/file_manager/gallery/js/image_editor/image_encoder.js (right):

https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode156
ui/file_manager/gallery/js/image_editor/image_encoder.js:156: * Convert
string to an array buffer.
On 2014/11/18 01:43:55, hirono wrote:
> nit: Converts

Done.

https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode219
ui/file_manager/gallery/js/image_editor/image_encoder.js:219: * Return a
range where the metadata is (or should be) located.
On 2014/11/18 01:43:55, hirono wrote:
> Please fix it with 'Returns'.

Done.

https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode221
ui/file_manager/gallery/js/image_editor/image_encoder.js:221: * @return
{!{from:number, to:number}} An object with from and to properties.
On 2014/11/18 01:43:55, hirono wrote:
> I think we don't need to add '!'. Record type (such as {from:number,
to:number})
> is non-nullable.

Done.

https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode227
ui/file_manager/gallery/js/image_editor/image_encoder.js:227: * Return
serialized metadata ready to write to an image file.
On 2014/11/18 01:43:55, hirono wrote:
> ditto.

Done.

https://codereview.chromium.org/716093003/diff/1/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode229
ui/file_manager/gallery/js/image_editor/image_encoder.js:229: * @return
{ArrayBuffer} Serialized metadata.
On 2014/11/18 01:43:55, hirono wrote:
> !ArrayBuffer ?

Done.

https://codereview.chromium.org/716093003/

hir...@chromium.org

unread,
Nov 18, 2014, 12:00:59 AM11/18/14
to yaw...@chromium.org, chromium...@chromium.org, rginda...@chromium.org, mtomas...@chromium.org, yoshik...@chromium.org
lgtm with a nit. Thanks!


https://codereview.chromium.org/716093003/diff/20001/ui/file_manager/gallery/js/image_editor/image_encoder.js
File ui/file_manager/gallery/js/image_editor/image_encoder.js (right):

https://codereview.chromium.org/716093003/diff/20001/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode158
ui/file_manager/gallery/js/image_editor/image_encoder.js:158: * @param
{number} from Strt index.
Strt -> Start?

https://codereview.chromium.org/716093003/

yaw...@chromium.org

unread,
Nov 18, 2014, 12:05:23 AM11/18/14
to hir...@chromium.org, chromium...@chromium.org, rginda...@chromium.org, mtomas...@chromium.org, yoshik...@chromium.org
Thank you!


https://codereview.chromium.org/716093003/diff/20001/ui/file_manager/gallery/js/image_editor/image_encoder.js
File ui/file_manager/gallery/js/image_editor/image_encoder.js (right):

https://codereview.chromium.org/716093003/diff/20001/ui/file_manager/gallery/js/image_editor/image_encoder.js#newcode158
ui/file_manager/gallery/js/image_editor/image_encoder.js:158: * @param
{number} from Strt index.
On 2014/11/18 05:00:58, hirono wrote:
> Strt -> Start?

Done.

https://codereview.chromium.org/716093003/

commi...@chromium.org

unread,
Nov 18, 2014, 12:06:18 AM11/18/14
to yaw...@chromium.org, hir...@chromium.org, chromium...@chromium.org, rginda...@chromium.org, mtomas...@chromium.org, yoshik...@chromium.org

commi...@chromium.org

unread,
Nov 18, 2014, 12:47:36 AM11/18/14
to yaw...@chromium.org, hir...@chromium.org, chromium...@chromium.org, rginda...@chromium.org, mtomas...@chromium.org, yoshik...@chromium.org
Committed patchset #3 (id:40001)

https://codereview.chromium.org/716093003/

commi...@chromium.org

unread,
Nov 18, 2014, 12:48:31 AM11/18/14
to yaw...@chromium.org, hir...@chromium.org, chromium...@chromium.org, rginda...@chromium.org, mtomas...@chromium.org, yoshik...@chromium.org
Patchset 3 (id:??) landed as
https://crrev.com/2711eaf138dabfb689b74d6c1b074e5578787385
Cr-Commit-Position: refs/heads/master@{#304567}

https://codereview.chromium.org/716093003/
Reply all
Reply to author
Forward
0 new messages