Add support for accessible tables with missing/bad data.

Previously we relied on Blink to correctly fill in the
row and column index of every cell in a table. This
tends to be expensive to do in Blink because it
requires invalidating and reserializing the entire table
if any cell changes.

Instead, try to defer as much of those calculations to
downstream to be computed when AT actually requests
table attributes.

This is more efficient, because Blink accessibility code
needs to run any time the page is updated, like when a
table is being built dynamically using JS - whereas AT
typically only queries table attributes when users
explore a table explicitly, so it makes sense to defer
computation until then.

This is also more accurate and less error-prone, as it
allows us to completely validate the table in a single
pass and fix any errors in table indexes.

Finally, a potential added benefit is that this makes it
easier to handle tables from Views or Arc++ that
don't have all of the table attributes set, we can
compute them automatically.

A follow-up change will remove the code that computes
some of these table attributes from Blink.

Bug: 832289

Change-Id: I252a592c854e47cf6d6e5bd6b7c4bc5d51775e72
Reviewed-on: https://chromium-review.googlesource.com/c/1307840Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606940}

ui/accessibility/ax_node.cc

@@ -199,6 +199,22 @@ int32_t AXNode::GetTableRowCount() const {
   return table_info->row_count;
 }
 
+int32_t AXNode::GetTableAriaColCount() const {
+  AXTableInfo* table_info = tree_->GetTableInfo(this);
+  if (!table_info)
+    return 0;
+
+  return table_info->aria_col_count;
+}
+
+int32_t AXNode::GetTableAriaRowCount() const {
+  AXTableInfo* table_info = tree_->GetTableInfo(this);
+  if (!table_info)
+    return 0;
+
+  return table_info->aria_row_count;
+}
+
 int32_t AXNode::GetTableCellCount() const {
   AXTableInfo* table_info = tree_->GetTableInfo(this);
   if (!table_info)
@@ -319,17 +335,27 @@ int32_t AXNode::GetTableCellIndex() const {
 }
 
 int32_t AXNode::GetTableCellColIndex() const {
-  // TODO(dmazzoni): Compute from AXTableInfo. http://crbug.com/832289
-  int32_t col_index = 0;
-  GetIntAttribute(ax::mojom::IntAttribute::kTableCellColumnIndex, &col_index);
-  return col_index;
+  AXTableInfo* table_info = GetAncestorTableInfo();
+  if (!table_info)
+    return 0;
+
+  int32_t index = GetTableCellIndex();
+  if (index == -1)
+    return 0;
+
+  return table_info->cell_data_vector[index].col_index;
 }
 
 int32_t AXNode::GetTableCellRowIndex() const {
-  // TODO(dmazzoni): Compute from AXTableInfo. http://crbug.com/832289
-  int32_t row_index = 0;
-  GetIntAttribute(ax::mojom::IntAttribute::kTableCellRowIndex, &row_index);
-  return row_index;
+  AXTableInfo* table_info = GetAncestorTableInfo();
+  if (!table_info)
+    return 0;
+
+  int32_t index = GetTableCellIndex();
+  if (index == -1)
+    return 0;
+
+  return table_info->cell_data_vector[index].row_index;
 }
 
 int32_t AXNode::GetTableCellColSpan() const {
@@ -360,15 +386,27 @@ int32_t AXNode::GetTableCellRowSpan() const {
 }
 
 int32_t AXNode::GetTableCellAriaColIndex() const {
-  int32_t col_index = 0;
-  GetIntAttribute(ax::mojom::IntAttribute::kAriaCellColumnIndex, &col_index);
-  return col_index;
+  AXTableInfo* table_info = GetAncestorTableInfo();
+  if (!table_info)
+    return -0;
+
+  int32_t index = GetTableCellIndex();
+  if (index == -1)
+    return 0;
+
+  return table_info->cell_data_vector[index].aria_col_index;
 }
 
 int32_t AXNode::GetTableCellAriaRowIndex() const {
-  int32_t row_index = 0;
-  GetIntAttribute(ax::mojom::IntAttribute::kAriaCellRowIndex, &row_index);
-  return row_index;
+  AXTableInfo* table_info = GetAncestorTableInfo();
+  if (!table_info)
+    return -0;
+
+  int32_t index = GetTableCellIndex();
+  if (index == -1)
+    return 0;
+
+  return table_info->cell_data_vector[index].aria_row_index;
 }
 
 void AXNode::GetTableCellColHeaderNodeIds(

ui/accessibility/ax_node.h

@@ -191,14 +191,22 @@ class AX_EXPORT AXNode final {
   // Most of these functions construct and cache an AXTableInfo behind
   // the scenes to infer many properties of tables.
   //
-  // TODO(dmazzoni): Make these const - not trivial because AXTableInfo
-  // does need to modify the AXTree.
+  // These interfaces use attributes provided by the source of the
+  // AX tree where possible, but fills in missing details and ignores
+  // specified attributes when they're bad or inconsistent. That way
+  // you're guaranteed to get a valid, consistent table when using these
+  // interfaces.
   //
 
-  // Table-like nodes (including grids).
+  // Table-like nodes (including grids). All indices are 0-based except
+  // ARIA indices are all 1-based. In other words, the top-left corner
+  // of the table is row 0, column 0, cell index 0 - but that same cell
+  // has a minimum ARIA row index of 1 and column index of 1.
   bool IsTable() const;
   int32_t GetTableColCount() const;
   int32_t GetTableRowCount() const;
+  int32_t GetTableAriaColCount() const;
+  int32_t GetTableAriaRowCount() const;
   int32_t GetTableCellCount() const;
   AXNode* GetTableCellFromIndex(int32_t index) const;
   AXNode* GetTableCellFromCoords(int32_t row_index, int32_t col_index) const;

ui/accessibility/ax_table_info.cc

@@ -9,27 +9,50 @@
 #include "ui/accessibility/ax_tree.h"
 #include "ui/gfx/geometry/rect_f.h"
 
+using ax::mojom::IntAttribute;
+
 namespace ui {
 
 namespace {
 
-void FindCells(AXNode* node, std::vector<AXNode*>* cells) {
+// Given a node representing a table row, search its children
+// recursively to find any cells or table headers, and append
+// them to |cells|.
+//
+// We recursively check generic containers like <div> and any
+// nodes that are ignored, but we don't search any other roles
+// in-between a table row and its cells.
+void FindCellsInRow(AXNode* node, std::vector<AXNode*>* cell_nodes) {
   for (AXNode* child : node->children()) {
     if (child->data().HasState(ax::mojom::State::kIgnored) ||
         child->data().role == ax::mojom::Role::kGenericContainer)
-      FindCells(child, cells);
+      FindCellsInRow(child, cell_nodes);
     else if (IsCellOrTableHeader(child->data().role))
-      cells->push_back(child);
+      cell_nodes->push_back(child);
   }
 }
 
-void FindRowsAndThenCells(AXNode* node, std::vector<AXNode*>* cells) {
+// Given a node representing a table/grid, search its children
+// recursively to find any rows and append them to |row_nodes|, then
+// for each row find its cells and add them to |cell_nodes_per_row| as a
+// 2-dimensional array.
+//
+// We recursively check generic containers like <div> and any
+// nodes that are ignored, but we don't search any other roles
+// in-between a table and its rows.
+void FindRowsAndThenCells(
+    AXNode* node,
+    std::vector<AXNode*>* row_nodes,
+    std::vector<std::vector<AXNode*>>* cell_nodes_per_row) {
   for (AXNode* child : node->children()) {
     if (child->data().HasState(ax::mojom::State::kIgnored) ||
-        child->data().role == ax::mojom::Role::kGenericContainer)
-      FindRowsAndThenCells(child, cells);
-    else if (child->data().role == ax::mojom::Role::kRow)
-      FindCells(child, cells);
+        child->data().role == ax::mojom::Role::kGenericContainer) {
+      FindRowsAndThenCells(child, row_nodes, cell_nodes_per_row);
+    } else if (child->data().role == ax::mojom::Role::kRow) {
+      row_nodes->push_back(child);
+      cell_nodes_per_row->push_back(std::vector<AXNode*>());
+      FindCellsInRow(child, &cell_nodes_per_row->back());
+    }
   }
 }
 
@@ -59,38 +82,176 @@ AXTableInfo* AXTableInfo::Create(AXTree* tree, AXNode* table_node) {
 }
 
 bool AXTableInfo::Update() {
-  if (!IsTableLike(table_node_->data().role))
+  const AXNodeData& node_data = table_node_->data();
+  if (!IsTableLike(node_data.role))
     return false;
 
+  ClearVectors();
+
+  std::vector<AXNode*> row_nodes;
+  std::vector<std::vector<AXNode*>> cell_nodes_per_row;
+  FindRowsAndThenCells(table_node_, &row_nodes, &cell_nodes_per_row);
+  DCHECK_EQ(cell_nodes_per_row.size(), row_nodes.size());
+
+  // Get the optional row and column count from the table. If we encounter
+  // a cell with an index or span larger than this, we'll update the
+  // table row and column count to be large enough to fit all cells.
+  row_count =
+      std::max(0, node_data.GetIntAttribute(IntAttribute::kTableRowCount));
+  col_count =
+      std::max(0, node_data.GetIntAttribute(IntAttribute::kTableColumnCount));
+  aria_row_count =
+      std::max(0, node_data.GetIntAttribute(IntAttribute::kAriaRowCount));
+  aria_col_count =
+      std::max(0, node_data.GetIntAttribute(IntAttribute::kAriaColumnCount));
+
+  // Iterate over the cells and build up an array of CellData
+  // entries, one for each cell. Compute the actual row and column
+  BuildCellDataVectorFromRowAndCellNodes(row_nodes, cell_nodes_per_row);
+
+  // At this point we have computed valid row and column indices for
+  // every cell in the table, and an accurate row and column count for the
+  // whole table that fits every cell and its spans. The final step is to
+  // fill in a 2-dimensional array that lets us look up an individual cell
+  // by its (row, column) coordinates, plus arrays to hold row and column
+  // headers.
+  BuildCellAndHeaderVectorsFromCellData();
+
+  // On Mac, we add a few extra nodes to the table - see comment
+  // at the top of UpdateExtraMacNodes for details.
+  if (tree_->enable_extra_mac_nodes())
+    UpdateExtraMacNodes();
+
+  // The table metadata is now valid, any table queries will now be
+  // fast. Any time a node in the table is updated, we'll have to
+  // recompute all of this.
+  valid_ = true;
+  return true;
+}
+
+void AXTableInfo::Invalidate() {
+  valid_ = false;
+}
+
+void AXTableInfo::ClearVectors() {
   col_headers.clear();
   row_headers.clear();
   all_headers.clear();
   cell_ids.clear();
   unique_cell_ids.clear();
+  cell_data_vector.clear();
+}
+
+void AXTableInfo::BuildCellDataVectorFromRowAndCellNodes(
+    const std::vector<AXNode*>& row_nodes,
+    const std::vector<std::vector<AXNode*>>& cell_nodes_per_row) {
+  // Iterate over the cells and build up an array of CellData
+  // entries, one for each cell. Compute the actual row and column
+  // indices for each cell by taking the specified row and column
+  // index in the accessibility tree if legal, but replacing it with
+  // valid table coordinates otherwise.
+  int32_t cell_index = 0;
+  int32_t current_row_index = 0;
+  int32_t current_aria_row_index = 1;
+  for (size_t i = 0; i < cell_nodes_per_row.size(); i++) {
+    auto& cell_nodes_in_this_row = cell_nodes_per_row[i];
+    AXNode* row_node = row_nodes[i];
+    bool is_first_cell_in_row = true;
+    int32_t current_col_index = 0;
+    int32_t current_aria_col_index = 1;
+    for (AXNode* cell : cell_nodes_in_this_row) {
+      // Fill in basic info in CellData.
+      CellData cell_data;
+      unique_cell_ids.push_back(cell->id());
+      cell_id_to_index[cell->id()] = cell_index++;
+      cell_data.cell = cell;
+
+      // Get table cell accessibility attributes - note that these may
+      // be missing or invalid, we'll correct them next.
+      const AXNodeData& node_data = cell->data();
+      cell_data.row_index =
+          node_data.GetIntAttribute(IntAttribute::kTableCellRowIndex);
+      cell_data.row_span =
+          node_data.GetIntAttribute(IntAttribute::kTableCellRowSpan);
+      cell_data.aria_row_index =
+          node_data.GetIntAttribute(IntAttribute::kAriaCellRowIndex);
+      cell_data.col_index =
+          node_data.GetIntAttribute(IntAttribute::kTableCellColumnIndex);
+      cell_data.aria_col_index =
+          node_data.GetIntAttribute(IntAttribute::kAriaCellColumnIndex);
+      cell_data.col_span =
+          node_data.GetIntAttribute(IntAttribute::kTableCellColumnSpan);
+
+      // The col span and row span must be at least 1.
+      cell_data.row_span = std::max(1, cell_data.row_span);
+      cell_data.col_span = std::max(1, cell_data.col_span);
+
+      // Ensure the column index must always be incrementing.
+      cell_data.col_index = std::max(cell_data.col_index, current_col_index);
+
+      if (is_first_cell_in_row) {
+        is_first_cell_in_row = false;
+
+        // If it's the first cell in the row, ensure the row index is
+        // incrementing. The rest of the cells in this row will be force to
+        // have the same row index.
+        cell_data.row_index = std::max(cell_data.row_index, current_row_index);
+        current_row_index = cell_data.row_index;
+
+        // The starting ARIA row and column index might be specified in
+        // the row node, we should check there.
+        if (!cell_data.aria_row_index) {
+          cell_data.aria_row_index =
+              row_node->data().GetIntAttribute(IntAttribute::kAriaCellRowIndex);
+        }
+        if (!cell_data.aria_col_index) {
+          cell_data.aria_col_index = row_node->data().GetIntAttribute(
+              IntAttribute::kAriaCellColumnIndex);
+        }
+        cell_data.aria_row_index =
+            std::max(cell_data.aria_row_index, current_aria_row_index);
+        current_aria_row_index = cell_data.aria_row_index;
+      } else {
+        // Don't allow the row index to change after the beginning
+        // of a row.
+        cell_data.row_index = current_row_index;
+        cell_data.aria_row_index = current_aria_row_index;
+      }
 
-  std::vector<AXNode*> cells;
-  FindRowsAndThenCells(table_node_, &cells);
-
-  // Compute the actual row and column count, and the set of all unique cell ids
-  // in the table.
-  row_count = table_node_->data().GetIntAttribute(
-      ax::mojom::IntAttribute::kTableRowCount);
-  col_count = table_node_->data().GetIntAttribute(
-      ax::mojom::IntAttribute::kTableColumnCount);
-  for (AXNode* cell : cells) {
-    int row_index = cell->data().GetIntAttribute(
-        ax::mojom::IntAttribute::kTableCellRowIndex);
-    int row_span = std::max(1, cell->data().GetIntAttribute(
-                                   ax::mojom::IntAttribute::kTableCellRowSpan));
-    row_count = std::max(row_count, row_index + row_span);
-    int col_index = cell->data().GetIntAttribute(
-        ax::mojom::IntAttribute::kTableCellColumnIndex);
-    int col_span =
-        std::max(1, cell->data().GetIntAttribute(
-                        ax::mojom::IntAttribute::kTableCellColumnSpan));
-    col_count = std::max(col_count, col_index + col_span);
+      // Ensure the ARIA col index is incrementing.
+      cell_data.aria_col_index =
+          std::max(cell_data.aria_col_index, current_aria_col_index);
+
+      // Update the row count and col count for the whole table to make
+      // sure they're large enough to fit this cell, including its spans.
+      // The -1 in the ARIA calcluations is because ARIA indices are 1-based,
+      // whereas all other indices are zero-based.
+      row_count = std::max(row_count, cell_data.row_index + cell_data.row_span);
+      col_count = std::max(col_count, cell_data.col_index + cell_data.col_span);
+      aria_row_count = std::max(
+          aria_row_count, current_aria_row_index + cell_data.row_span - 1);
+      aria_col_count = std::max(
+          aria_col_count, current_aria_col_index + cell_data.col_span - 1);
+
+      // Update |current_col_index| to reflect the next available index after
+      // this cell including its colspan. The next column index in this row
+      // must be at least this large. Same for the current ARIA col index.
+      current_col_index = cell_data.col_index + cell_data.col_span;
+      current_aria_col_index = cell_data.aria_col_index + cell_data.col_span;
+
+      // Add this cell to our vector.
+      cell_data_vector.push_back(cell_data);
+    }
+
+    // At the end of each row, increment |current_row_index| to reflect the next
+    // available index after this row. The next row index must be at least this
+    // large. Same for the current ARIA row index.
+    current_row_index++;
+    current_aria_row_index++;
   }
+}
 
+void AXTableInfo::BuildCellAndHeaderVectorsFromCellData() {
   // Allocate space for the 2-D array of cell IDs and 1-D
   // arrays of row headers and column headers.
   row_headers.resize(row_count);
@@ -99,31 +260,24 @@ bool AXTableInfo::Update() {
   for (auto& row : cell_ids)
     row.resize(col_count);
 
-  // Now iterate over the cells and fill in the cell IDs, row headers,
-  // and column headers based on the index and span of each cell.
-  int32_t cell_index = 0;
-  for (AXNode* cell : cells) {
-    unique_cell_ids.push_back(cell->id());
-    cell_id_to_index[cell->id()] = cell_index++;
-    int row_index = cell->data().GetIntAttribute(
-        ax::mojom::IntAttribute::kTableCellRowIndex);
-    int row_span = std::max(1, cell->data().GetIntAttribute(
-                                   ax::mojom::IntAttribute::kTableCellRowSpan));
-    int col_index = cell->data().GetIntAttribute(
-        ax::mojom::IntAttribute::kTableCellColumnIndex);
-    int col_span =
-        std::max(1, cell->data().GetIntAttribute(
-                        ax::mojom::IntAttribute::kTableCellColumnSpan));
-
-    // Cells must contain a 0-based row index and col index.
-    if (row_index < 0 || col_index < 0)
-      continue;
-
-    for (int r = row_index; r < row_index + row_span; r++) {
+  // Fill in the arrays.
+  //
+  // At this point we have computed valid row and column indices for
+  // every cell in the table, and an accurate row and column count for the
+  // whole table that fits every cell and its spans. The final step is to
+  // fill in a 2-dimensional array that lets us look up an individual cell
+  // by its (row, column) coordinates, plus arrays to hold row and column
+  // headers.
+  for (auto& cell_data : cell_data_vector) {
+    for (int r = cell_data.row_index;
+         r < cell_data.row_index + cell_data.row_span; r++) {
       DCHECK_LT(r, row_count);
-      for (int c = col_index; c < col_index + col_span; c++) {
+      for (int c = cell_data.col_index;
+           c < cell_data.col_index + cell_data.col_span; c++) {
         DCHECK_LT(c, col_count);
+        AXNode* cell = cell_data.cell;
         cell_ids[r][c] = cell->id();
+
         if (cell->data().role == ax::mojom::Role::kColumnHeader) {
           col_headers[c].push_back(cell->id());
           all_headers.push_back(cell->id());
@@ -134,16 +288,6 @@ bool AXTableInfo::Update() {
       }
     }
   }
-
-  if (tree_->enable_extra_mac_nodes())
-    UpdateExtraMacNodes();
-
-  valid_ = true;
-  return true;
-}
-
-void AXTableInfo::Invalidate() {
-  valid_ = false;
 }
 
 void AXTableInfo::UpdateExtraMacNodes() {
@@ -228,11 +372,11 @@ void AXTableInfo::UpdateExtraMacColumnNodeAttributes(int col_index) {
   data.int_attributes.clear();
 
   // Update the column index.
-  data.AddIntAttribute(ax::mojom::IntAttribute::kTableColumnIndex, col_index);
+  data.AddIntAttribute(IntAttribute::kTableColumnIndex, col_index);
 
   // Update the column header.
   if (!col_headers[col_index].empty()) {
-    data.AddIntAttribute(ax::mojom::IntAttribute::kTableColumnHeaderId,
+    data.AddIntAttribute(IntAttribute::kTableColumnHeaderId,
                          col_headers[col_index][0]);
   }
 

ui/accessibility/ax_table_info.h

@@ -19,6 +19,17 @@ class AXNode;
 // This helper class computes info about tables and grids in AXTrees.
 class AX_EXPORT AXTableInfo {
  public:
+  struct CellData {
+    AXNode* cell;
+    int32_t cell_id;
+    int32_t col_index;
+    int32_t row_index;
+    int32_t col_span;
+    int32_t row_span;
+    int32_t aria_col_index;
+    int32_t aria_row_index;
+  };
+
   // Returns nullptr if the node is not a valid table or grid node.
   static AXTableInfo* Create(AXTree* tree, AXNode* table_node);
 
@@ -57,6 +68,9 @@ class AX_EXPORT AXTableInfo {
   // really is missing from the table.
   std::vector<std::vector<int32_t>> cell_ids;
 
+  // Array of cell data for every unique cell in the table.
+  std::vector<CellData> cell_data_vector;
+
   // Set of all unique cell node IDs in the table.
   std::vector<int32_t> unique_cell_ids;
 
@@ -68,9 +82,19 @@ class AX_EXPORT AXTableInfo {
   // Map from each cell's node ID to its index in unique_cell_ids.
   base::hash_map<int32_t, int32_t> cell_id_to_index;
 
+  // The ARIA row count and column count, if any ARIA table or grid
+  // attributes are used in the table at all.
+  int32_t aria_row_count = 0;
+  int32_t aria_col_count = 0;
+
  private:
   AXTableInfo(AXTree* tree, AXNode* table_node);
 
+  void ClearVectors();
+  void BuildCellDataVectorFromRowAndCellNodes(
+      const std::vector<AXNode*>& row_nodes,
+      const std::vector<std::vector<AXNode*>>& cell_nodes_per_row);
+  void BuildCellAndHeaderVectorsFromCellData();
   void UpdateExtraMacNodes();
   void ClearExtraMacNodes();
   AXNode* CreateExtraMacColumnNode(int col_index);

ui/accessibility/ax_table_info_unittest.cc

@@ -502,4 +502,322 @@ TEST_F(AXTableInfoTest, ExtraMacNodes) {
   EXPECT_EQ(5, indirect_child_ids[1]);
 }
 
+TEST_F(AXTableInfoTest, TableWithNoIndices) {
+  AXTreeUpdate initial_state;
+  initial_state.root_id = 1;
+  initial_state.nodes.resize(7);
+  initial_state.nodes[0].id = 1;
+  initial_state.nodes[0].role = ax::mojom::Role::kTable;
+  initial_state.nodes[0].child_ids = {2, 3};
+  initial_state.nodes[1].id = 2;
+  initial_state.nodes[1].role = ax::mojom::Role::kRow;
+  initial_state.nodes[1].child_ids = {4, 5};
+  initial_state.nodes[2].id = 3;
+  initial_state.nodes[2].role = ax::mojom::Role::kRow;
+  initial_state.nodes[2].child_ids = {6, 7};
+  initial_state.nodes[3].id = 4;
+  initial_state.nodes[3].role = ax::mojom::Role::kColumnHeader;
+  initial_state.nodes[4].id = 5;
+  initial_state.nodes[4].role = ax::mojom::Role::kColumnHeader;
+  initial_state.nodes[5].id = 6;
+  initial_state.nodes[5].role = ax::mojom::Role::kCell;
+  initial_state.nodes[6].id = 7;
+  initial_state.nodes[6].role = ax::mojom::Role::kCell;
+
+  AXTree tree(initial_state);
+  AXNode* table = tree.root();
+
+  EXPECT_TRUE(table->IsTable());
+  EXPECT_FALSE(table->IsTableRow());
+  EXPECT_FALSE(table->IsTableCellOrHeader());
+  EXPECT_EQ(2, table->GetTableColCount());
+  EXPECT_EQ(2, table->GetTableRowCount());
+
+  EXPECT_EQ(4, table->GetTableCellFromCoords(0, 0)->id());
+  EXPECT_EQ(5, table->GetTableCellFromCoords(0, 1)->id());
+  EXPECT_EQ(6, table->GetTableCellFromCoords(1, 0)->id());
+  EXPECT_EQ(7, table->GetTableCellFromCoords(1, 1)->id());
+  EXPECT_EQ(nullptr, table->GetTableCellFromCoords(2, 1));
+  EXPECT_EQ(nullptr, table->GetTableCellFromCoords(1, -1));
+
+  EXPECT_EQ(4, table->GetTableCellFromIndex(0)->id());
+  EXPECT_EQ(5, table->GetTableCellFromIndex(1)->id());
+  EXPECT_EQ(6, table->GetTableCellFromIndex(2)->id());
+  EXPECT_EQ(7, table->GetTableCellFromIndex(3)->id());
+  EXPECT_EQ(nullptr, table->GetTableCellFromIndex(-1));
+  EXPECT_EQ(nullptr, table->GetTableCellFromIndex(4));
+
+  AXNode* cell_0_0 = tree.GetFromId(4);
+  EXPECT_EQ(0, cell_0_0->GetTableCellRowIndex());
+  EXPECT_EQ(0, cell_0_0->GetTableCellColIndex());
+  AXNode* cell_0_1 = tree.GetFromId(5);
+  EXPECT_EQ(0, cell_0_1->GetTableCellRowIndex());
+  EXPECT_EQ(1, cell_0_1->GetTableCellColIndex());
+  AXNode* cell_1_0 = tree.GetFromId(6);
+  EXPECT_EQ(1, cell_1_0->GetTableCellRowIndex());
+  EXPECT_EQ(0, cell_1_0->GetTableCellColIndex());
+  AXNode* cell_1_1 = tree.GetFromId(7);
+  EXPECT_EQ(1, cell_1_1->GetTableCellRowIndex());
+  EXPECT_EQ(1, cell_1_1->GetTableCellColIndex());
+}
+
+TEST_F(AXTableInfoTest, TableWithPartialIndices) {
+  // Start with a table with no indices.
+  AXTreeUpdate initial_state;
+  initial_state.root_id = 1;
+  initial_state.nodes.resize(7);
+  initial_state.nodes[0].id = 1;
+  initial_state.nodes[0].role = ax::mojom::Role::kTable;
+  initial_state.nodes[0].child_ids = {2, 3};
+  initial_state.nodes[1].id = 2;
+  initial_state.nodes[1].role = ax::mojom::Role::kRow;
+  initial_state.nodes[1].child_ids = {4, 5};
+  initial_state.nodes[2].id = 3;
+  initial_state.nodes[2].role = ax::mojom::Role::kRow;
+  initial_state.nodes[2].child_ids = {6, 7};
+  initial_state.nodes[3].id = 4;
+  initial_state.nodes[3].role = ax::mojom::Role::kColumnHeader;
+  initial_state.nodes[4].id = 5;
+  initial_state.nodes[4].role = ax::mojom::Role::kColumnHeader;
+  initial_state.nodes[5].id = 6;
+  initial_state.nodes[5].role = ax::mojom::Role::kCell;
+  initial_state.nodes[6].id = 7;
+  initial_state.nodes[6].role = ax::mojom::Role::kCell;
+
+  AXTree tree(initial_state);
+  AXNode* table = tree.root();
+
+  EXPECT_EQ(2, table->GetTableColCount());
+  EXPECT_EQ(2, table->GetTableRowCount());
+
+  AXNode* cell_0_0 = tree.GetFromId(4);
+  EXPECT_EQ(0, cell_0_0->GetTableCellRowIndex());
+  EXPECT_EQ(0, cell_0_0->GetTableCellColIndex());
+  AXNode* cell_0_1 = tree.GetFromId(5);
+  EXPECT_EQ(0, cell_0_1->GetTableCellRowIndex());
+  EXPECT_EQ(1, cell_0_1->GetTableCellColIndex());
+  AXNode* cell_1_0 = tree.GetFromId(6);
+  EXPECT_EQ(1, cell_1_0->GetTableCellRowIndex());
+  EXPECT_EQ(0, cell_1_0->GetTableCellColIndex());
+  AXNode* cell_1_1 = tree.GetFromId(7);
+  EXPECT_EQ(1, cell_1_1->GetTableCellRowIndex());
+  EXPECT_EQ(1, cell_1_1->GetTableCellColIndex());
+
+  AXTreeUpdate update = initial_state;
+  update.nodes[0].AddIntAttribute(ax::mojom::IntAttribute::kTableColumnCount,
+                                  5);
+  update.nodes[0].AddIntAttribute(ax::mojom::IntAttribute::kTableRowCount, 2);
+  update.nodes[5].AddIntAttribute(ax::mojom::IntAttribute::kTableCellRowIndex,
+                                  2);
+  update.nodes[5].AddIntAttribute(
+      ax::mojom::IntAttribute::kTableCellColumnIndex, 0);
+  update.nodes[6].AddIntAttribute(ax::mojom::IntAttribute::kTableCellRowIndex,
+                                  2);
+  update.nodes[6].AddIntAttribute(
+      ax::mojom::IntAttribute::kTableCellColumnIndex, 2);
+  EXPECT_TRUE(tree.Unserialize(update));
+
+  // The largest column index in the table is 2, but the
+  // table claims it has a column count of 5. That's allowed.
+  EXPECT_EQ(5, table->GetTableColCount());
+
+  // While the table claims it has a row count of 2, the
+  // last row has an index of 2, so the correct row count is 3.
+  EXPECT_EQ(3, table->GetTableRowCount());
+
+  // All of the specified row and cell indices are legal
+  // so they're respected.
+  EXPECT_EQ(0, cell_0_0->GetTableCellRowIndex());
+  EXPECT_EQ(0, cell_0_0->GetTableCellColIndex());
+  EXPECT_EQ(0, cell_0_1->GetTableCellRowIndex());
+  EXPECT_EQ(1, cell_0_1->GetTableCellColIndex());
+  EXPECT_EQ(2, cell_1_0->GetTableCellRowIndex());
+  EXPECT_EQ(0, cell_1_0->GetTableCellColIndex());
+  EXPECT_EQ(2, cell_1_1->GetTableCellRowIndex());
+  EXPECT_EQ(2, cell_1_1->GetTableCellColIndex());
+
+  // Fetching cells by coordinates works.
+  EXPECT_EQ(4, table->GetTableCellFromCoords(0, 0)->id());
+  EXPECT_EQ(5, table->GetTableCellFromCoords(0, 1)->id());
+  EXPECT_EQ(6, table->GetTableCellFromCoords(2, 0)->id());
+  EXPECT_EQ(7, table->GetTableCellFromCoords(2, 2)->id());
+  EXPECT_EQ(nullptr, table->GetTableCellFromCoords(0, 2));
+  EXPECT_EQ(nullptr, table->GetTableCellFromCoords(1, 0));
+  EXPECT_EQ(nullptr, table->GetTableCellFromCoords(1, 1));
+  EXPECT_EQ(nullptr, table->GetTableCellFromCoords(2, 1));
+}
+
+TEST_F(AXTableInfoTest, BadRowIndicesIgnored) {
+  // The table claims it has two rows and two columns, but
+  // the cell indices for both the first and second rows
+  // are for row 2 (zero-based).
+  //
+  // The cell indexes for the first row should be
+  // respected, and for the second row it will get the
+  // next row index.
+  AXTreeUpdate initial_state;
+  initial_state.root_id = 1;
+  initial_state.nodes.resize(7);
+  MakeTable(&initial_state.nodes[0], 1, 2, 2);
+  initial_state.nodes[0].child_ids = {2, 3};
+  MakeRow(&initial_state.nodes[1], 2, 0);
+  initial_state.nodes[1].child_ids = {4, 5};
+  MakeRow(&initial_state.nodes[2], 3, 0);
+  initial_state.nodes[2].child_ids = {6, 7};
+  MakeCell(&initial_state.nodes[3], 4, 2, 0);
+  MakeCell(&initial_state.nodes[4], 5, 2, 1);
+  MakeCell(&initial_state.nodes[5], 6, 2, 0);
+  MakeCell(&initial_state.nodes[6], 7, 2, 1);
+  AXTree tree(initial_state);
+  AXNode* table = tree.root();
+
+  EXPECT_EQ(2, table->GetTableColCount());
+  EXPECT_EQ(4, table->GetTableRowCount());
+
+  AXNode* cell_id_4 = tree.GetFromId(4);
+  EXPECT_EQ(2, cell_id_4->GetTableCellRowIndex());
+  EXPECT_EQ(0, cell_id_4->GetTableCellColIndex());
+  AXNode* cell_id_5 = tree.GetFromId(5);
+  EXPECT_EQ(2, cell_id_5->GetTableCellRowIndex());
+  EXPECT_EQ(1, cell_id_5->GetTableCellColIndex());
+  AXNode* cell_id_6 = tree.GetFromId(6);
+  EXPECT_EQ(3, cell_id_6->GetTableCellRowIndex());
+  EXPECT_EQ(0, cell_id_6->GetTableCellColIndex());
+  AXNode* cell_id_7 = tree.GetFromId(7);
+  EXPECT_EQ(3, cell_id_7->GetTableCellRowIndex());
+  EXPECT_EQ(1, cell_id_7->GetTableCellColIndex());
+}
+
+TEST_F(AXTableInfoTest, BadColIndicesIgnored) {
+  // The table claims it has two rows and two columns, but
+  // the cell indices for the columns either repeat or
+  // go backwards.
+  AXTreeUpdate initial_state;
+  initial_state.root_id = 1;
+  initial_state.nodes.resize(7);
+  MakeTable(&initial_state.nodes[0], 1, 2, 2);
+  initial_state.nodes[0].child_ids = {2, 3};
+  MakeRow(&initial_state.nodes[1], 2, 0);
+  initial_state.nodes[1].child_ids = {4, 5};
+  MakeRow(&initial_state.nodes[2], 3, 0);
+  initial_state.nodes[2].child_ids = {6, 7};
+  MakeCell(&initial_state.nodes[3], 4, 0, 1);
+  MakeCell(&initial_state.nodes[4], 5, 0, 1);
+  MakeCell(&initial_state.nodes[5], 6, 1, 2);
+  MakeCell(&initial_state.nodes[6], 7, 1, 1);
+  AXTree tree(initial_state);
+  AXNode* table = tree.root();
+
+  EXPECT_EQ(4, table->GetTableColCount());
+  EXPECT_EQ(2, table->GetTableRowCount());
+
+  AXNode* cell_id_4 = tree.GetFromId(4);
+  EXPECT_EQ(0, cell_id_4->GetTableCellRowIndex());
+  EXPECT_EQ(1, cell_id_4->GetTableCellColIndex());
+  AXNode* cell_id_5 = tree.GetFromId(5);
+  EXPECT_EQ(0, cell_id_5->GetTableCellRowIndex());
+  EXPECT_EQ(2, cell_id_5->GetTableCellColIndex());
+  AXNode* cell_id_6 = tree.GetFromId(6);
+  EXPECT_EQ(1, cell_id_6->GetTableCellRowIndex());
+  EXPECT_EQ(2, cell_id_6->GetTableCellColIndex());
+  AXNode* cell_id_7 = tree.GetFromId(7);
+  EXPECT_EQ(1, cell_id_7->GetTableCellRowIndex());
+  EXPECT_EQ(3, cell_id_7->GetTableCellColIndex());
+}
+
+TEST_F(AXTableInfoTest, AriaIndicesinferred) {
+  // Note that ARIA indices are 1-based, whereas the rest of
+  // the table indices are zero-based.
+  AXTreeUpdate initial_state;
+  initial_state.root_id = 1;
+  initial_state.nodes.resize(13);
+  MakeTable(&initial_state.nodes[0], 1, 3, 3);
+  initial_state.nodes[0].AddIntAttribute(ax::mojom::IntAttribute::kAriaRowCount,
+                                         5);
+  initial_state.nodes[0].AddIntAttribute(
+      ax::mojom::IntAttribute::kAriaColumnCount, 5);
+  initial_state.nodes[0].child_ids = {2, 3, 4};
+  MakeRow(&initial_state.nodes[1], 2, 0);
+  initial_state.nodes[1].child_ids = {5, 6, 7};
+  MakeRow(&initial_state.nodes[2], 3, 1);
+  initial_state.nodes[2].AddIntAttribute(
+      ax::mojom::IntAttribute::kAriaCellRowIndex, 4);
+  initial_state.nodes[2].child_ids = {8, 9, 10};
+  MakeRow(&initial_state.nodes[3], 4, 2);
+  initial_state.nodes[3].AddIntAttribute(
+      ax::mojom::IntAttribute::kAriaCellRowIndex, 4);
+  initial_state.nodes[3].child_ids = {11, 12, 13};
+  MakeCell(&initial_state.nodes[4], 5, 0, 0);
+  initial_state.nodes[4].AddIntAttribute(
+      ax::mojom::IntAttribute::kAriaCellRowIndex, 2);
+  initial_state.nodes[4].AddIntAttribute(
+      ax::mojom::IntAttribute::kAriaCellColumnIndex, 2);
+  MakeCell(&initial_state.nodes[5], 6, 0, 1);
+  MakeCell(&initial_state.nodes[6], 7, 0, 2);
+  MakeCell(&initial_state.nodes[7], 8, 1, 0);
+  MakeCell(&initial_state.nodes[8], 9, 1, 1);
+  MakeCell(&initial_state.nodes[9], 10, 1, 2);
+  MakeCell(&initial_state.nodes[10], 11, 2, 0);
+  initial_state.nodes[10].AddIntAttribute(
+      ax::mojom::IntAttribute::kAriaCellColumnIndex, 3);
+  MakeCell(&initial_state.nodes[11], 12, 2, 1);
+  initial_state.nodes[11].AddIntAttribute(
+      ax::mojom::IntAttribute::kAriaCellColumnIndex, 2);
+  MakeCell(&initial_state.nodes[12], 13, 2, 2);
+  initial_state.nodes[12].AddIntAttribute(
+      ax::mojom::IntAttribute::kAriaCellColumnIndex, 1);
+  AXTree tree(initial_state);
+  AXNode* table = tree.root();
+
+  EXPECT_EQ(5, table->GetTableAriaColCount());
+  EXPECT_EQ(5, table->GetTableAriaRowCount());
+
+  // The first row has the first cell ARIA row and column index
+  // specified as (2, 2). The rest of the row is inferred.
+
+  AXNode* cell_0_0 = tree.GetFromId(5);
+  EXPECT_EQ(2, cell_0_0->GetTableCellAriaRowIndex());
+  EXPECT_EQ(2, cell_0_0->GetTableCellAriaColIndex());
+
+  AXNode* cell_0_1 = tree.GetFromId(6);
+  EXPECT_EQ(2, cell_0_1->GetTableCellAriaRowIndex());
+  EXPECT_EQ(3, cell_0_1->GetTableCellAriaColIndex());
+
+  AXNode* cell_0_2 = tree.GetFromId(7);
+  EXPECT_EQ(2, cell_0_2->GetTableCellAriaRowIndex());
+  EXPECT_EQ(4, cell_0_2->GetTableCellAriaColIndex());
+
+  // The next row has the ARIA row index set to 4 on the row
+  // element. The rest is inferred.
+
+  AXNode* cell_1_0 = tree.GetFromId(8);
+  EXPECT_EQ(4, cell_1_0->GetTableCellAriaRowIndex());
+  EXPECT_EQ(1, cell_1_0->GetTableCellAriaColIndex());
+
+  AXNode* cell_1_1 = tree.GetFromId(9);
+  EXPECT_EQ(4, cell_1_1->GetTableCellAriaRowIndex());
+  EXPECT_EQ(2, cell_1_1->GetTableCellAriaColIndex());
+
+  AXNode* cell_1_2 = tree.GetFromId(10);
+  EXPECT_EQ(4, cell_1_2->GetTableCellAriaRowIndex());
+  EXPECT_EQ(3, cell_1_2->GetTableCellAriaColIndex());
+
+  // The last row has the ARIA row index set to 4 again, which is
+  // illegal so we should get 5. The cells have column indices of
+  // 3, 2, 1 which is illegal so we ignore the latter two and should
+  // end up with column indices of 3, 4, 5.
+
+  AXNode* cell_2_0 = tree.GetFromId(11);
+  EXPECT_EQ(5, cell_2_0->GetTableCellAriaRowIndex());
+  EXPECT_EQ(3, cell_2_0->GetTableCellAriaColIndex());
+
+  AXNode* cell_2_1 = tree.GetFromId(12);
+  EXPECT_EQ(5, cell_2_1->GetTableCellAriaRowIndex());
+  EXPECT_EQ(4, cell_2_1->GetTableCellAriaColIndex());
+
+  AXNode* cell_2_2 = tree.GetFromId(13);
+  EXPECT_EQ(5, cell_2_2->GetTableCellAriaRowIndex());
+  EXPECT_EQ(5, cell_2_2->GetTableCellAriaColIndex());
+}
+
 }  // namespace ui