Site Settings Desktop: Show the type of cookie in dropdown (instead of url).

chrome/browser/resources/settings/site_settings/cookie_tree_node.js

Index: chrome/browser/resources/settings/site_settings/cookie_tree_node.js
diff --git a/chrome/browser/resources/settings/site_settings/cookie_tree_node.js b/chrome/browser/resources/settings/site_settings/cookie_tree_node.js
index d4591dfc26b2b66ecc8fbb005582476fbcdc90b9..0afdeffa1107b4b07f04e6fa9441adcede7cca9d 100644
--- a/chrome/browser/resources/settings/site_settings/cookie_tree_node.js
+++ b/chrome/browser/resources/settings/site_settings/cookie_tree_node.js
@@ -39,6 +39,43 @@ var CookieList;
  */
 var CookieRemovePacket;
 
+/**
+ * Retrieves the human friendly text to show for the type of cookie.
+ * @param {string} dataType The datatype to look up.
+ * @param {number} cookieCount How many cookies are involved.
+ * @param {string} totalUsage How much data is being consumed.
+ * @return {string} The human-friendly description for this cookie.
+ */
+function getCookieDataCategoryText(dataType, cookieCount, totalUsage) {
+  var category = '';
+  if (dataType == 'cookie') {
+    if (cookieCount > 1)
+      category = loadTimeData.getStringF('cookiePlural', cookieCount);
+    else
+      category = loadTimeData.getString('cookieSingular');
+  } else if (dataType == 'database') {
+    category = loadTimeData.getString('cookieDatabaseStorage');
+  } else if (dataType == 'local_storage' || dataType == 'indexed_db') {
+    category = loadTimeData.getString('cookieLocalStorage');
+  } else if (dataType == 'app_cache') {
+    category = loadTimeData.getString('cookieAppCache');
+  } else if (dataType == 'file_system') {
+    category = loadTimeData.getString('cookieFileSystem');
+  } else if (dataType == 'quota') {
+    category = totalUsage;
+  } else if (dataType == 'channel_id') {
+    category = loadTimeData.getString('cookieChannelId');
+  } else if (dataType == 'service_worker') {
+    category = loadTimeData.getString('cookieServiceWorker');
+  } else if (dataType == 'cache_storage') {
+    category = loadTimeData.getString('cookieCacheStorage');
+  } else if (dataType == 'flash_lso') {
+    category = loadTimeData.getString('cookieFlashLso');
+  }

[dschuyler, 2016/08/23 18:53:39]

Suggestion: The above is ok, but it might be nicer
to read and quicker to execute if it were changed
to a dictionary/object with entries like:

  'cookie': loadTimeData.getString('cookieSingular'),
  'database': loadTimeData.getString('cookieDatabaseStorage'),
  'local_storage': loadTimeData.getString('cookieLocalStorage'),
  'indexed_db': loadTimeData.getString('cookieLocalStorage'),
  [...]

and then the if/else chain were shorter, like: 

if (dataType == 'cookie' && cookieCount > 1)
  category = loadTimeData.getStringF('cookiePlural', cookieCount);
else if (dataType == 'quota')
  category = totalUsage;
else
  category = this.categoryLabels[dataType];

Just an optional suggestion.

[Finnur, 2016/08/24 11:31:14]

I like that suggestion. Closure, doesn't quite like the use of |this| in a
function like that, but I moved the dictionary object outside the function and
that works fine. Will check in that way, but happy to augment if you think it is
necessary.

+
+  return category;
+}
+
 cr.define('settings', function() {
   'use strict';
 
@@ -153,34 +190,10 @@ cr.define('settings', function() {
           else
             dataType = descriptionNode.children_[0].data_.type;
 
-          var category = '';
-          if (dataType == 'cookie') {
-            var cookieCount = descriptionNode.children_.length;
-            if (cookieCount > 1)
-              category = loadTimeData.getStringF('cookiePlural', cookieCount);
-            else
-              category = loadTimeData.getString('cookieSingular');
-          } else if (dataType == 'database') {
-            category = loadTimeData.getString('cookieDatabaseStorage');
-          } else if (dataType == 'local_storage' || dataType == 'indexed_db') {
-            category = loadTimeData.getString('cookieLocalStorage');
-          } else if (dataType == 'app_cache') {
-            category = loadTimeData.getString('cookieAppCache');
-          } else if (dataType == 'file_system') {
-            category = loadTimeData.getString('cookieFileSystem');
-          } else if (dataType == 'quota') {
-            category = descriptionNode.data_.totalUsage;
-          } else if (dataType == 'channel_id') {
-            category = loadTimeData.getString('cookieChannelId');
-          } else if (dataType == 'service_worker') {
-            category = loadTimeData.getString('cookieServiceWorker');
-          } else if (dataType == 'cache_storage') {
-            category = loadTimeData.getString('cookieCacheStorage');
-          } else if (dataType == 'flash_lso') {
-            category = loadTimeData.getString('cookieFlashLso');
-          }
-
-          description += category;
+          var count =
+              (dataType == 'cookie') ? descriptionNode.children_.length : 0;
+          description += getCookieDataCategoryText(
+              dataType, count, descriptionNode.data_.totalUsage);
         }
         list.push({ site: title, id: id, localData: description });
       }