Fix HasDataEditor.createEditorForTraversal so it doesn't cause side effects.
(Previously it would shorten the list each time it's traversed.)
Also adds assertions to make sure editor indexes aren't negative.
Review at http://gwt-code-reviews.appspot.com/1587803/
Fixes issue: 6959
Contributed by: t.broyer
Review by: mdempsky@google.com
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@11462 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/editor/client/adapters/EditorSource.java b/user/src/com/google/gwt/editor/client/adapters/EditorSource.java
index 6a1d779..e2991fa 100644
--- a/user/src/com/google/gwt/editor/client/adapters/EditorSource.java
+++ b/user/src/com/google/gwt/editor/client/adapters/EditorSource.java
@@ -22,7 +22,7 @@
/**
* An entity capable of creating and destroying instances of Editors. This type
- * is used by Editors which operate on ordered data, sich as {@link ListEditor}.
+ * is used by Editors which operate on ordered data, such as {@link ListEditor}.
*
* @param <E> the type of Editor required
* @see com.google.gwt.editor.client.testing.FakeEditorSource
@@ -45,6 +45,7 @@
* @return a List of {@link Editor}s of type E
*/
public List<E> create(int count, int index) {
+ assert index >= 0;
List<E> toReturn = new ArrayList<E>(count);
for (int i = 0; i < count; i++) {
toReturn.add(create(index + i));
@@ -53,6 +54,22 @@
}
/**
+ * Creates a temporary sub-Editor to use for traversal.
+ * <p>
+ * For backwards compatibility with GWT 2.5.0 and earlier, the default implementation calls
+ * {@code create(0)} and {@link #dispose(Editor) disposes} the editor right away.
+ *
+ * @return an {@link Editor} of type E
+ * @see ListEditor#createEditorForTraversal()
+ * @see com.google.gwt.editor.client.EditorContext#traverseSyntheticCompositeEditor
+ */
+ public E createEditorForTraversal() {
+ E toReturn = create(0);
+ dispose(toReturn);
+ return toReturn;
+ }
+
+ /**
* Called when an Editor no longer requires a sub-Editor. The default
* implementation is a no-op.
*
@@ -68,5 +85,6 @@
* @param index the index of the Editor
*/
public void setIndex(E editor, int index) {
+ assert index >= 0;
}
}
\ No newline at end of file
diff --git a/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java b/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
index f2bb40f..a2a781f 100644
--- a/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
+++ b/user/src/com/google/gwt/editor/client/adapters/HasDataEditor.java
@@ -35,21 +35,30 @@
@Override
public IndexedEditor<T> create(int index) {
+ assert index >= 0;
return new IndexedEditor<T>(index, data);
}
@Override
+ public LeafValueEditor<T> createEditorForTraversal() {
+ return new IndexedEditor<T>(-1, null);
+ }
+
+ @Override
public void dispose(LeafValueEditor<T> subEditor) {
+ // We use a negative index as flag in createEditorForTraversal
+ assert ((IndexedEditor<T>) subEditor).index >= 0;
data.setRowCount(data.getRowCount() - 1);
}
@Override
public void setIndex(LeafValueEditor<T> editor, int index) {
+ assert index >= 0;
((IndexedEditor<T>) editor).setIndex(index);
}
}
- static class IndexedEditor<Q> implements LeafValueEditor<Q> {
+ private static class IndexedEditor<Q> implements LeafValueEditor<Q> {
private int index;
private Q value;
private final HasData<Q> data;
@@ -59,22 +68,27 @@
this.data = data;
}
+ @Override
public Q getValue() {
return value;
}
- public void setIndex(int index) {
- this.index = index;
- push();
- }
-
+ @Override
public void setValue(Q value) {
this.value = value;
push();
}
+ void setIndex(int index) {
+ assert index >= 0;
+ this.index = index;
+ push();
+ }
+
private void push() {
- data.setRowData(index, Collections.singletonList(value));
+ if (data != null) {
+ data.setRowData(index, Collections.singletonList(value));
+ }
}
}
@@ -92,7 +106,7 @@
/**
* Prevent subclassing.
*/
- HasDataEditor(HasData<T> data) {
+ private HasDataEditor(HasData<T> data) {
super(new HasDataEditorSource<T>(data));
}
}
diff --git a/user/src/com/google/gwt/editor/client/adapters/ListEditor.java b/user/src/com/google/gwt/editor/client/adapters/ListEditor.java
index 262ad66..0befca9 100644
--- a/user/src/com/google/gwt/editor/client/adapters/ListEditor.java
+++ b/user/src/com/google/gwt/editor/client/adapters/ListEditor.java
@@ -66,9 +66,7 @@
* @return an {@link Editor} of type E
*/
public E createEditorForTraversal() {
- E toReturn = editorSource.create(0);
- editorSource.dispose(toReturn);
- return toReturn;
+ return editorSource.createEditorForTraversal();
}
public void flush() {
diff --git a/user/src/com/google/gwt/editor/client/testing/FakeEditorSource.java b/user/src/com/google/gwt/editor/client/testing/FakeEditorSource.java
index 073aec3..5deff76 100644
--- a/user/src/com/google/gwt/editor/client/testing/FakeEditorSource.java
+++ b/user/src/com/google/gwt/editor/client/testing/FakeEditorSource.java
@@ -48,6 +48,11 @@
}
@Override
+ public FakeLeafValueEditor<T> createEditorForTraversal() {
+ return new FakeLeafValueEditor<T>();
+ }
+
+ @Override
public void dispose(FakeLeafValueEditor<T> subEditor) {
lastKnownPosition.put(subEditor, DISPOSED);
}
diff --git a/user/test/com/google/gwt/editor/client/adapters/HasDataEditorTest.java b/user/test/com/google/gwt/editor/client/adapters/HasDataEditorTest.java
index 9e400fa..8a5481e 100644
--- a/user/test/com/google/gwt/editor/client/adapters/HasDataEditorTest.java
+++ b/user/test/com/google/gwt/editor/client/adapters/HasDataEditorTest.java
@@ -16,6 +16,8 @@
package com.google.gwt.editor.client.adapters;
import com.google.gwt.core.client.GWT;
+import com.google.gwt.editor.client.EditorContext;
+import com.google.gwt.editor.client.EditorVisitor;
import com.google.gwt.editor.client.SimpleBeanEditorDriver;
import com.google.gwt.event.shared.GwtEvent;
import com.google.gwt.event.shared.HandlerRegistration;
@@ -205,6 +207,26 @@
assertEquals(expectedValue, editor.getList());
assertEquals(expectedValue, hasData.getRowData());
}
+
+ /**
+ * See <a href="http://code.google.com/p/google-web-toolkit/issues/detail?id=6959">issue 6959</a>
+ */
+ public void testTraverseSyntheticCompositeEditor() {
+ List<Integer> expectedValue = Arrays.asList(1, 2, 3, 4, 5);
+
+ EditorVisitor visitor = new SyntheticVisitor();
+
+ // check that it won't throw
+ driver.accept(visitor);
+
+ driver.edit(expectedValue);
+
+ // Shouldn't affect the editor and HasData
+ driver.accept(visitor);
+
+ assertEquals(expectedValue, editor.getList());
+ assertEquals(expectedValue, hasData.getRowData());
+ }
@Override
protected void gwtSetUp() throws Exception {
@@ -213,4 +235,14 @@
driver = GWT.create(HasDataEditorDriver.class);
driver.initialize(editor);
}
+
+ /** A visitor that visits synthetic composite editors. */
+ private static class SyntheticVisitor extends EditorVisitor {
+ public <T> boolean visit(EditorContext<T> ctx) {
+ if (ctx.asCompositeEditor() != null) {
+ ctx.traverseSyntheticCompositeEditor(this);
+ }
+ return true;
+ }
+ }
}
