Refactor setIcon to allow for more general use of imageData

extensions/renderer/resources/set_icon.js

Index: extensions/renderer/resources/set_icon.js
diff --git a/extensions/renderer/resources/set_icon.js b/extensions/renderer/resources/set_icon.js
index 883fd673997f849eaef0dc3514f6291131dadfb6..0df1c6939643728fced9ed1bd22634c1f5bc2921 100644
--- a/extensions/renderer/resources/set_icon.js
+++ b/extensions/renderer/resources/set_icon.js
@@ -49,10 +49,10 @@ function verifyImageData(imageData, iconSize) {
   }
 }
 
-function setIcon(details, callback, name, parameters, actionType) {
+function setIcon(details, actionType, callback) {

[not at google - send to devlin, 2014/08/19 00:04:43]

It looks like the only reason we need to thread through |actionType| is to have
a nicer error message. Let's just forget about that nicer error message (it's
pretty clear from the context - you only have one or the other) - and delete it.
Simpler everywhere.

[gpdavis, 2014/08/19 00:54:15]

Done.

   var iconSizes = [19, 38];
   if ('iconIndex' in details) {
-    sendRequest(name, [details, callback], parameters);
+    callback(details);

[not at google - send to devlin, 2014/08/19 00:04:43]

This code is such a nightmare. Sheesh. Such a nest of braces :(

Let's fix it, starting here:

if ('iconIndex' in details) {
  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(..);
      isEmpty = false;
    }
  }

  if (isEmpty) {
    // If details.imageData isnot a dictionary with keys...
    var...;
    verifyImageData(..);
  }

  callback(SetIconCommon(details));
  return;
}

... etc.

[gpdavis, 2014/08/19 00:54:15]

Done.

   } else if ('imageData' in details) {
     if (typeof details.imageData == 'object') {

[not at google - send to devlin, 2014/08/19 00:04:43]

Checks like these are so unnecessary, it's exactly what the schema validation
will have sorted out.

[gpdavis, 2014/08/19 00:54:15]

Done.

       var isEmpty = true;
@@ -65,8 +65,7 @@ function setIcon(details, callback, name, parameters, actionType) {
       }
 
       if (!isEmpty) {
-        sendRequest(name, [details, callback], parameters,
-                    {nativeFunction: SetIconCommon});
+        callback(SetIconCommon(details));
       } else {
         // If details.imageData is not dictionary with keys in set {'19', '38'},
         // it must be an ImageData object.
@@ -75,8 +74,7 @@ function setIcon(details, callback, name, parameters, actionType) {
         details.imageData = {};
         details.imageData[sizeKey] = imageData;
         verifyImageData(details.imageData[sizeKey], iconSizes[0]);
-        sendRequest(name, [details, callback], parameters,
-                    {nativeFunction: SetIconCommon});
+        callback(SetIconCommon(details));
      }
     } else {
       throw new Error('imageData property has unexpected type.');
@@ -90,9 +88,7 @@ function setIcon(details, callback, name, parameters, actionType) {
           delete details.path;
           if (isEmpty)
             throw new Error('The path property must not be empty.');
-          sendRequest(name, [details, callback], parameters,
-                      {nativeFunction: SetIconCommon});
-          return;
+          callback(SetIconCommon(details));
         }
         var sizeKey = iconSizes[index].toString();
         if (!(sizeKey in details.path)) {
@@ -115,8 +111,7 @@ function setIcon(details, callback, name, parameters, actionType) {
           function(imageData) {
             details.imageData[sizeKey] = imageData;
             delete details.path;
-            sendRequest(name, [details, callback], parameters,
-                        {nativeFunction: SetIconCommon});
+            callback(SetIconCommon(details));
       });
     } else {
       throw new Error('The path property should contain either string or ' +