[Offline pages] Create offline internals page for Android

chrome/browser/resources/offline_pages/offline_internals.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.
+
+cr.define('offlineInternals', function() {
+  'use strict';
+
+  var STORED_PAGE_ATTRIBUTES = ['onlineUrl', 'namespace', 'size'];
+  var QUEUE_ATTRIBUTES = ['onlineUrl', 'creation time', 'status'];

[Dan Beam, 2016/05/31 22:00:59]

can you potentially using closure compilation typechecking instead of strings
and obj[key] notation?  the code, as written, will never be typesafe :(

[chili, 2016/06/02 02:38:53]

Done.

+
+  /**
+   * Clear the specified table.

[Dan Beam, 2016/05/31 22:00:59]

@param

[chili, 2016/06/02 02:38:53]

Done.

+   */
+  function clearTable(tableId) {
+    var element = $(tableId);
+
+    element.textContent = '';

[Dan Beam, 2016/05/31 22:00:59]

nit: $(tableId).textContent = '';

[chili, 2016/06/02 02:38:53]

Done.

+  }
+
+  /**
+   * Fill stored pages table.
+   * @param {HTMLElement} element A HTML element
+   * @param {!Array} data An array object representing stored offline pages

[Dan Beam, 2016/05/31 22:00:59]

nit: end with .

[chili, 2016/06/02 02:38:53]

Done.

+   */
+  function fillStoredPages(element, data) {
+     for (var i = 0; i < data.length; i++) {
+      var row = document.createElement('tr');

[Dan Beam, 2016/05/31 22:00:59]

indent off

[chili, 2016/06/02 02:38:54]

Done.

+
+      var checkboxCell = document.createElement('td');
+      var checkbox = document.createElement('input');
+      checkbox.setAttribute('type', 'checkbox');
+      checkbox.setAttribute('name', 'stored');
+      checkbox.setAttribute('value', data[i]['id']);

[Dan Beam, 2016/05/31 22:00:59]

nit: ['id'] -> .id

[chili, 2016/06/02 02:38:53]

Done.

+
+      checkboxCell.appendChild(checkbox);
+      row.appendChild(checkboxCell);
+
+      for (let key of STORED_PAGE_ATTRIBUTES) {
+        var cell = document.createElement('td');
+        cell.textContent = data[i][key];
+        row.appendChild(cell);
+      }
+      element.appendChild(row);
+    }
+  }
+
+  /**
+   * Fill requests table.
+   * @param {HTMLElement} element A HTML element
+   * @param {!Array} data An array object representing the request queue
+   */
+  function fillRequestQueue(element, data) {
+    for (var i = 0; i < data.length; i++) {
+      var row = document.createElement('tr');
+
+      for (let key of QUEUE_ATTRIBUTES) {
+        var cell = document.createElement('td');
+        cell.textContent = data[i][key];
+        row.appendChild(cell);
+      }
+      element.appendChild(row);
+    }
+  }
+
+  /**
+   * Refresh all displayed information.
+   */
+  function refreshAll() {
+    chrome.send('getOfflineInternalsInfo');
+  }
+
+  /**
+   * Delete all pages in the offline store.
+   */
+  function deleteAllPages() {
+    chrome.send('deleteAllPages');
+  }
+
+  /**
+   * Callback when pages are deleted.
+   */
+  function pagesDeleted() {
+    // TODO(chili): decide what to do here. Perhaps a refresh of just
+    // the stored pages table?
+  }
+
+  /**
+   * Callback when information is loaded.
+   * @param {{AllPages: !Array, Queue:!Array}} info An object containing both

[Dan Beam, 2016/05/31 22:00:59]

Queue: !Array

[chili, 2016/06/02 02:38:53]

Done.

+   *     stored pages and request queue status.
+   */
+  function setOfflineInternalsInfo(info) {
+    clearTable('stored-pages');
+    clearTable('request-queue');
+
+    fillStoredPages($('stored-pages'), info.AllPages);
+    fillRequestQueue($('request-queue'), info.Queue);
+  }
+
+  /**
+   * Delete selected pages from the offline store.
+   */
+  function deleteSelectedPages() {
+    var selectedIds = new Array();

[Dan Beam, 2016/05/31 22:00:59]

new Array() -> []

[chili, 2016/06/02 02:38:54]

Done.

+    var checkboxes = document.getElementsByName('stored');
+
+    for (let box of checkboxes) {
+      if (box.checked) {

[Dan Beam, 2016/05/31 22:00:59]

no curlies

[chili, 2016/06/02 02:38:53]

Java-land habit >"<
Done

+        selectedIds.push(box.value);
+      }
+    }

[Dan Beam, 2016/05/31 22:00:59]

nit: if you're gonna use ES6...

var selectedIds = [].filter.call(checkboxes, b => b.checked).map(b => b.id);

but general note that ES6 support is bad on iOS and in our presubmit linter

[chili, 2016/06/02 02:38:54]

I don't think this works, because checkboxes is a NodeList, not an Array. 
Javascript will throw error.

[dpapad, 2016/06/02 17:33:16]

Drive-by suggestion. If you want to take advantage of the very useful array
methods (forEach, map, etc...), you can always convert the NodeList to an array
first

var array = Array.prototype.slice.call(nodelist);
Then you can follow previous suggestion (with or without ES6) of combining
filter() and map() calls.
var selectedIds = checkboxes.filter(....).map(....);

+
+    chrome.send('deleteSelectedPages', [selectedIds]);

[Dan Beam, 2016/05/31 22:00:59]

instead of doing this dance where you say

chrome.send('name', params);

// export global name
// call global name from C++

can you use cr.sendWithPromise() instead?  this can handle responses and params
and is more testable

[chili, 2016/06/02 02:38:53]

Done.

+  }
+
+  function initialize() {
+    $('clear-all').onclick = deleteAllPages;
+    $('clear-selected').onclick = deleteSelectedPages;
+    $('refresh').onclick = refreshAll;
+    chrome.send('getOfflineInternalsInfo');
+  }
+
+  // Return an object with all of the exports.
+  return {
+    initialize: initialize,

[Dan Beam, 2016/05/31 22:00:59]

why is `initialize` externally exposed?

[chili, 2016/06/02 02:38:53]

Because otherwise I can't seem to call offlineInternals.initialize in line
131...

+    setOfflineInternalsInfo: setOfflineInternalsInfo,
+    pagesDeleted: pagesDeleted
+  };
+});
+
+document.addEventListener('DOMContentLoaded', offlineInternals.initialize);