Fix iterator remove bug.
See the test case for details of the bug.
To put an end to these bugs I decided to simplify with a small
additional cost on removal.
Simpler is saner...
Change-Id: If64056d90b71c226b9c07ee99ab5c8ff76ded57d
Review-Link: https://gwt-review.googlesource.com/#/c/13489/
diff --git a/user/super/com/google/gwt/emul/java/util/InternalHashCodeMap.java b/user/super/com/google/gwt/emul/java/util/InternalHashCodeMap.java
index 19a0011..1d78932 100644
--- a/user/super/com/google/gwt/emul/java/util/InternalHashCodeMap.java
+++ b/user/super/com/google/gwt/emul/java/util/InternalHashCodeMap.java
@@ -75,6 +75,7 @@
Entry<K, V> entry = chain[i];
if (host.equals(key, entry.getKey())) {
if (chain.length == 1) {
+ ArrayHelper.setLength(chain, 0);
// remove the whole array
backingMap.delete(hashCode);
} else {
@@ -112,7 +113,6 @@
final InternalJsIterator<Object> chains = backingMap.entries();
int itemIndex = 0;
Entry<K, V>[] chain = newEntryChain();
- Entry<K, V>[] lastChain = null;
Entry<K, V> lastEntry = null;
@Override
@@ -134,7 +134,6 @@
public Entry<K, V> next() {
checkElement(hasNext());
- lastChain = chain;
lastEntry = chain[itemIndex++];
return lastEntry;
}
@@ -143,14 +142,11 @@
public void remove() {
checkState(lastEntry != null);
- boolean isLastChain = itemIndex == lastChain.length;
InternalHashCodeMap.this.remove(lastEntry.getKey());
-
- // If this was not the last item in the chain, our itemIndex just jumped an item.
- if (!isLastChain) {
+ // Unless we are in a new chain, all items have shifted so our itemIndex should as well...
+ if (itemIndex != 0) {
itemIndex--;
}
-
lastEntry = null;
}
};
diff --git a/user/test/com/google/gwt/emultest/java/util/HashMapTest.java b/user/test/com/google/gwt/emultest/java/util/HashMapTest.java
index 28f232f..81054aa 100644
--- a/user/test/com/google/gwt/emultest/java/util/HashMapTest.java
+++ b/user/test/com/google/gwt/emultest/java/util/HashMapTest.java
@@ -758,7 +758,7 @@
}
// see: https://github.com/gwtproject/gwt/issues/9184
- public void testIterationWithCollidingHashCodes() {
+ public void testIteration_withCollidingHashCodes() {
Object firstKey = createObjectWithHashCode(1);
Object secondKey = createObjectWithHashCode(1);
@@ -793,7 +793,7 @@
}
// see: https://github.com/gwtproject/gwt/issues/9184
- public void testIterationWithCollidingHashCodes_FullIteration() {
+ public void testIteration_withCollidingHashCodesAndFullIteration() {
List<Object> keys = new ArrayList<>();
HashMap<Object, String> testMap = new HashMap<>();
@@ -818,6 +818,22 @@
assertTrue(keys.isEmpty());
}
+ public void testIteration_hasNextThenRemove() {
+ HashMap<Object, String> testMap = new HashMap<>();
+ testMap.put(createObjectWithHashCode(1), "one");
+ testMap.put(createObjectWithHashCode(2), "two");
+
+ Iterator<Object> it = testMap.keySet().iterator();
+ it.next();
+ assertTrue("iterator should have next after first", it.hasNext());
+ it.remove();
+ assertTrue("iterator should have next after removal", it.hasNext());
+
+ it.next();
+ it.remove();
+ assertFalse(it.hasNext());
+ }
+
private Object createObjectWithHashCode(final int hashCode) {
return new Object() {
@Override
