Deferring updating column widths until the redraw loop executes in a finally command. Currently, every time the user adds or removes a column, we refresh the widths of all columns. This leads to an O(n^2) creation time for the table. Resetting the column widths in the redraw loop reduces that to O(n), and is consistent with the rendering implementation of the table itself. We use a dirty bit to indicate that the column widths need to be updated, to avoid unnecessary work in the redraw loop. When setting or clearing column widths, we update the affectected col element synchronously.
Review at http://gwt-code-reviews.appspot.com/1509808
Review by: pengzhuang@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10517 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java b/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
index 0619dcd..8287184 100644
--- a/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
+++ b/user/src/com/google/gwt/user/cellview/client/AbstractCellTable.java
@@ -952,6 +952,7 @@
private boolean cellIsEditing;
private final List<Column<T, ?>> columns = new ArrayList<Column<T, ?>>();
private final Map<Column<T, ?>, String> columnWidths = new HashMap<Column<T, ?>, String>();
+ private boolean columnWidthsDirty;
private final Map<Integer, String> columnWidthsByIndex = new HashMap<Integer, String>();
/**
@@ -1134,7 +1135,7 @@
*/
public void clearColumnWidth(Column<T, ?> column) {
columnWidths.remove(column);
- refreshColumnWidths();
+ updateColumnWidthImpl(column, null);
}
/**
@@ -1144,7 +1145,10 @@
*/
public void clearColumnWidth(Integer column) {
columnWidthsByIndex.remove(column);
- refreshColumnWidths();
+ // TODO(jlabanca): Compare to realColumnCount when headerBuilder lands.
+ if (column < getColumnCount()) {
+ doSetColumnWidth(column, null);
+ }
}
/**
@@ -1359,7 +1363,7 @@
}
CellBasedWidgetImpl.get().sinkEvents(this, consumedEvents);
- redraw();
+ refreshColumnsAndRedraw();
}
/**
@@ -1414,12 +1418,6 @@
insertColumn(beforeIndex, col, new SafeHtmlHeader(headerHtml), new SafeHtmlHeader(footerHtml));
}
- @Override
- public void redraw() {
- refreshColumnWidths();
- super.redraw();
- }
-
/**
* Redraw the table's footers.
*/
@@ -1466,7 +1464,7 @@
}
// Redraw the table asynchronously.
- redraw();
+ refreshColumnsAndRedraw();
// We don't unsink events because other handlers or user code may have sunk
// them intentionally.
@@ -1490,7 +1488,7 @@
*/
public void setColumnWidth(Column<T, ?> column, String width) {
columnWidths.put(column, width);
- refreshColumnWidths();
+ updateColumnWidthImpl(column, width);
}
/**
@@ -1525,7 +1523,10 @@
*/
public void setColumnWidth(int column, String width) {
columnWidthsByIndex.put(column, width);
- refreshColumnWidths();
+ // TODO(jlabanca): Compare to realColumnCount when headerBuilder lands.
+ if (column < getColumnCount()) {
+ doSetColumnWidth(column, width);
+ }
}
/**
@@ -2003,8 +2004,7 @@
@Override
protected void replaceAllChildren(List<T> values, SafeHtml html) {
- // Render the headers and footers.
- createHeadersAndFooters();
+ refreshHeadersAndColumnsImpl();
/*
* If html is not null, then the user overrode renderRowValues() and
@@ -2024,7 +2024,7 @@
@SuppressWarnings("deprecation")
@Override
protected void replaceChildren(List<T> values, int start, SafeHtml html) {
- createHeadersAndFooters();
+ refreshHeadersAndColumnsImpl();
/*
* If html is not null, then the user override renderRowValues() and
@@ -2570,6 +2570,28 @@
return (cellId == null) || (cellId.length() == 0) ? null : cellId;
}
+ /**
+ * Mark the column widths as dirty and redraw the table.
+ */
+ private void refreshColumnsAndRedraw() {
+ columnWidthsDirty = true;
+ redraw();
+ }
+
+ /**
+ * Refresh the headers and column widths.
+ */
+ private void refreshHeadersAndColumnsImpl() {
+ // Refresh the column widths if needed.
+ if (columnWidthsDirty) {
+ columnWidthsDirty = false;
+ refreshColumnWidths();
+ }
+
+ // Render the headers and footers.
+ createHeadersAndFooters();
+ }
+
private <C> boolean resetFocusOnCellImpl(int row, int col, HasCell<T, C> column,
Element cellParent) {
T value = getVisibleItem(row);
@@ -2595,4 +2617,21 @@
setStyleName(cells.getItem(i), cellStyle, add);
}
}
+
+ /**
+ * Update the width of all instances of the specified column. A column
+ * instance may appear multiple times in the table.
+ *
+ * @param column the column to update
+ * @param width the width of the column, or null to clear the width
+ */
+ private void updateColumnWidthImpl(Column<T, ?> column, String width) {
+ // TODO(jlabanca): Use realColumnCount when headerBuilder lands.
+ int columnCount = getColumnCount();
+ for (int i = 0; i < columnCount; i++) {
+ if (columns.get(i) == column) {
+ doSetColumnWidth(i, width);
+ }
+ }
+ }
}
diff --git a/user/test/com/google/gwt/user/cellview/client/CellTableTest.java b/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
index 3cee9aa..2f3ad78 100644
--- a/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
+++ b/user/test/com/google/gwt/user/cellview/client/CellTableTest.java
@@ -192,9 +192,11 @@
// Remove column 1.
table.removeColumn(column1);
+ table.getPresenter().flush();
assertEquals("0px", col1.getStyle().getWidth());
}
+ @Override
public void testSetColumnWidth() {
CellTable<String> table = createAbstractHasData(new TextCell());
Column<String, ?> column0 = table.getColumn(0);
