[MD settings] cleanup display local data

This CL removes the start/count parameters from getDisplayList for local data (which is not used)
and adds a loading message during the data transfer (from C++ to JS).

Bug: 730777, 776190
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I75d1728a84cc262b040e4ed3959a46914b48a040
Reviewed-on: https://chromium-review.googlesource.com/683467
Commit-Queue: Dave Schuyler <dschuyler@chromium.org>
Reviewed-by: Hector Carmona <hcarmona@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509957}

chrome/app/settings_strings.grdp

@@ -2574,6 +2574,9 @@
   <message name="IDS_SETTINGS_SITE_SETTINGS_SITE_RESET_DIALOG_TITLE" desc="Title of the dialog that warns about resetting all permissions for a site.">
     Reset settings for this site?
   </message>
+  <message name="IDS_SETTINGS_LOADING_LOCAL_DATA_LIST" desc="Explanation shown while transferring cookies and local data.">
+    Loading, please wait...
+  </message>
   <message name="IDS_SETTINGS_SITE_SETTINGS_SITE_RESET_CONFIRMATION" desc="Text for the dialog that warns about resetting all permissions for a site.">
     Chrome will reset permissions to their defaults
   </message>

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

@@ -28,7 +28,6 @@ var LocalDataItem;
  * TODO(dschuyler): add |filter| and |order|.
  * @typedef {{
  *   items: !Array<!LocalDataItem>,
- *   start: number,
  *   total: number,
  * }}
  */
