[gwt-contrib] Re: EnumOrdinalizer cleanup (issue1426804)
LGTM + nits (I've run tests on some big compiles, and ordinalization results look good) http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java File dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java (right): http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java#newcode195 dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java:195: /* This comment is no longer true, and can be removed http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java#newcode671 dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java:671: "}"); Is there a reason for this change? I've tested with and without this change, and tests pass. http://gwt-code-reviews.appspot.com/1426804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: EnumOrdinalizer cleanup (issue1426804)
http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java File dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java (right): http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode444 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:444: maybe add a test for this case in EnumOrdinalizerTest http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode646 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:646: * qualifying instance. This is ok, I think, since it is safe to assume that the endVisit(JFieldRef,...) and endVisit(JMethodCall,..) methods below will always be called after their instance expressions have been visited by TypeRemapper, or by one of the visit() methods below. This seems born out by JFieldRef.traverse() and JMethodCall.traverse()Make sense? http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode653 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:653: Can this comment be worded a bit more clearly? How about: "Replace any references to an enum ordinal field with the qualifying instance, if that instance has been ordinalized (e.g. change 4.ordinal with 4). http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode659 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:659: public void endVisit(JFieldRef x, Context ctx) { This super method call won't actually do anything in this case, but I assume you are adding it for future code-safety, should TypeRemapper choose to override it later? (same for the endVisit(JMethodCall,...) case below). http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode670 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:670: ditto http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode676 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:676: public void endVisit(JMethodCall x, Context ctx) { ditto http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode793 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:793: ++removeIndex; Feel free, looks good. Maybe also add in the tests I had for testing an empty/unused enum. http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java File dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java (right): http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java#newcode69 dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java:69: x.setType(modRemap(x.getType())); Hmmm, I'm not sure this should apply to x.getResultTypes(). For a particular permutation, I would think the remapping would apply equally to generated code, and so if one of the resultTypes needs to be remapped it would happen after the generators have run for a particular permutation. If it's called prior to the ReplaceRebinds visitor, it would be necessary to change mappings for which type to generate for which permutation, and this implies affecting the generate-with rules, etc. http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java File dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java (right): http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java#newcode86 dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java:86: public CharSequence getContent() { This comment no longer applies? http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java File dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java (right): http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java#newcode22 dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java:22: /** add tests for enums used in an instanceof, and for empty enums. can leave as a TODO and I can take a look later. http://gwt-code-reviews.appspot.com/1426804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing setInnerHTML calls on attach/detach sections. (issue1422811)
LGTM On 2011/04/26 23:35:28, hermes wrote: http://gwt-code-reviews.appspot.com/1422811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] GWT SDK 2.3.0.RC1
Hey GWTC folks, We have a GWT SDK 2.3.0.RC1 build that we would love feedback on. A big change since M1 is the move of AutoBean and RequestFactory to a new package, com.google.web.bindery. The old locations of AutoBean and RequestFactory should still work, but are deprecated. Fixing the deprecation warnings for the most part should be as simple as changing some import lines. If early adopters could verify that assumption, we would be grateful. The RC1 download can be found here: http://code.google.com/p/google-web-toolkit/downloads/detail?name=gwt-2.3.0.rc1.zip -- Chris/Ray, on behalf of the GWT team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding drag and drop support to the mobile web app. The desktop TaskEditView now has a list of t... (issue1420811)
http://gwt-code-reviews.appspot.com/1420811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] GWT 2.2.1: SimplePager fix
Hi, com.google.gwt.user.cellview.client.SimplePager doesn't disable the "fastForward" button in "setFastForwardDisabled" so you can *always* page forward many rows leading to display inconsistencies. Original code:- private void setFastForwardDisabled(boolean disabled) { if (fastForward == null) { return; } if (disabled) { fastForward.setResource(resources.simplePagerFastForwardDisabled()); fastForward.getElement().getParentElement().addClassName( style.disabledButton()); } else { fastForward.setResource(resources.simplePagerFastForward()); fastForward.getElement().getParentElement().removeClassName( style.disabledButton()); } } Patched code:- private void setFastForwardDisabled(boolean disabled) { if (fastForward == null) { return; } if (disabled) { fastForward.setResource(resources.simplePagerFastForwardDisabled()); fastForward.getElement().getParentElement().addClassName( style.disabledButton()); } else { fastForward.setResource(resources.simplePagerFastForward()); fastForward.getElement().getParentElement().removeClassName( style.disabledButton()); } //---> Fix fastForward.setDisabled( disabled ); } With kind regards, Mike -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fixing setInnerHTML calls on attach/detach sections. (issue1422811)
Reviewers: rdcastro, rjrjr, Description: Fixing setInnerHTML calls on attach/detach sections. Please review this at http://gwt-code-reviews.appspot.com/1422811/ Affected files: M user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java M user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java Index: user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java === --- user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java (revision 10081) +++ user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java (working copy) @@ -182,14 +182,16 @@ if (uiWriter.useLazyWidgetBuilders()) { if (idIsHasText.contains(idHolder)) { -fieldManager.require(childField).addAttachStatement( -"%s.setText(%s.getElementById(%s).getInnerText());", childField, -fieldManager.convertFieldToGetter(fieldName), +fieldManager.require(fieldName).addAttachStatement( +"%s.setText(%s.getElementById(%s).getInnerText());", +fieldManager.convertFieldToGetter(childField), +fieldName, fieldManager.convertFieldToGetter(idHolder)); } else if (idIsHasHTML.contains(idHolder)) { -fieldManager.require(childField).addAttachStatement( -"%s.setHTML(%s.getElementById(%s).getInnerHTML());", childField, -fieldManager.convertFieldToGetter(fieldName), +fieldManager.require(fieldName).addAttachStatement( +"%s.setHTML(%s.getElementById(%s).getInnerHTML());", +fieldManager.convertFieldToGetter(childField), +fieldName, fieldManager.convertFieldToGetter(idHolder)); } } else { Index: user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java === --- user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java (revision 10081) +++ user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java (working copy) @@ -274,6 +274,13 @@ } w.newline(); +// If we forced an attach, we should always detach, regardless of whether +// there are any detach statements. +if (attachedVar != null) { + w.write("// Detach section."); + w.write("%s.detach();", attachedVar); +} + if (detachStatements.size() > 0) { if (isAttachable) { w.write("%s.detachedInitializationCallback = ", getName()); @@ -283,14 +290,12 @@ w.outdent(); w.write("@Override public void execute() {"); w.indent(); - } else if (attachedVar != null) { -w.write("// Detach section."); -w.write("%s.detach();", attachedVar); } for (String s : detachStatements) { w.write(s); } + if (isAttachable) { w.outdent(); w.write("}"); -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Using the Editor framework to edit tasks in the MobileWebApp sample. The DateButton widget is li... (issue1425808)
http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/build.xml File samples/mobilewebapp/build.xml (left): http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/build.xml#oldcode12 samples/mobilewebapp/build.xml:12: On 2011/04/26 21:27:37, rchandia wrote: Actually this validation jar entry is not used. My bad. Removed. http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/build.xml File samples/mobilewebapp/build.xml (right): http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/build.xml#newcode16 samples/mobilewebapp/build.xml:16: On 2011/04/26 21:27:37, rchandia wrote: These entries do not seem to have an effect on the build. Probably because MobileWebApp sample is being compiled as "source" so no libs are ever copied. Removed. http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java File samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java (right): http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java#newcode125 samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java:125: } On 2011/04/26 21:27:37, rchandia wrote: Can we make failures more user friendly? Handle failure, advise of problem, navigate to Task List place Done. http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java#newcode166 samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java:166: } On 2011/04/26 21:27:37, rchandia wrote: onFailure()? Done. http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java#newcode203 samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java:203: private void handleConstraintViolations(Set> violations) { On 2011/04/26 21:52:52, bobv wrote: Can you use EditorDriver.setConstraintViolations() to do this instead? This would be more in keeping with how larger UIs would be validated. The editor(s) over in TaskEditView would need to implement HasEditorErrors in order to receive the EditorErrors, but you can get away from having this parse-and-dispatch logic here. Done. Pretty cool, I didn't know about that. http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java File samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java (right): http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java#newcode130 samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java:130: // TODO(jlabanca): Record and show a help video tutorial. On 2011/04/26 21:52:52, bobv wrote: Embed a YouTube video? The goal is to show how to use the HTML5 video element. http://gwt-code-reviews.appspot.com/1425808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Using the Editor framework to edit tasks in the MobileWebApp sample. The DateButton widget is li... (issue1425808)
http://gwt-code-reviews.appspot.com/1425808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing a bug in CustomScrollPanel that leads to an infinite loop in Safari 3 because the browser... (issue1421806)
committed as r10085 http://gwt-code-reviews.appspot.com/1421806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: SafeHtmlRenderer code gen for UiBinder (issue1426803)
Turns out the useLazyWidget stuff isn't passing all of the UiBinder tests yet. Ignoring that path for now seems reasonable. Sorry for the flip flop. On Mon, Apr 25, 2011 at 3:19 PM, wrote: > Oh, the base class exists already: > > com.google.gwt.text.shared.AbstractSafeHtmlRenderer > > > http://gwt-code-reviews.appspot.com/1426803/ > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10085 committed - Fixing a bug in CustomScrollPanel that leads to an infinite loop in Sa...
Revision: 10085 Author: jlaba...@google.com Date: Tue Apr 26 12:54:13 2011 Log: Fixing a bug in CustomScrollPanel that leads to an infinite loop in Safari 3 because the browser updates the scroll position of the outer element, which triggers a scroll event, which repeats the loop. We now check the scroll positions before updating them. Review at http://gwt-code-reviews.appspot.com/1421806 http://code.google.com/p/google-web-toolkit/source/detail?r=10085 Modified: /trunk/user/src/com/google/gwt/user/client/ui/CustomScrollPanel.java === --- /trunk/user/src/com/google/gwt/user/client/ui/CustomScrollPanel.java Tue Apr 26 05:06:08 2011 +++ /trunk/user/src/com/google/gwt/user/client/ui/CustomScrollPanel.java Tue Apr 26 12:54:13 2011 @@ -527,12 +527,16 @@ } /* - * Ensure that the viewport is anchored to the corner. If the user click and - * drags the content, its possible to shift the viewport and reveal the + * Ensure that the viewport is anchored to the corner. If the user clicks + * and drags the content, its possible to shift the viewport and reveal the * hidden scrollbars. */ -getElement().setScrollLeft(0); -getElement().setScrollTop(0); +if (getElement().getScrollLeft() != 0) { + getElement().setScrollLeft(0); +} +if (getElement().getScrollTop() != 0) { + getElement().setScrollTop(0); +} } /** -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10084 committed - This fix a bug in WidgetPlaceholderInterpreter (missing convertField)...
Revision: 10084 Author: her...@google.com Date: Tue Apr 26 12:50:53 2011 Log: This fix a bug in WidgetPlaceholderInterpreter (missing convertField) and create a new FieldWriterType to substitute the ugly setBuildPrecedence(). It also make LazyDomElement used in more places. This saved a few bytes. Review at http://gwt-code-reviews.appspot.com/1420810 http://code.google.com/p/google-web-toolkit/source/detail?r=10084 Modified: /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java === --- /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java Tue Apr 26 11:22:45 2011 +++ /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java Tue Apr 26 12:50:53 2011 @@ -26,6 +26,7 @@ import com.google.gwt.uibinder.attributeparsers.BundleAttributeParser; import com.google.gwt.uibinder.attributeparsers.BundleAttributeParsers; import com.google.gwt.uibinder.client.UiBinder; +import com.google.gwt.uibinder.client.UiBinderUtil.LazyDomElement; import com.google.gwt.uibinder.elementparsers.AttributeMessageParser; import com.google.gwt.uibinder.elementparsers.BeanParser; import com.google.gwt.uibinder.elementparsers.ElementParser; @@ -370,13 +371,11 @@ String name = declareDomIdHolder(); if (useLazyWidgetBuilders) { - // Initialize and add the removeAttribute('id') statement for the new - // DOM field. + // Create and initialize the dom field with LazyDomElement. FieldWriter field = fieldManager.require(fieldName); - field.setInitializer(formatCode( - "com.google.gwt.dom.client.Document.get().getElementById(%s).cast()", + field.setInitializer(formatCode("new %s(%s).get().cast()", + LazyDomElement.class.getCanonicalName(), fieldManager.convertFieldToGetter(name))); - field.addStatement("%s.removeAttribute(\"id\");", fieldName); // The dom must be created by its ancestor. fieldManager.require(ancestorField).addAttachStatement( -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Handle SafeHtml as return type in ui:text (issue1409802)
Still need to add some testing, but the other comments have been addressed. http://gwt-code-reviews.appspot.com/1409802/diff/3001/user/src/com/google/gwt/uibinder/elementparsers/ComputedAttributeInterpreter.java File user/src/com/google/gwt/uibinder/elementparsers/ComputedAttributeInterpreter.java (right): http://gwt-code-reviews.appspot.com/1409802/diff/3001/user/src/com/google/gwt/uibinder/elementparsers/ComputedAttributeInterpreter.java#newcode62 user/src/com/google/gwt/uibinder/elementparsers/ComputedAttributeInterpreter.java:62: if (att.getName().equals("from")) { On 2011/04/13 19:43:50, rjrjr wrote: We can't make a magic attribute name like this. Agreed. The problem is composed attributes, like "{foo.name} {bar.name}". Composed attributes like this won't have a return type of SafeHtml, so I added a check for composed attributes which defaults to String as the return type. http://gwt-code-reviews.appspot.com/1409802/diff/3001/user/src/com/google/gwt/uibinder/elementparsers/ComputedAttributeInterpreter.java#newcode66 user/src/com/google/gwt/uibinder/elementparsers/ComputedAttributeInterpreter.java:66: if (returnValue.equals("SafeHtml")) { On 2011/04/13 19:43:50, rjrjr wrote: Should be using JType, not a string. This logic should be in a delegate passed in to the CAI instance, or at least a protected method that subclass can override. See more detailed notes in UiTextInterpreter Done. http://gwt-code-reviews.appspot.com/1409802/diff/3001/user/src/com/google/gwt/uibinder/elementparsers/UiTextInterpreter.java File user/src/com/google/gwt/uibinder/elementparsers/UiTextInterpreter.java (right): http://gwt-code-reviews.appspot.com/1409802/diff/3001/user/src/com/google/gwt/uibinder/elementparsers/UiTextInterpreter.java#newcode27 user/src/com/google/gwt/uibinder/elementparsers/UiTextInterpreter.java:27: public class UiTextInterpreter implements XMLElement.Interpreter { On 2011/04/13 19:43:50, rjrjr wrote: This thing is a hack, relying on ComputedAttributeInterpreter to have run first, and doing nothing to verify that fact. Instead HTMLInterpreter and TextInterpreter should re-order things to run UiTextInterpreter first. Further, UiTextInterpreter should wrap its own instance of ComputedAttributeInterpreter to do its dirty work — when a UiTextInterpreter finishes, there should be nothing left for a downstream ComputedAttributeInterpreter to do. First, call XMLAttribute#hasComputedValue and barf if it's false. Refactor ComputedAttributeInterpreter to accept a delegate that makes the writer call, and which is given the returnType. If the returnType is not String, barf. Now make a separate UiTextInHtmlInterpreter, perhaps a subclass of this one, to be used by HtmlInterpreter. It should also accept a SafeHtml return type, and should make the decision to call tokenForSafeHtmlMethod or tokenForStringExpression. Done; let me know if this is not what you had in mind. http://gwt-code-reviews.appspot.com/1409802/diff/3001/user/src/com/google/gwt/uibinder/rebind/XMLAttribute.java File user/src/com/google/gwt/uibinder/rebind/XMLAttribute.java (right): http://gwt-code-reviews.appspot.com/1409802/diff/3001/user/src/com/google/gwt/uibinder/rebind/XMLAttribute.java#newcode38 user/src/com/google/gwt/uibinder/rebind/XMLAttribute.java:38: public String computedReturnValue(FieldManager fieldManager, On 2011/04/13 19:43:50, rjrjr wrote: JType getComputedValueType, return null if this is not a computed value. Done. http://gwt-code-reviews.appspot.com/1409802/diff/3001/user/src/com/google/gwt/uibinder/rebind/XMLAttribute.java#newcode41 user/src/com/google/gwt/uibinder/rebind/XMLAttribute.java:41: String[] parts = attrValue.substring(1, attrValue.length() - 1).split("\\."); On 2011/04/13 19:43:50, rjrjr wrote: This split logic already exists in FieldReference, keep it there. Make a new method on FieldManager that looks up an existing FieldReference, or creates a temporary one if need be, and returns FieldReference#findReturnType. Let FieldManager do the logging and you won't have to pass in a logger. Done. http://gwt-code-reviews.appspot.com/1409802/diff/3001/user/src/com/google/gwt/uibinder/rebind/XMLAttribute.java#newcode46 user/src/com/google/gwt/uibinder/rebind/XMLAttribute.java:46: return type.getSimpleSourceName(); On 2011/04/13 19:43:50, rjrjr wrote: return type, not a string. Done. http://gwt-code-reviews.appspot.com/1409802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Handle SafeHtml as return type in ui:text (issue1409802)
http://gwt-code-reviews.appspot.com/1409802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fixing a bug in CustomScrollPanel that leads to an infinite loop in Safari 3 because the browser... (issue1421806)
Reviewers: Frank, Description: Fixing a bug in CustomScrollPanel that leads to an infinite loop in Safari 3 because the browser updates the scroll position of the outer element, which triggers a scroll event, which repeats the loop. We now check the scroll positions before updating them. Please review this at http://gwt-code-reviews.appspot.com/1421806/ Affected files: M user/src/com/google/gwt/user/client/ui/CustomScrollPanel.java Index: user/src/com/google/gwt/user/client/ui/CustomScrollPanel.java === --- user/src/com/google/gwt/user/client/ui/CustomScrollPanel.java (revision 10081) +++ user/src/com/google/gwt/user/client/ui/CustomScrollPanel.java (working copy) @@ -527,12 +527,16 @@ } /* - * Ensure that the viewport is anchored to the corner. If the user click and - * drags the content, its possible to shift the viewport and reveal the + * Ensure that the viewport is anchored to the corner. If the user clicks + * and drags the content, its possible to shift the viewport and reveal the * hidden scrollbars. */ -getElement().setScrollLeft(0); -getElement().setScrollTop(0); +if (getElement().getScrollLeft() != 0) { + getElement().setScrollLeft(0); +} +if (getElement().getScrollTop() != 0) { + getElement().setScrollTop(0); +} } /** -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds some diagnostics to an exception thrown in CompiledClass. (issue1425810)
To recap f2f discussion, there are 2 issues here: - The first is the problem I was trying to patch - in that ClassFormatException are not handled the same way in findType(). - The second issue is that the app I was running actually has 2 modules loading in hosted mode. We speculate the second time through, hosted mode is taking a different code path in findType(), trying to getNameEnvironmentAnswer() out of bytecode (the first time it might have pulled it directly out of JDT data strutures.) http://gwt-code-reviews.appspot.com/1425810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: This fix a bug in WidgetPlaceholderInterpreter (missing convertField) (issue1420810)
LGTM http://gwt-code-reviews.appspot.com/1420810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: This fix a bug in WidgetPlaceholderInterpreter (missing convertField) (issue1420810)
LGTM http://gwt-code-reviews.appspot.com/1420810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: This fix a bug in WidgetPlaceholderInterpreter (missing convertField) (issue1420810)
On 2011/04/26 22:11:30, hermes wrote: Yes, Rafa won the race! The bug is already fixed and FieldWriterType doesn't bring any big gain. So I reverted the most of the files. The only change here is the use of LazyDomElement to make all places using the same pattern. http://gwt-code-reviews.appspot.com/1420810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: This fix a bug in WidgetPlaceholderInterpreter (missing convertField) (issue1420810)
http://gwt-code-reviews.appspot.com/1420810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Using the Editor framework to edit tasks in the MobileWebApp sample. The DateButton widget is li... (issue1425808)
http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java File samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java (right): http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java#newcode203 samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java:203: private void handleConstraintViolations(Set> violations) { Can you use EditorDriver.setConstraintViolations() to do this instead? This would be more in keeping with how larger UIs would be validated. The editor(s) over in TaskEditView would need to implement HasEditorErrors in order to receive the EditorErrors, but you can get away from having this parse-and-dispatch logic here. http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java File samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java (right): http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java#newcode130 samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java:130: // TODO(jlabanca): Record and show a help video tutorial. Embed a YouTube video? http://gwt-code-reviews.appspot.com/1425808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Using the Editor framework to edit tasks in the MobileWebApp sample. The DateButton widget is li... (issue1425808)
http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java File samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java (right): http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java#newcode113 samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java:113: @Override Disregard. I was compiling against stale GWT binaries. On 2011/04/26 21:27:37, rchandia wrote: This fails to compile with ant. It is because user-build.xml is using Java 1.5 bytecode generation. We are deprecating Java 1.5 (and this is an AppEngine app, after all) so it would be best to modify user-build.xml to use Java 1.6. http://gwt-code-reviews.appspot.com/1425808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10082 committed - create javadoc directory for GWT 2.3 release
Revision: 10082 Author: mrruss...@google.com Date: Tue Apr 26 14:40:57 2011 Log: create javadoc directory for GWT 2.3 release http://code.google.com/p/google-web-toolkit/source/detail?r=10082 Added: /javadoc/2.3 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10081 committed - Introduce the Attachable interface, and add support to UiBinder's HTML...
Revision: 10081 Author: rdcas...@google.com Date: Tue Apr 26 11:22:45 2011 Log: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. Review at http://gwt-code-reviews.appspot.com/1426805 http://code.google.com/p/google-web-toolkit/source/detail?r=10081 Added: /trunk/user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java /trunk/user/src/com/google/gwt/uibinder/elementparsers/AttachableInterpreter.java /trunk/user/src/com/google/gwt/user/client/ui/Attachable.java /trunk/user/src/com/google/gwt/user/client/ui/AttachableComposite.java /trunk/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java Modified: /trunk/user/src/com/google/gwt/uibinder/elementparsers/InterpreterPipe.java /trunk/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java /trunk/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java /trunk/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java /trunk/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java /trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java === --- /dev/null +++ /trunk/user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java Tue Apr 26 11:22:45 2011 @@ -0,0 +1,107 @@ +/* + * 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.uibinder.elementparsers; + +import com.google.gwt.core.ext.UnableToCompleteException; +import com.google.gwt.core.ext.typeinfo.JClassType; +import com.google.gwt.uibinder.elementparsers.HtmlMessageInterpreter.PlaceholderInterpreterProvider; +import com.google.gwt.uibinder.rebind.UiBinderWriter; +import com.google.gwt.uibinder.rebind.XMLElement; +import com.google.gwt.uibinder.rebind.messages.MessageWriter; +import com.google.gwt.uibinder.rebind.messages.PlaceholderInterpreter; + +/** + * Parses {@link com.google.gwt.user.client.ui.AttachableHTMLPanel} widgets. + */ +public class AttachableHTMLPanelParser implements ElementParser { + + public void parse(XMLElement elem, String fieldName, JClassType type, + final UiBinderWriter writer) throws UnableToCompleteException { + +assert writer.useLazyWidgetBuilders(); +writer.getFieldManager().lookup(fieldName) +.setAttachable(true); + +/* + * Gathers up elements that indicate nested Attachable objects. + */ +AttachableInterpreter attachableInterpreter = null; +attachableInterpreter = new AttachableInterpreter( +fieldName, writer); + +/* + * Gathers up elements that indicate nested widgets (but only those that are + * not inside msg elements). + */ +WidgetInterpreter widgetInterpreter = new WidgetInterpreter(fieldName, +writer); + +/* + * Handles non-widget elements like msg, and dom elements with ui:field + * attributes. There may be widgets inside a msg, which is why the + * construction in makeHtmlInterpreter is so complicated. + */ +HtmlInterpreter htmlInterpreter = makeHtmlInterpreter(fieldName, writer); + +writer.beginAttachedSection(fieldName); + +final InterpreterPipe interpreters; +interpreters = InterpreterPipe.newPipe( +attachableInterpreter, widgetInterpreter, htmlInterpreter); +String html = elem.consumeInnerHtml(interpreters); + +writer.endAttachedSection(); + +/* + * AttachableHTMLPanel has no no-arg ctor, so we have to generate our own, using the + * element's innerHTML and perhaps its tag attribute. Do this in a way that + * will not break subclasses if they happen to have the same constructor + * signature (by passing in type). + */ +String customTag = elem.consumeStringAttribute("tag", null); + +// TODO(rdcastro): Add support for custom tags in AttachableHTMLPanel. +if (customTag != null) { + writer.getLogger().die( + "AttachableHTMLPanel does not support custom root elements yet."); +} + +writer.setFieldInitializerAsConstructor(fieldName, type, writer.declareTemplateCall(html)); + } + + /** + * Creates an HtmlInterpreter with our specialized placeholder interpreter, + * which will allow widget instances to be declared inside of ui:msg elements. + */ + private HtmlInterpreter makeHtmlInterpreter(final String fieldName, + final UiBinderWriter uiWriter) { +final S
[gwt-contrib] Re: Adds some diagnostics to an exception thrown in CompiledClass. (issue1425810)
http://gwt-code-reviews.appspot.com/1425810/diff/1/dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java File dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java (right): http://gwt-code-reviews.appspot.com/1425810/diff/1/dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java#newcode309 dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java:309: CompilationProblemReporter.logMissingTypeErrorWithHints(logger, InternalName On 2011/04/26 21:07:53, scottb wrote: Can you explain? This seems a very surprising choice for dealing with ClassFormatException. I know from looking at this error that the problem was fixed by adding back a missing source file. I could have just printed out this unit. Interestingly it is only dev mode that had a problem. Compiling in web mode seemed to work fine. Utimately, I think the best solution might be to allow the exception to be simply ignored, as it is below http://gwt-code-reviews.appspot.com/1425810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Using the Editor framework to edit tasks in the MobileWebApp sample. The DateButton widget is li... (issue1425808)
http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/build.xml File samples/mobilewebapp/build.xml (left): http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/build.xml#oldcode12 samples/mobilewebapp/build.xml:12: Actually this validation jar entry is not used. My bad. http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/build.xml File samples/mobilewebapp/build.xml (right): http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/build.xml#newcode16 samples/mobilewebapp/build.xml:16: These entries do not seem to have an effect on the build. Probably because MobileWebApp sample is being compiled as "source" so no libs are ever copied. http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java File samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java (right): http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java#newcode113 samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java:113: @Override This fails to compile with ant. It is because user-build.xml is using Java 1.5 bytecode generation. We are deprecating Java 1.5 (and this is an AppEngine app, after all) so it would be best to modify user-build.xml to use Java 1.6. http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java#newcode125 samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java:125: } Can we make failures more user friendly? Handle failure, advise of problem, navigate to Task List place http://gwt-code-reviews.appspot.com/1425808/diff/1/samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java#newcode166 samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/activity/TaskEditActivity.java:166: } onFailure()? http://gwt-code-reviews.appspot.com/1425808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: EnumOrdinalizer cleanup (issue1426804)
Updated based on feedback. http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java File dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java (left): http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#oldcode865 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:865: } Without the whole 'ignored cast operations' this part isn't needed. http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#oldcode902 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:902: } Ditto. http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java File dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java (right): http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode646 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:646: * qualifying instance. Merged in the other visitor. http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode793 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:793: ++removeIndex; Went ahead and merged in your change too. I can revert this part if you'd rather submit yours. http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java File dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java (right): http://gwt-code-reviews.appspot.com/1426804/diff/3002/dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java#newcode101 dev/core/src/com/google/gwt/dev/jjs/impl/TypeRemapper.java:101: return result; Implemented the madeChanges() as you mentioned. http://gwt-code-reviews.appspot.com/1426804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding drag and drop support to the mobile web app. The desktop TaskEditView now has a list of t... (issue1420811)
On Tue, Apr 26, 2011 at 4:57 PM, Jeff Larsen wrote: > Drag n Drop doesn't work in ie8 (expected). Perhaps use deferred binding to > get rid of the templates portion for all versions of ie<9. Otherwise > those templates are pretty useless. DragEvent can implement PartialSupport and provide a static isSupported() method so we can check if drag and drop is supported. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds some diagnostics to an exception thrown in CompiledClass. (issue1425810)
High-level, the intent is that the JdtCompiler has only two failure modes. Either there is an error with the input, in which case we report a compilation problem on the unit itself, or else we encounter an error so severe that we terminate. I would have thought this would fall squarely into the latter. A less invasive change would be to add more information onto the RuntimeException detail message, would this give you what you need to diagnose the problem? http://gwt-code-reviews.appspot.com/1425810/diff/1/dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java File dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java (right): http://gwt-code-reviews.appspot.com/1425810/diff/1/dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java#newcode309 dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java:309: CompilationProblemReporter.logMissingTypeErrorWithHints(logger, InternalName Can you explain? This seems a very surprising choice for dealing with ClassFormatException. http://gwt-code-reviews.appspot.com/1425810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10080 committed - remove bad copy
Revision: 10080 Author: mrruss...@google.com Date: Tue Apr 26 14:00:10 2011 Log: remove bad copy http://code.google.com/p/google-web-toolkit/source/detail?r=10080 Deleted: /javadoc/2.3 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds some diagnostics to an exception thrown in CompiledClass. (issue1425810)
Another possibility to consider top deal with this issue is to just catch the ClassFormatException and return null. That's exactly what is done at the bottom of JdtCompiler.INameEnvironment.findType(char[][]) http://gwt-code-reviews.appspot.com/1425810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds cache of CollectClassData to make refresh faster. (issue1420809)
http://gwt-code-reviews.appspot.com/1420809/diff/5001/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java File dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java (right): http://gwt-code-reviews.appspot.com/1420809/diff/5001/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java#newcode464 dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java:464: Map values = Maps.newHashMap(annotData.getValues()); Okay, gotcha. Please add a comment "Make a copy before we mutate the collection". Also, please just use new HashMap() to follow the existing GWT style. If we think there's unique value in heavily utilizing guava for this, that should be a larger discussion. http://gwt-code-reviews.appspot.com/1420809/diff/5001/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java#newcode626 dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java:626: private CollectClassData processClass(TypeData typeData) { +1 http://gwt-code-reviews.appspot.com/1420809/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10079 committed - copy the javadocs
Revision: 10079 Author: mrruss...@google.com Date: Tue Apr 26 13:57:14 2011 Log: copy the javadocs http://code.google.com/p/google-web-toolkit/source/detail?r=10079 Added: /javadoc/2.3/2.2 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding drag and drop support to the mobile web app. The desktop TaskEditView now has a list of t... (issue1420811)
Drag n Drop doesn't work in ie8 (expected). Perhaps use deferred binding to get rid of the templates portion for all versions of ie<9. Otherwise those templates are pretty useless. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds cache of CollectClassData to make refresh faster. (issue1420809)
http://gwt-code-reviews.appspot.com/1420809/diff/5001/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java File dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java (right): http://gwt-code-reviews.appspot.com/1420809/diff/5001/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java#newcode626 dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java:626: private CollectClassData processClass(TypeData typeData) { Make this a method on TypeData: getCollectClassData()? http://gwt-code-reviews.appspot.com/1420809/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds some diagnostics to an exception thrown in CompiledClass. (issue1425810)
This patch deserves some explanation: I saw a case where the ClassFormatException that was formally caught in CompiledClass was emitted from the compiler. Adding some missing source to the source path fixed the problem. I was unable to reproduce the problem in a small example. I'm proposing this patch because I'm not sure how to prevent the invalid bytecode from being created in the first place. Instead, the patch prints out as much relevant information as possible so that if missing source is the problem, the developer has a clue of how to fix it. I am not sure why, but JdtCompiler did not contain any references to TreeLogger before this point. I feel there may be a design reason for that http://gwt-code-reviews.appspot.com/1425810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Adds some diagnostics to an exception thrown in CompiledClass. (issue1425810)
Reviewers: scottb, jbrosenberg, tobyr, Description: Adds some diagnostics to an exception thrown in CompiledClass. Please review this at http://gwt-code-reviews.appspot.com/1425810/ Affected files: M dev/core/src/com/google/gwt/dev/javac/CompilationProblemReporter.java M dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java M dev/core/src/com/google/gwt/dev/javac/CompiledClass.java M dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10078 committed - new directory for 2.3 javadocs
Revision: 10078 Author: mrruss...@google.com Date: Tue Apr 26 13:27:11 2011 Log: new directory for 2.3 javadocs http://code.google.com/p/google-web-toolkit/source/detail?r=10078 Added: /javadoc/2.3 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: EnumOrdinalizer cleanup & better interop. (issue1426804)
Based on your comments, I'm going to rework the patch. You're right, I was totally confused about the difference between ReplaceEnumTypesWithInteger & ReplaceOrdinalFieldAndMethodRefsWithOrdinal. I think the JField / JFieldRef replacements in the former are what threw me. I'll rework the patch. By valid state of the AST, I had run into a case in some work I was doing where you had by analogy the equivalent of EnumType x = 3, and TypeTightener was needing to clean up after. But I think now that what I needed to do in my patch was update TypeRemapper instead. Lemme rework this and see what falls out. On Tue, Apr 26, 2011 at 3:12 PM, wrote: > Scott, would you mind, for my own edification, outlining how this > improves the "valid state" of the AST after EnumOrdinalizer runs? > Thanks. > > > http://gwt-code-reviews.appspot.com/1426804/ > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Adding drag and drop support to the mobile web app. The desktop TaskEditView now has a list of t... (issue1420811)
Reviewers: rice, Description: Adding drag and drop support to the mobile web app. The desktop TaskEditView now has a list of templates. Users can drag a template over the edit form to populate the fields. I found a few holes in the drag and drop API, which are also fixed in this patch. I added the missing DragStartEvent, DragEvent, and DragEndEvent, which are part of the HTML5 spec. Also, the native dragexit event is deprecated and only used by older versions of firefox, so I switch the DragExitEvent to use the correct event type "dragleave". I modified sinkEvents to handle the new bitless drag event types. I also added the DataTransfer JSO, which is required to actually use drag and drop (I didn't implement the entire API until I have more time to test the other methods in DataTransfer, but the important ones are there). I updated DragAndDropEventsSinkTest to use a WidgetCreator instead of duplicating code all over the place. Also, we don't need to test every subclass of FocusWidget, since FocusWidget itself adds the drag handlers. Demo at http://gwt-cloudtasks.appspot.com/. Note that in IE9, you have to select text before you can trigger a drag event, and you must drop directly over the input boxes in the form. In other browsers, this is not the case. Please review this at http://gwt-code-reviews.appspot.com/1420811/ Affected files: A samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/DesktopTaskEditView.css M samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/DesktopTaskEditView.java M samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/DesktopTaskEditView.ui.xml M samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/client/desktop/MobileWebAppShellDesktop.java M samples/mobilewebapp/src/com/google/gwt/sample/mobilewebapp/shared/TaskProxyImpl.java M user/src/com/google/gwt/dom/client/DOMImpl.java M user/src/com/google/gwt/dom/client/DOMImplWebkit.java A user/src/com/google/gwt/dom/client/DataTransfer.java M user/src/com/google/gwt/dom/client/Element.java M user/src/com/google/gwt/dom/client/NativeEvent.java A user/src/com/google/gwt/event/dom/client/DragEndEvent.java A user/src/com/google/gwt/event/dom/client/DragEndHandler.java M user/src/com/google/gwt/event/dom/client/DragEnterEvent.java A user/src/com/google/gwt/event/dom/client/DragEvent.java A user/src/com/google/gwt/event/dom/client/DragEventBase.java M user/src/com/google/gwt/event/dom/client/DragExitEvent.java A user/src/com/google/gwt/event/dom/client/DragHandler.java M user/src/com/google/gwt/event/dom/client/DragOverEvent.java A user/src/com/google/gwt/event/dom/client/DragStartEvent.java A user/src/com/google/gwt/event/dom/client/DragStartHandler.java M user/src/com/google/gwt/event/dom/client/DropEvent.java M user/src/com/google/gwt/event/dom/client/HasAllDragAndDropHandlers.java A user/src/com/google/gwt/event/dom/client/HasDragEndHandlers.java A user/src/com/google/gwt/event/dom/client/HasDragHandlers.java A user/src/com/google/gwt/event/dom/client/HasDragStartHandlers.java M user/src/com/google/gwt/user/cellview/client/CellBasedWidgetImpl.java M user/src/com/google/gwt/user/client/impl/DOMImplIE9.java M user/src/com/google/gwt/user/client/impl/DOMImplMozilla.java M user/src/com/google/gwt/user/client/impl/DOMImplStandard.java M user/src/com/google/gwt/user/client/ui/FocusPanel.java M user/src/com/google/gwt/user/client/ui/FocusWidget.java M user/src/com/google/gwt/user/client/ui/HTMLTable.java M user/src/com/google/gwt/user/client/ui/Image.java M user/src/com/google/gwt/user/client/ui/Label.java M user/src/com/google/gwt/user/client/ui/Widget.java M user/test/com/google/gwt/user/client/DragAndDropEventsSinkTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10077 committed - Include all needed javax.validation classes for the api checker....
Revision: 10077 Author: ncha...@google.com Date: Tue Apr 26 09:52:21 2011 Log: Include all needed javax.validation classes for the api checker. Review at http://gwt-code-reviews.appspot.com/1425802 Review by: rchan...@google.com http://code.google.com/p/google-web-toolkit/source/detail?r=10077 Modified: /trunk/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiCompatibilityChecker.java === --- /trunk/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiCompatibilityChecker.java Fri Apr 15 08:00:11 2011 +++ /trunk/tools/api-checker/src/com/google/gwt/tools/apichecker/ApiCompatibilityChecker.java Tue Apr 26 09:52:21 2011 @@ -175,7 +175,9 @@ } /** - * Class that specifies a set of {@link CompilationUnit} read from jar files. + * Class that specifies a set of + * {@link com.google.gwt.dev.javac.CompilationUnit CompilationUnit} read from + * jar files. */ private static class JarFileResources extends Resources { private static final String MOCK_PREFIX = "/mock/"; @@ -286,7 +288,8 @@ } /** - * Abstract internal class that specifies a set of {@link CompilationUnit}. + * Abstract internal class that specifies a set of + * {@link com.google.gwt.dev.javac.CompilationUnit CompilationUnit}. */ private abstract static class Resources { protected final TreeLogger logger; @@ -344,8 +347,9 @@ } /** - * Class that specifies a set of {@link CompilationUnit} read from the - * file-system. + * Class that specifies a set of + * {@link com.google.gwt.dev.javac.CompilationUnit CompilationUnit} read from + * the file-system. */ private static class SourceFileResources extends Resources { private final ZipScanner excludeScanner; @@ -966,10 +970,23 @@ throws UnableToCompleteException, NotFoundException, IOException { Set resources = new HashSet(); if (extraSourceJars != null) { - Resources extra = - new JarFileResources(extraSourceJars, Collections.singleton(""), new HashSet( - Arrays.asList("javax/validation/Validation.java", - "javax/validation/constraints/Pattern.java")), logger); + Resources extra = new JarFileResources( + extraSourceJars, + Collections.singleton(""), + new HashSet(Arrays.asList( + "javax/validation/Configuration.java", + "javax/validation/MessageInterpolator.java", + "javax/validation/Validation.java", + "javax/validation/ValidatorContext.java", + "javax/validation/ValidatorFactory.java", + "javax/validation/ValidationProviderResolver.java", + "javax/validation/bootstrap/GenericBootstrap.java", + "javax/validation/bootstrap/ProviderSpecificBootstrap.java", + "javax/validation/constraints/Pattern.java", + "javax/validation/spi/BootstrapState.java", + "javax/validation/spi/ConfigurationState.java", + "javax/validation/spi/ValidationProvider.java")), + logger); Set loaded = extra.getResources(); System.out.println("Found " + loaded.size() + " new resources"); resources.addAll(loaded); -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing a bug with ScrollImplTrident where it fires a synthetic scroll event even if the browser ... (issue1426806)
LGTM On 2011/04/26 19:20:10, jlabanca wrote: committed as r10076 http://gwt-code-reviews.appspot.com/1426806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing a bug with ScrollImplTrident where it fires a synthetic scroll event even if the browser ... (issue1426806)
committed as r10076 http://gwt-code-reviews.appspot.com/1426806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10076 committed - Fixing a bug with ScrollImplTrident where it fires a synthetic scroll ...
Revision: 10076 Author: jlaba...@google.com Date: Tue Apr 26 08:38:21 2011 Log: Fixing a bug with ScrollImplTrident where it fires a synthetic scroll event even if the browser fires a native scroll event when an element is resized. We now delay firing the synthetic scroll event until the browser has had a chance to fire a native scroll event. It appears that RC versions of IE fires the native event, but final versions do not. All other browser fire the native event, so IE may fix this in the future. Review at http://gwt-code-reviews.appspot.com/1426806 http://code.google.com/p/google-web-toolkit/source/detail?r=10076 Modified: /trunk/user/src/com/google/gwt/user/client/ui/ScrollImpl.java === --- /trunk/user/src/com/google/gwt/user/client/ui/ScrollImpl.java Tue Apr 26 05:06:08 2011 +++ /trunk/user/src/com/google/gwt/user/client/ui/ScrollImpl.java Tue Apr 26 08:38:21 2011 @@ -49,13 +49,17 @@ // Detect if the scrollable element or the container within it changes // size, either of which could affect the scroll position. var resizeHandler = $entry(function() { -// Trigger a synthetic scroll event the scroll position changes. -if (scrollableElem.scrollTop != scrollableElem.__lastScrollTop || -scrollableElem.scrollLeft != scrollableElem.__lastScrollLeft) { - scrollHandler(); // Update scroll positions. - @com.google.gwt.user.client.ui.ScrollImpl.ScrollImplTrident::triggerScrollEvent(Lcom/google/gwt/dom/client/Element;) -(scrollableElem); -} +// Give the browser a chance to fire a native scroll event before +// synthesizing one. +setTimeout($entry(function() { + // Trigger a synthetic scroll event if the scroll position changes. + if (scrollableElem.scrollTop != scrollableElem.__lastScrollTop || + scrollableElem.scrollLeft != scrollableElem.__lastScrollLeft) { +scrollHandler(); // Update scroll positions. + @com.google.gwt.user.client.ui.ScrollImpl.ScrollImplTrident::triggerScrollEvent(Lcom/google/gwt/dom/client/Element;) + (scrollableElem); + } +}), 1); }); scrollable.attachEvent('onresize', resizeHandler); container.attachEvent('onresize', resizeHandler); -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: EnumOrdinalizer cleanup & better interop. (issue1426804)
Scott, would you mind, for my own edification, outlining how this improves the "valid state" of the AST after EnumOrdinalizer runs? Thanks. http://gwt-code-reviews.appspot.com/1426804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10075 committed - Fix build break introduced by previous change
Revision: 10075 Author: gwt.mirror...@gmail.com Date: Tue Apr 26 12:08:36 2011 Log: Fix build break introduced by previous change http://code.google.com/p/google-web-toolkit/source/detail?r=10075 Modified: /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java === --- /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java Tue Apr 26 08:02:24 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java Tue Apr 26 12:08:36 2011 @@ -308,7 +308,7 @@ errorCount++; } } - if (suppressErrors && errorCount > 0 && !logger.isLoggable(TreeLogger.TRACE)) + if (suppressErrors && errorCount > 0 && !logger.isLoggable(TreeLogger.TRACE) && logger.isLoggable(TreeLogger.INFO)) { logger.log(TreeLogger.INFO, "Ignored " + errorCount + " unit" + (errorCount > 1 ? "s" : "") + " with compilation errors in first pass. Specify -logLevel DEBUG to see all errors"); -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: This fix a bug in WidgetPlaceholderInterpreter (missing convertField) (issue1420810)
LGTM You and Rafa are on a collision course around the field precedence thing. Let the race begin! http://gwt-code-reviews.appspot.com/1420810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. (issue1426805)
LGTM++ http://gwt-code-reviews.appspot.com/1426805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10074 committed - create a tag for the gwt 2.3rc1 release
Revision: 10074 Author: mrruss...@google.com Date: Tue Apr 26 11:49:09 2011 Log: create a tag for the gwt 2.3rc1 release http://code.google.com/p/google-web-toolkit/source/detail?r=10074 Added: /tags/2.3.0-rc1 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: EnumOrdinalizer cleanup & better interop. (issue1426804)
In looking at EnumOrdinalizer now, I think there is some confusion wrt the 2nd and 3rd visitors (ReplaceEnumTypesWithInteger & ReplaceOrdinalFieldAndMethodRefsWithOrdinal). It seems that RETWI, which extends TypeRemapper, is already doing some of the assignments (perhaps not completely correctly), that you've added to ROFAMFWO. I'm wondering now if the 2 visitors can be merged, or simplified. Thoughts? That being said, the current form does appear to working correctly, I've tested it on several large apps, and in one case, it's afforded an additional ordinalization opportunity that was previously being hidden by the extraneous (Enum) casts that are now removed! I'm ok with going ahead with these mods, and I can revisit/refactor subsequently, if you want. http://gwt-code-reviews.appspot.com/1426804/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java File dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java (right): http://gwt-code-reviews.appspot.com/1426804/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode448 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:448: Yes, I think this is necessary now, if we can't rely on there being an upcast to Enum on the args. http://gwt-code-reviews.appspot.com/1426804/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode776 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:776: I believe this should already be happening in TypeRemapper (which is extended by ReplaceEnumTypesWithInteger) http://gwt-code-reviews.appspot.com/1426804/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode786 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:786: I believe this should also already be happening in TypeRemapper (for endVisit(JVariable,...), which should also cover the case for JLocal and JParameter. http://gwt-code-reviews.appspot.com/1426804/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode796 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:796: Already happening in TypeRemapper http://gwt-code-reviews.appspot.com/1426804/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode892 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:892: I'm interested in the call to madeChanges() here, this probably needs to be added everywhere within TypeRemapper() (assuming you agree with my above comments, and we don't need this updateVar() method here anymore). http://gwt-code-reviews.appspot.com/1426804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. (issue1426805)
http://gwt-code-reviews.appspot.com/1426805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] This fix a bug in WidgetPlaceholderInterpreter (missing convertField) (issue1420810)
Reviewers: rdcastro, rjrjr, Description: This fix a bug in WidgetPlaceholderInterpreter (missing convertField) and create a new FieldWriterType to substitute the ugly setBuildPrecedence(). It also make LazyDomElement used in more places. This saved a few bytes. Please review this at http://gwt-code-reviews.appspot.com/1420810/ Affected files: M user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java M user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java M user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java M user/src/com/google/gwt/uibinder/rebind/FieldManager.java M user/src/com/google/gwt/uibinder/rebind/FieldWriter.java A user/src/com/google/gwt/uibinder/rebind/FieldWriterType.java M user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. (issue1426805)
http://gwt-code-reviews.appspot.com/1426805/diff/6001/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java (right): http://gwt-code-reviews.appspot.com/1426805/diff/6001/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode124 user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:124: public void setStyleName(String styleName) { On 2011/04/26 17:57:42, rjrjr wrote: Are you sure this is the funnel point for widget style operations? Looking at UIObject, seems like you need to override all the methods there that call the static setStyleName and setStylePrimaryName methods. Really, there are going to be a lot of such wrappers needed. I suspect that attachable support is going to find its way into UIObject. Okay to TODO for now, I know you're trying to get to critical mass to start measuring things, but the TODO list is getting long. I totally agree. But I think that to move such a disruptive change to such basic objects we need both a very strong case for the performance improvements and a very nice, stable API. That's why this is all scattered around here. Hopefully until the end of the week we'll have the perf numbers, and I'll address at least a couple of the TODOs I added. Also, once we have a fairly large set of complicated orkut widgets implemented on top of this, I think we'll have a good idea of what the "right" API is, and we can work on it next week. At least that's the plan :-) http://gwt-code-reviews.appspot.com/1426805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Additional infrastructure for generating source code outside of a (issue1421805)
Reviewers: unnurg, Description: Additional infrastructure for generating source code outside of a GWT generator. Review by: unn...@google.com Please review this at http://gwt-code-reviews.appspot.com/1421805/ Affected files: A user/src/com/google/gwt/codegen/rebind/GwtCodeGenContext.java A user/src/com/google/gwt/codegen/server/AbortablePrintWriter.java A user/src/com/google/gwt/codegen/server/CodeGenContext.java A user/src/com/google/gwt/codegen/server/CodeGenLogger.java A user/src/com/google/gwt/codegen/server/CodeGenUtils.java A user/src/com/google/gwt/codegen/server/JavaSourceWriter.java A user/src/com/google/gwt/codegen/server/SourceWriter.java A user/src/com/google/gwt/codegen/server/SourceWriterBase.java A user/src/com/google/gwt/codegen/server/SourceWriterBuilder.java A user/src/com/google/gwt/codegen/server/StringSourceWriter.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. (issue1426805)
http://gwt-code-reviews.appspot.com/1426805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10072 committed - Autoformats a few files before patch...
Revision: 10072 Author: gwt.mirror...@gmail.com Date: Tue Apr 26 10:55:15 2011 Log: Autoformats a few files before patch Review at http://gwt-code-reviews.appspot.com/1425809 http://code.google.com/p/google-web-toolkit/source/detail?r=10072 Modified: /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationProblemReporter.java /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java /trunk/dev/core/src/com/google/gwt/dev/javac/CompiledClass.java /trunk/dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java === --- /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationProblemReporter.java Thu Apr 14 07:13:39 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationProblemReporter.java Tue Apr 26 10:55:15 2011 @@ -189,14 +189,14 @@ return false; } TreeLogger branch = -CompilationProblemReporter.reportErrors(logger, unit.getProblems(), -unit.getResourceLocation(), unit.isError(), new SourceFetcher() { - - public String getSource() { -return unit.getSource(); - } - -}, unit.getTypeName(), suppressErrors); +CompilationProblemReporter.reportErrors(logger, unit.getProblems(), unit +.getResourceLocation(), unit.isError(), new SourceFetcher() { + + public String getSource() { +return unit.getSource(); + } + +}, unit.getTypeName(), suppressErrors); return branch != null; } === --- /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java Tue Apr 19 07:13:00 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java Tue Apr 26 10:55:15 2011 @@ -146,7 +146,7 @@ private final Map allValidClasses = new HashMap(); private final GwtAstBuilder astBuilder = new GwtAstBuilder(); - + private final boolean suppressErrors; private transient LinkedBlockingQueue buildQueue; @@ -300,13 +300,12 @@ logger = logger.branch(TreeLogger.DEBUG, "Validating newly compiled units"); int errorCount = 0; for (CompilationUnit unit : resultUnits) { -if (CompilationProblemReporter.reportErrors(logger, unit, suppressErrors)) { +if (CompilationProblemReporter.reportErrors(logger, unit, suppressErrors)) { errorCount++; } } if (suppressErrors && errorCount > 0 && !logger.isLoggable(TreeLogger.TRACE)) { -logger.log(TreeLogger.INFO, "Ignored " + errorCount + " unit" -+ (errorCount > 1 ? "s" : "") +logger.log(TreeLogger.INFO, "Ignored " + errorCount + " unit" + (errorCount > 1 ? "s" : "") + " with compilation errors in first pass. Specify -logLevel DEBUG to see all errors"); } return resultUnits; @@ -320,12 +319,12 @@ } public static CompilationState buildFrom(TreeLogger logger, Set resources, - AdditionalTypeProviderDelegate delegate) { + AdditionalTypeProviderDelegate delegate) { return buildFrom(logger, resources, delegate, false); } - + public static CompilationState buildFrom(TreeLogger logger, Set resources, - AdditionalTypeProviderDelegate delegate, boolean suppressErrors) { + AdditionalTypeProviderDelegate delegate, boolean suppressErrors) { Event event = SpeedTracerLogger.start(DevModeEventType.CSB_BUILD_FROM_ORACLE); try { return instance.doBuildFrom(logger, resources, delegate, suppressErrors); @@ -333,7 +332,6 @@ event.end(); } } - public static CompilationStateBuilder get() { return instance; @@ -399,7 +397,8 @@ return new CompilationState(logger, resultUnits, compileMoreLater); } - public CompilationState doBuildFrom(TreeLogger logger, Set resources, boolean suppressErrors) { + public CompilationState doBuildFrom(TreeLogger logger, Set resources, + boolean suppressErrors) { return doBuildFrom(logger, resources, null, suppressErrors); } @@ -409,7 +408,7 @@ * TODO: maybe use a finer brush than to synchronize the whole thing. */ synchronized Collection doBuildGeneratedTypes(TreeLogger logger, - Collection generatedUnits, CompileMoreLater compileMoreLater, + Collection generatedUnits, CompileMoreLater compileMoreLater, boolean suppressErrors) { // Units we definitely want to build. === --- /trunk/dev/core/src/com/google/gwt/dev/javac/CompiledClass.java Thu Mar 24 15:47:57 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/CompiledClass.java Tue Apr 26 10:55:15 2011 @@ -55,12 +55,12 @@ * @param enclosingClass - outer class * @param isLocal Is this class a local class? (See the JLS rev 2 section * 14.3) - * @param internalName the internal binary name for this class. e.g. {@code - * java/util/Map$Entry
[gwt-contrib] Re: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. (issue1426805)
LGTM with yet another TODO Also, the very next patch will need to be an integration test. http://gwt-code-reviews.appspot.com/1426805/diff/6001/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java File user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java (right): http://gwt-code-reviews.appspot.com/1426805/diff/6001/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java#newcode124 user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java:124: public void setStyleName(String styleName) { Are you sure this is the funnel point for widget style operations? Looking at UIObject, seems like you need to override all the methods there that call the static setStyleName and setStylePrimaryName methods. Really, there are going to be a lot of such wrappers needed. I suspect that attachable support is going to find its way into UIObject. Okay to TODO for now, I know you're trying to get to critical mass to start measuring things, but the TODO list is getting long. http://gwt-code-reviews.appspot.com/1426805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fixing a bug with ScrollImplTrident where it fires a synthetic scroll event even if the browser ... (issue1426806)
Reviewers: rchandia, Description: Fixing a bug with ScrollImplTrident where it fires a synthetic scroll event even if the browser fires a native scroll event when an element is resized. We now delay firing the synthetic scroll event until the browser has had a chance to fire a native scroll event. It appears that RC versions of IE fires the native event, but final versions do not. All other browser fire the native event, so IE may fix this in the future. Please review this at http://gwt-code-reviews.appspot.com/1426806/ Affected files: M user/src/com/google/gwt/user/client/ui/ScrollImpl.java Index: user/src/com/google/gwt/user/client/ui/ScrollImpl.java === --- user/src/com/google/gwt/user/client/ui/ScrollImpl.java (revision 10071) +++ user/src/com/google/gwt/user/client/ui/ScrollImpl.java (working copy) @@ -49,13 +49,17 @@ // Detect if the scrollable element or the container within it changes // size, either of which could affect the scroll position. var resizeHandler = $entry(function() { -// Trigger a synthetic scroll event the scroll position changes. -if (scrollableElem.scrollTop != scrollableElem.__lastScrollTop || -scrollableElem.scrollLeft != scrollableElem.__lastScrollLeft) { - scrollHandler(); // Update scroll positions. - @com.google.gwt.user.client.ui.ScrollImpl.ScrollImplTrident::triggerScrollEvent(Lcom/google/gwt/dom/client/Element;) -(scrollableElem); -} +// Give the browser a chance to fire a native scroll event before +// synthesizing one. +setTimeout($entry(function() { + // Trigger a synthetic scroll event if the scroll position changes. + if (scrollableElem.scrollTop != scrollableElem.__lastScrollTop || + scrollableElem.scrollLeft != scrollableElem.__lastScrollLeft) { +scrollHandler(); // Update scroll positions. + @com.google.gwt.user.client.ui.ScrollImpl.ScrollImplTrident::triggerScrollEvent(Lcom/google/gwt/dom/client/Element;) + (scrollableElem); + } +}), 1); }); scrollable.attachEvent('onresize', resizeHandler); container.attachEvent('onresize', resizeHandler); -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds cache of CollectClassData to make refresh faster. (issue1420809)
Actually after initial cache filling all these units are used. I see preparing new TypeData only for changed units, when I modify them and hit Refresh in DevMode browser. I've added lazy TypeData into CompiledClass and filling TypeData information instead of cache. I'm not sure however if adding TypeData into CompiledClass is perfect, it seems that we add information from one layer into other layer. http://gwt-code-reviews.appspot.com/1420809/diff/1/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java File dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java (right): http://gwt-code-reviews.appspot.com/1420809/diff/1/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java#newcode249 dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java:249: new ReferenceMap(AbstractReferenceMap.HARD, AbstractReferenceMap.SOFT); On 2011/04/26 15:30:36, scottb wrote: Needs to be synchronized? This code was removed. http://gwt-code-reviews.appspot.com/1420809/diff/1/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java#newcode467 dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java:467: Map values = Maps.newHashMap(annotData.getValues()); What's the motivation here? This methods updates this annotation values map, so makes shared cached state invalid. So, we want to update copy instead. http://gwt-code-reviews.appspot.com/1420809/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. (issue1426805)
Please hold reviewing this for a sec, I'm fixing a compile error I introduced. On Tue, Apr 26, 2011 at 2:39 PM, wrote: > Thanks, guys. Another look? > > > > > http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java > File > > user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java > (right): > > > http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java#newcode76 > > user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java:76: > if (null == customTag) { > On 2011/04/26 17:13:47, rjrjr wrote: > >> Please check for appropriate constructor with >> > > > com.google.gwt.uibinder.rebind.TypeOracleUtils.hasCompatibleConstructor(JClassType, > >> JType...), and call logger.die if none is found. >> > > Actually, I totally missed this, but supporting custom root elements is > something I still have to add to AttachableHTMLPanel. It's not hard, but > since it's not blocking for 99% of our tests, I'd like to put it > together with the other 2 dozen of TODOs I'm adding with this CL... I > promise I'll get to them. Scout's honor. :-) > > > > http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java > File > user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java > (right): > > > http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java#newcode84 > user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java:84: > childFieldWriter.setBuildPrecendence(2); > On 2011/04/26 17:13:47, rjrjr wrote: > >> This looks brittle. Shouldn't you ask for this widget's precedence and >> > set the > >> child's to +1? >> > > Done. > > > > http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java#newcode99 > user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java:99: > elementWriter.setBuildPrecendence(2); > On 2011/04/26 17:13:47, rjrjr wrote: > >> ditto >> > > Done. > > > > http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java > File > > user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java > (right): > > > http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java#newcode123 > > user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java:123: > childFieldWriter.setBuildPrecendence(2); > On 2011/04/26 17:13:47, rjrjr wrote: > >> Again with the magic constant >> > > Done. > > > > http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java#newcode133 > > user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java:133: > elementWriter.setBuildPrecendence(2); > On 2011/04/26 17:13:47, rjrjr wrote: > >> ditto >> > > Done. > > > > http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java > File user/src/com/google/gwt/uibinder/rebind/FieldWriter.java (right): > > > http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java#newcode133 > user/src/com/google/gwt/uibinder/rebind/FieldWriter.java:133: * doesn't > scale well. Centralize these values, using enums or constants. > On 2011/04/26 17:13:47, rjrjr wrote: > >> FYI, field Writers already model a dependency chain, see >> > needs(FieldWriter) > >> above. Would that let us drop the precedence thing? Should have called >> > this out > >> during Hermes's review, oh well. >> > > Hermes is fixing this in a follow-up CL, but I don't think he's actually > using that info. I think he's just creating enums for these constants. > > > http://gwt-code-reviews.appspot.com/1426805/ > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds cache of CollectClassData to make refresh faster. (issue1420809)
http://gwt-code-reviews.appspot.com/1420809/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10071 committed - Introducing ServiceHelper, a inner class of RemoteServiceProxy that...
Revision: 10071 Author: gwt.mirror...@gmail.com Date: Tue Apr 26 10:46:07 2011 Log: Introducing ServiceHelper, a inner class of RemoteServiceProxy that makes ProxyCreator generates less code. Review at http://gwt-code-reviews.appspot.com/1423808 Review by: robertvaw...@google.com http://code.google.com/p/google-web-toolkit/source/detail?r=10071 Modified: /trunk/user/src/com/google/gwt/user/client/rpc/impl/RemoteServiceProxy.java /trunk/user/src/com/google/gwt/user/rebind/rpc/ProxyCreator.java === --- /trunk/user/src/com/google/gwt/user/client/rpc/impl/RemoteServiceProxy.java Mon Mar 21 12:22:19 2011 +++ /trunk/user/src/com/google/gwt/user/client/rpc/impl/RemoteServiceProxy.java Tue Apr 26 10:46:07 2011 @@ -46,6 +46,63 @@ * The content type to be used in HTTP requests. */ private static final String RPC_CONTENT_TYPE = "text/x-gwt-rpc; charset=utf-8"; + + /** + * A helper class that prepares the service to serialize data. + */ + public class ServiceHelper { + +private final String fullServiceName; +private final String methodName; +private final RpcStatsContext statsContext; +private SerializationStreamWriter streamWriter; + +public ServiceHelper(String serviceName, String methodName) { + this.fullServiceName = serviceName + "." + methodName; + this.methodName = methodName; + this.statsContext = new RpcStatsContext(); +} + +/** + * Finishes the serialization. + */ +public Request finish(AsyncCallback callback, ResponseReader responseHeader) +throws SerializationException { + String payload = streamWriter.toString(); + boolean toss = statsContext.isStatsAvailable() + && statsContext.stats(statsContext.timeStat(fullServiceName, "requestSerialized")); + return doInvoke(responseHeader, fullServiceName, statsContext, payload, callback); +} + +/** + * Finishes the serialization and return a RequestBuilder. + */ +public RequestBuilder finishForRequestBuilder(AsyncCallback callback, +ResponseReader responseHeader) throws SerializationException { + String payload = streamWriter.toString(); + boolean toss = statsContext.isStatsAvailable() + && statsContext.stats(statsContext.timeStat(fullServiceName, "requestSerialized")); + return doPrepareRequestBuilder( + responseHeader, fullServiceName, statsContext, payload, callback); +} + +/** + * Starts the serialization. + */ +public SerializationStreamWriter start(String remoteServiceInterfaceName, +int paramCount) throws SerializationException { + boolean toss = statsContext.isStatsAvailable() + && statsContext.stats(statsContext.timeStat(fullServiceName, "begin")); + streamWriter = createStreamWriter(); + if (getRpcToken() != null) { +streamWriter.writeObject(getRpcToken()); + } + streamWriter.writeString(remoteServiceInterfaceName); + streamWriter.writeString(methodName); + streamWriter.writeInt(paramCount); + return streamWriter; +} + } /** * @deprecated use {@link RpcStatsContext}. === --- /trunk/user/src/com/google/gwt/user/rebind/rpc/ProxyCreator.java Tue Feb 8 05:06:33 2011 +++ /trunk/user/src/com/google/gwt/user/rebind/rpc/ProxyCreator.java Tue Apr 26 10:46:07 2011 @@ -284,7 +284,7 @@ // Determine the set of serializable types Event event = SpeedTracerLogger.start(CompilerEventType.GENERATOR_RPC_STOB); - + SerializableTypeOracleBuilder typesSentFromBrowserBuilder = new SerializableTypeOracleBuilder( logger, propertyOracle, context); typesSentFromBrowserBuilder.setTypeFilter(blacklistTypeFilter); @@ -334,7 +334,7 @@ rpcLog = stringWriter.toString(); } event.end(); - + generateTypeHandlers(logger, context, typesSentFromBrowser, typesSentToBrowser); @@ -514,36 +514,23 @@ w.println(") {"); w.indent(); -String statsContextName = nameFactory.createName("statsContext"); -generateRpcStatsContext(w, syncMethod, asyncMethod, statsContextName); - -String statsMethodExpr = getProxySimpleName() + "." + syncMethod.getName(); -String tossName = nameFactory.createName("toss"); -w.println("boolean %s = %s.isStatsAvailable() && %s.stats(%s.timeStat(\"%s\", \"begin\"));", -tossName, statsContextName, statsContextName, statsContextName, statsMethodExpr); -w.print(SerializationStreamWriter.class.getSimpleName()); -w.print(" "); -String streamWriterName = nameFactory.createName("streamWriter"); -w.println(streamWriterName + " = createStreamWriter();"); -w.println("// createStreamWriter() prepared the stream"); +String helperName = nameFactory.createName("helper"); +String helperClassName = RemoteServiceProxy.ServiceHelper.class.getCanonicalName(); +w.p
[gwt-contrib] Re: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. (issue1426805)
Thanks, guys. Another look? http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java File user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java (right): http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java#newcode76 user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java:76: if (null == customTag) { On 2011/04/26 17:13:47, rjrjr wrote: Please check for appropriate constructor with com.google.gwt.uibinder.rebind.TypeOracleUtils.hasCompatibleConstructor(JClassType, JType...), and call logger.die if none is found. Actually, I totally missed this, but supporting custom root elements is something I still have to add to AttachableHTMLPanel. It's not hard, but since it's not blocking for 99% of our tests, I'd like to put it together with the other 2 dozen of TODOs I'm adding with this CL... I promise I'll get to them. Scout's honor. :-) http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java File user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java (right): http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java#newcode84 user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java:84: childFieldWriter.setBuildPrecendence(2); On 2011/04/26 17:13:47, rjrjr wrote: This looks brittle. Shouldn't you ask for this widget's precedence and set the child's to +1? Done. http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java#newcode99 user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java:99: elementWriter.setBuildPrecendence(2); On 2011/04/26 17:13:47, rjrjr wrote: ditto Done. http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java File user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java (right): http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java#newcode123 user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java:123: childFieldWriter.setBuildPrecendence(2); On 2011/04/26 17:13:47, rjrjr wrote: Again with the magic constant Done. http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java#newcode133 user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java:133: elementWriter.setBuildPrecendence(2); On 2011/04/26 17:13:47, rjrjr wrote: ditto Done. http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java File user/src/com/google/gwt/uibinder/rebind/FieldWriter.java (right): http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java#newcode133 user/src/com/google/gwt/uibinder/rebind/FieldWriter.java:133: * doesn't scale well. Centralize these values, using enums or constants. On 2011/04/26 17:13:47, rjrjr wrote: FYI, field Writers already model a dependency chain, see needs(FieldWriter) above. Would that let us drop the precedence thing? Should have called this out during Hermes's review, oh well. Hermes is fixing this in a follow-up CL, but I don't think he's actually using that info. I think he's just creating enums for these constants. http://gwt-code-reviews.appspot.com/1426805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. (issue1426805)
http://gwt-code-reviews.appspot.com/1426805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. (issue1426805)
Reviewers: rjrjr, hermes, Description: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. Please review this at http://gwt-code-reviews.appspot.com/1426805/ Affected files: A user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java A user/src/com/google/gwt/uibinder/elementparsers/AttachableInterpreter.java M user/src/com/google/gwt/uibinder/elementparsers/InterpreterPipe.java M user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java M user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java M user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java M user/src/com/google/gwt/uibinder/rebind/FieldWriter.java M user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java A user/src/com/google/gwt/user/client/ui/Attachable.java A user/src/com/google/gwt/user/client/ui/AttachableComposite.java A user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Phase 1 of GWT Dashboard. This includes an interface in gwt-dev for posting to a dashboard (defa... (issue1427807)
Reviewers: tobyr, Description: Phase 1 of GWT Dashboard. This includes an interface in gwt-dev for posting to a dashboard (default implementation is a no-op). Added calls to this interface in a couple of places. Created factory class that can provide the no-op implementation or a real implementation (based on system property). Updated unit test for SpeedTracerLogger to include a test that verifies that it is posting events to dashboard as expected. Review by: to...@google.com Please review this at http://gwt-code-reviews.appspot.com/1427807/ Affected files: M dev/core/src/com/google/gwt/dev/shell/OophmSessionHandler.java A dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifier.java A dev/core/src/com/google/gwt/dev/util/log/dashboard/DashboardNotifierFactory.java A dev/core/src/com/google/gwt/dev/util/log/dashboard/NoOpDashboardNotifier.java M dev/core/src/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLogger.java A dev/core/test/com/google/gwt/dev/util/log/dashboard/SpeedTracerLoggerTestMockNotifier.java M dev/core/test/com/google/gwt/dev/util/log/speedtracer/SpeedTracerLoggerTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Autoformats a few files before patch (issue1425809)
LGTM http://gwt-code-reviews.appspot.com/1425809/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduce the Attachable interface, and add support to UiBinder's HTMLPanel parser. (issue1426805)
http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java File user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java (right): http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java#newcode76 user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java:76: if (null == customTag) { Please check for appropriate constructor with com.google.gwt.uibinder.rebind.TypeOracleUtils.hasCompatibleConstructor(JClassType, JType...), and call logger.die if none is found. http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java File user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java (right): http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java#newcode84 user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java:84: childFieldWriter.setBuildPrecendence(2); This looks brittle. Shouldn't you ask for this widget's precedence and set the child's to +1? http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java#newcode99 user/src/com/google/gwt/uibinder/elementparsers/WidgetInterpreter.java:99: elementWriter.setBuildPrecendence(2); ditto http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java File user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java (right): http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java#newcode123 user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java:123: childFieldWriter.setBuildPrecendence(2); Again with the magic constant http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java#newcode133 user/src/com/google/gwt/uibinder/elementparsers/WidgetPlaceholderInterpreter.java:133: elementWriter.setBuildPrecendence(2); ditto http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java File user/src/com/google/gwt/uibinder/rebind/FieldWriter.java (right): http://gwt-code-reviews.appspot.com/1426805/diff/1/user/src/com/google/gwt/uibinder/rebind/FieldWriter.java#newcode133 user/src/com/google/gwt/uibinder/rebind/FieldWriter.java:133: * doesn't scale well. Centralize these values, using enums or constants. FYI, field Writers already model a dependency chain, see needs(FieldWriter) above. Would that let us drop the precedence thing? Should have called this out during Hermes's review, oh well. http://gwt-code-reviews.appspot.com/1426805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Autoformats a few files before patch (issue1425809)
Reviewers: scottb, jbrosenberg, Description: Autoformats a few files before patch Please review this at http://gwt-code-reviews.appspot.com/1425809/ Affected files: M dev/core/src/com/google/gwt/dev/javac/CompilationProblemReporter.java M dev/core/src/com/google/gwt/dev/javac/CompilationStateBuilder.java M dev/core/src/com/google/gwt/dev/javac/CompiledClass.java M dev/core/src/com/google/gwt/dev/javac/JdtCompiler.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introducing ServiceHelper, a inner class of RemoteServiceProxy that (issue1423808)
LGTM http://gwt-code-reviews.appspot.com/1423808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10069 committed - Cherry picking r10017 into releases/2.3m1 for public issue...
Revision: 10069 Author: gwt.mirror...@gmail.com Date: Tue Apr 26 09:33:54 2011 Log: Cherry picking r10017 into releases/2.3m1 for public issue http://gwt-code-reviews.appspot.com/1423801/ http://code.google.com/p/google-web-toolkit/source/detail?r=10069 Modified: /releases/2.3/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java === --- /releases/2.3/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java Tue Apr 12 13:51:55 2011 +++ /releases/2.3/user/src/com/google/gwt/uibinder/rebind/UiBinderGenerator.java Tue Apr 26 09:33:54 2011 @@ -47,8 +47,6 @@ private static final String TEMPLATE_SUFFIX = ".ui.xml"; - private static final String ELEMENT_FACTORY_PROPERTY = "uibinder.html.elementfactory"; - private static final String XSS_SAFE_CONFIG_PROPERTY = "UiBinder.useSafeHtmlTemplates"; private static boolean xssWarningGiven = false; -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10068 committed - Cherry pick r10025 into gwt/releases/2.3
Revision: 10068 Author: zun...@google.com Date: Tue Apr 19 09:50:32 2011 Log: Cherry pick r10025 into gwt/releases/2.3 http://code.google.com/p/google-web-toolkit/source/detail?r=10068 Modified: /releases/2.3/dev/core/src/com/google/gwt/dev/cfg/ConditionWhenTypeAssignableTo.java /releases/2.3/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java === --- /releases/2.3/dev/core/src/com/google/gwt/dev/cfg/ConditionWhenTypeAssignableTo.java Tue Apr 19 09:13:32 2011 +++ /releases/2.3/dev/core/src/com/google/gwt/dev/cfg/ConditionWhenTypeAssignableTo.java Tue Apr 19 09:50:32 2011 @@ -105,7 +105,7 @@ if (!warnedMissingValidationJar) { warnedMissingValidationJar = true; logger.log(TreeLogger.WARN, "Detected warnings related to '" + typeName + "'. " -+ " Is validation-api--sources.jar on the classpath?"); ++ " Are validation-api-.jar and validation-api--sources.jar on the classpath?"); logger.log(TreeLogger.INFO, "Specify -logLevel DEBUG to see all errors."); // Show the first error that matches return false; === --- /releases/2.3/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java Wed Feb 9 07:49:06 2011 +++ /releases/2.3/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java Tue Apr 19 09:50:32 2011 @@ -25,15 +25,15 @@ import com.google.gwt.dev.asm.signature.SignatureReader; import com.google.gwt.dev.asm.util.TraceClassVisitor; import com.google.gwt.dev.javac.asm.CollectAnnotationData; +import com.google.gwt.dev.javac.asm.CollectAnnotationData.AnnotationData; import com.google.gwt.dev.javac.asm.CollectClassData; +import com.google.gwt.dev.javac.asm.CollectClassData.AnnotationEnum; import com.google.gwt.dev.javac.asm.CollectFieldData; import com.google.gwt.dev.javac.asm.CollectMethodData; import com.google.gwt.dev.javac.asm.CollectTypeParams; import com.google.gwt.dev.javac.asm.ResolveClassSignature; import com.google.gwt.dev.javac.asm.ResolveMethodSignature; import com.google.gwt.dev.javac.asm.ResolveTypeSignature; -import com.google.gwt.dev.javac.asm.CollectAnnotationData.AnnotationData; -import com.google.gwt.dev.javac.asm.CollectClassData.AnnotationEnum; import com.google.gwt.dev.javac.typemodel.JAbstractMethod; import com.google.gwt.dev.javac.typemodel.JArrayType; import com.google.gwt.dev.javac.typemodel.JClassType; @@ -239,6 +239,11 @@ */ private static final boolean TRACE_CLASSES = false; + /** + * Suppress some warnings related to missing valiation.jar on classpath. + */ + private static boolean warnedMissingValidationJar = false; + private static JTypeParameter[] collectTypeParams(String signature) { if (signature != null) { List params = new ArrayList(); @@ -541,22 +546,27 @@ private Class getAnnotationClass(TreeLogger logger, AnnotationData annotData) { Type type = Type.getType(annotData.getDesc()); +String typeName = type.getClassName(); try { - Class clazz = Class.forName(type.getClassName(), false, + Class clazz = Class.forName(typeName, false, Thread.currentThread().getContextClassLoader()); if (!Annotation.class.isAssignableFrom(clazz)) { -logger.log(TreeLogger.ERROR, "Type " + type.getClassName() +logger.log(TreeLogger.ERROR, "Type " + typeName + " is not an annotation"); return null; } return clazz.asSubclass(Annotation.class); } catch (ClassNotFoundException e) { - logger.log(TreeLogger.WARN, "Ignoring unresolvable annotation type " - + type.getClassName()); + TreeLogger.Type level = TreeLogger.WARN; + if (shouldSuppressUnresolvableAnnotation(logger, typeName)) { +level = TreeLogger.DEBUG; + } + logger.log(level, "Ignoring unresolvable annotation type " + + typeName); return null; } } - + @SuppressWarnings("unused") private Class getClassLiteralForPrimitive(Type type) { switch (type.getSort()) { @@ -583,7 +593,7 @@ return null; } } - + /** * Map a bitset onto a different bitset. * @@ -1164,4 +1174,27 @@ return null; } } -} + + /** + * Suppress multiple validation related messages and replace with a hint. + * + * @param typeName fully qualified type name to check for filtering + */ + // TODO(zundel): Can be removed when javax.validation is included in the JRE + private boolean shouldSuppressUnresolvableAnnotation(TreeLogger logger, String typeName) { +if (typeName.startsWith("javax.validation.") +|| typeName.startsWith("com.google.gwt.validation.")) { + if (!warnedMissingValidationJar) { +warnedMissingValidationJar = true; +logger.log(TreeLogger.WARN, "Detected warnings related to '" + typeName + "'. " ++ " Is validation-.jar on the classpath?");
[gwt-contrib] Re: Allow enum ordinalization to work correctly for enum classes with no values (i.e. empty enums). (issue1423809)
LGTM w/ nits http://gwt-code-reviews.appspot.com/1423809/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java File dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java (right): http://gwt-code-reviews.appspot.com/1423809/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java#newcode810 dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java:810: toRemove = Lists.add(toRemove,index); Why not do the remove inline? I think there's a fairly common pattern where you just index-- at the removal site to offset the ++ below. http://gwt-code-reviews.appspot.com/1423809/diff/1/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java File dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java (right): http://gwt-code-reviews.appspot.com/1423809/diff/1/dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java#newcode250 dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java:250: } If I'm scanning this correctly, I think the sort order is backwards. http://gwt-code-reviews.appspot.com/1423809/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introducing CustomScrollPanel, a subclass of ScrollPanel that lets users define their own scroll... (issue1427804)
committed as r10067 http://gwt-code-reviews.appspot.com/1427804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix cast normalizer to properly deal with multidimensional JSO arrays. (issue1428803)
LGTM http://gwt-code-reviews.appspot.com/1428803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds cache of CollectClassData to make refresh faster. (issue1420809)
High-level, it seems like we might want to think about how to tie the lifecycle of the data to the lifecycle of the associated CompilationUnit/CompiledClass. Maybe this should actually be moved into a lazy-initialized transient field in CompiledClass instead of having to use a map. Also, now that we're going to be caching these in memory, these classes are in sore need of a "remove unused data" pass. Much of the contained data isn't used in practice. http://gwt-code-reviews.appspot.com/1420809/diff/1/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java File dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java (right): http://gwt-code-reviews.appspot.com/1420809/diff/1/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java#newcode249 dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java:249: new ReferenceMap(AbstractReferenceMap.HARD, AbstractReferenceMap.SOFT); Needs to be synchronized? http://gwt-code-reviews.appspot.com/1420809/diff/1/dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java#newcode467 dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java:467: Map values = Maps.newHashMap(annotData.getValues()); What's the motivation here? http://gwt-code-reviews.appspot.com/1420809/diff/1/dev/core/src/com/google/gwt/dev/util/ByteArrayWrapper.java File dev/core/src/com/google/gwt/dev/util/ByteArrayWrapper.java (right): http://gwt-code-reviews.appspot.com/1420809/diff/1/dev/core/src/com/google/gwt/dev/util/ByteArrayWrapper.java#newcode36 dev/core/src/com/google/gwt/dev/util/ByteArrayWrapper.java:36: && Arrays.equals(bytes, ((ByteArrayWrapper) obj).bytes); Why not compare hashCode before the expensive byte compare? http://gwt-code-reviews.appspot.com/1420809/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Allow enum ordinalization to work correctly for enum classes with no values (i.e. empty enums). (issue1423809)
When an enum has no enum constants, it's clinit only has 1 statement, and the statements list is implemented as a Collections.SingletonList, which is immuable, and so the 'it.remove()' calls were failing. This change allows the code to proceed. This issue was exposed as part of another change I'm working on, which will allow some more ordinalization opportunities. http://gwt-code-reviews.appspot.com/1423809/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Allow enum ordinalization to work correctly for enum classes with no values (i.e. empty enums). (issue1423809)
Reviewers: scottb, Description: Allow enum ordinalization to work correctly for enum classes with no values (i.e. empty enums). Please review this at http://gwt-code-reviews.appspot.com/1423809/ Affected files: M dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java M dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java Index: dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java === --- dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java (revision 10048) +++ dev/core/src/com/google/gwt/dev/jjs/impl/EnumOrdinalizer.java (working copy) @@ -19,6 +19,7 @@ import com.google.gwt.dev.jjs.SourceInfo; import com.google.gwt.dev.jjs.ast.Context; import com.google.gwt.dev.jjs.ast.JArrayType; +import com.google.gwt.dev.jjs.ast.JBlock; import com.google.gwt.dev.jjs.ast.JCastOperation; import com.google.gwt.dev.jjs.ast.JClassLiteral; import com.google.gwt.dev.jjs.ast.JClassType; @@ -42,6 +43,7 @@ import com.google.gwt.dev.jjs.ast.JVariableRef; import com.google.gwt.dev.jjs.ast.js.JsniFieldRef; import com.google.gwt.dev.jjs.ast.js.JsniMethodRef; +import com.google.gwt.dev.util.collect.Lists; import com.google.gwt.dev.util.log.speedtracer.CompilerEventType; import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger; import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event; @@ -790,8 +792,11 @@ * clinit for an ordinalizable enum. */ private void updateClinit(JMethod method) { - List stmts = ((JMethodBody) method.getBody()).getStatements(); + JBlock stmtBlock = ((JMethodBody) method.getBody()).getBlock(); + List stmts = stmtBlock.getStatements(); Iterator it = stmts.iterator(); + List toRemove = Lists.create(); + int index = 0; // look for statements of the form EnumValueField = ... while (it.hasNext()) { JStatement stmt = it.next(); @@ -800,16 +805,19 @@ if (ref instanceof JFieldRef) { JFieldRef enumRef = (JFieldRef) ref; if (enumRef.getField().getEnclosingType() == method.getEnclosingType()) { - // see if LHS is a field ref that corresponds to the class of this - // method + // see if LHS is a field ref that corresponds to the class of this method if (enumRef.getField() instanceof JEnumField) { -it.remove(); +toRemove = Lists.add(toRemove,index); } else if (enumRef.getField().getName().equals("$VALUES")) { -it.remove(); +toRemove = Lists.add(toRemove,index); } } } } +index++; + } + for (int i = toRemove.size() - 1; i >= 0; i--) { +stmtBlock.removeStmt(toRemove.get(i)); } } } Index: dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java === --- dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java (revision 10048) +++ dev/core/test/com/google/gwt/dev/jjs/impl/EnumOrdinalizerTest.java (working copy) @@ -225,6 +225,30 @@ assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit")); } + public void testOrdinalizeUnusedEnum() + throws UnableToCompleteException { +EnumOrdinalizer.resetTracker(); + +setupFruitEnum(); + +optimize("void", "Fruit myEnum;"); + +EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker(); +assertTrue(tracker.isOrdinalized("test.EntryPoint$Fruit")); + } + + public void testOrdinalizeUnusedEmptyEnum() + throws UnableToCompleteException { +EnumOrdinalizer.resetTracker(); + +setupEmptyEnum(); + +optimize("void", "EmptyEnum myEnum;"); + +EnumOrdinalizer.Tracker tracker = EnumOrdinalizer.getTracker(); +assertTrue(tracker.isOrdinalized("test.EntryPoint$EmptyEnum")); + } + public void testNotOrdinalizableClassLiteralReference() throws UnableToCompleteException { EnumOrdinalizer.resetTracker(); @@ -796,6 +820,10 @@ assertFalse(tracker.isOrdinalized("test.EntryPoint$Vegetable")); } + private void setupEmptyEnum() { +addSnippetClassDecl("public enum EmptyEnum {}"); + } + private void setupFruitEnum() { addSnippetClassDecl("public enum Fruit {APPLE, ORANGE}"); setupExtraDummyEnum(); -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Adds cache of CollectClassData to make refresh faster. (issue1420809)
Reviewers: zundel, Description: Adds cache of CollectClassData to make refresh faster. This gives about 10% performance gain on big projects. Please review this at http://gwt-code-reviews.appspot.com/1420809/ Affected files: M dev/core/src/com/google/gwt/dev/javac/TypeOracleMediator.java A dev/core/src/com/google/gwt/dev/util/ByteArrayWrapper.java A dev/core/test/com/google/gwt/dev/util/ByteArrayWrapperTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Re-architect how overrides are handled. (issue1422810)
http://gwt-code-reviews.appspot.com/1422810/diff/1/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java File dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java (right): http://gwt-code-reviews.appspot.com/1422810/diff/1/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java#newcode830 dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java:830: } Note to self: the method name doesn't match the function. Either rename the method, or else go ahead and reduce the returned set to non-abstract, instantiable types. http://gwt-code-reviews.appspot.com/1422810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Re-architect how overrides are handled. (issue1422810)
It actually does fix the abstract getClass() problem, I was noticing in the generated output. I'll mark my change as fixing this issue. On Tue, Apr 26, 2011 at 4:03 AM, wrote: > Hey, it looks like it would fix this issue, cool! > http://code.google.com/p/google-web-toolkit/issues/detail?id=4893 > According to my tests at the time, it would enable pruning of most > getClass() methods (at a minimum all those for abstract classes, which > are always overridden in the concrete classes, and then never executed): > http://gwt-code-reviews.appspot.com/609801/show > > > http://gwt-code-reviews.appspot.com/1422810/ > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Wrap low-priorty log calls with an 'if' test to avoid unnecessary calls (issue1425807)
LGTM http://gwt-code-reviews.appspot.com/1425807/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Re-architect how overrides are handled. (issue1422810)
Hey, it looks like it would fix this issue, cool! http://code.google.com/p/google-web-toolkit/issues/detail?id=4893 According to my tests at the time, it would enable pruning of most getClass() methods (at a minimum all those for abstract classes, which are always overridden in the concrete classes, and then never executed): http://gwt-code-reviews.appspot.com/609801/show http://gwt-code-reviews.appspot.com/1422810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors