[gwt-contrib] Re: RR : Overhaul the client-side RequestFactory code (issue924801)
Committed at r8918. Final diffstat: 136 files changed, 6712 insertions(+), 7889 deletions(-) Thanks for turning around a huge code review in a day. http://gwt-code-reviews.appspot.com/924801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: RR : Overhaul the client-side RequestFactory code (issue924801)
Feedback implemented. @RayC, I'll look at your patch before answering the question about the @Service validation. http://gwt-code-reviews.appspot.com/924801/diff/2001/3007 File user/src/com/google/gwt/app/place/AbstractProxyListActivity.java (right): http://gwt-code-reviews.appspot.com/924801/diff/2001/3007#newcode114 user/src/com/google/gwt/app/place/AbstractProxyListActivity.java:114: // Operation.CREATE)); On 2010/10/01 17:49:28, rjrjr wrote: I'm pretty sure that ID is never used, this is vestigial. Make it null and I believe nothing bad will happen. At the moment this is only exercised by addon-gwt. I don't want to hold up your patch getting that fixed, so you'll have to take the fix on faith and I'll have to follow up your submit with the banishment patch I've been working on. Done. http://gwt-code-reviews.appspot.com/924801/diff/2001/3008 File user/src/com/google/gwt/app/place/PropertyColumn.java (right): http://gwt-code-reviews.appspot.com/924801/diff/2001/3008#newcode74 user/src/com/google/gwt/app/place/PropertyColumn.java:74: public String getValue(R object) { On 2010/10/01 17:49:28, rjrjr wrote: Indeed, a smelly hack and a loophole the size of a barn door that would lead to a generation of unoptimizable GWT apps. As we discussed offline, delete this method and have its uses in samples/expenses implement it as needed. But *don't* eliminate the property name stuff. Despite the DRY violation, that's needed downstream by code that fills out the with() clauses when building requests Done. http://gwt-code-reviews.appspot.com/924801/diff/2001/3010 File user/src/com/google/gwt/editor/client/AutoBean.java (right): http://gwt-code-reviews.appspot.com/924801/diff/2001/3010#newcode21 user/src/com/google/gwt/editor/client/AutoBean.java:21: * PFM. On 2010/10/01 23:31:07, rjrjr wrote: ahem It happens to be true. http://gwt-code-reviews.appspot.com/924801/diff/2001/3010#newcode87 user/src/com/google/gwt/editor/client/AutoBean.java:87: * A clone operation only ever produces a shallow copy of its tags, since the On 2010/10/01 23:31:07, rjrjr wrote: Having trouble squaring this sentence with the deep flag. Updated the javadoc. All that I really mean to say is newBean.tags = new HashMap(toClone.tags); regardless of deep. http://gwt-code-reviews.appspot.com/924801/diff/2001/3010#newcode98 user/src/com/google/gwt/editor/client/AutoBean.java:98: List getOperations(); Removed Operations entirely, since I didn't need them. They were left over from an earlier sketch. http://gwt-code-reviews.appspot.com/924801/diff/2001/3012 File user/src/com/google/gwt/editor/client/AutoBeanUtils.java (right): http://gwt-code-reviews.appspot.com/924801/diff/2001/3012#newcode39 user/src/com/google/gwt/editor/client/AutoBeanUtils.java:39: public static Map diff(AutoBean a, AutoBean b) { Contents of the diff are used in AbstractRequestContext.makePayload(). I need the property names to be able to send them to the server, although going the deRPC route and using Impl.nameOf() would be possible. http://gwt-code-reviews.appspot.com/924801/diff/2001/3016 File user/src/com/google/gwt/editor/rebind/AutoBeanFactoryGenerator.java (right): http://gwt-code-reviews.appspot.com/924801/diff/2001/3016#newcode92 user/src/com/google/gwt/editor/rebind/AutoBeanFactoryGenerator.java:92: { On 2010/10/01 23:31:07, rjrjr wrote: You're not using these braces for anything, it's all in the for loop Done. http://gwt-code-reviews.appspot.com/924801/diff/2001/3023 File user/src/com/google/gwt/editor/rebind/model/ModelUtils.java (right): http://gwt-code-reviews.appspot.com/924801/diff/2001/3023#newcode32 user/src/com/google/gwt/editor/rebind/model/ModelUtils.java:32: public class ModelUtils { On 2010/10/01 23:31:07, rjrjr wrote: public? Used by Editor and RF generators. http://gwt-code-reviews.appspot.com/924801/diff/2001/3043 File user/src/com/google/gwt/requestfactory/client/impl/AbstractRequest.java (right): http://gwt-code-reviews.appspot.com/924801/diff/2001/3043#newcode93 user/src/com/google/gwt/requestfactory/client/impl/AbstractRequest.java:93: System.out.println(responseText); On 2010/10/01 23:31:07, rjrjr wrote: oops Done. http://gwt-code-reviews.appspot.com/924801/diff/2001/3043#newcode161 user/src/com/google/gwt/requestfactory/client/impl/AbstractRequest.java:161: // Really want Class.isInstance() On 2010/10/01 23:31:07, rjrjr wrote: Hear hear. And isAssignableFrom() while we're at it Done. http://gwt-code-reviews.appspot.com/924801/diff/2001/3043#newcode200 user/src/com/google/gwt/requestfactory/client/impl/AbstractRequest.java:200: if (receiver != null) { On 2010/10/01 23:31:07, rjrjr wrote: Should we have a default receiver that throws an RTE on unhandled violations or failures? It's awfully easy to forget to provide a callback and then miss what's going on when things fail silently. That's why we previously required the receiver arg in fire, to make that mistake much
[gwt-contrib] Re: RR : Overhaul the client-side RequestFactory code (issue924801)
LGTM. Other than the comments on wave that you have already looked at, I don't have anything new. I carefully looked at the EntityProxy and stableId stuff, and it looks great. I also skimmed over the generator code. On Fri, Oct 1, 2010 at 5:50 PM, wrote: > > Small nit. Besides validation of the context methods themselves, they > also need to be matched up against the service class, I've already done > the @ProxyFor matching part in the patch I sent, so I could do the > @Service matching if you want. > > > > http://gwt-code-reviews.appspot.com/924801/diff/13001/7088 > File > > user/src/com/google/gwt/requestfactory/rebind/model/RequestFactoryModel.java > (right): > > http://gwt-code-reviews.appspot.com/924801/diff/13001/7088#newcode288 > > user/src/com/google/gwt/requestfactory/rebind/model/RequestFactoryModel.java:288: > > This validates that the return type is transportable, but does not > validate that the parameters are transportable. > > > http://gwt-code-reviews.appspot.com/924801/show > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: editor framework and radio buttons
> On Fri, Oct 1, 2010 at 7:32 PM, Patrick Julien wrote: > > > True, but at the same time, it's a check box, validation shouldn't be > > an issue anyway Except if you have validators such as "this shouldn't be checked if you have value X in field Y" On 2 oct, 01:33, Patrick Julien wrote: > Is there a wave or doc somewhere about the editor framework? I'm > looking to see if this can be used to create child un-owned > relationships too The only one that I know of: https://wave.google.com/wave/waveref/googlewave.com/w+T8klBocZA AFAIK there shouldn't be any issue with unowned relationships (the concern is orthogonal to the Editor) -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Intern strings in the compiler AST to reduce memory footprint. (issue946801)
On 2010/10/02 00:36:10, conroy wrote: Turns out most of the things we need to intern get captured through JType's constructor, so the changes here are pretty minimal. http://gwt-code-reviews.appspot.com/946801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Intern strings in the compiler AST to reduce memory footprint. (issue946801)
Reviewers: scottb, zundel, Description: Intern strings in the compiler AST to reduce memory footprint. Please review this at http://gwt-code-reviews.appspot.com/946801/show Affected files: M dev/core/src/com/google/gwt/dev/jjs/ast/JAnnotation.java M dev/core/src/com/google/gwt/dev/jjs/ast/JLabel.java M dev/core/src/com/google/gwt/dev/jjs/ast/JParameter.java M dev/core/src/com/google/gwt/dev/jjs/ast/JPrimitiveType.java M dev/core/src/com/google/gwt/dev/jjs/ast/JType.java M dev/core/src/com/google/gwt/dev/jjs/ast/JVariable.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: RR : Overhaul the client-side RequestFactory code (issue924801)
http://gwt-code-reviews.appspot.com/924801/diff/2001/3012 File user/src/com/google/gwt/editor/client/AutoBeanUtils.java (right): http://gwt-code-reviews.appspot.com/924801/diff/2001/3012#newcode39 user/src/com/google/gwt/editor/client/AutoBeanUtils.java:39: public static Map diff(AutoBean a, AutoBean b) { In the old JSO model, in theory they could of, but in practice, RecordSchema would add every property to a collection, whether it was used or not, so it kept them live. It would be exceedingly difficult to employ any kind of reflective metadata and then prune unused metadata. ;-( On 2010/10/01 23:31:07, rjrjr wrote: Do you actually use the diff anywhere, or just the fact that things are different? Is this another spot to fret about unoptimizable string based property games? E.g. I can use this to get the full property list by comparing a bean to an empty one. Really, fundamentally, can unused auto bean properties ever be dead stripped? Not saying it's a deal breaker if they can't, but we should be conscious of the choice and document it. http://gwt-code-reviews.appspot.com/924801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: editor framework and radio buttons
Is there a wave or doc somewhere about the editor framework? I'm looking to see if this can be used to create child un-owned relationships too On Fri, Oct 1, 2010 at 7:32 PM, Patrick Julien wrote: > On Fri, Oct 1, 2010 at 7:28 PM, Thomas Broyer wrote: >> >> >> On 2 oct, 01:17, Patrick Julien wrote: >>> Ah, now I know what you're saying, I was trying to wrap this thing in >>> a decorator, you just need to use it directly since it's an editor >> >> AFAICT, the only thing you won't have is the display of validation >> errors (which ValueBoxEditorDecorator takes care of; in other words, >> ValueBoxEditorDecorator has no other use than adding this feature to >> any ValueBoxBase, by implementing both HasEditorErrors and IsEditor) > > > True, but at the same time, it's a check box, validation shouldn't be > an issue anyway > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: editor framework and radio buttons
On Fri, Oct 1, 2010 at 7:28 PM, Thomas Broyer wrote: > > > On 2 oct, 01:17, Patrick Julien wrote: >> Ah, now I know what you're saying, I was trying to wrap this thing in >> a decorator, you just need to use it directly since it's an editor > > AFAICT, the only thing you won't have is the display of validation > errors (which ValueBoxEditorDecorator takes care of; in other words, > ValueBoxEditorDecorator has no other use than adding this feature to > any ValueBoxBase, by implementing both HasEditorErrors and IsEditor) True, but at the same time, it's a check box, validation shouldn't be an issue anyway -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: editor framework and radio buttons
On 2 oct, 01:17, Patrick Julien wrote: > Ah, now I know what you're saying, I was trying to wrap this thing in > a decorator, you just need to use it directly since it's an editor AFAICT, the only thing you won't have is the display of validation errors (which ValueBoxEditorDecorator takes care of; in other words, ValueBoxEditorDecorator has no other use than adding this feature to any ValueBoxBase, by implementing both HasEditorErrors and IsEditor) -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] JsonRequestProcessor.validateKeys
The problem is inside in JsonRequestProcessor in the method updatePropertyTypes It's using entity.getDeclaredFields() which will omit all inherited fields. On Fri, Oct 1, 2010 at 5:24 PM, Pascal Patry wrote: > I'm currently having 2 issues with "JsonRequestProcessor.validateKeys()". > These requests are coming from updates that we want to perform on entities > that are driven by EntityProxy. > > The first issue is with inheritance. We have a base class for all our > entities on the server side in order to not have to replicate common > fields like the last update timestamp, the creation timestamp or even > the container id. So, on the concrete Entity, there are no containerId > field, but the client can send this data to the server in the update > request. When this happens, we encounter an exception on the server. > > "Error: key containerId is not permitted to be set" > > It seems that the validator is not picking up properties coming from the > inherited base entity class. getContainerId() and setContainerId() with > proper signature are present on the concrete entity. > > > The second issue we are getting is related to the same validation and > is producing the same exception. We have transient functions in some > entities to fill our needs. Let's take an example where the UserAccount > has a birth date set on the entity, but we only want to expose the age > of it when a client list these entities. We then have the proper getAge() > function on UserAccount that is a transient property that we compute on > the fly. When some changes are applied to a UserAccountEntityProxy from > an Editor, we use RequestFactoryEditorDriver to drive the update which > will carry this transient property back to the server. So, when > validation occurs on this field, it will be rejected because "the key > is not permitted to be set", which is true, but which we really don't care > about. I can create a dummy setter and field to get passed the issue, > but this is really annoying and misleading. > > -- > http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: editor framework and radio buttons
Ah, now I know what you're saying, I was trying to wrap this thing in a decorator, you just need to use it directly since it's an editor On Fri, Oct 1, 2010 at 7:03 PM, Patrick Julien wrote: > On Fri, Oct 1, 2010 at 6:51 PM, Thomas Broyer wrote: >> >> >> On 1 oct, 18:14, Patrick Julien wrote: >>> yeah, CheckBox doesn't work out of the box either because it needs to >>> implement ValueBase >> >> Because you try to wrap it in a ValueBoxEditorDecorator? > > Yep, I have, you need ValueBase in order to use a ValueBox > >> Judging from the code (i.e. I haven't tested to confirm what I'm >> saying here), CheckBox should work as a "simple" IsEditor (i.e. >> @UiField CheckBox myBooleanPropertyEditor) > > That works but the driver can't find this editor, so you don't get > binding from your backing type to the UI type > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: editor framework and radio buttons
On Fri, Oct 1, 2010 at 6:51 PM, Thomas Broyer wrote: > > > On 1 oct, 18:14, Patrick Julien wrote: >> yeah, CheckBox doesn't work out of the box either because it needs to >> implement ValueBase > > Because you try to wrap it in a ValueBoxEditorDecorator? Yep, I have, you need ValueBase in order to use a ValueBox > Judging from the code (i.e. I haven't tested to confirm what I'm > saying here), CheckBox should work as a "simple" IsEditor (i.e. > @UiField CheckBox myBooleanPropertyEditor) That works but the driver can't find this editor, so you don't get binding from your backing type to the UI type -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: editor framework and radio buttons
On 1 oct, 18:14, Patrick Julien wrote: > yeah, CheckBox doesn't work out of the box either because it needs to > implement ValueBase Because you try to wrap it in a ValueBoxEditorDecorator? Judging from the code (i.e. I haven't tested to confirm what I'm saying here), CheckBox should work as a "simple" IsEditor (i.e. @UiField CheckBox myBooleanPropertyEditor) -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add Support for server side script selection in linker (issue941802)
Reviewers: jgw, Description: Add Support for server side script selection in linker Please review this at http://gwt-code-reviews.appspot.com/941802/show Affected files: A dev/core/src/com/google/gwt/core/ext/linker/impl/PermutationsUtil.java A dev/core/src/com/google/gwt/core/ext/linker/impl/PropertiesMappingArtifact.java A dev/core/src/com/google/gwt/core/ext/linker/impl/ResourceInjectionUtil.java M dev/core/src/com/google/gwt/core/ext/linker/impl/SelectionScriptLinker.java M dev/core/src/com/google/gwt/core/ext/linker/impl/computeScriptBase.js M dev/core/src/com/google/gwt/core/ext/linker/impl/installLocationIframe.js A dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptCommon.js A dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptDirect.js A dev/core/src/com/google/gwt/core/ext/linker/impl/installScriptEarlyDownload.js M dev/core/src/com/google/gwt/core/ext/linker/impl/permutations.js M dev/core/src/com/google/gwt/core/ext/linker/impl/processMetas.js M dev/core/src/com/google/gwt/core/ext/linker/impl/waitForBodyLoaded.js M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeLinker.java M dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js M dev/core/src/com/google/gwt/core/linker/SingleScriptLinker.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] JsonRequestProcessor.validateKeys
I'm currently having 2 issues with "JsonRequestProcessor.validateKeys()". These requests are coming from updates that we want to perform on entities that are driven by EntityProxy. The first issue is with inheritance. We have a base class for all our entities on the server side in order to not have to replicate common fields like the last update timestamp, the creation timestamp or even the container id. So, on the concrete Entity, there are no containerId field, but the client can send this data to the server in the update request. When this happens, we encounter an exception on the server. "Error: key containerId is not permitted to be set" It seems that the validator is not picking up properties coming from the inherited base entity class. getContainerId() and setContainerId() with proper signature are present on the concrete entity. The second issue we are getting is related to the same validation and is producing the same exception. We have transient functions in some entities to fill our needs. Let's take an example where the UserAccount has a birth date set on the entity, but we only want to expose the age of it when a client list these entities. We then have the proper getAge() function on UserAccount that is a transient property that we compute on the fly. When some changes are applied to a UserAccountEntityProxy from an Editor, we use RequestFactoryEditorDriver to drive the update which will carry this transient property back to the server. So, when validation occurs on this field, it will be rejected because "the key is not permitted to be set", which is true, but which we really don't care about. I can create a dummy setter and field to get passed the issue, but this is really annoying and misleading. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fixing a ConcurrentModificationException in JsonRequestProcessor that occurs when you use a sett... (issue945801)
Reviewers: robertvawter, Description: Fixing a ConcurrentModificationException in JsonRequestProcessor that occurs when you use a setter that sets an Entity from null to a value. Also fixes a bug in DeltaValueStoreJsonImpl#retainValue() where we only retain the first element of a List instead of all elements in the list because the return statement is in the wrong place. This revealed other NPEs on the server that are also fixed. Please review this at http://gwt-code-reviews.appspot.com/945801/show Affected files: M user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java M user/src/com/google/gwt/requestfactory/server/JsonRequestProcessor.java M user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java M user/test/com/google/gwt/requestfactory/server/JsonRequestProcessorTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: RR : Overhaul the client-side RequestFactory code (issue924801)
http://gwt-code-reviews.appspot.com/924801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixes some bugs in CellBrowser. CellBrowser#setChildState() was exited early too aggressively, ... (issue942801)
committed as r8917 http://gwt-code-reviews.appspot.com/942801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r8917 committed - Fixes some bugs in CellBrowser. CellBrowser#setChildState() was exite...
Revision: 8917 Author: jlaba...@google.com Date: Fri Oct 1 09:41:26 2010 Log: Fixes some bugs in CellBrowser. CellBrowser#setChildState() was exited early too aggressively, which could put CellBrowser in an invalid state if you mix leaf and non-leaf nodes. CellBrowser was overriding onFocus() to open children, but it isn't focusable if keyboard selection is disabled. We now override a package protected method that is triggered on focus or mousedown. Also, we would apply the keyboard selected style to the 0th row if keyboard selection is disabled. This bug has been fixed. I wrote tests for the mixed leaf/non leaf node cases, and I manually verified that the CellBrowser behaves correctly whether or not keyboard selection is enabled. Review at http://gwt-code-reviews.appspot.com/942801 Review by: r...@google.com http://code.google.com/p/google-web-toolkit/source/detail?r=8917 Modified: /trunk/user/src/com/google/gwt/user/cellview/client/CellBrowser.java /trunk/user/src/com/google/gwt/user/cellview/client/CellList.java /trunk/user/test/com/google/gwt/user/cellview/client/CellBrowserTest.java === --- /trunk/user/src/com/google/gwt/user/cellview/client/CellBrowser.java Thu Sep 23 15:11:20 2010 +++ /trunk/user/src/com/google/gwt/user/cellview/client/CellBrowser.java Fri Oct 1 09:41:26 2010 @@ -1,12 +1,12 @@ /* * Copyright 2010 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 @@ -63,7 +63,7 @@ /** * A "browsable" view of a tree in which only a single node per level may be * open at one time. - * + * * * This widget will only work in standards mode, which requires that * the HTML page in which it is run have an explicit @@ -180,11 +180,11 @@ } /** - * A custom version of cell list used by the browser. - * + * A custom version of cell list used by the browser. Visible for testing. + * * @param the data type of list items */ - private class BrowserCellList extends CellList { + class BrowserCellList extends CellList { /** * The level of this list view. @@ -192,14 +192,19 @@ private final int level; /** - * The key of the currently open item. + * The key of the currently focused item. */ -private Object openKey; +private Object focusedKey; /** - * The value of the currently open item. + * The value of the currently focused item. */ -private T openValue; +private T focusedValue; + +/** + * Indicates whether or not the focused value is open. + */ +private boolean isFocusedOpen; public BrowserCellList(final Cell cell, int level, ProvidesKey keyProvider) { @@ -238,18 +243,6 @@ } } } - -@Override -protected void onFocus() { - super.onFocus(); - - // Open the selected row. - int selectedRow = getKeyboardSelectedRow(); - if (isRowWithinBounds(selectedRow)) { -T value = getDisplayedItem(selectedRow); -setChildState(this, value, true, true, true); - } -} @Override protected void renderRowValues(SafeHtmlBuilder sb, List values, @@ -261,8 +254,7 @@ String openItem = " " + style.cellBrowserOpenItem(); String evenItem = style.cellBrowserEvenItem(); String oddItem = style.cellBrowserOddItem(); - int keyboardSelectedRow = Math.max(0, getKeyboardSelectedRow() - + getPageStart()); + int keyboardSelectedRow = getKeyboardSelectedRow() + getPageStart(); int length = values.size(); int end = start + length; for (int i = start; i < end; i++) { @@ -270,7 +262,8 @@ Object key = getValueKey(value); boolean isSelected = selectionModel == null ? false : selectionModel.isSelected(value); -boolean isOpen = (openKey == null) ? false : openKey.equals(key); +boolean isOpen = (focusedKey == null || !isFocusedOpen) ? false +: focusedKey.equals(key); StringBuilder classesBuilder = new StringBuilder(); classesBuilder.append(i % 2 == 0 ? evenItem : oddItem); if (isOpen) { @@ -313,6 +306,15 @@ } } } + +@Override +void doKeyboardSelection(Event event, T value, int indexOnPage) { + super.doKeyboardSelection(event, value, indexOnPage); + + // Open the selected row. If keyboard selection updates the selection + // model, this is a no-op. + setChildState(this, value, true, true, true); +} /** *
[gwt-contrib] Re: RR : Overhaul the client-side RequestFactory code (issue924801)
Partial review, must get food. http://gwt-code-reviews.appspot.com/924801/diff/2001/3051 File user/src/com/google/gwt/requestfactory/client/impl/EntityCodex.java (right): http://gwt-code-reviews.appspot.com/924801/diff/2001/3051#newcode115 user/src/com/google/gwt/requestfactory/client/impl/EntityCodex.java:115: This doesn't seem like it would handle nulls in collections properly, since you'll return "null" http://gwt-code-reviews.appspot.com/924801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixes an NPE when setting an entity parameter to null. The NPE occurs in generated code because ... (issue944801)
committed as r8915 http://gwt-code-reviews.appspot.com/944801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r8916 committed - Fix nonstandard two method handler interface that probably would have...
Revision: 8916 Author: rj...@google.com Date: Fri Oct 1 09:39:58 2010 Log: Fix nonstandard two method handler interface that probably would have broken UiBinder. Review at http://gwt-code-reviews.appspot.com/941801 Review by: r...@google.com http://code.google.com/p/google-web-toolkit/source/detail?r=8916 Modified: /trunk/user/src/com/google/gwt/event/logical/shared/AttachEvent.java /trunk/user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java === --- /trunk/user/src/com/google/gwt/event/logical/shared/AttachEvent.java Mon Sep 13 16:10:55 2010 +++ /trunk/user/src/com/google/gwt/event/logical/shared/AttachEvent.java Fri Oct 1 09:39:58 2010 @@ -29,8 +29,7 @@ * Implemented by objects that handle {...@link AttachEvent}. */ public interface Handler extends EventHandler { -void onAttach(AttachEvent event); -void onDetach(AttachEvent event); +void onAttachOrDetach(AttachEvent event); } /** @@ -98,10 +97,6 @@ @Override protected void dispatch(AttachEvent.Handler handler) { -if (attached) { - handler.onAttach(this); -} else { - handler.onDetach(this); -} +handler.onAttachOrDetach(this); } } === --- /trunk/user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java Mon Sep 13 16:10:55 2010 +++ /trunk/user/test/com/google/gwt/user/client/ui/WidgetOnLoadTest.java Fri Oct 1 09:39:58 2010 @@ -111,11 +111,12 @@ int delegateAttachOrder; int delegateDetachOrder; - public void onAttach(AttachEvent event) { -delegateAttachOrder = ++orderIndex; - } - public void onDetach(AttachEvent event) { -delegateDetachOrder = ++orderIndex; + public void onAttachOrDetach(AttachEvent event) { +if (event.isAttached()) { + delegateAttachOrder = ++orderIndex; +} else { + delegateDetachOrder = ++orderIndex; +} } } -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r8915 committed - Fixes an NPE when setting an entity parameter to null. The NPE occurs ...
Revision: 8915 Author: jlaba...@google.com Date: Fri Oct 1 09:16:52 2010 Log: Fixes an NPE when setting an entity parameter to null. The NPE occurs in generated code because we try to get the entity in wire format. Review at http://gwt-code-reviews.appspot.com/944801 http://code.google.com/p/google-web-toolkit/source/detail?r=8915 Modified: /trunk/user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java /trunk/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java === --- /trunk/user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java Thu Sep 30 18:35:57 2010 +++ /trunk/user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java Fri Oct 1 09:16:52 2010 @@ -1041,22 +1041,25 @@ if (sb.length() > 0) { sb.append(", "); } - JClassType classType = parameter.getType().isClassOrInterface(); - JType paramType = parameter.getType(); + JClassType classType = paramType.isClassOrInterface(); + String paramName = parameter.getName(); + if (paramType.getQualifiedSourceName().equals( EntityProxyId.class.getName())) { -sb.append("factory.getWireFormat(" + parameter.getName() + ")"); +sb.append("factory.getWireFormat(" + paramName + ")"); continue; } if (classType != null && classType.isAssignableTo(entityProxyType)) { +sb.append(paramName + " == null ? null : "); sb.append("((" + classType.getQualifiedBinaryName() + "Impl" + ")"); - } - sb.append(parameter.getName()); - if (classType != null && classType.isAssignableTo(entityProxyType)) { +sb.append(paramName); sb.append(").wireFormatId()"); - } +continue; + } + + sb.append(paramName); } return "new Object[] {" + sb.toString() + "}"; } === --- /trunk/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java Thu Sep 30 12:36:38 2010 +++ /trunk/user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java Fri Oct 1 09:16:52 2010 @@ -499,7 +499,7 @@ /** * Test that a null value can be sent in a request. */ - public void disabledTestNullSimpleFooRequest() { + public void testNullSimpleFooRequest() { delayTestFinish(5000); final Request fooReq = req.simpleFooRequest().receiveNullSimpleFoo(null); fooReq.fire(new Receiver() { -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
And one further minor suggestion: for all the places where you have a comment like "// implemented in the translatable version of this class", may I suggest adding some additional information on where the source might be located (such as Scott's instructions above). If I had had those, I probably would have figured out the uncaught exception on my own and never started this thread. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
> ReturnMessage.isException = true was returned. It would be nice to > have a stack trace for the code that created this ReturnMessage in > response to an exception. I agree with Scott that the EmulateJsStack pass could be applied to the native code fragments injected into the browser. The native code is already executed in a wrapper that returns an array of [didThrow, value], so $stack could be added as another member. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixes some bugs in CellBrowser. CellBrowser#setChildState() was exited early too aggressively, ... (issue942801)
LGTM http://gwt-code-reviews.appspot.com/942801/diff/1/3 File user/src/com/google/gwt/user/cellview/client/CellList.java (right): http://gwt-code-reviews.appspot.com/942801/diff/1/3#newcode472 user/src/com/google/gwt/user/cellview/client/CellList.java:472: * Called when the user selects a cell with the mouse of tab key. of -> or http://gwt-code-reviews.appspot.com/942801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
On Oct 1, 12:32 pm, Scott Blum wrote: > It sounds like the problem really boils down to less-than-useful stack > traces when a native exception is thrown in Dev mode. Ironically, if you > ran this in Prod mode, you'd probably get a better stack trace, because we > emulate the stack over there. > > We could probably implement the same sort of stack trace emulation feature > in Dev mode + some other stack cleanup to make the exception traceback much > nicer. Well, I'd say there are two problems (well three if you include the fact that this exception wasn't being thrown on 2.0.4). The first is that the stack trace is not very friendly. The second is this problem of allowing outstanding commands to keep executing even after a test has finished. Fixing the original exception would make the second problem go away but I would have been able to focus in on the first exception more quickly if it hadn't been for the secondary exceptions that occurred as a result of the first. Definitely providing more information in the stack trace would be great. For example, with a debugger, I can see that the call that was invoked (that threw the exception) is "@com.google.gwt.dom.client.DOMImplTrident::dispatchEvent(Lcom/google/ gwt/dom/client/Element;Lcom/google/gwt/dom/client/NativeEvent;)". That tells me a little more than I knew before although I don't know yet if that's useful. It's only telling me that something somewhere inside dispatchEvent threw an exception and a ReturnMessage.isException = true was returned. It would be nice to have a stack trace for the code that created this ReturnMessage in response to an exception. This may be a slight tangent, but I know that we often get these types of exceptions in production (so not dev mode here) that are caught by our uncaught exception handler and some of them don't even include a line number or file name. When your logs say an error occurred and the entire message is literally "Unspecified error", there's just no way to debug it. I thought I had heard you guys had some way of creating a stack trace of the native javascript exception and then some means of de-obfuscating it. Did I imagine that? -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
On Fri, Oct 1, 2010 at 1:59 PM, Damon Lundin wrote: > On Oct 1, 12:51 pm, John Tamplin wrote: > > If you are only using GWT-supplied APIs, I am not sure how you aren't > > getting the uncaught exception handler called on those calls. Can you > > create a small repro case that illustrates the problem? > I think you are misunderstanding. The uncaught exception handle *is* > getting called. The uncaught exception handler that is setup by the > gwttestcase is catching the exception and calling finishTest. The > problem is that there are outstanding commands that continue to get > executed even though that particular test case just failed and is done. Ok, then just have try/finally around your code to cleanup the state in the test before the exception catching code finishes the test. -- John A. Tamplin Software Engineer (GWT), Google -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r8914 committed - Fixes NPE in GenerateJavaScriptAST when SOYC is enabled....
Revision: 8914 Author: sco...@google.com Date: Fri Oct 1 07:57:13 2010 Log: Fixes NPE in GenerateJavaScriptAST when SOYC is enabled. Review by: d...@google.com http://code.google.com/p/google-web-toolkit/source/detail?r=8914 Modified: /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java === --- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java Mon Aug 30 04:31:11 2010 +++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java Fri Oct 1 07:57:13 2010 @@ -376,8 +376,10 @@ polymorphicJsFunctions.add(jsFunction); } methodBodyMap.put(x.getBody(), jsFunction); - jsFunction.getSourceInfo().addCorrelation( - program.getCorrelator().by(globalName)); + if (globalName != null) { +jsFunction.getSourceInfo().addCorrelation( +program.getCorrelator().by(globalName)); + } push(jsFunction.getScope()); if (program.getIndexedMethods().contains(x)) { -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
On Oct 1, 12:51 pm, John Tamplin wrote: > If you are only using GWT-supplied APIs, I am not sure how you aren't > getting the uncaught exception handler called on those calls. Can you > create a small repro case that illustrates the problem? > > John A. Tamplin > Software Engineer (GWT), Google I think you are misunderstanding. The uncaught exception handle *is* getting called. The uncaught exception handler that is setup by the gwttestcase is catching the exception and calling finishTest. The problem is that there are outstanding commands that continue to get executed even though that particular test case just failed and is done. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
On Fri, Oct 1, 2010 at 1:41 PM, Damon Lundin wrote: > I'm not sure I entirely understand. Are you suggesting that I need to > convert all of my DeferredCommand calls to ones that use some new > native method implementation so I can wrap my callbacks in $entry? > No, if you are only using GWT APIs that handle the callback from the browser event loop then that is already taken care of. You didn't say what code you were using, so I thought you might be doing something yourself that needed to be wrapped. If you are only using GWT-supplied APIs, I am not sure how you aren't getting the uncaught exception handler called on those calls. Can you create a small repro case that illustrates the problem? I'm not sure how to cancel outstanding RPC calls if that's even possible. [unrelated] Define your async service interface with a return value of Request instead of void, and then you can call request.cancel() to terminate it. -- John A. Tamplin Software Engineer (GWT), Google -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
On Oct 1, 12:12 pm, John Tamplin wrote: > If your test code ultimately involves callbacks from the browser event loop > from your own code, you need to wrap all such calls in $entry, which hooks > up the uncaught exception handler and the Scheduler hooks. All of the GWT > widgets/etc that do this already take care of it, but if you write your own > you have to handle it. > > Look at JsopRequest for an example, but basically if you set a callback > foo.callback = function()... then that should instead be > foo.callback=$entry(function()...). > > -- > John A. Tamplin > Software Engineer (GWT), Google I'm not sure I entirely understand. Are you suggesting that I need to convert all of my DeferredCommand calls to ones that use some new native method implementation so I can wrap my callbacks in $entry? I'm not sure what that accomplishes since DeferredCommand already does that. When it starts a timer, the timer code wraps the callback in $entry. It also seems like a heavy burden to impose on the test developers when GWT is already maintaining a list of callbacks. If there was a DeferredCommand.cancelAll, I could call that manually myself although I'm suggesting that perhaps the GWT test case should do that for us. That would also catch any DeferredCommands that may have been executed in the production code. I'm not sure how to cancel outstanding RPC calls if that's even possible. Also, I see the calls to $entry in GWT source but I don't really have any idea what it does other than hook at the uncaught exception handler as you just noted. And BTW, my apologies for starting this thread so early. I generally try to avoid getting help for things I should have been able to figure out on my own :-( -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: Test timeout in JUnitShell ? (GWT 2.0.0)
On Fri, Oct 1, 2010 at 12:32 PM, Ed wrote: > Thanks for the detailed explanation, that sure helps. > > > I have seen the timeout message happen due to a compile error in the GWT > > code, but you'd see those errors in the log also. > Could it have something to do with issue 4700 ?: > http://code.google.com/p/google-web-toolkit/issues/detail?id=4700 > Nominally unrelated... 4700 is about benign-but-scary error messages resulting from sloppy gwt.xml specifications (and/or sloppy code arrangement) interacting with GWT's need to sniff around for "all available" classes rather than loading things only as referenced from your entry point. *However*, those benign-but-scary ones look just like less benign messages for actually referenced code, and *that* would be a fatal compilation error that, if I remember right, may indeed result in this timeout. Precisely, again if I remember, the client connects to the web server, gets a synthetic module that starts by making a GWT RPC to get the first test(s) to be run, and that RPC is what GWT didn't get in the one-minute timeout. If your code didn't compile, that RPC can't happen, and we time out. Failure to find source for classes we touch is one way to fail to compile (but identical-seeming errors for classes we don't touch are 4700). > + I run the tests in noserver mode against an external tomcat server > that runs the backend that is started by the cargo maven plugin. I use > a proxy servlet in the internal tomcat server that is started by GWT > like explained in issue 4615: > http://code.google.com/p/google-web-toolkit/issues/detail?id=4615 > Ah. I haven't attempted that pattern. More thoughts below, though. > What is the relation between HTMLUnit and the internal tomcat that GWT > uses ?.. > None whatsoever: tomcat is a web server and servlet container; htmlunit is a web browser and JavaScript engine that happens to be (a) implemented in Java and (b) bundled into GWT. It sounds as though the RPC request for the junitshell servlet (i.e. JUnitHostImpl, implementing JUnitHost/JUnitHostAsync as an RPC interface) is not getting to "our" servlet; I can't tell you why. It might be misconfiguration in your proxy, and being sent to your tomcat server, which either wouldn't handle it or would handle it but not be the test infrastructure waiting for the call, and so the timeout would happen. Something like Charles or wireshark, or just close-reading your logs and configurations, might help you work out what was happening there. I suppose it might be that HtmlUnit is tripping itself up on startup, too, and sometimes not even making that RPC... same tools to diagnose. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
It sounds like the problem really boils down to less-than-useful stack traces when a native exception is thrown in Dev mode. Ironically, if you ran this in Prod mode, you'd probably get a better stack trace, because we emulate the stack over there. We could probably implement the same sort of stack trace emulation feature in Dev mode + some other stack cleanup to make the exception traceback much nicer. CC'ing Kelly so he can see about prioritizing this. On Fri, Oct 1, 2010 at 1:00 PM, Damon Lundin wrote: > Thanks for the help Scott. The problem is not with the > delayTestFinish. Apparently our tests are throwing uncaught > exceptions which causes the test to finish. The exception does appear > in our logs, but it's one of these: > > Caused by: com.google.gwt.core.client.JavaScriptException: (Error): > Unspecified error. > number: -2147467259 > description: Unspecified error. >at > > com.google.gwt.dev.shell.BrowserChannelServer.invokeJavascript(BrowserChannelServer.java: > 237) > > Which are not terribly useful (if you have any useful suggestions on > dealing with these, I'm all ears) so I moved on to the next exception > which was an NPE in our code. That NPE is caused by the tests running > on top of each other. So I guess this was my fault for not paying > closer attention to the first exception. > > There is perhaps one possible recommendation that could come out of > this and maybe that is that when a test begins (or finishes) it > cancels all outstanding timers, commands and pending async callbacks > (perhaps optionally). The confusion occurred here because even though > the previous test threw an uncaught exception and finished allowing > the next test to begin, the outstanding commands started by the > previous test were still allowed to run and when they did, they messed > up the shared state used by the next test. > > Of course now I have to go figure out what's causing this uncaught > exception to be thrown :-(. And I'm also still confused why the tests > pass when run one at a time. > > -- > http://groups.google.com/group/Google-Web-Toolkit-Contributors > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
On Fri, Oct 1, 2010 at 1:00 PM, Damon Lundin wrote: > Thanks for the help Scott. The problem is not with the > delayTestFinish. Apparently our tests are throwing uncaught > exceptions which causes the test to finish. The exception does appear > in our logs, but it's one of these: > > Caused by: com.google.gwt.core.client.JavaScriptException: (Error): > Unspecified error. > number: -2147467259 > description: Unspecified error. >at > > com.google.gwt.dev.shell.BrowserChannelServer.invokeJavascript(BrowserChannelServer.java: > 237) > > Which are not terribly useful (if you have any useful suggestions on > dealing with these, I'm all ears) so I moved on to the next exception > which was an NPE in our code. That NPE is caused by the tests running > on top of each other. So I guess this was my fault for not paying > closer attention to the first exception. > > There is perhaps one possible recommendation that could come out of > this and maybe that is that when a test begins (or finishes) it > cancels all outstanding timers, commands and pending async callbacks > (perhaps optionally). The confusion occurred here because even though > the previous test threw an uncaught exception and finished allowing > the next test to begin, the outstanding commands started by the > previous test were still allowed to run and when they did, they messed > up the shared state used by the next test. > > Of course now I have to go figure out what's causing this uncaught > exception to be thrown :-(. And I'm also still confused why the tests > pass when run one at a time. If your test code ultimately involves callbacks from the browser event loop from your own code, you need to wrap all such calls in $entry, which hooks up the uncaught exception handler and the Scheduler hooks. All of the GWT widgets/etc that do this already take care of it, but if you write your own you have to handle it. Look at JsopRequest for an example, but basically if you set a callback foo.callback = function()... then that should instead be foo.callback=$entry(function()...). -- John A. Tamplin Software Engineer (GWT), Google -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] RR : Overhaul the client-side RequestFactory code (issue924801)
Reviewers: rjrjr, cromwellian, amitmanjhi, Message: This patch is against r8884. My plan is to merge up to trunk today to incorporate the new tests that have been going in, but at first glance, that shouldn't significantly change the overall code. Description: Overhaul the client-side RequestFactory code. Implements API changes per ROO-1456. Factors an AutoBean library out of RF for use later in the Editor framework Switches RF code-generator to be model-based. Patch by: bobv Review by: rjrjr, cromwellian, amitmanjhi Please review this at http://gwt-code-reviews.appspot.com/924801/show Affected files: M samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/DynaTableRf.java M samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/PersonEditorWorkflow.java M samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/events/EditPersonEvent.java M samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/widgets/SummaryWidget.java M samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/shared/DynaTableRequestFactory.java M user/src/com/google/gwt/app/place/AbstractProxyEditActivity.java M user/src/com/google/gwt/app/place/AbstractProxyListActivity.java M user/src/com/google/gwt/app/place/PropertyColumn.java M user/src/com/google/gwt/editor/Editor.gwt.xml A user/src/com/google/gwt/editor/client/AutoBean.java A user/src/com/google/gwt/editor/client/AutoBeanFactory.java A user/src/com/google/gwt/editor/client/AutoBeanUtils.java A user/src/com/google/gwt/editor/client/AutoBeanVisitor.java A user/src/com/google/gwt/editor/client/impl/AbstractAutoBean.java A user/src/com/google/gwt/editor/client/impl/AbstractAutoBeanFactory.java A user/src/com/google/gwt/editor/rebind/AutoBeanFactoryGenerator.java A user/src/com/google/gwt/editor/rebind/model/AutoBeanFactoryMethod.java A user/src/com/google/gwt/editor/rebind/model/AutoBeanFactoryModel.java A user/src/com/google/gwt/editor/rebind/model/AutoBeanMethod.java A user/src/com/google/gwt/editor/rebind/model/AutoBeanType.java M user/src/com/google/gwt/editor/rebind/model/EditorData.java M user/src/com/google/gwt/editor/rebind/model/EditorModel.java A user/src/com/google/gwt/editor/rebind/model/ModelUtils.java M user/src/com/google/gwt/requestfactory/client/RequestFactoryEditorDriver.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractBigDecimalRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractBigIntegerRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractBooleanRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractByteRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractCharacterRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractDateRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractDoubleRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractEnumRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractFloatRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractIntegerRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonListRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonObjectRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonProxyCollectionRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonProxyListRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonProxySetRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractJsonValueListRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractLongRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractPrimitiveRequest.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractRequest.java A user/src/com/google/gwt/requestfactory/client/impl/AbstractRequestContext.java A user/src/com/google/gwt/requestfactory/client/impl/AbstractRequestFactory.java M user/src/com/google/gwt/requestfactory/client/impl/AbstractRequestFactoryEditorDriver.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractShortRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractStringRequest.java D user/src/com/google/gwt/requestfactory/client/impl/AbstractVoidRequest.java D user/src/com/google/gwt/requestfactory/client/impl/DeltaValueStoreJsonImpl.java A user/src/com/google/gwt/requestfactory/client/impl/EntityCodex.java A user/src/com/google/gwt/requestfactory/client/impl/EntityProxyCategory.java D user/src/com/google/gwt/requestfactory/client/impl/EntityProxyIdImpl.java M user/src/com/google/gwt/requestfactory/client/impl/FindRequest.java D user/src/com/google/gwt/requestfactory/client/impl/FindRequestObjectImpl.java D
[gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
Thanks for the help Scott. The problem is not with the delayTestFinish. Apparently our tests are throwing uncaught exceptions which causes the test to finish. The exception does appear in our logs, but it's one of these: Caused by: com.google.gwt.core.client.JavaScriptException: (Error): Unspecified error. number: -2147467259 description: Unspecified error. at com.google.gwt.dev.shell.BrowserChannelServer.invokeJavascript(BrowserChannelServer.java: 237) Which are not terribly useful (if you have any useful suggestions on dealing with these, I'm all ears) so I moved on to the next exception which was an NPE in our code. That NPE is caused by the tests running on top of each other. So I guess this was my fault for not paying closer attention to the first exception. There is perhaps one possible recommendation that could come out of this and maybe that is that when a test begins (or finishes) it cancels all outstanding timers, commands and pending async callbacks (perhaps optionally). The confusion occurred here because even though the previous test threw an uncaught exception and finished allowing the next test to begin, the outstanding commands started by the previous test were still allowed to run and when they did, they messed up the shared state used by the next test. Of course now I have to go figure out what's causing this uncaught exception to be thrown :-(. And I'm also still confused why the tests pass when run one at a time. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Test timeout in JUnitShell ? (GWT 2.0.0)
Thanks for the detailed explanation, that sure helps. > I have seen the timeout message happen due to a compile error in the GWT > code, but you'd see those errors in the log also. Could it have something to do with issue 4700 ?: http://code.google.com/p/google-web-toolkit/issues/detail?id=4700 Some more details: + I run the the tests with the following jvm arguments: -Xms756m -Xmx756m -Dcatalina.base.create=${project.basedir} - Dgwt.args="-logLevel INFO -Xtries 1" + I run the tests in noserver mode against an external tomcat server that runs the backend that is started by the cargo maven plugin. I use a proxy servlet in the internal tomcat server that is started by GWT like explained in issue 4615: http://code.google.com/p/google-web-toolkit/issues/detail?id=4615 What is the relation between HTMLUnit and the internal tomcat that GWT uses ?.. On Oct 1, 4:08 pm, Freeland Abbott wrote: > That timeout means that GWT has launched a browser pointed at itself, to run > the test, but the browser didn't actually connect to us within 60 seconds. > The constant isn't directly configurable, no, but it's not about the size > of your test, just getting that first useful access from the browser (so > it's supposed to encompass browser start time + network latency; 60s should > be way over adequate!). > > I have seen the timeout message happen due to a compile error in the GWT > code, but you'd see those errors in the log also. > > I don't know much about the maven codehaus plugin, but here's roughly what > GWT tests do: > > 1. GWT sets itself up as a web server on some random port. JUnitShell > derives off the older GWTShell, so this should mean using our internal > tomcat as the server. (I'm not sure your "tomcat with the cargo plugin" is > relevant!) > 2. That server is set to serve a synthetic GWT module, which is created > via a generator that can reflect on your test code. > 3. Depending on the -runStyle flag, GWT chooses a browser to launch. By > default, we'd use HtmlUnit (mostly because we know where it is!), but there > are other options available, including Manual (which has no timeout, and > expects you to point a browser at GWT's server) and External (you give GWT > an executable that accepts a URL as an argument). > 4. If the browser we launch doesn't successfully access the web server we > have, on that random port, you will get the timeout you cite. If it does > access in time, the page should pull your code and start walking through > the > tests, reporting status and moving to the next test (or batch, depending on > your batch strategy). > > I hope that helps. HtmlUnit does have some known issues, so especially if > you see this happening "sometimes" you might want to experiment with either > -runStyle Manual or -runStyle External to see if those are better-behaved > for you. > > On Fri, Oct 1, 2010 at 9:06 AM, Ed wrote: > > Sometimes my GWT test's fail (during nightly build) and give me the > > following error. > > - > > [INFO] com.google.gwt.junit.client.TimeoutException: The browser did > > not contact the server within 6ms. > > [INFO] - NO RESPONSE: 192.168.1.65 / Mozilla/5.0 (Windows; U; Windows > > NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1 > > [INFO] - 1 client(s) haven't responded back to JUnitShell since the > > start of the test. > > [INFO] Actual time elapsed: 60.004 seconds. > > -- > > > This occurs when running my tests through the maven codehaus plugin, > > starting first tomcat with the cargo plugin. > > I played with the timeout settings I have in the maven and cargo > > plugin, but don't seem to be able to solve it. > > The maven plugin will run GWT test suites that extend from > > GWTTestSuite that contain test cases that extend GWTTestCase. > > The first GWT test suite always run with success and then the > > following GWT test suite gives the above error > > > The code in JUnitShell throwing the above error: > > > > } else if (testBeginTimeout < currentTimeMillis) { > > double elapsed = (currentTimeMillis - testBeginTime) / 1000.0; > > throw new TimeoutException( > > "The browser did not contact the server within " > > + TEST_BEGIN_TIMEOUT_MILLIS + "ms.\n" > > + messageQueue.getUnretrievedClients(currentTestInfo) > > + "\n Actual time elapsed: " + elapsed + " seconds.\n"); > > } > > > > > I don't understand well how the testing with server/client concept > > works. > > Why is this timeout generated and how can I solve this? > > Shouldn't the timeout be adjustable as I the timeout is contained in > > the constant TEST_BEGIN_TIMEOUT_MILLIS ? > > > -- > >http://groups.google.com/group/Google-Web-Toolkit-Contributors > > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: editor framework and radio buttons
yeah, CheckBox doesn't work out of the box either because it needs to implement ValueBase On Fri, Oct 1, 2010 at 8:36 AM, Patrick Julien wrote: > thanks for the info > > On Fri, Oct 1, 2010 at 5:27 AM, Thomas Broyer wrote: >> >> On Sep 30, 6:56 pm, Patrick Julien wrote: >>> Is there a class that integrates the two? same for check boxes? >> >> Hmm, CheckBox implements IsEditor> so it >> should work OK for a boolean value. >> >> RadioButton extends CheckBox so it's a "boolean editor", but radio >> buttons are not about boolean things... :-/ >> >> I think for checkbox lists (multi-valued selection) and radio button >> lists (and even a Cell-widget-based selector), you'd (currently) have >> to make your own Editor class. >> It seems hard to provide such editors out-of-the-box because there are >> many use cases, depending on whether you want the value be the "index" >> in a list of acceptable values, the "form value" of the checkbox/ >> radiobutton, or an object in a list of acceptable values (this is >> similar to why ListBox does not implement HasValue); not to mention >> how you'd like your checkboxes/radiobuttons be laid out! >> >> (Also noteworthy: SimpleCheckBox implements neither IsEditor or even >> HasValue; and for single selection within a dropdown list, there's >> ValueListBox) >> >> -- >> http://groups.google.com/group/Google-Web-Toolkit-Contributors > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r8913 committed - Clarifying the JavaDoc of CellPanel (the parent of DockPanel and Horiz...
Revision: 8913 Author: gwt.mirror...@gmail.com Date: Fri Oct 1 05:51:52 2010 Log: Clarifying the JavaDoc of CellPanel (the parent of DockPanel and Horizontal/VerticalPanel) so that people do not mistake it for a Cell based data presentation widget. Review by: r...@google.com http://code.google.com/p/google-web-toolkit/source/detail?r=8913 Modified: /trunk/user/src/com/google/gwt/user/client/ui/CellPanel.java === --- /trunk/user/src/com/google/gwt/user/client/ui/CellPanel.java Mon Nov 30 20:51:14 2009 +++ /trunk/user/src/com/google/gwt/user/client/ui/CellPanel.java Fri Oct 1 05:51:52 2010 @@ -25,6 +25,12 @@ * cell's size may be set independently. Each child widget can take up a subset * of its cell and can be aligned within it. * + * + * Note: This class is not related to the + * {...@link com.google.gwt.cell.client.Cell} based data presentation widgets such + * as {...@link com.google.gwt.user.cellview.client.CellList} and + * {...@link com.google.gwt.user.cellview.client.CellTable}. + * * Use in UiBinder Templates * * When working with CellPanel subclasses in -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fixes an NPE when setting an entity parameter to null. The NPE occurs in generated code because ... (issue944801)
Reviewers: cromwellian, Description: Fixes an NPE when setting an entity parameter to null. The NPE occurs in generated code because we try to get the entity in wire format. Please review this at http://gwt-code-reviews.appspot.com/944801/show Affected files: M user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java M user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java Index: user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java === --- user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java (revision 8912) +++ user/src/com/google/gwt/requestfactory/rebind/RequestFactoryGenerator.java (working copy) @@ -1041,22 +1041,25 @@ if (sb.length() > 0) { sb.append(", "); } - JClassType classType = parameter.getType().isClassOrInterface(); - JType paramType = parameter.getType(); + JClassType classType = paramType.isClassOrInterface(); + String paramName = parameter.getName(); + if (paramType.getQualifiedSourceName().equals( EntityProxyId.class.getName())) { -sb.append("factory.getWireFormat(" + parameter.getName() + ")"); +sb.append("factory.getWireFormat(" + paramName + ")"); continue; } if (classType != null && classType.isAssignableTo(entityProxyType)) { +sb.append(paramName + " == null ? null : "); sb.append("((" + classType.getQualifiedBinaryName() + "Impl" + ")"); - } - sb.append(parameter.getName()); - if (classType != null && classType.isAssignableTo(entityProxyType)) { +sb.append(paramName); sb.append(").wireFormatId()"); - } +continue; + } + + sb.append(paramName); } return "new Object[] {" + sb.toString() + "}"; } Index: user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java === --- user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java (revision 8912) +++ user/test/com/google/gwt/requestfactory/client/RequestFactoryTest.java (working copy) @@ -499,7 +499,7 @@ /** * Test that a null value can be sent in a request. */ - public void disabledTestNullSimpleFooRequest() { + public void testNullSimpleFooRequest() { delayTestFinish(5000); final Request fooReq = req.simpleFooRequest().receiveNullSimpleFoo(null); fooReq.fire(new Receiver() { -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
Ok, that's pretty strange, but there is a way to debug. - Unzip gwt-user.jar somewhere. - Inside where you extracted it, there should be a folder "com/google/gwt/junit/translatable/" - Go edit your launch config and add that folder to the Source Lookup Path, under the "Source" tab, "Add." a "File System Directory", and select that folder named "translatable". Move it UP above the default path. - Put a breakpoint where you call delayTestFinish() - Launch You should now be able to step into delayTestFinish() and/or step out of your test method and see what the client code is doing. On Fri, Oct 1, 2010 at 11:13 AM, Damon Lundin wrote: > On Oct 1, 9:11 am, Scott Blum wrote: > > Is this in dev mode or prod mode? Do the tests succeed if you run them > > individually? > > This is in dev mode. If I disable all of the tests and run them > individually they all pass. I've never tried running tests in prod > mode but I think I know how and I can give that a try. The next thing > to try as well would be to chain all the test methods together. So > instead of having test01...testN, I can have the end of test00 start > test01 and the end of test01 start test02, etc. Of course that's > problematic if a test fails, the next one won't run. And if the > delayTestFinish isn't working, the whole thing might just stop before > it gets all the way through. But something to try. > > -- > http://groups.google.com/group/Google-Web-Toolkit-Contributors > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
On Oct 1, 9:11 am, Scott Blum wrote: > Is this in dev mode or prod mode? Do the tests succeed if you run them > individually? This is in dev mode. If I disable all of the tests and run them individually they all pass. I've never tried running tests in prod mode but I think I know how and I can give that a try. The next thing to try as well would be to chain all the test methods together. So instead of having test01...testN, I can have the end of test00 start test01 and the end of test01 start test02, etc. Of course that's problematic if a test fails, the next one won't run. And if the delayTestFinish isn't working, the whole thing might just stop before it gets all the way through. But something to try. -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Converting MultiWordSuggestOracle to use SafeHtml to escape suggestions, and fixing fixing broke... (issue938801)
LGTM http://gwt-code-reviews.appspot.com/938801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix nonstandard two method handler interface that probably would have (issue941801)
LGTM http://gwt-code-reviews.appspot.com/941801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] delayTestFinish allowing the next test to begin before the previous one finishes (2.1.0.m3)?
Is this in dev mode or prod mode? Do the tests succeed if you run them individually? On Thu, Sep 30, 2010 at 7:54 PM, Damon Lundin wrote: > We are trying to upgrade from 2.0.4 to 2.1.0.m3 because of some bugs > in 2.0.4 but in doing so our GWT test cases have begun failing and I > have determined it's because the next test is beginning before the > previous one is complete and they are messing each other up. I know > that delayTestFinish is meant to allow for tests to make use of async > calls and it's been working for us but now it's not. > > We have a number of tests that roughly look like this: > > public void test03 { > delayTestFinish(12); > rpcService.someServerCall(new AsyncCallback() { >public void onSuccess() { > // Do some stuff > DeferredCommand.addCommand(new Command() { >public void execute() { > // More stuff > finishTest(); >} > } >} > } > } > > public void test04 { > delayTestFinish(12); > ... > } > > Our code actually uses DeferredCommands in a long chain of calls but > in short, test04 begins executing before we call finishTest() in the > previous test and I don't think that's supposed to happen. > > I could further debug this if someone could explain to me what > "implemented in the translatable version of this class" means and how > I would debug this "translatable version". Any advice on how to > proceed? > > -- > http://groups.google.com/group/Google-Web-Toolkit-Contributors > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Test timeout in JUnitShell ? (GWT 2.0.0)
That timeout means that GWT has launched a browser pointed at itself, to run the test, but the browser didn't actually connect to us within 60 seconds. The constant isn't directly configurable, no, but it's not about the size of your test, just getting that first useful access from the browser (so it's supposed to encompass browser start time + network latency; 60s should be way over adequate!). I have seen the timeout message happen due to a compile error in the GWT code, but you'd see those errors in the log also. I don't know much about the maven codehaus plugin, but here's roughly what GWT tests do: 1. GWT sets itself up as a web server on some random port. JUnitShell derives off the older GWTShell, so this should mean using our internal tomcat as the server. (I'm not sure your "tomcat with the cargo plugin" is relevant!) 2. That server is set to serve a synthetic GWT module, which is created via a generator that can reflect on your test code. 3. Depending on the -runStyle flag, GWT chooses a browser to launch. By default, we'd use HtmlUnit (mostly because we know where it is!), but there are other options available, including Manual (which has no timeout, and expects you to point a browser at GWT's server) and External (you give GWT an executable that accepts a URL as an argument). 4. If the browser we launch doesn't successfully access the web server we have, on that random port, you will get the timeout you cite. If it does access in time, the page should pull your code and start walking through the tests, reporting status and moving to the next test (or batch, depending on your batch strategy). I hope that helps. HtmlUnit does have some known issues, so especially if you see this happening "sometimes" you might want to experiment with either -runStyle Manual or -runStyle External to see if those are better-behaved for you. On Fri, Oct 1, 2010 at 9:06 AM, Ed wrote: > Sometimes my GWT test's fail (during nightly build) and give me the > following error. > - > [INFO] com.google.gwt.junit.client.TimeoutException: The browser did > not contact the server within 6ms. > [INFO] - NO RESPONSE: 192.168.1.65 / Mozilla/5.0 (Windows; U; Windows > NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1 > [INFO] - 1 client(s) haven't responded back to JUnitShell since the > start of the test. > [INFO] Actual time elapsed: 60.004 seconds. > -- > > This occurs when running my tests through the maven codehaus plugin, > starting first tomcat with the cargo plugin. > I played with the timeout settings I have in the maven and cargo > plugin, but don't seem to be able to solve it. > The maven plugin will run GWT test suites that extend from > GWTTestSuite that contain test cases that extend GWTTestCase. > The first GWT test suite always run with success and then the > following GWT test suite gives the above error > > The code in JUnitShell throwing the above error: > >} else if (testBeginTimeout < currentTimeMillis) { > double elapsed = (currentTimeMillis - testBeginTime) / 1000.0; > throw new TimeoutException( > "The browser did not contact the server within " > + TEST_BEGIN_TIMEOUT_MILLIS + "ms.\n" > + messageQueue.getUnretrievedClients(currentTestInfo) > + "\n Actual time elapsed: " + elapsed + " seconds.\n"); >} > > > I don't understand well how the testing with server/client concept > works. > Why is this timeout generated and how can I solve this? > Shouldn't the timeout be adjustable as I the timeout is contained in > the constant TEST_BEGIN_TIMEOUT_MILLIS ? > > -- > http://groups.google.com/group/Google-Web-Toolkit-Contributors > -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Test timeout in JUnitShell ? (GWT 2.0.0)
Sometimes my GWT test's fail (during nightly build) and give me the following error. - [INFO] com.google.gwt.junit.client.TimeoutException: The browser did not contact the server within 6ms. [INFO] - NO RESPONSE: 192.168.1.65 / Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.1) Gecko/2008070208 Firefox/3.0.1 [INFO] - 1 client(s) haven't responded back to JUnitShell since the start of the test. [INFO] Actual time elapsed: 60.004 seconds. -- This occurs when running my tests through the maven codehaus plugin, starting first tomcat with the cargo plugin. I played with the timeout settings I have in the maven and cargo plugin, but don't seem to be able to solve it. The maven plugin will run GWT test suites that extend from GWTTestSuite that contain test cases that extend GWTTestCase. The first GWT test suite always run with success and then the following GWT test suite gives the above error The code in JUnitShell throwing the above error: } else if (testBeginTimeout < currentTimeMillis) { double elapsed = (currentTimeMillis - testBeginTime) / 1000.0; throw new TimeoutException( "The browser did not contact the server within " + TEST_BEGIN_TIMEOUT_MILLIS + "ms.\n" + messageQueue.getUnretrievedClients(currentTestInfo) + "\n Actual time elapsed: " + elapsed + " seconds.\n"); } I don't understand well how the testing with server/client concept works. Why is this timeout generated and how can I solve this? Shouldn't the timeout be adjustable as I the timeout is contained in the constant TEST_BEGIN_TIMEOUT_MILLIS ? -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: editor framework and radio buttons
thanks for the info On Fri, Oct 1, 2010 at 5:27 AM, Thomas Broyer wrote: > > On Sep 30, 6:56 pm, Patrick Julien wrote: >> Is there a class that integrates the two? same for check boxes? > > Hmm, CheckBox implements IsEditor> so it > should work OK for a boolean value. > > RadioButton extends CheckBox so it's a "boolean editor", but radio > buttons are not about boolean things... :-/ > > I think for checkbox lists (multi-valued selection) and radio button > lists (and even a Cell-widget-based selector), you'd (currently) have > to make your own Editor class. > It seems hard to provide such editors out-of-the-box because there are > many use cases, depending on whether you want the value be the "index" > in a list of acceptable values, the "form value" of the checkbox/ > radiobutton, or an object in a list of acceptable values (this is > similar to why ListBox does not implement HasValue); not to mention > how you'd like your checkboxes/radiobuttons be laid out! > > (Also noteworthy: SimpleCheckBox implements neither IsEditor or even > HasValue; and for single selection within a dropdown list, there's > ValueListBox) > > -- > http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: editor framework and radio buttons
On Sep 30, 6:56 pm, Patrick Julien wrote: > Is there a class that integrates the two? same for check boxes? Hmm, CheckBox implements IsEditor> so it should work OK for a boolean value. RadioButton extends CheckBox so it's a "boolean editor", but radio buttons are not about boolean things... :-/ I think for checkbox lists (multi-valued selection) and radio button lists (and even a Cell-widget-based selector), you'd (currently) have to make your own Editor class. It seems hard to provide such editors out-of-the-box because there are many use cases, depending on whether you want the value be the "index" in a list of acceptable values, the "form value" of the checkbox/ radiobutton, or an object in a list of acceptable values (this is similar to why ListBox does not implement HasValue); not to mention how you'd like your checkboxes/radiobuttons be laid out! (Also noteworthy: SimpleCheckBox implements neither IsEditor or even HasValue; and for single selection within a dropdown list, there's ValueListBox) -- http://groups.google.com/group/Google-Web-Toolkit-Contributors