@@ -39,13 +38,9 @@ cr.define('settings', function() {
   class LocalDataBrowserProxy {
     /**
      * @param {string} filter Search filter (use "" for none).
-     * @param {number} begin Which element to start with. (Similar to 'offset'
-     *     in SQL). The first item is at 0.
-     * @param {number} count How many list elements are displayed. (Similar to
-     *     'limit' in SQL). Pass -1 to get all remaining items.
      * @return {!Promise<!LocalDataList>}
      */
-    getDisplayList(filter, begin, count) {}
+    getDisplayList(filter) {}
 
     /**
      * Removes all local data (local storage, cookies, etc.).
@@ -93,9 +88,8 @@ cr.define('settings', function() {
    */
   class LocalDataBrowserProxyImpl {
     /** @override */
-    getDisplayList(filter, begin, count) {
-      return cr.sendWithPromise(
-          'localData.getDisplayList', filter, begin, count);
+    getDisplayList(filter) {
+      return cr.sendWithPromise('localData.getDisplayList', filter);
     }
 
     /** @override */

chrome/browser/resources/settings/site_settings/site_data.html

@@ -31,7 +31,9 @@
       }
     </style>
     <div class="settings-box continuation">
-      <paper-button class="secondary-button" id="removeShowingSites"
+      <div hidden$="[[!isLoading_]]">$i18n{loadingLocalDataList}</div>
+      <paper-button class="secondary-button"
+          disabled$="[[isLoading_]]" id="removeShowingSites"
           on-tap="onRemoveShowingSitesTap_" hidden$="[[!sites.length]]">
         [[computeRemoveLabel_(filter)]]
       </paper-button>

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

@@ -60,6 +60,8 @@ Polymer({
       observer: 'focusConfigChanged_',
     },
 
+    isLoading_: Boolean,
+
     /** @type {!Array<!LocalDataItem>} */
     sites: {
       type: Array,
@@ -99,6 +101,7 @@ Polymer({
     settings.GlobalScrollTargetBehaviorImpl.currentRouteChanged.call(
         this, currentRoute);
     if (currentRoute == settings.routes.SITE_SETTINGS_SITE_DATA) {
+      this.isLoading_ = true;
       this.browserProxy_.reloadCookies().then(this.updateSiteList_.bind(this));
     }
   },
@@ -138,12 +141,12 @@ Polymer({
    * @private
    */
   updateSiteList_: function() {
-    this.browserProxy_
-        .getDisplayList(this.filter, 0 /* start */, -1 /* count */)
-        .then((listInfo) => {
-          this.sites = listInfo.items;
-          this.fire('site-data-list-complete');
-        });
+    this.isLoading_ = true;
+    this.browserProxy_.getDisplayList(this.filter).then((listInfo) => {
+      this.sites = listInfo.items;
+      this.isLoading_ = false;
+      this.fire('site-data-list-complete');
+    });
   },
 
   /**

chrome/browser/ui/webui/settings/md_settings_localized_strings_provider.cc

@@ -1925,6 +1925,7 @@ void AddSiteSettingsStrings(content::WebUIDataSource* html_source,
      IDS_SETTINGS_SITE_SETTINGS_SITE_RESET_CONFIRMATION},
     {"siteSettingsSiteResetDialogTitle",
      IDS_SETTINGS_SITE_SETTINGS_SITE_RESET_DIALOG_TITLE},
+    {"loadingLocalDataList", IDS_SETTINGS_LOADING_LOCAL_DATA_LIST},
     {"thirdPartyCookie", IDS_SETTINGS_SITE_SETTINGS_THIRD_PARTY_COOKIE},
     {"thirdPartyCookieSublabel",
      IDS_SETTINGS_SITE_SETTINGS_THIRD_PARTY_COOKIE_SUBLABEL},

chrome/browser/ui/webui/settings/settings_cookies_view_handler.cc

@@ -134,8 +134,7 @@ CookiesViewHandler::Request::Request() {
 }
 
 void CookiesViewHandler::Request::Clear() {
-  start_index = 0;
-  index_count = 0;
+  should_send_list = false;
   callback_id_.clear();
 }
 
@@ -237,7 +236,7 @@ void CookiesViewHandler::TreeModelEndBatch(CookiesTreeModel* model) {
   DCHECK(batch_update_);
   batch_update_ = false;
   if (IsJavascriptAllowed()) {
-    if (request_.index_count) {
+    if (request_.should_send_list) {
       SendLocalDataList(model->GetRoot());
     } else if (!request_.callback_id_.empty()) {
       ResolveJavascriptCallback(base::Value(request_.callback_id_),
@@ -303,20 +302,13 @@ void CookiesViewHandler::HandleGetCookieDetails(const base::ListValue* args) {
 
 void CookiesViewHandler::HandleGetDisplayList(const base::ListValue* args) {
   CHECK(request_.callback_id_.empty());
-  CHECK_EQ(4U, args->GetSize());
+  CHECK_EQ(2U, args->GetSize());
   CHECK(args->GetString(0, &request_.callback_id_));
   base::string16 filter;
   CHECK(args->GetString(1, &filter));
-  CHECK(args->GetInteger(2, &request_.start_index));
-  CHECK(args->GetInteger(3, &request_.index_count));
-  if (request_.index_count == -1) {
-    // Requesting *all* items.
-    request_.index_count = INT_MAX;
-  }
 
   AllowJavascript();
-  CHECK_GE(request_.start_index, 0);
-  CHECK_GE(request_.index_count, 0);
+  request_.should_send_list = true;
   // Resetting the filter is a heavy operation, avoid unnecessary filtering.
   if (filter != filter_) {
     filter_ = filter;
@@ -392,7 +384,7 @@ void CookiesViewHandler::HandleRemoveItem(const base::ListValue* args) {
 
 void CookiesViewHandler::SendLocalDataList(const CookieTreeNode* parent) {
   CHECK(cookies_tree_model_.get());
-  CHECK(request_.index_count);
+  CHECK(request_.should_send_list);
   const int parent_child_count = parent->child_count();
   if (sorted_sites_.empty()) {
     // Sort the list by site.
@@ -405,12 +397,6 @@ void CookiesViewHandler::SendLocalDataList(const CookieTreeNode* parent) {
   }
 
   const int list_item_count = sorted_sites_.size();
-  const int start = request_.start_index;
-  int limit = std::min(start + request_.index_count, list_item_count);
-  // Check for integer wraparound/overflow. The client is not expected to know
-  // the child_count, so this wraparound is not an error.
-  if (limit < 0)
-    limit = list_item_count;
   // The layers in the CookieTree are:
   //   root - Top level.
   //   site - www.google.com, example.com, etc.
@@ -418,7 +404,7 @@ void CookiesViewHandler::SendLocalDataList(const CookieTreeNode* parent) {
   //   item - Info on the actual thing.
   // Gather list of sites with some highlights of the categories and items.
   std::unique_ptr<base::ListValue> site_list(new base::ListValue);
-  for (int i = start; i < limit; ++i) {
+  for (int i = 0; i < list_item_count; ++i) {
     const CookieTreeNode* site = parent->GetChild(sorted_sites_[i].second);
     std::string description;
     for (int k = 0; k < site->child_count(); ++k) {
@@ -457,7 +443,6 @@ void CookiesViewHandler::SendLocalDataList(const CookieTreeNode* parent) {
 
   base::DictionaryValue response;
   response.Set(kItems, std::move(site_list));
-  response.Set(kStart, base::MakeUnique<base::Value>(start));
   response.Set(kTotal, base::MakeUnique<base::Value>(list_item_count));
 
   ResolveJavascriptCallback(base::Value(request_.callback_id_), response);

chrome/browser/ui/webui/settings/settings_cookies_view_handler.h

@@ -93,10 +93,8 @@ class CookiesViewHandler : public SettingsPageUIHandler,
     Request();
     void Clear();
 
-    // The top list item in view.
-    int start_index;
-    // How many list items are in view.
-    int index_count;
+    // Whether the request expects a list response.
+    bool should_send_list;
     // The callback ID for the current outstanding request.
     std::string callback_id_;
   };

chrome/test/data/webui/settings/test_local_data_browser_proxy.js

@@ -46,14 +46,11 @@ class TestLocalDataBrowserProxy extends TestBrowserProxy {
   }
 
   /** @override */
-  getDisplayList(filter, begin, count) {
+  getDisplayList(filter) {
     if (filter === undefined)
       filter = '';
-    if (count == -1)
-      count = this.cookieList_.length;
     let output = [];
-    let end = Math.min(begin + count, this.cookieList_.length);
-    for (let i = begin; i < end; ++i) {
+    for (let i = 0; i < this.cookieList_.length; ++i) {
       if (this.cookieList_[i].site.indexOf(filter) >= 0) {
         output.push(this.filteredCookieList_[i]);
       }