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 20 matching lines @@
      */
     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.requests_ = 1;
+    this.browserProxy.reloadCookies();

[dschuyler, 2016/08/18 00:44:03]

Can we change reloadCookies() and removeAllCookies()
to return a promise and remove the this.requests_ 
tracking entirely?

[Finnur, 2016/08/18 16:20:30]

I've converted to promises, but it still doesn't alleviate the need to track
requests. It does, however, make the tracking a bit saner.

The reason I need to track the number of outstanding requests is to figure out
when I'm done fetching data so that I'm not calling getSummaryList() repeatedly
while the data is being populated -- it slows down the page loading.

The CookieTreeModel, when reset (as in reload-all-cookies), will give you the
top-level nodes. It is up to the client to decide what to do with it. We want
all the data, so we keep issuing more requests for more details until we have
everything.

I only want to update the UI when that is complete. So, I have to keep track of
the requests to know when. It feels more natural to do this at the JS layer
because the C++ layer doesn't really care if more requests are coming in and it
doesn't feel right to have it try to decipher what the client will do (or tell
the client when it is done).

Does this make sense?

+  },
+
+  /**
+   * Returns whether remove all should be shown.
+   * @param {Array<CookieDataSummaryItem>} sites The sites list to use to
+   * determine whether the button should be visible.
+   * @private
+   */
+  removeAllIsVisible_: function(sites) {
+    return sites.length > 0;
+  },
+
+  /**
+   * Called when the cookie list is ready to be shown.
+   * @private
+   */
   loadChildren_: function(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.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);
       }
     }
 
     if (--this.requests_ == 0)
       this.sites = this.treeNodes_.getSummaryList();
+
+    // 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.
    */
   onTreeItemRemoved_: function(args) {
     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() {
+    // removeAllCookies will result in the client list being updated, so update
+    // the list of outstanding requests.
+    this.requests_++;

[dschuyler, 2016/08/18 00:44:03]

nit: please use pre-increment unless post-increment
is needed for the specific case. i.e.
++this.requests_;

[Finnur, 2016/08/18 16:20:30]

Acknowledged.

+    this.browserProxy.removeAllCookies();

[dschuyler, 2016/08/18 00:44:03]

Same comment:

Can we change reloadCookies() and removeAllCookies()
to return a promise and remove the this.requests_ 
tracking entirely?

+  },
+
+  /**
+   * @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();
     });
   },
 });