Site Settings Desktop: Polish the Site Data details dialog.

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

 // Copyright 2016 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.
 
 /**
  * @typedef {{hasChildren: boolean,
  *            id: string,
  *            title: string,
  *            totalUsage: string,
  *            type: string}}
@@ skipping 157 matching lines @@
 
         for (var j = 0; j < siteEntry.children_.length; ++j) {
           var descriptionNode = siteEntry.children_[j];
           if (j > 0)
             description += ', ';
 
           // Some types, like quota, have no description nodes.
           var dataType = '';
           if (descriptionNode.data_.type != undefined)
             dataType = descriptionNode.data_.type;
-          else
-            dataType = descriptionNode.children_[0].data_.type;
+          else {

[dschuyler, 2016/09/29 19:40:27]

If the |else| has braces {} the |if| should also
(even though it's a one line block).

[Finnur, 2016/09/30 09:19:42]

Ah, yes, I indeed.

+            // A description node can have no children when in the process of
+            // being deleted.

[dschuyler, 2016/09/29 19:40:27]

optional:
// A description node might not have children when it's deleted.

(The 'can have no' on line 179 could mean 
'might not have' or it could
mean 'absolutely will not have'. The difference
can be discerned by context, so it's still correct
to leave it as is. It's just nicer to not have 
to work out what is intended). :)

[Finnur, 2016/09/30 09:19:42]

Done.

+            if (descriptionNode.children_.length > 0)
+              dataType = descriptionNode.children_[0].data_.type;

[Finnur, 2016/09/29 15:54:01]

This is not listed in the bug, but was discovered while debugging the original
issue. Some background:

A CookieTreeNode for a site consists of a root node for the site and a container
node (called descriptionNode here) that contains all the actual data nodes of a
specific type, i.e. ChannelID. 

As in:

- Site
  - Generic container
    - Leaf node (Cookie)
    - Leaf node (Cookie)
  - Generic container
    - Leaf node (ChannelID)

Each time a leaf node is deleted, the underlying Site Data list gets notified
and updates the summary list. But when the last/only data node within a
container node is deleted by the Site Data Dialog, the container node is also
deleted.

This means we'll get two delete notifications: One for the leaf node and another
for the container (in that order). However, since we label the container nodes
(in the summary list) by the data type of the nodes it contains we get an error
in the console about 'data_ not being defined on undefined'. Which makes sense,
because before the second delete comes in we'll have a container node that is
empty but about to be destroyed. But since the container node is empty and
therefore has no data type, the data type can be blank.

[dschuyler, 2016/09/29 19:40:27]

Thanks for the explanation!

+          }
 
           var count =
               (dataType == 'cookie') ? descriptionNode.children_.length : 0;
           if (count > 1) {
             description += loadTimeData.getStringF('cookiePlural', count);
           } else {
             description += getCookieDataCategoryText(
                 dataType, descriptionNode.data_.totalUsage);
           }
 
@@ skipping 50 matching lines @@
           root.appendChild(content);
         }
       }
     },
   };
 
   return {
     CookieTreeNode: CookieTreeNode,
   };
 });