Revision: 6307 Author: [email protected] Date: Tue Oct 6 14:43:09 2009 Log: UiBinder Optimizations + Decrease number of attachments to get fields by id General algorithm is: - Attach the html to the DOM - Pluck out all DOM fields - Pluck out all fields for widget replacement - Detach the HTML - Finalize replacement of elements with widgets
patch by: tstanis at google.com review by: rjrjr at google.com http://code.google.com/p/google-web-toolkit/source/detail?r=6307 Modified: /trunk/tools/api-checker/config/gwt16_20userApi.conf /trunk/user/src/com/google/gwt/uibinder/client/UiBinderUtil.java /trunk/user/src/com/google/gwt/uibinder/parsers/DomElementParser.java /trunk/user/src/com/google/gwt/uibinder/parsers/HTMLPanelParser.java /trunk/user/src/com/google/gwt/uibinder/parsers/HasHTMLParser.java /trunk/user/src/com/google/gwt/uibinder/parsers/WidgetInterpreter.java /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java /trunk/user/src/com/google/gwt/user/client/ui/HTMLPanel.java /trunk/user/test/com/google/gwt/user/client/ui/HTMLPanelTest.java ======================================= --- /trunk/tools/api-checker/config/gwt16_20userApi.conf Wed Aug 5 20:27:52 2009 +++ /trunk/tools/api-checker/config/gwt16_20userApi.conf Tue Oct 6 14:43:09 2009 @@ -90,3 +90,5 @@ com.google.gwt.i18n.client.LocaleInfo::getCurrentLocale() FINAL_ADDED com.google.gwt.i18n.client.LocaleInfo::getLocaleName() FINAL_ADDED com.google.gwt.i18n.client.LocaleInfo::isRTL() FINAL_ADDED +# added addAndReplaceElement(Widget, Element) in 2.0 +com.google.gwt.user.client.ui.HTMLPanel::addAndReplaceElement(Lcom/google/gwt/user/client/ui/Widget;Ljava/lang/String;) OVERLOADED_METHOD_CALL ======================================= --- /trunk/user/src/com/google/gwt/uibinder/client/UiBinderUtil.java Wed Aug 5 20:27:52 2009 +++ /trunk/user/src/com/google/gwt/uibinder/client/UiBinderUtil.java Tue Oct 6 14:43:09 2009 @@ -26,12 +26,49 @@ * so please don't use them for non-UiBinder code. */ public class UiBinderUtil { + /** + * Temporary attachment record that keeps track of where an element was + * before attachment. Use the detach method to put things back. + * + */ + public static class TempAttachment { + private final Element element; + private final Element origParent; + private final Element origSibling; + + private TempAttachment(Element origParent, Element origSibling, + Element element) { + this.origParent = origParent; + this.origSibling = origSibling; + this.element = element; + } + + /** + * Restore to previous DOM state before attachment. + */ + public void detach() { + // Put the panel's element back where it was. + if (origParent != null) { + origParent.insertBefore(element, origSibling); + } else { + orphan(element); + } + } + } + private static Element hiddenDiv; - - public static Element attachToDomAndGetChild(Element element, String id) { + + /** + * Attaches the element to the dom temporarily. Keeps track of where it is + * attached so that things can be put back latter. + * + * @return attachment record which can be used for reverting back to previous + * DOM state + */ + public static TempAttachment attachToDom(Element element) { // TODO(rjrjr) This is copied from HTMLPanel. Reconcile ensureHiddenDiv(); - + // Hang on to the panel's original parent and sibling elements so that it // can be replaced. Element origParent = element.getParentElement(); @@ -39,19 +76,10 @@ // Attach the panel's element to the hidden div. hiddenDiv.appendChild(element); - - // Now that we're attached to the DOM, we can use getElementById. - Element child = Document.get().getElementById(id); - - // Put the panel's element back where it was. - if (origParent != null) { - origParent.insertBefore(element, origSibling); - } else { - orphan(element); - } - - return child; - } + + return new TempAttachment(origParent, origSibling, element); + } + public static Element fromHtml(String html) { ensureHiddenDiv(); ======================================= --- /trunk/user/src/com/google/gwt/uibinder/parsers/DomElementParser.java Tue Sep 1 15:14:29 2009 +++ /trunk/user/src/com/google/gwt/uibinder/parsers/DomElementParser.java Tue Oct 6 14:43:09 2009 @@ -23,22 +23,24 @@ /** * Parses a dom element and all of its children. Note that this parser does not * make recursive calls to parse child elements, unlike what goes on with widget - * parsers. Instead, we consume the inner html of the given element into - * a single string literal, used to instantiate the dom tree at run time. + * parsers. Instead, we consume the inner html of the given element into a + * single string literal, used to instantiate the dom tree at run time. */ public class DomElementParser implements ElementParser { public void parse(XMLElement elem, String fieldName, JClassType type, UiBinderWriter writer) throws UnableToCompleteException { - HtmlInterpreter interpreter = - new HtmlInterpreter(writer, fieldName, new HtmlMessageInterpreter(writer, - fieldName)); + HtmlInterpreter interpreter = new HtmlInterpreter(writer, fieldName, + new HtmlMessageInterpreter(writer, fieldName)); interpreter.interpretElement(elem); + writer.beginAttachedSection(fieldName); String html = elem.consumeOpeningTag() + elem.consumeInnerHtml(interpreter) - + elem.getClosingTag(); + + elem.getClosingTag(); + writer.endAttachedSection(); writer.setFieldInitializer(fieldName, String.format( - "(%1$s) UiBinderUtil.fromHtml(\"%2$s\")", type.getQualifiedSourceName(), html)); + "(%1$s) UiBinderUtil.fromHtml(\"%2$s\")", + type.getQualifiedSourceName(), html)); } } ======================================= --- /trunk/user/src/com/google/gwt/uibinder/parsers/HTMLPanelParser.java Tue Sep 8 22:07:33 2009 +++ /trunk/user/src/com/google/gwt/uibinder/parsers/HTMLPanelParser.java Tue Oct 6 14:43:09 2009 @@ -46,8 +46,10 @@ */ HtmlInterpreter htmlInterpreter = makeHtmlInterpreter(fieldName, writer); + writer.beginAttachedSection(fieldName + ".getElement()"); String html = elem.consumeInnerHtml(InterpreterPipe.newPipe(widgetInterpreter, htmlInterpreter)); + writer.endAttachedSection(); /* * HTMLPanel has no no-arg ctor, so we have to generate our own, using the ======================================= --- /trunk/user/src/com/google/gwt/uibinder/parsers/HasHTMLParser.java Wed Aug 5 20:27:52 2009 +++ /trunk/user/src/com/google/gwt/uibinder/parsers/HasHTMLParser.java Tue Oct 6 14:43:09 2009 @@ -30,8 +30,9 @@ HtmlInterpreter interpreter = HtmlInterpreter.newInterpreterForUiObject(writer, fieldName); + writer.beginAttachedSection(fieldName + ".getElement()"); String html = elem.consumeInnerHtml(interpreter); - + writer.endAttachedSection(); // TODO(jgw): throw an error if there's a conflicting 'html' attribute. if (html.trim().length() > 0) { writer.genStringPropertySet(fieldName, "HTML", html); ======================================= --- /trunk/user/src/com/google/gwt/uibinder/parsers/WidgetInterpreter.java Tue Sep 1 15:14:29 2009 +++ /trunk/user/src/com/google/gwt/uibinder/parsers/WidgetInterpreter.java Tue Oct 6 14:43:09 2009 @@ -18,7 +18,6 @@ import com.google.gwt.core.ext.UnableToCompleteException; import com.google.gwt.uibinder.rebind.UiBinderWriter; import com.google.gwt.uibinder.rebind.XMLElement; -import com.google.gwt.uibinder.rebind.XMLElement.PostProcessingInterpreter; import java.util.Collections; import java.util.HashMap; @@ -29,7 +28,7 @@ * instances. Declares the appropriate widget, and replaces them in the markup * with a <span>. */ -class WidgetInterpreter implements PostProcessingInterpreter<String> { +class WidgetInterpreter implements XMLElement.Interpreter<String> { private static final Map<String, String> LEGAL_CHILD_ELEMENTS; private static final String DEFAULT_CHILD_ELEMENT = "span"; @@ -58,11 +57,6 @@ return tag; } - /** - * A map of widget idHolder field names to the element it references. - */ - private final Map<String, XMLElement> idToWidgetElement = - new HashMap<String, XMLElement>(); private final String fieldName; private final UiBinderWriter uiWriter; @@ -72,14 +66,28 @@ this.uiWriter = writer; } - public String interpretElement(XMLElement elem) throws UnableToCompleteException { + public String interpretElement(XMLElement elem) + throws UnableToCompleteException { if (uiWriter.isWidgetElement(elem)) { // Allocate a local variable to hold the dom id for this widget. Note // that idHolder is a local variable reference, not a string id. We // have to generate the ids at runtime, not compile time, or else // we'll reuse ids for any template rendered more than once. String idHolder = uiWriter.declareDomIdHolder(); - idToWidgetElement.put(idHolder, elem); + String childField = uiWriter.parseElementToField(elem); + uiWriter.ensureFieldAttached(fieldName); + + String elementPointer = idHolder + "Element"; + uiWriter.addInitStatement( + "com.google.gwt.user.client.Element %s = " + + "com.google.gwt.dom.client.Document.get().getElementById(%s).cast();", + elementPointer, idHolder); + // Delay replacing the placeholders with the widgets until after + // detachment so as not to end up attaching the widget to the DOM + // unnecessarily + uiWriter.addDetachStatement( + "%1$s.addAndReplaceElement(%2$s, %3$s);", + fieldName, childField, elementPointer); // Create an element to hold the widget. String tag = getLegalPlaceholderTag(elem); @@ -87,25 +95,4 @@ } return null; } - - /** - * Called by {...@link XMLElement#consumeInnerHtml} after all elements have - * been handed to {...@link #interpretElement}. Parses each widget element - * that was seen, and generates a - * {...@link com.google.gwt.user.client.ui.HTMLPanel#addAndReplaceElement} for - * each. - */ - public String postProcess(String consumedHtml) throws UnableToCompleteException { - /* - * Generate an addAndReplaceElement(widget, idHolder) for each widget found - * while parsing the element's inner HTML. - */ - for (String idHolder : idToWidgetElement.keySet()) { - XMLElement childElem = idToWidgetElement.get(idHolder); - String childField = uiWriter.parseElementToField(childElem); - uiWriter.addInitStatement("%1$s.addAndReplaceElement(%2$s, %3$s);", fieldName, - childField, idHolder); - } - return consumedHtml; - } -} +} ======================================= --- /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java Tue Sep 22 14:34:43 2009 +++ /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java Tue Oct 6 14:43:09 2009 @@ -241,6 +241,22 @@ private String rendered; + /** + * Stack of element variable names that have been attached. + */ + private final LinkedList<String> attachSectionElements = new LinkedList<String>(); + /** + * Maps from field element name to the temporary attach record variable name. + */ + private final Map<String, String> attachedVars = new HashMap<String, String>(); + private int nextAttachVar = 0; + + /** + * Stack of statements to be executed after we detach the current attach + * section. + */ + private final LinkedList<List<String>> detachStatementsStack = new LinkedList<List<String>>(); + UiBinderWriter(JClassType baseClass, String implClassName, String templatePath, TypeOracle oracle, MortalLogger logger) throws UnableToCompleteException { @@ -264,6 +280,18 @@ handlerEvaluator = new HandlerEvaluator(ownerClass, logger, oracle); fieldManager = new FieldManager(logger); } + + /** + * Statements to be excuted right after the current attached element is + * detached. This is useful for doing things that might be expensive while the + * element is attached to the DOM. + * + * @param format + * @param args + */ + public void addDetachStatement(String format, Object... args) { + detachStatementsStack.getFirst().add(String.format(format, args)); + } /** * Add a statement to be run after everything has been instantiated, in the @@ -280,6 +308,17 @@ public void addStatement(String format, Object... args) { statements.add(String.format(format, args)); } + + /** + * Begin a section where a new attachable element is being parsed. Note that + * attachment is only done when actually needed. + * + * @param element to be attached for this section + */ + public void beginAttachedSection(String element) { + attachSectionElements.addFirst(element); + detachStatementsStack.addFirst(new ArrayList<String>()); + } /** * Declare a field that will hold an Element instance. Returns a token that @@ -298,11 +337,12 @@ */ public String declareDomField(String fieldName, String parentElementExpression) throws UnableToCompleteException { + ensureAttached(parentElementExpression); String name = declareDomIdHolder(); setFieldInitializer(fieldName, "null"); addInitStatement( - "%s = UiBinderUtil.attachToDomAndGetChild(%s, %s).cast();", fieldName, - parentElementExpression, name); + "%s = com.google.gwt.dom.client.Document.get().getElementById(%s).cast();", + fieldName, name); addInitStatement("%s.removeAttribute(\"id\");", fieldName); return tokenForExpression(name); } @@ -389,6 +429,48 @@ throws UnableToCompleteException { logger.die(message, params); } + + /** + * End the current attachable section. This will detach the element if it was + * ever attached and execute any detach statements. + */ + public void endAttachedSection() { + String elementVar = attachSectionElements.removeFirst(); + List<String> detachStatements = detachStatementsStack.removeFirst(); + if (attachedVars.containsKey(elementVar)) { + String attachedVar = attachedVars.remove(elementVar); + addInitStatement("%s.detach();", attachedVar); + for (String statement : detachStatements) { + addInitStatement(statement); + } + } + } + + /** + * Ensure that the specified element is attached to the DOM. + * + * @param element variable name of element to be attached + */ + public void ensureAttached(String element) { + String attachSectionElement = attachSectionElements.getFirst(); + if (!attachedVars.containsKey(attachSectionElement)) { + String attachedVar = "attachRecord" + nextAttachVar; + addInitStatement( + "UiBinderUtil.TempAttachment %s = UiBinderUtil.attachToDom(%s);", + attachedVar, attachSectionElement); + attachedVars.put(attachSectionElement, attachedVar); + nextAttachVar++; + } + } + + /** + * Ensure that the specified field is attached to the DOM. + * + * @param field variable name of the field to be attached + */ + public void ensureFieldAttached(String field) { + ensureAttached(field + ".getElement()"); + } /** * Finds the JClassType that corresponds to this XMLElement, which must be a @@ -736,6 +818,23 @@ return b.toString(); } + + /** + * Ensures that all of the internal data structures are cleaned up correctly + * at the end of parsing the document. + * + * @throws UnableToCompleteException + */ + private void ensureAttachmentCleanedUp() throws UnableToCompleteException { + if (!attachSectionElements.isEmpty()) { + throw new IllegalStateException("Attachments not cleaned up: " + + attachSectionElements); + } + if (!detachStatementsStack.isEmpty()) { + throw new IllegalStateException("Detach not cleaned up: " + + detachStatementsStack); + } + } /** * Given a DOM tag name, return the corresponding @@ -898,9 +997,9 @@ StringWriter stringWriter = new StringWriter(); IndentedWriter niceWriter = new IndentedWriter( new PrintWriter(stringWriter)); - writeBinder(niceWriter, rootField); + ensureAttachmentCleanedUp(); return stringWriter.toString(); } @@ -986,7 +1085,6 @@ addWidgetParser("RadioButton"); addWidgetParser("CellPanel"); addWidgetParser("CustomButton"); - addWidgetParser("DockLayoutPanel"); addWidgetParser("StackLayoutPanel"); ======================================= --- /trunk/user/src/com/google/gwt/user/client/ui/HTMLPanel.java Tue Jul 21 07:11:57 2009 +++ /trunk/user/src/com/google/gwt/user/client/ui/HTMLPanel.java Tue Oct 6 14:43:09 2009 @@ -107,6 +107,20 @@ super.add(widget, elem); } + /** + * Adds a child widget to the panel, replacing the HTML element. + * + * @param widget the widget to be added + * @param toReplace the element to be replaced by the widget + */ + public void addAndReplaceElement(Widget widget, Element toReplace) { + // Logic pulled from super.add(), replacing the element rather than adding. + widget.removeFromParent(); + getChildren().add(widget); + toReplace.getParentNode().replaceChild(widget.getElement(), toReplace); + adopt(widget); + } + /** * Adds a child widget to the panel, replacing the HTML element specified by a * given id. @@ -120,14 +134,10 @@ if (toReplace == null) { throw new NoSuchElementException(id); } - - // Logic pulled from super.add(), replacing the element rather than adding. - widget.removeFromParent(); - getChildren().add(widget); - toReplace.getParentNode().replaceChild(widget.getElement(), toReplace); - adopt(widget); - } - + + addAndReplaceElement(widget, toReplace); + } + /** * Finds an {...@link Element element} within this panel by its id. * ======================================= --- /trunk/user/test/com/google/gwt/user/client/ui/HTMLPanelTest.java Wed Sep 23 10:15:52 2009 +++ /trunk/user/test/com/google/gwt/user/client/ui/HTMLPanelTest.java Tue Oct 6 14:43:09 2009 @@ -118,6 +118,27 @@ Node next = button.getElement().getNextSibling(); assertEquals("bar", next.getNodeValue()); } + + /** + * Ensures that addAndReplaceChild() puts the widget in exactly the right place in the DOM. + */ + public void testAddAndReplaceElementForElement() { + HTMLPanel hp = new HTMLPanel("<div id='parent'>foo<span id='placeholder'></span>bar</div>"); + + RootPanel.get().add(hp); + com.google.gwt.user.client.Element placeholder = hp.getElementById("placeholder"); + + Button button = new Button("my button"); + hp.addAndReplaceElement(button, placeholder); + + assertEquals("parent", button.getElement().getParentElement().getId()); + + Node prev = button.getElement().getPreviousSibling(); + assertEquals("foo", prev.getNodeValue()); + + Node next = button.getElement().getNextSibling(); + assertEquals("bar", next.getNodeValue()); + } /** * Tests table root tag. --~--~---------~--~----~------------~-------~--~----~ http://groups.google.com/group/Google-Web-Toolkit-Contributors -~----------~----~----~----~------~----~------~--~---
