Fixing IsRenderable API step 2: pass in a stamper instead of the actual ID. As we discussed a few weeks ago, the goal here is to make the exact scheme by which elements are marked and retrieved opaque to the IsRenderable implementation. I thought I went about this in small steps: 1-) Fix the API and UiBinder to use a RenderableStamper class. This class supports stamping SafeHtml and ElementBuilder, but also exposes the token explicitly for backwards-compatibility (to keep the patch at a manageable size) 2-) Fix all uses of RenderableStamper.geToken() to use either SafeHtml or ElementBuilder. In particular I'm worried about performance regressions if we use SafeHtml. 3-) Profit. Review at http://gwt-code-reviews.appspot.com/1472801 git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10421 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/uibinder/elementparsers/IsRenderableInterpreter.java b/user/src/com/google/gwt/uibinder/elementparsers/IsRenderableInterpreter.java index b1d92cf..c9855ae 100644 --- a/user/src/com/google/gwt/uibinder/elementparsers/IsRenderableInterpreter.java +++ b/user/src/com/google/gwt/uibinder/elementparsers/IsRenderableInterpreter.java
@@ -44,21 +44,15 @@ return null; } - String idHolder = uiWriter.declareDomIdHolder(); + String stamper = uiWriter.declareRenderableStamper(); FieldManager fieldManager = uiWriter.getFieldManager(); FieldWriter fieldWriter = fieldManager.require(fieldName); - FieldWriter childFieldWriter = uiWriter.parseElementToFieldWriter(elem); - String elementPointer = idHolder + "Element"; fieldWriter.addAttachStatement( - "com.google.gwt.user.client.Element %s = " + - "com.google.gwt.dom.client.Document.get().getElementById(%s).cast();", - elementPointer, fieldManager.convertFieldToGetter(idHolder)); - fieldWriter.addAttachStatement( - "%s.claimElement(%s);", + "%s.claimElement(%s.findStampedElement());", fieldManager.convertFieldToGetter(childFieldWriter.getName()), - elementPointer); + fieldManager.convertFieldToGetter(stamper)); // Some operations are more efficient when the Widget isn't attached to // the document. Perform them here. @@ -73,7 +67,7 @@ // TODO(rdcastro): use the render() call that receives the SafeHtmlBuilder String elementHtml = fieldManager.convertFieldToGetter(childFieldWriter.getName()) + ".render(" - + fieldManager.convertFieldToGetter(idHolder) + ")"; + + fieldManager.convertFieldToGetter(stamper) + ")"; return uiWriter.tokenForSafeHtmlExpression(elementHtml); } }
diff --git a/user/src/com/google/gwt/uibinder/rebind/FieldWriterType.java b/user/src/com/google/gwt/uibinder/rebind/FieldWriterType.java index df4e88c..cb24f35 100644 --- a/user/src/com/google/gwt/uibinder/rebind/FieldWriterType.java +++ b/user/src/com/google/gwt/uibinder/rebind/FieldWriterType.java
@@ -21,10 +21,11 @@ */ enum FieldWriterType { - GENERATED_BUNDLE(5), - GENERATED_CSS(4), - IMPORTED(3), // ui:with clauses. - DOM_ID_HOLDER(2), + GENERATED_BUNDLE(6), + GENERATED_CSS(5), + IMPORTED(4), // ui:with clauses. + DOM_ID_HOLDER(3), + RENDERABLE_STAMPER(2), DEFAULT(1); /**
diff --git a/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java b/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java index 2d5f550..0649c5d 100644 --- a/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java +++ b/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
@@ -43,6 +43,7 @@ import com.google.gwt.uibinder.rebind.model.OwnerField; import com.google.gwt.user.client.ui.IsRenderable; import com.google.gwt.user.client.ui.IsWidget; +import com.google.gwt.user.client.ui.RenderableStamper; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -220,6 +221,8 @@ private String gwtPrefix; + private int renderableStamper = 0; + private String rendered; /** * Stack of element variable names that have been attached. @@ -511,6 +514,23 @@ } /** + * Declare a {@link RenderableStamper} instance that will be filled at runtime + * with a unique token. This instance can then be used to stamp a single {@link IsRenderable}. + * + * @return that variable's name. + */ + public String declareRenderableStamper() throws UnableToCompleteException { + String renderableStamperName = "renderableStamper" + renderableStamper++; + FieldWriter domField = fieldManager.registerField(FieldWriterType.RENDERABLE_STAMPER, + oracle.findType(RenderableStamper.class.getName()), renderableStamperName); + domField.setInitializer(formatCode( + "new %s(com.google.gwt.dom.client.Document.get().createUniqueId())", + RenderableStamper.class.getName())); + + return renderableStamperName; + } + + /** * Writes a new SafeHtml template to the generated BinderImpl. * * @return The invocation of the SafeHtml template function with the arguments
diff --git a/user/src/com/google/gwt/user/client/ui/Composite.java b/user/src/com/google/gwt/user/client/ui/Composite.java index 6fbbeb1..779755a 100644 --- a/user/src/com/google/gwt/user/client/ui/Composite.java +++ b/user/src/com/google/gwt/user/client/ui/Composite.java
@@ -93,22 +93,25 @@ } @Override - public final SafeHtml render(String id) { + public final SafeHtml render(RenderableStamper stamper) { if (renderable != null) { - return renderable.render(id); + return renderable.render(stamper); } else { SafeHtmlBuilder builder = new SafeHtmlBuilder(); - render(id, builder); + render(stamper, builder); return builder.toSafeHtml(); } } @Override - public final void render(String id, SafeHtmlBuilder builder) { + @SuppressWarnings("deprecation") + public final void render(RenderableStamper stamper, SafeHtmlBuilder builder) { if (renderable != null) { - renderable.render(id, builder); + renderable.render(stamper, builder); } else { - builder.append(TEMPLATE.renderWithId(id)); + // TODO(rdcastro): Investigate whether SafeHtml or ElementBuilder stamping should be used + // to avoid any performance regressions. + builder.append(TEMPLATE.renderWithId(stamper.getToken())); } }
diff --git a/user/src/com/google/gwt/user/client/ui/IsRenderable.java b/user/src/com/google/gwt/user/client/ui/IsRenderable.java index 15a12be..d4bf0fe 100644 --- a/user/src/com/google/gwt/user/client/ui/IsRenderable.java +++ b/user/src/com/google/gwt/user/client/ui/IsRenderable.java
@@ -42,14 +42,15 @@ void initializeClaimedElement(); /** - * @see #render(String, SafeHtmlBuilder) + * @see #render(RendearbleStamper, SafeHtmlBuilder) * TODO(rdcastro): Remove this once UiBinder doesn't rely on it anymore. */ - SafeHtml render(String id); + SafeHtml render(RenderableStamper stamper); /** * Tells this object to render itself as HTML and append it to the given builder. - * The root element of the HTML must be identifies by the given id. + * If the implementation expects to be able to claim an element later, it must be + * marked by the given stamper. */ - void render(String id, SafeHtmlBuilder builder); + void render(RenderableStamper stamper, SafeHtmlBuilder builder); }
diff --git a/user/src/com/google/gwt/user/client/ui/RenderablePanel.java b/user/src/com/google/gwt/user/client/ui/RenderablePanel.java index 4d26c2f..d3c8857 100644 --- a/user/src/com/google/gwt/user/client/ui/RenderablePanel.java +++ b/user/src/com/google/gwt/user/client/ui/RenderablePanel.java
@@ -186,15 +186,20 @@ } @Override - public SafeHtml render(String id) { + public SafeHtml render(RenderableStamper stamper) { SafeHtmlBuilder builder = new SafeHtmlBuilder(); - render(id, builder); + render(stamper, builder); return builder.toSafeHtml(); } @Override - public void render(String id, SafeHtmlBuilder builder) { + @SuppressWarnings("deprecation") + public void render(RenderableStamper stamper, SafeHtmlBuilder builder) { String styleName = getStyleName(); + + // TODO(rdcastro): Investigate whether SafeHtml or ElementBuilder stamping should be used here + // to avoid any performance regressions. + String id = stamper.getToken(); if (styleName != null) { builder.append(TEMPLATE.renderWithIdAndClass(id, styleName, getInnerHtml())); styleName = null;
diff --git a/user/src/com/google/gwt/user/client/ui/RenderableStamper.java b/user/src/com/google/gwt/user/client/ui/RenderableStamper.java new file mode 100644 index 0000000..d16516c --- /dev/null +++ b/user/src/com/google/gwt/user/client/ui/RenderableStamper.java
@@ -0,0 +1,100 @@ +/* + * Copyright 2011 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.google.gwt.user.client.ui; + +import com.google.gwt.dom.builder.shared.ElementBuilderBase; +import com.google.gwt.dom.client.Document; +import com.google.gwt.dom.client.Element; +import com.google.gwt.safehtml.shared.SafeHtml; +import com.google.gwt.safehtml.shared.SafeHtmlUtils; + +/** + * Used by {@link IsRenderable} to mark their root element in such a way that + * they can be later retrieved. This class abstracts the exact details of how + * the element is marked and retrieved, so that we can always use the best + * method avaialable without having to change all implementations of + * {@link IsRenderable}. + * <p> + * The expected flow is for the {@link IsRenderable} object to use one of the + * {@link #stamp} methods below to mark their HTML. At a later point, its parent + * widget will use the {@link #findStampedElement} to retrieve the right element. + */ +public class RenderableStamper { + + // The token used to stamp IsRenderable objects. + private final String token; + + /** + * Creates a stamper that will be use the given token, which is assumed + * to be unique and will be escaped before being used. + */ + public RenderableStamper(String token) { + this.token = SafeHtmlUtils.htmlEscape(token); + } + + /** + * Finds the element that was previously stamped in the DOM. + * For this to work properly the element must be attached to the document. + */ + public Element findStampedElement() { + // TODO(rdcastro): Add a DEV-only check to make sure the element is attached. + return Document.get().getElementById(token); + } + + /** + * Exposes the token used for stamping {@link IsRenderable} objects. + * @deprecated This method is born deprecated, and should not be used in new code. + */ + @Deprecated + public String getToken() { + return token; + } + + /** + * Stamps an HTML element in such a way that it can be later found in the DOM tree. + * To be used by {@link IsRenderable} objects built using {@link SafeHtml} directly, this assumes + * the element to be stamped is the first found in the given {@link SafeHtml}. + * Returns safeHtml untouched if it does not being with a tag. + */ + public SafeHtml stamp(SafeHtml safeHtml) { + String html = safeHtml.asString().trim(); + if (!html.startsWith("<")) { + return safeHtml; + } + + int endOfFirstTag = html.indexOf('>'); + // TODO(rdcastro): Maybe add a DEV-only check to make sure endOfFirstTag != -1 + if (html.charAt(endOfFirstTag - 1) == '/') { + endOfFirstTag--; + } + StringBuilder htmlBuilder = new StringBuilder() + .append(html.substring(0, endOfFirstTag)) + .append(" id ='") + .append(token) + .append("'") + .append(html.substring(endOfFirstTag)); + return SafeHtmlUtils.fromTrustedString(htmlBuilder.toString()); + } + + /** + * Stamps an HTML element in such a way that it can be later found in the DOM tree. + * To be used by {@link IsRenderable} objects built using ElementBuilder, this assumes + * the given elementBuilder is for the root element that should later be claimed. + */ + public void stamp(ElementBuilderBase<?> elementBuilder) { + elementBuilder.id(token); + } +}