Refactor setIcon to allow for more general use of imageData

extensions/renderer/resources/set_icon.js

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 var SetIconCommon = requireNative('setIcon').SetIconCommon;
 var sendRequest = require('sendRequest').sendRequest;
 
-function loadImagePath(path, iconSize, actionType, callback) {
+function loadImagePath(path, iconSize, callback) {
   var img = new Image();
   img.onerror = function() {
-    console.error('Could not load ' + actionType + ' icon \'' +
+    console.error('Could not load action icon \'' +
                   path + '\'.');

[not at google - send to devlin, 2014/08/19 17:32:29]

This should fit on 1 line now.

[gpdavis, 2014/08/19 22:05:07]

Sweet deal!

   };
   img.onload = function() {
     var canvas = document.createElement('canvas');
     canvas.width = img.width > iconSize ? iconSize : img.width;
     canvas.height = img.height > iconSize ? iconSize : img.height;
 
     var canvas_context = canvas.getContext('2d');
     canvas_context.clearRect(0, 0, canvas.width, canvas.height);
     canvas_context.drawImage(img, 0, 0, canvas.width, canvas.height);
     var imageData = canvas_context.getImageData(0, 0, canvas.width,
@@ skipping 18 matching lines @@
         ' dictionary of ImageData objects.');
   }
 
   if (imageData.width > iconSize ||
       imageData.height > iconSize) {
     throw new Error(
         'The imageData property must contain an ImageData object that ' +
         'is no larger than ' + iconSize + ' pixels square.');
   }
 }
 

[not at google - send to devlin, 2014/08/19 17:32:29]

Add comment:

/**
 * Normalizes |details| to a format suitable for sending to the browser,
 * for example converting ImageData to a binary representation.
 *
 * @param {ImageDetails} details
 *   The ImageDetails passed into an extension action-style API.
 */

[gpdavis, 2014/08/19 22:05:07]

Done.

-function setIcon(details, callback, name, parameters, actionType) {
+function setIcon(details, callback) {
   var iconSizes = [19, 38];
   if ('iconIndex' in details) {
-    sendRequest(name, [details, callback], parameters);
-  } else if ('imageData' in details) {
-    if (typeof details.imageData == 'object') {
-      var isEmpty = true;
-      for (var i = 0; i < iconSizes.length; i++) {
-        var sizeKey = iconSizes[i].toString();
-        if (sizeKey in details.imageData) {
-          verifyImageData(details.imageData[sizeKey], iconSizes[i]);
-          isEmpty =false;
-        }
+    callback(details);
+    return;
+  }
+
+  if ('imageData' in details) {
+    var isEmpty = true;
+    for (var i = 0; i < iconSizes.length; i++) {
+      var sizeKey = iconSizes[i].toString();
+      if (sizeKey in details.imageData) {
+        verifyImageData(details.imageData[sizeKey], iconSizes[i]);
+        isEmpty =false;
       }
+    }
 
-      if (!isEmpty) {
-        sendRequest(name, [details, callback], parameters,
-                    {nativeFunction: SetIconCommon});
-      } else {
-        // If details.imageData is not dictionary with keys in set {'19', '38'},
-        // it must be an ImageData object.
-        var sizeKey = iconSizes[0].toString();
-        var imageData = details.imageData;
-        details.imageData = {};
-        details.imageData[sizeKey] = imageData;
-        verifyImageData(details.imageData[sizeKey], iconSizes[0]);
-        sendRequest(name, [details, callback], parameters,
-                    {nativeFunction: SetIconCommon});
-     }
-    } else {
-      throw new Error('imageData property has unexpected type.');
+    if (isEmpty) {
+      // If details.imageData is not dictionary with keys in set {'19', '38'},
+      // it must be an ImageData object.
+      var sizeKey = iconSizes[0].toString();
+      var imageData = details.imageData;
+      details.imageData = {};
+      details.imageData[sizeKey] = imageData;
+      verifyImageData(details.imageData[sizeKey], iconSizes[0]);
     }
-  } else if ('path' in details) {
+    callback(SetIconCommon(details));
+    return;
+  }

[not at google - send to devlin, 2014/08/19 17:32:29]

nit: blank line here.

[gpdavis, 2014/08/19 22:05:07]

Done.

+  if ('path' in details) {
     if (typeof details.path == 'object') {
       details.imageData = {};
       var isEmpty = true;
       var processIconSize = function(index) {
         if (index == iconSizes.length) {
           delete details.path;
           if (isEmpty)
             throw new Error('The path property must not be empty.');
-          sendRequest(name, [details, callback], parameters,
-                      {nativeFunction: SetIconCommon});
+          callback(SetIconCommon(details));
           return;
         }
         var sizeKey = iconSizes[index].toString();
         if (!(sizeKey in details.path)) {
           processIconSize(index + 1);
           return;
         }
         isEmpty = false;
-        loadImagePath(details.path[sizeKey], iconSizes[index], actionType,
+        loadImagePath(details.path[sizeKey], iconSizes[index],
           function(imageData) {
             details.imageData[sizeKey] = imageData;
             processIconSize(index + 1);
           });
       }
 

[not at google - send to devlin, 2014/08/19 17:32:29]

nit: no blank line here :)

[gpdavis, 2014/08/19 22:05:07]

Done.

       processIconSize(0);
     } else if (typeof details.path == 'string') {
       var sizeKey = iconSizes[0].toString();
       details.imageData = {};
-      loadImagePath(details.path, iconSizes[0], actionType,
+      loadImagePath(details.path, iconSizes[0],
           function(imageData) {
             details.imageData[sizeKey] = imageData;
             delete details.path;
-            sendRequest(name, [details, callback], parameters,
-                        {nativeFunction: SetIconCommon});
+            callback(SetIconCommon(details));
+            return;
       });
-    } else {
-      throw new Error('The path property should contain either string or ' +
-                      'dictionary of strings.');
     }

[not at google - send to devlin, 2014/08/19 17:32:29]

return after this.

[gpdavis, 2014/08/19 22:05:07]

Done.

-  } else {
-    throw new Error(
-        'Either the path or imageData property must be specified.');
   }

[not at google - send to devlin, 2014/08/19 17:32:29]

You still need to throw an Error out here.

[gpdavis, 2014/08/19 22:05:07]

Ah, right, because we can't validate that one of them exists with the schema. 
We can just validate that if they do exist, they are the correct type.  Right?

[not at google - send to devlin, 2014/08/20 14:42:18]

Yeah exactly, there is no way to say in the schema "exactly one of these must
exist".

 }
 
 exports.setIcon = setIcon;