LGTM
http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java File dev/core/src/com/google/gwt/dev/util/collect/HashMap.java (right): http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode54 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:54: throw new ConcurrentModificationException(); I wonder how many places we'll find that start throwing this exception after this patch? http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode111 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:111: HashMap.this.ensureSizeFor(Math.max(size(), c.size())); At first glance, this change doesn't make sense = couldn't we end up with size() + c.size() items in the worst case? (c is all new entries). OK, so we don't know until we see if the entries are actually in the map already. But do we really need this here? Is addAll() going to end up calling EntrySet.add()? We don't worry about ensureSizeFor there - we could just let HashMap.this.put() worry about it. My guess is that this is a heuristic to make sure we don't resize the table multiple times in the middle of addAll(). Comment please. http://gwt-code-reviews.appspot.com/1384804/diff/1/dev/core/src/com/google/gwt/dev/util/collect/HashMap.java#newcode482 dev/core/src/com/google/gwt/dev/util/collect/HashMap.java:482: HashMap.this.ensureSizeFor(Math.max(size(), m.size())); So here again, this looks odd http://gwt-code-reviews.appspot.com/1384804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
