Site Settings Desktop: Implement individual cookie removal and RemoveAll.

chrome/browser/resources/settings/site_settings/site_data.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.
 
 /**
  * @fileoverview
  * 'site-data' handles showing the local storage summary list for all sites.
  */
 
 Polymer({
@@ skipping 15 matching lines @@
      */
     treeNodes_: Object,
 
     /**
      * Keeps track of how many outstanding requests for more data there are.
      */
     requests_: Number,
   },
 
   ready: function() {
-    this.addWebUIListener('loadChildren', this.loadChildren_.bind(this));
     this.addWebUIListener('onTreeItemRemoved',
         this.onTreeItemRemoved_.bind(this));
     this.treeNodes_ = new settings.CookieTreeNode(null);
     // Start the initial request.
-    this.browserProxy.reloadCookies();
-    this.requests_ = 1;
+    this.reloadCookies_();
   },
 
+  /**
+   * Reloads the whole cookie list.
+   * @private
+   */
+  reloadCookies_: function() {
+    this.browserProxy.reloadCookies().then(function(list) {
+      this.loadChildren_(list);
+    }.bind(this));
+  },
+
+  /**
+   * Returns whether remove all should be shown.
+   * @param {Array<CookieDataSummaryItem>} sites The sites list to use to
+   * determine whether the button should be visible.

[dschuyler, 2016/08/18 22:51:10]

nit: indent 'determine...'

[Finnur, 2016/08/19 12:18:32]

Done.

+   * @private
+   */
+  removeAllIsVisible_: function(sites) {
+    return sites.length > 0;
+  },
+
+  /**
+   * Called when the cookie list is ready to be shown.
+   * @private
+   */
   loadChildren_: function(list) {

[dschuyler, 2016/08/18 22:51:10]

@param for list.

     var parentId = list[0];
     var data = list[1];
 
     if (parentId == null) {
+      // New root being added, clear the list and add the nodes.
+      this.sites = [];
+      this.requests_ = 0;
       this.treeNodes_.addChildNodes(this.treeNodes_, data);
     } else {
       this.treeNodes_.populateChildNodes(parentId, this.treeNodes_, data);
     }
 
     for (var i = 0; i < data.length; ++i) {
       var prefix = parentId == null ? '' : parentId + ', ';
       if (data[i].hasChildren) {
-        this.requests_ += 1;
-        this.browserProxy.loadCookieChildren(prefix + data[i].id);
+        ++this.requests_;
+        this.browserProxy.loadCookieChildren(
+            prefix + data[i].id).then(function(list) {
+          --this.requests_;
+          this.loadChildren_(list);
+        }.bind(this));
       }
     }
 
-    if (--this.requests_ == 0)
+    if (this.requests_ == 0)
       this.sites = this.treeNodes_.getSummaryList();

[dschuyler, 2016/08/18 22:51:10]

It looks like this may be called more often
than once. i.e. it may trigger once per level
in the tree (*I think*). If the goal is just
to reduce the calls to getSummaryList, this 
may be fine. If getSummeryList should not
be called more than once (in a strict sense)
then there /may/ be an issue here.

[Finnur, 2016/08/18 23:27:38]

I don't think this is an issue, but will check tomorrow when I get in to work
again.

[Finnur, 2016/08/19 12:18:32]

I verified this again and getSummaryList is called only once (at the end, when
all levels have finished updating). 

+
+    // If this reaches below zero then we're forgetting to increase the
+    // outstanding request count and the summary list won't be updated at the
+    // end.
+    assert(this.requests_ >= 0);
   },
 
   /**
-   * Called when an item is removed.
+   * Called when a single item has been removed (not during delete all).
    */
   onTreeItemRemoved_: function(args) {

[dschuyler, 2016/08/18 22:51:10]

Can we @param the args?

[Finnur, 2016/08/18 23:27:38]

Umm, yeah -- |we| probably can. :) I, however, already made an attempt at it
yesterday and gave up. I'm just not sure how to represent the data types
involved to Closure. Your input here would be valuable.

Essentially, I wanted to represent that |args| always looks like this:

Array<String, Number, Number>

In other words, it is a list value with always the same three data types.

But Closure doesn't like that kind of definition. I also tried something like
Array<String|Number>, but then Closure complains about the call to
removeByParentId below (because it wants String but gets String|Number.

What's the right way to do this?

[Finnur, 2016/08/19 12:18:32]

Actually, scratch that...

Coming in this morning with a fresh mind I realize that sending the data the way
the handler is doing it is a bit dumb. So I changed it to a dictionary and added
the @params.

     this.treeNodes_.removeByParentId(args[0], args[1], args[2]);
     this.sites = this.treeNodes_.getSummaryList();
   },
 
   /**
-   * @param {!{model: !{item: !{title: string, id: string}}}} event
+   * Deletes all site data for a given site.
+   * @param {!{model: !{item: CookieDataSummaryItem}}} event
+   * @private
+   */
+  onDeleteSite_: function(event) {
+    this.browserProxy.removeCookie(event.model.item.id);
+  },
+
+  /**
+   * Deletes site data for all sites.
+   * @private
+   */
+  onDeleteAllSites_: function() {
+    this.browserProxy.removeAllCookies().then(function(list) {
+      this.loadChildren_(list);
+    }.bind(this));
+  },
+
+  /**
+   * @param {!{model: !{item: CookieDataSummaryItem}}} event
    * @private
    */
   onSiteTap_: function(event) {
     var dialog = document.createElement('site-data-details-dialog');
     dialog.category = this.category;
     this.shadowRoot.appendChild(dialog);
 
     var node = this.treeNodes_.fetchNodeById(event.model.item.id, false);
     dialog.open(node);
 
     dialog.addEventListener('close', function(event) {
       dialog.remove();
     });
   },
 });