LGTM, but I think we really need Joel to double-check (well, we also need him to commit the change, so... ;-) )
Also, maybe the overall process and DOM structure should be documented once in the class javadoc, rather than having it scattered in the class? Something like: "For the parent layout and for each child layer, we create a create a style ruler used to measure decorations. The ruler is always the first child of its parent so it can be easily accessed without the need for a direct reference (an earlier version stored it in an expando but that creates a memory leak)." Then go on briefly describing that there are two elements (in addition to the style ruler) created for each child layer; how resizes are detected and what kind of treatments they trigger, with a small note about what can happen when we're not attached, when we're attached, etc. and how we overcome memory leaks (by creating and clearing the __layer expandos when attached/detached, similar to the DOM.setEventListener and its __listener expando) http://gwt-code-reviews.appspot.com/1601804/diff/13003/user/src/com/google/gwt/layout/client/LayoutImplIE6.java File user/src/com/google/gwt/layout/client/LayoutImplIE6.java (right): http://gwt-code-reviews.appspot.com/1601804/diff/13003/user/src/com/google/gwt/layout/client/LayoutImplIE6.java#newcode41 user/src/com/google/gwt/layout/client/LayoutImplIE6.java:41: * dynamically detects IE7 and punts to the super implementation. I thought you removed that note? http://gwt-code-reviews.appspot.com/1601804/diff/13003/user/src/com/google/gwt/layout/client/LayoutImplIE6.java#newcode69 user/src/com/google/gwt/layout/client/LayoutImplIE6.java:69: * Sets __decoWidth/__decoWidth on {@code element} using its ruler. Should be "__decoWidth/__decoHeight" http://gwt-code-reviews.appspot.com/1601804/diff/13003/user/src/com/google/gwt/layout/client/LayoutImplIE6.java#newcode114 user/src/com/google/gwt/layout/client/LayoutImplIE6.java:114: private static native void resizeRelativeToParent(Element parent) /*-{ I'm not found of these argument renames: the method's goal is to "resize an element relative to its parent", hence resizeRelativeToParent(elem). If you rename elem to parent, the name of the method becomes confusing. Same in LayoutImpl with fillParent(elem): "make the given element fill its parent", fillParent(parent) doesn't mean much thing. I understand where you come from (name the argument after the value that will be passed, so we know which element in the small subtrees we're managing we're dealing with), but IMO it's more confusing. If you want to provide such hint to the next developer reading the code, then do it in the javadoc. http://gwt-code-reviews.appspot.com/1601804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
