Bug 1326182 - Send all the console.table items in the console message. r=Honza.

This allows us to not have to reach for the server
again from the client to get all the data we need
to render the table.

We still need to keep the old function to handle
connections to older servers. We'll use Bug 1597955
in Firefox 74 to clean the bits we don't need anymore.

A test is added to ensure the bug is fixed and we don't
regress; stubs are updated.

Differential Revision: https://phabricator.services.mozilla.com/D53962

--HG--
extra : moz-landing-system : lando

devtools/client/webconsole/components/Output/ConsoleTable.js

@@ -12,6 +12,7 @@ const actions = require("devtools/client/webconsole/actions/messages");
 const {
   l10n,
   getArrayTypeNames,
+  getDescriptorValue,
 } = require("devtools/client/webconsole/utils/messages");
 loader.lazyGetter(this, "MODE", function() {
   return require("devtools/client/shared/components/reps/reps").MODE;
@@ -37,6 +38,7 @@ class ConsoleTable extends Component {
       parameters: PropTypes.array.isRequired,
       serviceContainer: PropTypes.object.isRequired,
       id: PropTypes.string.isRequired,
+      // Remove in Firefox 74. See Bug 1597955.
       tableData: PropTypes.object,
     };
   }
@@ -47,10 +49,13 @@ class ConsoleTable extends Component {
     this.getRows = this.getRows.bind(this);
   }
 
+  // Remove in Firefox 74. See Bug 1597955.
   componentWillMount() {
     const { id, dispatch, parameters, tableData } = this.props;
 
-    if (!Array.isArray(parameters) || parameters.length === 0 || tableData) {
+    const data = tableData || (parameters[0] && parameters[0].ownProperties);
+
+    if (!Array.isArray(parameters) || parameters.length === 0 || data) {
       return;
     }
 
@@ -110,20 +115,27 @@ class ConsoleTable extends Component {
 
   render() {
     const { parameters, tableData } = this.props;
-    const headersGrip = parameters[1];
+    const [valueGrip, headersGrip] = parameters;
     const headers =
       headersGrip && headersGrip.preview ? headersGrip.preview.items : null;
 
-    // if tableData is nullable, we don't show anything.
+    const data =
-    if (!tableData) {
+      valueGrip && valueGrip.ownProperties
+        ? valueGrip.ownProperties
+        : tableData;
+
+    // if we don't have any data, don't show anything.
+    if (!data) {
       return null;
     }
 
-    const { columns, items } = getTableItems(
+    const dataType = getParametersDataType(parameters);
-      tableData,
+
-      getParametersDataType(parameters),
+    // Remove deprecatedGetTableItems in Firefox 74. See Bug 1597955.
-      headers
+    const { columns, items } =
-    );
+      valueGrip && valueGrip.ownProperties
+        ? getTableItems(data, dataType, headers)
+        : deprecatedGetTableItems(data, dataType, headers);
 
     return dom.div(
       {
@@ -146,10 +158,11 @@ function getParametersDataType(parameters = null) {
   return parameters[0].class;
 }
 
-function getTableItems(data = {}, type, headers = null) {
+const INDEX_NAME = "_index";
-  const INDEX_NAME = "_index";
+const VALUE_NAME = "_value";
-  const VALUE_NAME = "_value";
+
-  const namedIndexes = {
+function getNamedIndexes(type) {
+  return {
     [INDEX_NAME]: getArrayTypeNames()
       .concat("Object")
       .includes(type)
@@ -158,6 +171,19 @@ function getTableItems(data = {}, type, headers = null) {
     [VALUE_NAME]: l10n.getStr("table.value"),
     key: l10n.getStr("table.key"),
   };
+}
+
+function hasValidCustomHeaders(headers) {
+  return (
+    Array.isArray(headers) &&
+    headers.every(
+      header => typeof header === "string" || Number.isInteger(Number(header))
+    )
+  );
+}
+
+function getTableItems(data = {}, type, headers = null) {
+  const namedIndexes = getNamedIndexes(type);
 
   let columns = new Map();
   const items = [];
@@ -167,11 +193,7 @@ function getTableItems(data = {}, type, headers = null) {
     Object.keys(item).forEach(key => addColumn(key));
   };
 
-  const hasValidCustomHeaders =
+  const validCustomHeaders = hasValidCustomHeaders(headers);
-    Array.isArray(headers) &&
-    headers.every(
-      header => typeof header === "string" || Number.isInteger(Number(header))
-    );
 
   const addColumn = function(columnIndex) {
     const columnExists = columns.has(columnIndex);
@@ -180,7 +202,99 @@ function getTableItems(data = {}, type, headers = null) {
     if (
       !columnExists &&
       !hasMaxColumns &&
-      (!hasValidCustomHeaders ||
+      (!validCustomHeaders ||
+        headers.includes(columnIndex) ||
+        columnIndex === INDEX_NAME)
+    ) {
+      columns.set(columnIndex, namedIndexes[columnIndex] || columnIndex);
+    }
+  };
+
+  for (let [index, property] of Object.entries(data)) {
+    if (type !== "Object" && index == parseInt(index, 10)) {
+      index = parseInt(index, 10);
+    }
+
+    const item = {
+      [INDEX_NAME]: index,
+    };
+
+    const propertyValue = getDescriptorValue(property);
+
+    if (propertyValue && propertyValue.ownProperties) {
+      const entries = propertyValue.ownProperties;
+      for (const [key, entry] of Object.entries(entries)) {
+        item[key] = getDescriptorValue(entry);
+      }
+    } else if (
+      propertyValue &&
+      propertyValue.preview &&
+      (type === "Map" || type === "WeakMap")
+    ) {
+      item.key = propertyValue.preview.key;
+      item[VALUE_NAME] = propertyValue.preview.value;
+    } else {
+      item[VALUE_NAME] = propertyValue;
+    }
+
+    addItem(item);
+
+    if (items.length === TABLE_ROW_MAX_ITEMS) {
+      break;
+    }
+  }
+
+  // Some headers might not be present in the items, so we make sure to
+  // return all the headers set by the user.
+  if (validCustomHeaders) {
+    headers.forEach(header => addColumn(header));
+  }
+
+  // We want to always have the index column first
+  if (columns.has(INDEX_NAME)) {
+    const index = columns.get(INDEX_NAME);
+    columns.delete(INDEX_NAME);
+    columns = new Map([[INDEX_NAME, index], ...columns.entries()]);
+  }
+
+  // We want to always have the values column last
+  if (columns.has(VALUE_NAME)) {
+    const index = columns.get(VALUE_NAME);
+    columns.delete(VALUE_NAME);
+    columns.set(VALUE_NAME, index);
+  }
+
+  return {
+    columns,
+    items,
+  };
+}
+
+/**
+ * This function is handling console.table packets from pre-Firefox72 servers.
+ * Remove in Firefox 74. See Bug 1597955.
+ */
+function deprecatedGetTableItems(data = {}, type, headers = null) {
+  const namedIndexes = getNamedIndexes(type);
+
+  let columns = new Map();
+  const items = [];
+
+  const addItem = function(item) {
+    items.push(item);
+    Object.keys(item).forEach(key => addColumn(key));
+  };
+
+  const validCustomHeaders = hasValidCustomHeaders(headers);
+
+  const addColumn = function(columnIndex) {
+    const columnExists = columns.has(columnIndex);
+    const hasMaxColumns = columns.size == TABLE_COLUMN_MAX_ITEMS;
+
+    if (
+      !columnExists &&
+      !hasMaxColumns &&
+      (!validCustomHeaders ||
         headers.includes(columnIndex) ||
         columnIndex === INDEX_NAME)
     ) {
@@ -234,7 +348,7 @@ function getTableItems(data = {}, type, headers = null) {
 
   // Some headers might not be present in the items, so we make sure to
   // return all the headers set by the user.
-  if (hasValidCustomHeaders) {
+  if (validCustomHeaders) {
     headers.forEach(header => addColumn(header));
   }
 

devtools/client/webconsole/reducers/messages.js

@@ -47,6 +47,12 @@ loader.lazyRequireGetter(
   "devtools/client/webconsole/utils/messages",
   true
 );
+loader.lazyRequireGetter(
+  this,
+  "getDescriptorValue",
+  "devtools/client/webconsole/utils/messages",
+  true
+);
 loader.lazyRequireGetter(
   this,
   "getParentWarningGroupMessageId",
@@ -1364,6 +1370,18 @@ function isTextInParameter(text, regex, parameter) {
     }
   }
 
+  if (parameter && parameter.ownProperties) {
+    for (const [key, desc] of Object.entries(parameter.ownProperties)) {
+      if (matchStr(key)) {
+        return true;
+      }
+
+      if (isTextInParameter(text, regex, getDescriptorValue(desc))) {
+        return true;
+      }
+    }
+  }
+
   return false;
 }
 

devtools/client/webconsole/test/browser/browser.ini

@@ -326,6 +326,7 @@ skip-if = true # Bug XXX # Bug 1405250
 [browser_webconsole_console_group.js]
 [browser_webconsole_console_logging_workers_api.js]
 skip-if = e10s # SharedWorkers console events are not received on the current process because they could run on any process.
+[browser_webconsole_console_table_post_alterations.js]
 [browser_webconsole_console_table.js]
 [browser_webconsole_console_timeStamp.js]
 [browser_webconsole_console_trace_distinct.js]

devtools/client/webconsole/test/browser/browser_webconsole_console_table.js

@@ -258,7 +258,7 @@ add_task(async function() {
       input: [{ a: { b: 34 } }],
       expected: {
         columns: ["(index)", "a"],
-        rows: [["0", "Object { … }"]],
+        rows: [["0", "Object { b: 34 }"]],
       },
       additionalTest: async function(node) {
         info("Check that object in a cell can be expanded");
@@ -312,7 +312,7 @@ async function testItem(testCase, node) {
   is(
     JSON.stringify(testCase.expected.columns),
     JSON.stringify(columns.map(column => column.textContent)),
-    "table has the expected columns"
+    `${testCase.info} | table has the expected columns`
   );
 
   // We don't really have rows since we are using a CSS grid in order to have a sticky
@@ -321,14 +321,18 @@ async function testItem(testCase, node) {
   is(
     testCase.expected.rows.length,
     cells.length / columnsNumber,
-    "table has the expected number of rows"
+    `${testCase.info} | table has the expected number of rows`
   );
 
   testCase.expected.rows.forEach((expectedRow, rowIndex) => {
     const startIndex = rowIndex * columnsNumber;
     // Slicing the cells array so we can get the current "row".
     const rowCells = cells.slice(startIndex, startIndex + columnsNumber);
-    is(rowCells.map(x => x.textContent).join(" | "), expectedRow.join(" | "));
+    is(
+      rowCells.map(x => x.textContent).join(" | "),
+      expectedRow.join(" | "),
+      `${testCase.info} | row has the expected content`
+    );
   });
 
   if (testCase.expected.overflow) {

devtools/client/webconsole/test/browser/browser_webconsole_console_table_post_alterations.js

@@ -0,0 +1,63 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+// Check that calling console.table on a variable which is modified after the
+// console.table call only shows data for when the variable was logged.
+
+const TEST_URI = `data:text/html,Test console.table with modified variable`;
+
+add_task(async function() {
+  const hud = await openNewTabAndConsole(TEST_URI);
+
+  await ContentTask.spawn(gBrowser.selectedBrowser, null, () => {
+    const x = ["a", "b"];
+    content.wrappedJSObject.console.table(x);
+    x.push("c");
+    content.wrappedJSObject.console.table(x);
+    x.sort((a, b) => b - a);
+    content.wrappedJSObject.console.table(x);
+  });
+
+  const [table1, table2, table3] = await waitFor(() => {
+    const res = hud.ui.outputNode.querySelectorAll(
+      ".message .new-consoletable"
+    );
+    if (res.length === 3) {
+      return res;
+    }
+    return null;
+  });
+
+  info("Check the rows of the first table");
+  checkTable(table1, [[0, "a"], [1, "b"]]);
+
+  info("Check the rows of the table after adding an element to the array");
+  checkTable(table2, [[0, "a"], [1, "b"], [2, "c"]]);
+
+  info("Check the rows of the table after sorting the array");
+  checkTable(table3, [[0, "c"], [1, "b"], [2, "a"]]);
+});
+
+function checkTable(node, expectedRows) {
+  const columns = Array.from(node.querySelectorAll("[role=columnheader]"));
+  const columnsNumber = columns.length;
+  const cells = Array.from(node.querySelectorAll("[role=gridcell]"));
+
+  // We don't really have rows since we are using a CSS grid in order to have a sticky
+  // header on the table. So we check the "rows" by dividing the number of cells by the
+  // number of columns.
+  is(
+    cells.length / columnsNumber,
+    expectedRows.length,
+    "table has the expected number of rows"
+  );
+
+  expectedRows.forEach((expectedRow, rowIndex) => {
+    const startIndex = rowIndex * columnsNumber;
+    // Slicing the cells array so we can get the current "row".
+    const rowCells = cells.slice(startIndex, startIndex + columnsNumber);
+    is(rowCells.map(x => x.textContent).join(" | "), expectedRow.join(" | "));
+  });
+}

devtools/client/webconsole/test/node/fixtures/stubs/consoleApi.js

@@ -458,14 +458,26 @@ stubPackets.set(`console.table(['red', 'green', 'blue']);`, {
         "extensible": true,
         "frozen": false,
         "sealed": false,
-        "preview": {
+        "preview": null,
-          "kind": "ArrayLike",
+        "ownProperties": {
-          "length": 3,
+          "0": {
-          "items": [
+            "configurable": true,
-            "red",
+            "enumerable": true,
-            "green",
+            "writable": true,
-            "blue"
+            "value": "red"
-          ]
+          },
+          "1": {
+            "configurable": true,
+            "enumerable": true,
+            "writable": true,
+            "value": "green"
+          },
+          "2": {
+            "configurable": true,
+            "enumerable": true,
+            "writable": true,
+            "value": "blue"
+          }
         }
       }
     ],
@@ -1150,14 +1162,26 @@ stubPackets.set(`console.table(['a', 'b', 'c'])`, {
         "extensible": true,
         "frozen": false,
         "sealed": false,
-        "preview": {
+        "preview": null,
-          "kind": "ArrayLike",
+        "ownProperties": {
-          "length": 3,
+          "0": {
-          "items": [
+            "configurable": true,
-            "a",
+            "enumerable": true,
-            "b",
+            "writable": true,
-            "c"
+            "value": "a"
-          ]
+          },
+          "1": {
+            "configurable": true,
+            "enumerable": true,
+            "writable": true,
+            "value": "b"
+          },
+          "2": {
+            "configurable": true,
+            "enumerable": true,
+            "writable": true,
+            "value": "c"
+          }
         }
       }
     ],

devtools/client/webconsole/utils/messages.js

@@ -689,9 +689,25 @@ function getArrayTypeNames() {
   ];
 }
 
+function getDescriptorValue(descriptor) {
+  if (!descriptor) {
+    return descriptor;
+  }
+
+  if (Object.prototype.hasOwnProperty.call(descriptor, "safeGetterValues")) {
+    return descriptor.safeGetterValues;
+  }
+
+  if (Object.prototype.hasOwnProperty.call(descriptor, "value")) {
+    return descriptor.value;
+  }
+  return descriptor;
+}
+
 module.exports = {
   createWarningGroupMessage,
   getArrayTypeNames,
+  getDescriptorValue,
   getInitialMessageCountForViewport,
   getParentWarningGroupMessageId,
   getWarningGroupType,

devtools/server/actors/webconsole.js

@@ -17,6 +17,7 @@ const { ObjectActor } = require("devtools/server/actors/object");
 const { LongStringActor } = require("devtools/server/actors/string");
 const {
   createValueGrip,
+  isArray,
   stringIsLong,
 } = require("devtools/server/actors/object/utils");
 const DevToolsUtils = require("devtools/shared/DevToolsUtils");
@@ -2020,7 +2021,6 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, {
     const result = WebConsoleUtils.cloneObject(message);
 
     result.workerType = WebConsoleUtils.getWorkerType(result) || "none";
-
     result.sourceId = this.getActorIdForInternalSourceId(result.sourceId);
 
     delete result.wrappedJSObject;
@@ -2046,12 +2046,94 @@ const WebConsoleActor = ActorClassWithSpec(webconsoleSpec, {
       return this.createValueGrip(string);
     });
 
+    if (result.level === "table") {
+      const tableItems = this._getConsoleTableMessageItems(result);
+      if (tableItems) {
+        result.arguments[0].ownProperties = tableItems;
+        result.arguments[0].preview = null;
+      }
+
+      // Only return the 2 first params.
+      result.arguments = result.arguments.slice(0, 2);
+    }
+
     result.category = message.category || "webdev";
     result.innerWindowID = message.innerID;
 
     return result;
   },
 
+  /**
+   * Return the properties needed to display the appropriate table for a given
+   * console.table call.
+   * This function does a little more than creating an ObjectActor for the first
+   * parameter of the message. When layout out the console table in the output, we want
+   * to be able to look into sub-properties so the table can have a different layout (
+   * for arrays of arrays, objects with objects properties, arrays of objects, …).
+   * So here we need to retrieve the properties of the first parameter, and also all the
+   * sub-properties we might need.
+   *
+   * @param {Object} result: The console.table message.
+   * @returns {Object} An object containing the properties of the first argument of the
+   *                   console.table call.
+   */
+  _getConsoleTableMessageItems: function(result) {
+    if (
+      !result ||
+      !Array.isArray(result.arguments) ||
+      result.arguments.length == 0
+    ) {
+      return null;
+    }
+
+    const [tableItemGrip] = result.arguments;
+    const dataType = tableItemGrip.class;
+    const needEntries = ["Map", "WeakMap", "Set", "WeakSet"].includes(dataType);
+    const ignoreNonIndexedProperties = isArray(tableItemGrip);
+
+    const tableItemActor = this.getActorByID(tableItemGrip.actor);
+    if (!tableItemActor) {
+      return null;
+    }
+
+    // Retrieve the properties (or entries for Set/Map) of the console table first arg.
+    const iterator = needEntries
+      ? tableItemActor.enumEntries()
+      : tableItemActor.enumProperties({
+          ignoreNonIndexedProperties,
+        });
+    const { ownProperties } = iterator.all();
+
+    // The iterator returns a descriptor for each property, wherein the value could be
+    // in one of those sub-property.
+    const descriptorKeys = ["safeGetterValues", "getterValue", "value"];
+
+    Object.values(ownProperties).forEach(desc => {
+      if (typeof desc !== "undefined") {
+        descriptorKeys.forEach(key => {
+          if (desc && desc.hasOwnProperty(key)) {
+            const grip = desc[key];
+
+            // We need to load sub-properties as well to render the table in a nice way.
+            const actor = grip && this.getActorByID(grip.actor);
+            if (actor) {
+              const res = actor
+                .enumProperties({
+                  ignoreNonIndexedProperties: isArray(grip),
+                })
+                .all();
+              if (res && res.ownProperties) {
+                desc[key].ownProperties = res.ownProperties;
+              }
+            }
+          }
+        });
+      }
+    });
+
+    return ownProperties;
+  },
+
   /**
    * Find the XUL window that owns the content window.
    *