[gwt-contrib] Re: Implements * globbing for RequestFactory with(), the simpler half of (issue1520808)
LGTM. I like how simple the core diff is. http://gwt-code-reviews.appspot.com/1520808/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryPolymorphicTest.java File user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryPolymorphicTest.java (right): http://gwt-code-reviews.appspot.com/1520808/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryPolymorphicTest.java#newcode78 user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryPolymorphicTest.java:78: Extra whitespace. http://gwt-code-reviews.appspot.com/1520808/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryPolymorphicTest.java#newcode701 user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryPolymorphicTest.java:701: Extra whitespace here and elsewhere. http://gwt-code-reviews.appspot.com/1520808/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java File user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java (right): http://gwt-code-reviews.appspot.com/1520808/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode715 user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:715: .getFooField()); Does selfOneToManyField have more than one element in it? If so, it would be good to double-check the length of the field and make ensure the globbing works on more than just the first element. http://gwt-code-reviews.appspot.com/1520808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Collapse deferred-binding property values that depend on collapsed property values. (issue1525804)
Reviewers: rjrjr, jlabanca,
Message:
Actual changes marked in the review, the rest is autoformatting.
The basis for this change is to prevent properties that depend on other
collapsed properties from ballooning the number of hard (emitted)
permutations. For example, the html5-related deferred binding
properties prevent a simple collapse-property name=user.agent / from
actually reducing the number of permutations emitted.
The rule:
A dependency on a collapsed property forms an equivalence set for the
dependent value. In other words, Collapsing is contagious.
http://gwt-code-reviews.appspot.com/1525804/diff/1/dev/core/src/com/google/gwt/dev/cfg/BindingProperty.java
File dev/core/src/com/google/gwt/dev/cfg/BindingProperty.java (right):
http://gwt-code-reviews.appspot.com/1525804/diff/1/dev/core/src/com/google/gwt/dev/cfg/BindingProperty.java#newcode313
dev/core/src/com/google/gwt/dev/cfg/BindingProperty.java:313: void
normalizeCollapsedValues() {
Actual change is here.
http://gwt-code-reviews.appspot.com/1525804/diff/1/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java
File dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java
(right):
http://gwt-code-reviews.appspot.com/1525804/diff/1/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java#newcode230
dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java:230:
public String toString() {
Actual change here.
Description:
Collapse deferred-binding property values that depend on collapsed
property values.
Patch by: bobv
Review by: rjrjr
Please review this at http://gwt-code-reviews.appspot.com/1525804/
Affected files:
M dev/core/src/com/google/gwt/dev/cfg/BindingProperty.java
M dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java
M dev/core/test/com/google/gwt/dev/cfg/ModuleDefTest.java
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Support chained requests where the returned proxy type is not reachable from the canonical Reque... (issue1499810)
Reviewers: rjrjr, Description: Support chained requests where the returned proxy type is not reachable from the canonical RequestContext. Issue 6641. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1499810/ Affected files: M user/src/com/google/web/bindery/autobean/vm/impl/FactoryHandler.java M user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java M user/test/com/google/web/bindery/requestfactory/gwt/RequestFactorySuite.java A user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryChainedContextTest.java M user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java A user/test/com/google/web/bindery/requestfactory/server/RequestFactoryChainedContextJreTest.java M user/test/com/google/web/bindery/requestfactory/vm/RequestFactoryJreSuite.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Support chained requests where the returned proxy type is not reachable from the canonical Reque... (issue1499810)
PTAL http://gwt-code-reviews.appspot.com/1499810/diff/1/user/src/com/google/web/bindery/autobean/vm/impl/FactoryHandler.java File user/src/com/google/web/bindery/autobean/vm/impl/FactoryHandler.java (right): http://gwt-code-reviews.appspot.com/1499810/diff/1/user/src/com/google/web/bindery/autobean/vm/impl/FactoryHandler.java#newcode54 user/src/com/google/web/bindery/autobean/vm/impl/FactoryHandler.java:54: return method.invoke(this, args); On 2011/08/04 18:30:33, rjrjr wrote: Is this really related to this patch? What's it fixing? Removed. http://gwt-code-reviews.appspot.com/1499810/diff/1/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java File user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java (right): http://gwt-code-reviews.appspot.com/1499810/diff/1/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java#newcode505 user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java:505: AutoBeanT created; On 2011/08/04 18:30:33, rjrjr wrote: So when I append the a CarService request context to a DogService context, the car context will know how to make dogs, yes? Are we sure that's desirable? I guess I'm asking if it's a side effect of the fix that is not worth the work of masking, or if you're taking extra effort to make it happen. Presuming this side effect is happening because it needs to happen, we should explicitly test for it. Changed createProxy() so that the user-accessible create() method isn't subject to cross-contamination. However, there are implementation limitations in the jre implementation that currently make this impractical. Filed issue 6658, since the difference in behavior can be eliminated after the RfValidator patch lands. I also changed createProxy() to be a protected method. http://gwt-code-reviews.appspot.com/1499810/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryChainedContextTest.java File user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryChainedContextTest.java (right): http://gwt-code-reviews.appspot.com/1499810/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryChainedContextTest.java#newcode38 user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryChainedContextTest.java:38: * rechable proxy types. On 2011/08/04 18:30:33, rjrjr wrote: reachable Done. http://gwt-code-reviews.appspot.com/1499810/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryChainedContextTest.java#newcode91 user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryChainedContextTest.java:91: SimpleFooRequest c3 = c2.append(req.simpleFooRequest()); On 2011/08/04 18:30:33, rjrjr wrote: Would be easier to read of the context names reflected their type. barRc1, fooRc1 Done. http://gwt-code-reviews.appspot.com/1499810/diff/3002/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java File user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java (right): http://gwt-code-reviews.appspot.com/1499810/diff/3002/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java#newcode107 user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java:107: public SetAbstractRequestContext appendedContexts; Changed this to track the RequestContext instances since it's more general. http://gwt-code-reviews.appspot.com/1499810/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Replace RequestFactoryInterfaceValidator with an annotation-processor-based approach. (issue1503804)
http://gwt-code-reviews.appspot.com/1503804/diff/1/samples/dynatablerf/build.xml
File samples/dynatablerf/build.xml (right):
http://gwt-code-reviews.appspot.com/1503804/diff/1/samples/dynatablerf/build.xml#newcode8
samples/dynatablerf/build.xml:8: !-- Run the annotation processor --
On 2011/08/02 17:43:26, rjrjr wrote:
This is a sample, can you be a bit more verbose? Annotation processor
is an
implementation detail.
So really people only need to include this jar if they want compile
time
notification of RF errors. Which seems like a pretty big deal,
especially if the
rf server will no longer do such validation by default.
The server code won't accept the RequestFactory interface if it hasn't
been validated. A runtime error will occur, telling the user to run the
ValidationTool.
Should we do something to force people to include this jar, like
remove RF from
gwt-user? I suppose that's a pretty drastic thing to do to existing
users,
although frankly it's tempting. Will they see any kind of warning if
they don't
include it ?
I'm actually tempted to be drastic.
I've been thinking about pulling the requestfactory server and vm code
from gwt-[user|servlet].jar since that encourages deployment bloat. I
could go either way on pulling the client code from gwt-user.jar.
Would it be better to not include the annotation processor in
requestfactory-client.jar and instead require the user to add
requestfactory-apt.jar to their build path? It would slightly reduce
the amount of code in requestfactory-client.jar and would avoid any
confusion with adding requestfactory-client.jar to a server build
process.
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DeobfuscatorBuilder.java
File
user/src/com/google/web/bindery/requestfactory/apt/DeobfuscatorBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DeobfuscatorBuilder.java#newcode38
user/src/com/google/web/bindery/requestfactory/apt/DeobfuscatorBuilder.java:38:
* Visits a RequestFactory to create its associated DeobfuscatorBuilder
type.
On 2011/08/02 17:43:26, rjrjr wrote:
Visits an RF to create itself? ...its associated {@link
Deobfuscator}... yes?
Done.
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DescriptorBuilder.java
File
user/src/com/google/web/bindery/requestfactory/apt/DescriptorBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DescriptorBuilder.java#newcode34
user/src/com/google/web/bindery/requestfactory/apt/DescriptorBuilder.java:34:
* Builds descriptors from TypeMirrors for both simple types and methods.
On 2011/08/02 17:43:26, rjrjr wrote:
These descriptors are used by {@link ...}
Done.
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java
File
user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java
(right):
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java#newcode50
user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java:50:
* output jar and the binary names of RequestFactory interfaces that
should be
On 2011/07/29 00:53:51, tbroyer wrote:
Out of curiosity: why an output *jar* and not an output folder? (or
giving the
user the choice).
Using a folder would allow outputting to a WAR's WEB-INF/classes
(before
packaging the WAR of course) or, when building using Maven, outputting
to
target/classes or target/generated-classes (whatever is best for
Maven) so the
generated classes can be used by tests (which are run before the code
is
packaged into a JAR, and adding a JAR to the classpath that is not a
dependency
might not be practical, if at all possible; it's late here, so I
haven't checked
how it could work).
Updated so that if the first argument is a directory, the classes will
be emitted as individual files.
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java
File
user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java
(right):
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java#newcode27
user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java:27:
* Provides access to payload deobfuscation services.
On 2011/08/02 17:43:26, rjrjr wrote:
...for both servers and clients.
Is this the place to mention what GWT clients do instead?
Done.
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java#newcode37
user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java:37:
* processor as part of the build process.
On 2011/08/02 17:43:26,
[gwt-contrib] Re: Replace RequestFactoryInterfaceValidator with an annotation-processor-based approach. (issue1503804)
Per offline conversation, I've removed the annotation processor from requestfactory-client.jar. This means that the server code must be built with requestfactory-apt.jar, but doesn't need to be deployed with it. http://gwt-code-reviews.appspot.com/1503804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Prevent an AutoBean property named property from causing a compilation error. (issue1505804)
Reviewers: rjrjr,
Message:
Review requested.
Description:
Prevent an AutoBean property named property from causing a compilation
error.
Patch by: bobv
Review by: rjrjr
Please review this at http://gwt-code-reviews.appspot.com/1505804/
Affected files:
M
user/src/com/google/web/bindery/autobean/gwt/rebind/AutoBeanFactoryGenerator.java
M user/test/com/google/web/bindery/autobean/gwt/client/AutoBeanTest.java
Index:
user/src/com/google/web/bindery/autobean/gwt/rebind/AutoBeanFactoryGenerator.java
===
---
user/src/com/google/web/bindery/autobean/gwt/rebind/AutoBeanFactoryGenerator.java
(revision 10473)
+++
user/src/com/google/web/bindery/autobean/gwt/rebind/AutoBeanFactoryGenerator.java
(working copy)
@@ -231,8 +231,9 @@
String castType;
if (returnType.isPrimitive() != null) {
castType =
returnType.isPrimitive().getQualifiedBoxedSourceName();
-// Boolean toReturn = getOrReify(foo);
-sw.println(%s toReturn = getOrReify(\%s\);, castType,
method.getPropertyName());
+// Boolean toReturn = Outher.this.getOrReify(foo);
+sw.println(%s toReturn = %s.this.getOrReify(\%s\);,
castType, type
+.getSimpleSourceName(), method.getPropertyName());
// return toReturn == null ? false : toReturn;
sw.println(return toReturn == null ? %s : toReturn;,
returnType.isPrimitive()
.getUninitializedFieldExpression());
@@ -241,17 +242,19 @@
sw.println(return data.isNull(\%1$s\) ? null :
data.get(\%1$s\);, method
.getPropertyName());
} else {
-// return (ReturnType) values.getOrReify(\foo\);
+// return (ReturnType) Outer.this.getOrReify(\foo\);
castType = ModelUtils.getQualifiedBaseSourceName(returnType);
-sw.println(return (%s) getOrReify(\%s\);, castType,
method.getPropertyName());
+sw.println(return (%s) %s.this.getOrReify(\%s\);,
castType, type
+.getSimpleSourceName(), method.getPropertyName());
}
}
break;
case SET:
case SET_BUILDER: {
JParameter param = jmethod.getParameters()[0];
- // setProperty(foo, parameter);
- sw.println(setProperty(\%s\, %s);, method.getPropertyName(),
param.getName());
+ // Other.this.setProperty(foo, parameter);
+ sw.println(%s.this.setProperty(\%s\, %s);,
type.getSimpleSourceName(), method
+ .getPropertyName(), param.getName());
if (JBeanMethod.SET_BUILDER.equals(method.getAction())) {
sw.println(return this;);
}
Index:
user/test/com/google/web/bindery/autobean/gwt/client/AutoBeanTest.java
===
--- user/test/com/google/web/bindery/autobean/gwt/client/AutoBeanTest.java
(revision 10473)
+++ user/test/com/google/web/bindery/autobean/gwt/client/AutoBeanTest.java
(working copy)
@@ -15,14 +15,14 @@
*/
package com.google.web.bindery.autobean.gwt.client;
+import com.google.gwt.core.client.GWT;
+import com.google.gwt.junit.client.GWTTestCase;
import com.google.web.bindery.autobean.shared.AutoBean;
import com.google.web.bindery.autobean.shared.AutoBeanFactory;
import com.google.web.bindery.autobean.shared.AutoBeanFactory.Category;
import com.google.web.bindery.autobean.shared.AutoBeanUtils;
import com.google.web.bindery.autobean.shared.AutoBeanVisitor;
import
com.google.web.bindery.autobean.shared.AutoBeanVisitor.ParameterizationVisitor;
-import com.google.gwt.core.client.GWT;
-import com.google.gwt.junit.client.GWTTestCase;
import java.util.ArrayList;
import java.util.Collection;
@@ -42,6 +42,7 @@
public static Object seen;
public static T T __intercept(AutoBeanHasCall bean, T value) {
+ assertNotNull(bean);
seen = value;
return value;
}
@@ -129,9 +130,14 @@
interface Intf {
int getInt();
+String getProperty();
+
String getString();
void setInt(int number);
+
+// Avoid name conflicts in AbstractAutoBean
+void setProperty(String value);
void setString(String value);
}
@@ -150,6 +156,7 @@
static class RealIntf implements Intf {
int i;
+String property;
String string;
@Override
@@ -161,6 +168,11 @@
return i;
}
+@Override
+public String getProperty() {
+ return property;
+}
+
public String getString() {
return string;
}
@@ -172,6 +184,11 @@
public void setInt(int number) {
this.i = number;
+}
+
+@Override
+public void setProperty(String value) {
+ this.property = value;
}
public void setString(String value) {
@@ -473,7 +490,7 @@
if (int.equals(propertyName)) {
assertEquals(42, value);
assertEquals(int.class
[gwt-contrib] Replace RequestFactoryInterfaceValidator with an annotation-processor-based approach. (issue1503804)
/RequestFactoryInterfaceValidatorTest.java
File
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java
(left):
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java#oldcode46
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java:46:
public class RequestFactoryInterfaceValidatorTest extends TestCase {
RfValidatorTest was introduced in a previous commit and expanded in this
patch. This test can be retired since RFIV is being removed.
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java
File
user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java
(left):
http://gwt-code-reviews.appspot.com/1503804/diff/1/user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java#oldcode18
user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java:18:
import com.google.gwt.dev.util.collect.HashSet;
Weird import problem.
Description:
Replace RequestFactoryInterfaceValidator with an
annotation-processor-based approach.
Add a ValidationTool to precompute server and JRE-client metadata.
Patch by: bobv
Review by: rjrjr, tbroyer
Please review this at http://gwt-code-reviews.appspot.com/1503804/
Affected files:
M requestfactory/build.xml
M samples/dynatablerf/build.xml
M
user/src/com/google/web/bindery/requestfactory/apt/ClientToDomainMapper.java
A
user/src/com/google/web/bindery/requestfactory/apt/DeobfuscatorBuilder.java
A
user/src/com/google/web/bindery/requestfactory/apt/DescriptorBuilder.java
M user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java
A
user/src/com/google/web/bindery/requestfactory/apt/ExtraTypesScanner.java
M user/src/com/google/web/bindery/requestfactory/apt/Messages.java
A
user/src/com/google/web/bindery/requestfactory/apt/ReferredTypesCollector.java
M
user/src/com/google/web/bindery/requestfactory/apt/RequestContextScanner.java
M
user/src/com/google/web/bindery/requestfactory/apt/RequestFactoryScanner.java
D user/src/com/google/web/bindery/requestfactory/apt/RfApt.java
M user/src/com/google/web/bindery/requestfactory/apt/RfValidator.java
M user/src/com/google/web/bindery/requestfactory/apt/ScannerBase.java
M user/src/com/google/web/bindery/requestfactory/apt/State.java
A user/src/com/google/web/bindery/requestfactory/apt/TypeComparator.java
A user/src/com/google/web/bindery/requestfactory/apt/ValidationTool.java
D user/src/com/google/web/bindery/requestfactory/server/Deobfuscator.java
D user/src/com/google/web/bindery/requestfactory/server/OperationData.java
D
user/src/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidator.java
M
user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java
M
user/src/com/google/web/bindery/requestfactory/server/ResolverServiceLayer.java
M
user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestContext.java
M
user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestFactory.java
A user/src/com/google/web/bindery/requestfactory/vm/impl/Deobfuscator.java
A
user/src/com/google/web/bindery/requestfactory/vm/impl/OperationData.java
D
user/src/com/google/web/bindery/requestfactory/vm/impl/TypeTokenResolver.java
M
user/test/com/google/web/bindery/requestfactory/apt/MyRequestContext.java
M
user/test/com/google/web/bindery/requestfactory/apt/MyRequestFactory.java
M user/test/com/google/web/bindery/requestfactory/apt/RfValidatorTest.java
M
user/test/com/google/web/bindery/requestfactory/server/BoxesAndPrimitivesJreTest.java
D
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java
M
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryJreTest.java
M
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryPolymorphicJreTest.java
M user/test/com/google/web/bindery/requestfactory/server/SimpleBar.java
M
user/test/com/google/web/bindery/requestfactory/vm/RequestFactoryJreSuite.java
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Improve error message when a RequestFactory 2.3 request is received. (issue1503803)
Reviewers: rjrjr,
Description:
Improve error message when a RequestFactory 2.3 request is received.
Issue 6628.
Patch by: bobv
Review by: rjrjr
Please review this at http://gwt-code-reviews.appspot.com/1503803/
Affected files:
M
user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java
M
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
Index:
user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java
===
---
user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java
(revision 10252)
+++
user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java
(working copy)
@@ -126,7 +126,6 @@
try {
process(req, responseBean.as());
} catch (ReportableException e) {
- e.printStackTrace();
// Create a new response envelope, since the state is unknown
responseBean = FACTORY.response();
responseBean.as().setGeneralFailure(createFailureMessage(e).as());
@@ -200,7 +199,12 @@
final RequestState source = new RequestState(service);
// Make sure the RequestFactory is valid
-service.resolveRequestFactory(req.getRequestFactory());
+String requestFactoryToken = req.getRequestFactory();
+if (requestFactoryToken == null) {
+ // Tell old clients to go away
+ throw new ReportableException(The client payload version is out of
sync with the server);
+}
+service.resolveRequestFactory(requestFactoryToken);
// Apply operations
processOperationMessages(source, req);
Index:
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
===
---
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
(revision 10252)
+++
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
(working copy)
@@ -15,6 +15,7 @@
*/
package com.google.web.bindery.requestfactory.gwt.client;
+import com.google.web.bindery.autobean.shared.AutoBeanCodex;
import com.google.web.bindery.requestfactory.shared.EntityProxy;
import com.google.web.bindery.requestfactory.shared.EntityProxyChange;
import com.google.web.bindery.requestfactory.shared.EntityProxyId;
@@ -23,6 +24,8 @@
import com.google.web.bindery.requestfactory.shared.Receiver;
import com.google.web.bindery.requestfactory.shared.Request;
import com.google.web.bindery.requestfactory.shared.RequestContext;
+import com.google.web.bindery.requestfactory.shared.RequestTransport;
+import
com.google.web.bindery.requestfactory.shared.RequestTransport.TransportReceiver;
import com.google.web.bindery.requestfactory.shared.ServerFailure;
import com.google.web.bindery.requestfactory.shared.SimpleBarProxy;
import com.google.web.bindery.requestfactory.shared.SimpleBarRequest;
@@ -31,7 +34,10 @@
import com.google.web.bindery.requestfactory.shared.SimpleFooRequest;
import com.google.web.bindery.requestfactory.shared.SimpleValueContext;
import com.google.web.bindery.requestfactory.shared.SimpleValueProxy;
+import
com.google.web.bindery.requestfactory.shared.impl.MessageFactoryHolder;
import
com.google.web.bindery.requestfactory.shared.impl.SimpleEntityProxyId;
+import
com.google.web.bindery.requestfactory.shared.messages.ResponseMessage;
+import
com.google.web.bindery.requestfactory.shared.messages.ServerFailureMessage;
import java.math.BigDecimal;
import java.math.BigInteger;
@@ -565,6 +571,49 @@
}
/**
+ * Tests the server behavior when an empty JSON object is sent.
+ */
+ public void testEmptyRequestBlankObject() {
+delayTestFinish(DELAY_TEST_FINISH);
+RequestTransport transport = req.getRequestTransport();
+transport.send({}, new TransportReceiver() {
+ @Override
+ public void onTransportFailure(ServerFailure failure) {
+fail();
+ }
+
+ @Override
+ public void onTransportSuccess(String payload) {
+ResponseMessage resp =
+AutoBeanCodex.decode(MessageFactoryHolder.FACTORY,
ResponseMessage.class, payload).as();
+ServerFailureMessage failure = resp.getGeneralFailure();
+assertNotNull(failure);
+finishTestAndReset();
+ }
+});
+ }
+
+ /**
+ * Tests the server behavior when a zero-length payload is sent.
+ */
+ public void testEmptyRequestZeroLength() {
+delayTestFinish(DELAY_TEST_FINISH);
+RequestTransport transport = req.getRequestTransport();
+transport.send(, new TransportReceiver() {
+ @Override
+ public void onTransportFailure(ServerFailure failure) {
+// Expect a 500 since the payload is malformed
+finishTestAndReset();
+ }
+
+ @Override
+ public void onTransportSuccess(String payload) {
+fail(Should not have succeeded);
+ }
+});
+ }
+
+ /**
* Tests
[gwt-contrib] Add font-face support to CssResource. (issue1502806)
Reviewers: rjrjr, Message: This is a re-spin of a patch that was written back in October but that got dropped. The original patch is at http://gwt-code-reviews.appspot.com/943802. Description: Add font-face support to CssResource. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1502806/ Affected files: M user/src/com/google/gwt/resources/css/CssGenerationVisitor.java M user/src/com/google/gwt/resources/css/GenerateCssAst.java A user/src/com/google/gwt/resources/css/ast/CssFontFace.java M user/src/com/google/gwt/resources/css/ast/CssNodeCloner.java M user/src/com/google/gwt/resources/css/ast/CssVisitor.java M user/test/com/google/gwt/resources/client/CSSResourceTest.java M user/test/com/google/gwt/resources/client/test.css -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Only require @Service mapping for RequestContext types referenced from a RequestFactory. (issue1478803)
Reviewers: drfibonacci,
Description:
Only require @Service mapping for RequestContext types referenced from a
RequestFactory.
This allows RequestContext base types to be defined.
Patch by: bobv
Review by: drfibonacci
Please review this at http://gwt-code-reviews.appspot.com/1478803/
Affected files:
M
user/src/com/google/web/bindery/requestfactory/apt/RequestContextScanner.java
M
user/src/com/google/web/bindery/requestfactory/apt/RequestFactoryScanner.java
M user/src/com/google/web/bindery/requestfactory/apt/State.java
M
user/test/com/google/web/bindery/requestfactory/apt/MyRequestContext.java
M
user/test/com/google/web/bindery/requestfactory/apt/MyRequestFactory.java
Index:
user/src/com/google/web/bindery/requestfactory/apt/RequestContextScanner.java
===
---
user/src/com/google/web/bindery/requestfactory/apt/RequestContextScanner.java
(revision 10431)
+++
user/src/com/google/web/bindery/requestfactory/apt/RequestContextScanner.java
(working copy)
@@ -79,10 +79,6 @@
Service service = x.getAnnotation(Service.class);
ServiceName serviceName = x.getAnnotation(ServiceName.class);
JsonRpcService jsonRpcService = x.getAnnotation(JsonRpcService.class);
-if (service == null serviceName == null jsonRpcService == null) {
- state.poison(x,
Messages.contextMustBeAnnotated(state.requestContextType.asElement()
- .getSimpleName()));
-}
if (service != null) {
poisonIfAnnotationPresent(state, x, serviceName, jsonRpcService);
Index:
user/src/com/google/web/bindery/requestfactory/apt/RequestFactoryScanner.java
===
---
user/src/com/google/web/bindery/requestfactory/apt/RequestFactoryScanner.java
(revision 10431)
+++
user/src/com/google/web/bindery/requestfactory/apt/RequestFactoryScanner.java
(working copy)
@@ -41,7 +41,9 @@
if (!returnTypeElement.getKind().equals(ElementKind.INTERFACE)) {
state.poison(x,
Messages.factoryMustReturnInterface(returnTypeElement.getSimpleName()));
} else {
-state.maybeScanContext((TypeElement) returnTypeElement);
+TypeElement contextElement = (TypeElement) returnTypeElement;
+state.maybeScanContext(contextElement);
+state.requireMapping(contextElement);
}
} else {
state.poison(x,
Messages.factoryMustBeAssignable(state.requestContextType.asElement()
Index: user/src/com/google/web/bindery/requestfactory/apt/State.java
===
--- user/src/com/google/web/bindery/requestfactory/apt/State.java (revision
10431)
+++ user/src/com/google/web/bindery/requestfactory/apt/State.java (working
copy)
@@ -128,7 +128,7 @@
*/
private final MapElement, SetString previousMessages = new
HashMapElement, SetString();
- private final SetTypeElement proxiesRequiringMapping = new
LinkedHashSetTypeElement();
+ private final SetTypeElement typesRequiringMapping = new
LinkedHashSetTypeElement();
public State(ProcessingEnvironment processingEnv) {
clientToDomainMain = new HashMapTypeElement, TypeElement();
@@ -234,9 +234,14 @@
poison(job.element, sw.toString());
}
}
-for (TypeElement proxyElement : proxiesRequiringMapping) {
- if (!getClientToDomainMap().containsKey(proxyElement)) {
-poison(proxyElement, Messages.proxyMustBeAnnotated());
+for (TypeElement element : typesRequiringMapping) {
+ if (!getClientToDomainMap().containsKey(element)) {
+if (types.isAssignable(element.asType(), requestContextType)) {
+ poison(element,
Messages.contextMustBeAnnotated(types.asElement(requestContextType)
+ .getSimpleName()));
+} else {
+ poison(element, Messages.proxyMustBeAnnotated());
+}
}
}
}
@@ -331,8 +336,8 @@
}
}
- public void requireMapping(TypeElement proxyElement) {
-proxiesRequiringMapping.add(proxyElement);
+ public void requireMapping(TypeElement interfaceElement) {
+typesRequiringMapping.add(interfaceElement);
}
/**
Index:
user/test/com/google/web/bindery/requestfactory/apt/MyRequestContext.java
===
---
user/test/com/google/web/bindery/requestfactory/apt/MyRequestContext.java
(revision 10431)
+++
user/test/com/google/web/bindery/requestfactory/apt/MyRequestContext.java
(working copy)
@@ -23,7 +23,10 @@
import com.google.web.bindery.requestfactory.shared.Request;
import com.google.web.bindery.requestfactory.shared.RequestContext;
-@Expect(method = contextMustBeAnnotated, args = RequestContext)
+/*
+ * No error about a missing mapping expected because this type isn't
referenced
+ * from a RequestFactory.
+ */
interface MyRequestContext extends RequestContext {
@Expect(method = proxyMustBeAnnotated
[gwt-contrib] Use erasure to handle wildcard types in RfValidator. (issue1467814)
Reviewers: pquitslund,
Description:
Use erasure to handle wildcard types in RfValidator.
Restore FindRequest to 2.4 parameterization.
Patch by: bobv
Review by: pquitslund
Please review this at http://gwt-code-reviews.appspot.com/1467814/
Affected files:
M
user/src/com/google/web/bindery/requestfactory/apt/ClientToDomainMapper.java
M
user/src/com/google/web/bindery/requestfactory/apt/TransportableTypeVisitor.java
M user/src/com/google/web/bindery/requestfactory/apt/TypeSimplifier.java
M
user/src/com/google/web/bindery/requestfactory/shared/impl/FindRequest.java
Index:
user/src/com/google/web/bindery/requestfactory/apt/ClientToDomainMapper.java
===
---
user/src/com/google/web/bindery/requestfactory/apt/ClientToDomainMapper.java
(revision 10426)
+++
user/src/com/google/web/bindery/requestfactory/apt/ClientToDomainMapper.java
(working copy)
@@ -62,6 +62,12 @@
if (state.types.isAssignable(x, state.entityProxyType)
|| state.types.isAssignable(x, state.valueProxyType)) {
// FooProxy - FooDomain
+ /*
+ * TODO(bobv): This if statement should be widened to baseProxy to
support
+ * heterogenous collections of any proxy type. The BaseProxy
interface
+ * would also need to be annotated with an @ProxyFor mapping. This
can be
+ * done once RFIV is removed, since it only allows homogenous
collections.
+ */
TypeElement domainType =
state.getClientToDomainMap().get(state.types.asElement(x));
if (domainType == null) {
return defaultAction(x, state);
@@ -120,10 +126,7 @@
@Override
public TypeMirror visitWildcard(WildcardType x, State state) {
// Convert ? extends FooProxy to FooDomain
-if (x.getExtendsBound() != null) {
- return x.getExtendsBound().accept(this, state);
-}
-return defaultAction(x, state);
+return state.types.erasure(x).accept(this, state);
}
/**
Index:
user/src/com/google/web/bindery/requestfactory/apt/TransportableTypeVisitor.java
===
---
user/src/com/google/web/bindery/requestfactory/apt/TransportableTypeVisitor.java
(revision 10426)
+++
user/src/com/google/web/bindery/requestfactory/apt/TransportableTypeVisitor.java
(working copy)
@@ -105,7 +105,7 @@
@Override
public Boolean visitWildcard(WildcardType t, State state) {
// Allow List? extends FooProxy
-return t.getExtendsBound() != null t.getExtendsBound().accept(this,
state);
+return state.types.erasure(t).accept(this, state);
}
@Override
Index:
user/src/com/google/web/bindery/requestfactory/apt/TypeSimplifier.java
===
--- user/src/com/google/web/bindery/requestfactory/apt/TypeSimplifier.java
(revision 10426)
+++ user/src/com/google/web/bindery/requestfactory/apt/TypeSimplifier.java
(working copy)
@@ -102,10 +102,7 @@
@Override
public TypeMirror visitWildcard(WildcardType x, State state) {
-if (x.getExtendsBound() != null) {
- return x.getExtendsBound().accept(this, state);
-}
-return state.objectType;
+return state.types.erasure(x).accept(this, state);
}
@Override
Index:
user/src/com/google/web/bindery/requestfactory/shared/impl/FindRequest.java
===
---
user/src/com/google/web/bindery/requestfactory/shared/impl/FindRequest.java
(revision 10426)
+++
user/src/com/google/web/bindery/requestfactory/shared/impl/FindRequest.java
(working copy)
@@ -31,5 +31,5 @@
/**
* Use the implicit lookup in passing EntityProxy types to service
methods.
*/
- RequestEntityProxy find(EntityProxyId? extends EntityProxy proxy);
+ RequestEntityProxy find(EntityProxyId? proxy);
}
\ No newline at end of file
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Bypass RfValidatorTest when no JDK is available. (issue1465810)
Reviewers: rchandia,
Description:
Bypass RfValidatorTest when no JDK is available.
Patch by: bobv
Review by: rchandia
Please review this at http://gwt-code-reviews.appspot.com/1465810/
Affected files:
M user/test/com/google/web/bindery/requestfactory/apt/RfValidatorTest.java
Index:
user/test/com/google/web/bindery/requestfactory/apt/RfValidatorTest.java
===
---
user/test/com/google/web/bindery/requestfactory/apt/RfValidatorTest.java
(revision 10426)
+++
user/test/com/google/web/bindery/requestfactory/apt/RfValidatorTest.java
(working copy)
@@ -131,6 +131,10 @@
*/
private void testGeneratedMessages(Class?... classes) {
JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
+if (compiler == null) {
+ // This test is being run without a full JDK
+ return;
+}
ListJavaFileObject files = new
ArrayListJavaFileObject(classes.length);
for (Class? clazz : classes) {
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add compile-time tests for APT-based RequestFactory interface validator. (issue1473801)
Reviewers: keertip, pquitslund, tbroyer, Description: Add compile-time tests for APT-based RequestFactory interface validator. Move all validator message formatting to a single class. Patch by: bobv Review by: keertip,pquitslund, t.broyer Suggested by: t.broyer Please review this at http://gwt-code-reviews.appspot.com/1473801/ Affected files: M user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java A user/src/com/google/web/bindery/requestfactory/apt/Messages.java M user/src/com/google/web/bindery/requestfactory/apt/ProxyScanner.java M user/src/com/google/web/bindery/requestfactory/apt/RequestContextScanner.java M user/src/com/google/web/bindery/requestfactory/apt/RequestFactoryScanner.java M user/src/com/google/web/bindery/requestfactory/apt/RfValidator.java M user/src/com/google/web/bindery/requestfactory/apt/ScannerBase.java M user/src/com/google/web/bindery/requestfactory/apt/State.java M user/src/com/google/web/bindery/requestfactory/shared/SkipInterfaceValidation.java A user/test/com/google/web/bindery/requestfactory/apt/DiagnosticComparator.java A user/test/com/google/web/bindery/requestfactory/apt/EntityProxyCheckDomainMapping.java A user/test/com/google/web/bindery/requestfactory/apt/EntityProxyMismatchedFind.java A user/test/com/google/web/bindery/requestfactory/apt/EntityProxyMissingDomainLocatorMethods.java A user/test/com/google/web/bindery/requestfactory/apt/EntityProxyMissingDomainType.java A user/test/com/google/web/bindery/requestfactory/apt/Expect.java A user/test/com/google/web/bindery/requestfactory/apt/ExpectCollector.java A user/test/com/google/web/bindery/requestfactory/apt/Expected.java A user/test/com/google/web/bindery/requestfactory/apt/MyRequestContext.java A user/test/com/google/web/bindery/requestfactory/apt/MyRequestFactory.java A user/test/com/google/web/bindery/requestfactory/apt/RequestContextMissingDomainType.java A user/test/com/google/web/bindery/requestfactory/apt/RequestContextUsingUnmappedProxy.java A user/test/com/google/web/bindery/requestfactory/apt/RequestContextWithMismatchedInstance.java A user/test/com/google/web/bindery/requestfactory/apt/RfValidatorTest.java A user/test/com/google/web/bindery/requestfactory/apt/package-info.java M user/test/com/google/web/bindery/requestfactory/gwt/RequestFactoryGwtJreSuite.java M user/test/com/google/web/bindery/requestfactory/shared/TestFooPolymorphicRequest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add RequestFactory validator implemented as an annotation processor. (issue1467804)
Regarding the generics issue, I agree that the checker is behaving
correctly: the ListSubFooProxy would be converted to a ListSubFoo
when being checked against the domain type. A List? extends BaseFoo
can't be assigned to the type ListSubFoo since the list might contain
OtherFoo elements.
Since we've mainly be talking about edge-case details of the
implementation and it sounds like the checker mostly works with your
code base, I'm going to go ahead and check the code in with some
error-reporting fixes to make it less spammy when compiling with javac.
The findFoo() error is changed to a warning, although I'm thinking that
it might be better for the novice user if it's an error. For the
advanced user, an annotation processor option could be used to switch
that from an error to a warning.
I'll be on vacation the early part of next week and will be studiously
avoiding the computer. Thanks for your feedback, it's been very
helpful.
http://gwt-code-reviews.appspot.com/1467804/diff/6052/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java
File
user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java
(right):
http://gwt-code-reviews.appspot.com/1467804/diff/6052/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java#newcode340
user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java:340:
state.warn(warnTo, Cannot validate method (%s.%s) because the domain
mapping for the
On 2011/07/01 09:19:16, tbroyer wrote:
On 2011/06/30 15:16:48, bobv wrote:
This is more verbose, but it will unambiguously identify the
unchecked method
in
question.
This doesn't print what one would expect:
Another place where Eclipse and javac differ :-)
I don't mind having the warning on the getId method because of a
previous
warning on EmbeddedProxyId, but there should IMO be only one, not
hundreds of
them! (I actually have 52 such warnings exactly)
The duplicate messages are due to the way client interfaces are checked.
Each proxy or context type is examined individually as a flattened list
of all methods exposed by that type. If proxyA and proxyB both inherit
some interface X, the methods in X will be checked multiple times.
Admittedly this is more work than strictly necessary, but de-duplicating
the effort would complicate the checker code. Instead I've added some
code to the State object to prevent duplicate messages from being
emitted.
http://gwt-code-reviews.appspot.com/1467804/diff/6052/user/src/com/google/web/bindery/requestfactory/apt/TransportableTypeVisitor.java
File
user/src/com/google/web/bindery/requestfactory/apt/TransportableTypeVisitor.java
(right):
http://gwt-code-reviews.appspot.com/1467804/diff/6052/user/src/com/google/web/bindery/requestfactory/apt/TransportableTypeVisitor.java#newcode102
user/src/com/google/web/bindery/requestfactory/apt/TransportableTypeVisitor.java:102:
return state.types.erasure(t).accept(this, state);
On 2011/07/01 09:19:16, tbroyer wrote:
Have you tried with an intersection type? i.e. something like T
extends
MyInterface EntityProxy
I don't have that in my code base, and haven't tried it, but judging
from the
code of Eclipse's TypeVariableImpl#getUpperBound, it might very well
trigger the
bug too, in which case erasure() probably wouldn't be an appropriare
workaround.
I was thinking about looping on the Types#directSupertypes until one
is detected
as a transportable type:
for (TypeMirror type : state.types.directSupertypes(t)) {
if (type.accept(this, state)) {
return true;
}
}
return false;
that should work, because the expected type of getUpperBound is a
DeclaredType,
so visitDeclared should have been called, and it only uses
isAssignable checks
(so isAssignable(MyInterfaceEntityProxy, EntityProxy) would be
equivalent to
isAssignable(MyInterface,EntityProxy)||
isAssignable(EntityProxy,EntityProxy).
In the case of T extends EnumT, I guess it'd work the same.
I don't think that intersection types make sense in the most-derived
RequestFactory interfaces. Enums are the only concrete types that you
can specify in a RequestFactory interface. The implementations of all
other return types (proxies, etc) will necessarily be implemented by the
RequestFactory generator. The extra mix-in interfaces in the
intersection type probably won't be implemented by whatever generated,
concrete type is being returned. The only way that would work is if the
concrete type was generated from an interface that already implemented
the mix-in.
http://gwt-code-reviews.appspot.com/1467804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add RequestFactory validator implemented as an annotation processor. (issue1467804)
I was able to recreate the stack overflow with type variables as simple as T extends EnumT. The latest patch addresses this and I've changed the error-handling code to trap Throwables to fail better. http://gwt-code-reviews.appspot.com/1467804/diff/1028/user/src/com/google/web/bindery/requestfactory/apt/ClientToDomainMapper.java File user/src/com/google/web/bindery/requestfactory/apt/ClientToDomainMapper.java (right): http://gwt-code-reviews.appspot.com/1467804/diff/1028/user/src/com/google/web/bindery/requestfactory/apt/ClientToDomainMapper.java#newcode111 user/src/com/google/web/bindery/requestfactory/apt/ClientToDomainMapper.java:111: // Here, t would be NONE or PACKAGE, neither of which make sense On 2011/06/28 22:17:00, tbroyer wrote: s/t/x/ Done. http://gwt-code-reviews.appspot.com/1467804/diff/6052/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java File user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java (right): http://gwt-code-reviews.appspot.com/1467804/diff/6052/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java#newcode340 user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java:340: state.warn(warnTo, Cannot validate method (%s.%s) because the domain mapping for the This is more verbose, but it will unambiguously identify the unchecked method in question. http://gwt-code-reviews.appspot.com/1467804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add RequestFactory validator implemented as an annotation processor. (issue1467804)
On Wed, Jun 29, 2011 at 8:46 AM, t.bro...@gmail.com wrote: Still having stackoverflows when enabling the annotation processor in Eclipse. The error I got the last time I tried had a very similar stacktrace (see below). Do you have any self-parameterized types? FooQ extends FooQ kind of things going on? I'd assumed from your original report of stack exhaustion that it was the eager examination of types. This looks like a standard recursion break-condition bug. Running the tool from the Maven build gives a lot of warnings like those: Cannot validate this method because the domain mapping for the return type (xxx.xxx.Xxx) could not be resolved to a domain type\n\nAdd @SuppressWarnings(requestfactory) to dismiss. without pointing at the method. Ok, I'll tweak the error message to include the proxy/context type being scanned and the method signature. Fortunately, I know from my model that this is all due to a single method in a base-interface inherited by almost all our proxies. The few other warnings/errors are legitimate. Excellent. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Update XML doc to make it clearer that 'compiler.emulatedStack' should no longer be used. (issue1462804)
LGTM http://gwt-code-reviews.appspot.com/1462804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix ArrayStoreException in assignments to an element of an array of a single jso interface type. (issue1470801)
LGTM http://gwt-code-reviews.appspot.com/1470801/diff/1/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java File user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java (right): http://gwt-code-reviews.appspot.com/1470801/diff/1/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java#newcode531 user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java:531: Simple simpleTwo = (Simple) JavaScriptObject.createObject(); Should simpleTwo be a non-JSO Simple object to make sure that heterogenous arrays work correctly? http://gwt-code-reviews.appspot.com/1470801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add RequestFactory validator implemented as an annotation processor. (issue1467804)
1) I changed the State.maybeX() methods to add their jobs to a work-queue instead of processing them immediately. I think that should significantly reduce the stack depth. 2) Requiring a @ProxyFor annotation has also been deferred until after all types have been scanned. This should prevent the multiple-warning scenario. 3) The DiagnosticKind.NOTE just doesn't show up in the Eclipse editor. It will show up in the Error Log view, but that view doesn't provide a back-reference to the element. Regarding the 5926 case, were any of those methods defined in unchecked proxies or RequestContexts? I just want to make sure the problem is really fixed the first time around. Thanks for testing this out. http://gwt-code-reviews.appspot.com/1467804/diff/3002/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java File user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java (right): http://gwt-code-reviews.appspot.com/1467804/diff/3002/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java#newcode58 user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java:58: this.params = new ArrayListTypeMirror(params.size()); On 2011/06/28 00:29:30, tbroyer wrote: Wrap in a Collections.unmodifiableList() (as a safeguard) Done. http://gwt-code-reviews.appspot.com/1467804/diff/3002/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java#newcode187 user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java:187: state.poison(checkedType, Could not find domain method similar to %s, sb); On 2011/06/28 00:29:30, tbroyer wrote: Should pass clientMethodElement, otherwise @SkipInterfaceValidation will only be checked on the class, not on the method. Done. http://gwt-code-reviews.appspot.com/1467804/diff/3002/user/src/com/google/web/bindery/requestfactory/apt/ProxyScanner.java File user/src/com/google/web/bindery/requestfactory/apt/ProxyScanner.java (right): http://gwt-code-reviews.appspot.com/1467804/diff/3002/user/src/com/google/web/bindery/requestfactory/apt/ProxyScanner.java#newcode78 user/src/com/google/web/bindery/requestfactory/apt/ProxyScanner.java:78: state.addMapping(x, domain); On 2011/06/28 00:29:30, tbroyer wrote: This should be called even if domain==null, maybe with a special value, to take note that the proxy has been seen, so that TransportableTypeVisitor doesn't bail each time it sees it later. Done. http://gwt-code-reviews.appspot.com/1467804/diff/3002/user/src/com/google/web/bindery/requestfactory/apt/TypeSimplifier.java File user/src/com/google/web/bindery/requestfactory/apt/TypeSimplifier.java (right): http://gwt-code-reviews.appspot.com/1467804/diff/3002/user/src/com/google/web/bindery/requestfactory/apt/TypeSimplifier.java#newcode53 user/src/com/google/web/bindery/requestfactory/apt/TypeSimplifier.java:53: private boolean boxPrimitives; On 2011/06/28 00:29:30, tbroyer wrote: make 'final' Done. http://gwt-code-reviews.appspot.com/1467804/diff/3002/user/src/com/google/web/bindery/requestfactory/apt/TypeSimplifier.java#newcode64 user/src/com/google/web/bindery/requestfactory/apt/TypeSimplifier.java:64: ListTypeMirror newArgs = new ArrayListTypeMirror(); On 2011/06/28 00:29:30, tbroyer wrote: Initialize capacity at x.getTypeArguments().size(). Done. http://gwt-code-reviews.appspot.com/1467804/diff/3002/user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java File user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java (right): http://gwt-code-reviews.appspot.com/1467804/diff/3002/user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java#newcode45 user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java:45: // This annotation is temporary until RFIV and this test are removed On 2011/06/28 00:29:30, tbroyer wrote: Just so that it's clear for everyone, maybe add a note saying the RFIV only processes SkipInterfaceValidation on methods, not on types (even less containing types), and that this annotation here is for the annotation processor. Done. http://gwt-code-reviews.appspot.com/1467804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Removes .apt_generated from gwt-user classpath. Since Annotation Processor (issue1466807)
LGTM I wonder why the optional attribute of the classpath entry wasn't preventing this from causing problems. http://gwt-code-reviews.appspot.com/1466807/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add RequestFactory validator implemented as an annotation processor. (issue1467804)
Updated patch incorporating Thomas's feedback.
http://gwt-code-reviews.appspot.com/1467804/diff/1/user/build.xml
File user/build.xml (right):
http://gwt-code-reviews.appspot.com/1467804/diff/1/user/build.xml#newcode155
user/build.xml:155: exclude
name=com/google/web/bindery/requestfactory/apt/**/
On 2011/06/23 16:07:24, tbroyer wrote:
Why is this needed? I can understand that it makes iterating on the
APT code
easier (changes you make to it aren't taken into account when
determining
whether to precompile modules –which is slow and resource consuming–,
when
launching, e.g., unit tests) but you probably meant to only have it a
s a
temporary measure?
It's temporary, will revert before submitting.
http://gwt-code-reviews.appspot.com/1467804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java
File
user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java
(right):
http://gwt-code-reviews.appspot.com/1467804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java#newcode50
user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java:50:
private ExecutableElement found;
On 2011/06/23 16:07:24, tbroyer wrote:
shouldn't they all be 'final' ?
Changed lookFor() to a constructor, since instances of a MethoFinder
were never recycled.
http://gwt-code-reviews.appspot.com/1467804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java#newcode83
user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java:83:
TypeMirror paramType = maybeBox(paramElement.asType(), state);
On 2011/06/23 16:07:24, tbroyer wrote:
Correct me if I'm wrong, but this is a change from RFIV, where
argument types
weren't boxed before being compared. If this is a willful change,
then
ReflectiveServiceLayer would have to be updated as well when looking
up methods.
Similarly, return type is only boxed in RFIV for service methods, not
properties.
No longer looking for boxed parameters.
http://gwt-code-reviews.appspot.com/1467804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java#newcode96
user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java:96:
|| state.types.isSubsignature((ExecutableType) x.asType(),
(ExecutableType) found
On 2011/06/23 16:07:24, tbroyer wrote:
That's great, as it seems to fix issue 5926. (unfortunately, because
RFIV is
still used, an interface that validates with RfValidator at
compile-time can
fail with RFIV at runtime)
There will be a follow-up patch to this that deletes
RequestFactoryInterfaceValidator (the code's mostly written in another
git branch).
http://gwt-code-reviews.appspot.com/1467804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java#newcode111
user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java:111:
returnType = maybeBox(returnType, state);
On 2011/06/23 16:07:24, tbroyer wrote:
Why isn't this done in the constructor?
Done.
http://gwt-code-reviews.appspot.com/1467804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java#newcode115
user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java:115:
return scanAllInheritedMethods(x, state);
On 2011/06/23 16:07:24, tbroyer wrote:
Isn't that more or less the default behavior? (default behavior would
visit
fields and nested types too)
The default behavior is to scan all enclosed (declared) methods. In this
case, the code needs to consider all inherited methods, since methods
may be mixed in from non-proxy superinterfaces.
http://gwt-code-reviews.appspot.com/1467804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java#newcode149
user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java:149:
TypeMirror returnType = x.getReturnType().accept(new
ClientToDomainMapper(), state);
On 2011/06/23 16:07:24, tbroyer wrote:
x should probably be turned into an ExecutableType using
state.types.asMemberOf(domainType.asType(), x) to make sure we
preserve actual
type variables in the hierarchy (and similarly in MethodFinder).
I mean, if we have:
interface BaseFooProxyT {
T getT();
void setT(T);
}
and
interface FooProxy extends BaseFooProxyBarProxy { }
the ExecutableElement will have a formal type variable T and we only
know
about its bounds. What we'd like to check here is that the domain
object has
BarProxy getT() and void setT(BarProxy) methods, and only
ExecutableType
would give us the information.
There might be other places where that'd be beneficial.
...and that'd help fix issue 5926 in an elegant way:
http://code.google.com/p/google-web-toolkit/issues/detail?id=5926
Done.
http://gwt-code-reviews.appspot.com/1467804/diff/1/user/src/com/google/web/bindery/requestfactory/apt/Finder.java
File user/src/com/google/web/bindery/requestfactory/apt/Finder.java
(right):
[gwt-contrib] Add RequestFactory validator implemented as an annotation processor. (issue1467804)
Reviewers: pquislund_google.com, tbroyer, Message: This annotation processor will provide compile-time validation of RequestFactory interfaces. It produces red-squigglies in Eclipse on save. The plan is for this to replace the existing RequestFactoryInterfaceValidator by pre-generating all of the data needed for the Deobfuscator. The nascent RfApt type can also be removed. This is not slated for the 2.4 branch, since it's probably going to take some soak time to get everything running smoothly. Run ant requestfactory and copy build/lib/requestfactory-apt.jar into tools/lib/requesfactory. Close, re-open, and clean the gwt-user project (or whichever project the processor is being run against. Where the processor incorrectly flags an error, the @SkipInterfaceValidation annotation can be applied. Also, check Eclipse's Window - Show View - Error Log for any exceptions that escape the annotation processor. @Phil, Since Ray's on vacation, can you take the review on this? @Thomas, Your feedback would be very helpful, since you have an RF-based app. Description: Add RequestFactory validator implemented as an annotation processor. This will eventually replace the RequestFactoryInterfaceValidator and it's classfile-based approach. Patch by: bobv Review by: ??? Please review this at http://gwt-code-reviews.appspot.com/1467804/ Affected files: M eclipse/samples/DynaTableRf/.classpath A eclipse/samples/DynaTableRf/.factorypath M user/build.xml A user/src/com/google/web/bindery/requestfactory/apt/ClientToDomainMapper.java A user/src/com/google/web/bindery/requestfactory/apt/DomainChecker.java A user/src/com/google/web/bindery/requestfactory/apt/Finder.java A user/src/com/google/web/bindery/requestfactory/apt/HaltException.java A user/src/com/google/web/bindery/requestfactory/apt/ProxyScanner.java A user/src/com/google/web/bindery/requestfactory/apt/RequestContextScanner.java A user/src/com/google/web/bindery/requestfactory/apt/RequestFactoryScanner.java M user/src/com/google/web/bindery/requestfactory/apt/RfApt.java A user/src/com/google/web/bindery/requestfactory/apt/RfValidator.java A user/src/com/google/web/bindery/requestfactory/apt/ScannerBase.java A user/src/com/google/web/bindery/requestfactory/apt/State.java A user/src/com/google/web/bindery/requestfactory/apt/TransportableTypeVisitor.java M user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java M user/src/com/google/web/bindery/requestfactory/shared/SkipInterfaceValidation.java M user/src/com/google/web/bindery/requestfactory/shared/impl/FindRequest.java M user/src/com/google/web/bindery/requestfactory/shared/impl/ProxySerializerImpl.java M user/test/com/google/web/bindery/requestfactory/shared/BaseFooProxy.java M user/test/com/google/web/bindery/requestfactory/shared/SimpleFooProxy.java M user/test/com/google/web/bindery/requestfactory/shared/TestFooPolymorphicRequest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: RequestFactory annotation processor generates uncompilable code when there are nested proxies
On Wed, Jun 22, 2011 at 4:57 AM, Thomas Broyer t.bro...@gmail.com wrote: I'm trying to add a unit test for issue 5926 and I don't know how to deal with the TypeTokenResolver to easily run the test from within Eclipse. I enabled the requestfactory-apt.jar annotation processor on the gwt-user project and it generates a Java file that cannot be compiled, because it uses binary names for classes instead of source names, e.g. addTypeToken(6GWlio0rpqSic5Nce0g_FPANjqc=, com.google.web.bindery.requestfactory.server.BoxesAndPrimitivesJreTest$ProxyMismatchedGetterA.class.getName()); addTypeToken(6TPopfzq37rZzVxkFiyvasCck2Q=, com.google.web.bindery.requestfactory.server.RequestFactoryInterfaceValidatorTest$LocatorEntityProxy.class.getName()); I'll fix the code-gen for this. When you added the annotation processor to Eclipse, are you seeing a new classpath entry for .apt_generated in the package explorer? I'm seeing different behaviors between different Eclipse installations as to whether or not the apt source paths show up in the package explorer. In all cases, the generated type is available through the Open Type dialog, but it's not causing a red-X to show compilation failure. How do you run RF tests in Eclipse? (and are you doing it? or always running them through Ant?) I usually run the RequestFactoryJreSuite in Eclipse as a regular JUnit test for fast turnarounds and then use ant for the GWT-based tests. Why is it using class literals rather than just strings? to fail early if one of the classes cannot be found? (rather than failing only when InProcessRequestContext#createProxy resolves the type token) Yes. instead of just processing interfaces annotated with @ProxyFor or @ProxyForName (roundEnv.getElementsAnnotatedWith), given that proxies have to be annotated to be used with RF? That would be faster, will change this as well. Originally RfApt was going to look for RequestFactory interfaces. Am I missing something? Nope. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Issue 6504: AutoBean doesn't support enums with anonymous enum values (issue1467802)
LGTM. Will commit and integrate into 2.4 branch. http://gwt-code-reviews.appspot.com/1467802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: A general purpose script injection class for injecting a script directly (issue1451818)
http://gwt-code-reviews.appspot.com/1451818/diff/5001/user/src/com/google/gwt/core/client/ScriptInjector.java
File user/src/com/google/gwt/core/client/ScriptInjector.java (right):
http://gwt-code-reviews.appspot.com/1451818/diff/5001/user/src/com/google/gwt/core/client/ScriptInjector.java#newcode39
user/src/com/google/gwt/core/client/ScriptInjector.java:39: *
public void onFailure(Void reason) {
Leaving reason as Void means there's no way to provide any kind of
diagnostic message to the caller. You could make this parameterized by
Exception to at least be able to provide a message string, and possibly
a JavaScriptException if one is warranted.
http://gwt-code-reviews.appspot.com/1451818/diff/5001/user/src/com/google/gwt/core/client/ScriptInjector.java#newcode177
user/src/com/google/gwt/core/client/ScriptInjector.java:177: public
static final JavaScriptObject TOP_WINDOW = nativeTopWindow();
Why would an external caller use use TOP_WINDOW? If it's going to be
public, javadoc it.
http://gwt-code-reviews.appspot.com/1451818/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Ensure all ProxyAutoBeans not directly referenced from root object or via a shim can be garbage-... (issue1451819)
But I wonder: isn't this change making RequestFactory incompatible with AppEngine? Reverting to original behavior of calling cleanup() from get()/set(). -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Make TypeTokenResolver and RequestFactory's annotation processor easier to integrate with Adroid... (issue1462801)
Reviewers: rjrjr, rice,
Description:
Make TypeTokenResolver and RequestFactory's annotation processor easier
to integrate with Adroid ADT build process by generating a pre-populated
TypeTokenBuilder class.
This avoids the need to manipulate resource inclusion in the usual
project setup.
Patch by: bobv
Review by: rjrjr, rice
Please review this at http://gwt-code-reviews.appspot.com/1462801/
Affected files:
M user/src/com/google/web/bindery/requestfactory/apt/RfApt.java
M
user/src/com/google/web/bindery/requestfactory/vm/impl/TypeTokenResolver.java
Index: user/src/com/google/web/bindery/requestfactory/apt/RfApt.java
===
--- user/src/com/google/web/bindery/requestfactory/apt/RfApt.java (revision
10342)
+++ user/src/com/google/web/bindery/requestfactory/apt/RfApt.java (working
copy)
@@ -22,10 +22,13 @@
import com.google.web.bindery.requestfactory.vm.impl.TypeTokenResolver;
import java.io.IOException;
+import java.io.PrintWriter;
+import java.util.Map;
import java.util.Set;
import javax.annotation.processing.AbstractProcessor;
import javax.annotation.processing.Filer;
+import javax.annotation.processing.FilerException;
import javax.annotation.processing.ProcessingEnvironment;
import javax.annotation.processing.RoundEnvironment;
import javax.annotation.processing.SupportedAnnotationTypes;
@@ -41,6 +44,7 @@
import javax.lang.model.util.Types;
import javax.tools.Diagnostic.Kind;
import javax.tools.FileObject;
+import javax.tools.JavaFileObject;
import javax.tools.StandardLocation;
/**
@@ -108,7 +112,7 @@
// Extract data
new Finder().scan(ElementFilter.typesIn(roundEnv.getRootElements()),
null);
-
+
// On the last round, write out accumulated data
if (roundEnv.processingOver()) {
TypeTokenResolver d = builder.build();
@@ -118,6 +122,39 @@
d.store(res.openOutputStream());
} catch (IOException e) {
error(Could not write output: + e.getMessage());
+ }
+
+ /*
+ * Getting the TOKEN_MANIFEST resource into an Android APK generated
by
+ * the Android Eclipse plugin is non-trivial. (Users of ant and
apkbuilder
+ * can just use -rf to include the relevant file). To support the
common
+ * use-case, we'll generate a subclass of TypeTokenResolver.Builder
that
+ * has all of the data already baked into it. This synthetic subtype
is
+ * looked for first by TypeTokenResolver and any manifests that are
on the
+ * classpath will be added on top.
+ */
+ try {
+String packageName =
TypeTokenResolver.class.getPackage().getName();
+String simpleName = TypeTokenResolver.class.getSimpleName()
+ BuilderImpl;
+JavaFileObject classfile = filer.createSourceFile(packageName
+ . + simpleName);
+PrintWriter pw = new PrintWriter(classfile.openWriter());
+pw.println(package + packageName + ;);
+pw.println(public class + simpleName + extends
++ TypeTokenResolver.Builder.class.getCanonicalName() + {);
+pw.println(public + simpleName + () {);
+for (Map.EntryString, String entry :
d.getAllTypeTokens().entrySet()) {
+ if (elements.getTypeElement(entry.getValue()) != null) {
+pw.println(addTypeToken(\ + entry.getKey() + \, +
entry.getValue()
++ .class.getName()););
+ }
+}
+pw.println(});
+pw.println(});
+pw.close();
+ } catch (FilerException e) {
+log(Ignoring exception: %s, e.getMessage());
+ } catch (IOException e) {
+error(Could not write BuilderImpl: + e.getMessage());
}
log(Finished!);
}
Index:
user/src/com/google/web/bindery/requestfactory/vm/impl/TypeTokenResolver.java
===
---
user/src/com/google/web/bindery/requestfactory/vm/impl/TypeTokenResolver.java
(revision 10342)
+++
user/src/com/google/web/bindery/requestfactory/vm/impl/TypeTokenResolver.java
(working copy)
@@ -26,6 +26,8 @@
import java.util.Map;
import java.util.Properties;
import java.util.Set;
+import java.util.SortedMap;
+import java.util.TreeMap;
/**
* Resolves payload type tokens to binary class names.
@@ -75,10 +77,26 @@
* TypeTokenResolver.
*/
public static TypeTokenResolver loadFromClasspath() throws IOException {
-Builder builder = new Builder();
+Builder builder;
+boolean mustLoad = true;
+try {
+ // Look for a pre-cooked Builder type
+ Class? maybeBuilderImpl =
+ Class.forName(TypeTokenResolver.class.getName() + BuilderImpl,
false, Thread
+ .currentThread().getContextClassLoader());
+ builder = maybeBuilderImpl.asSubclass(Builder.class).newInstance();
+ mustLoad = false;
+} catch (ClassNotFoundException ignored) {
+ // Try manifest-based approach
+ builder = new
[gwt-contrib] Add some test framework code for RequestFactory to support simple load testing. (issue1453814)
Reviewers: rjrjr, Description: Add some test framework code for RequestFactory to support simple load testing. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1453814/ Affected files: M user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java A user/src/com/google/web/bindery/requestfactory/vm/testing/UrlRequestTransport.java M user/test/com/google/web/bindery/requestfactory/server/RequestFactoryJreTest.java A user/test/com/google/web/bindery/requestfactory/server/RequestFactoryTestServer.java M user/test/com/google/web/bindery/requestfactory/vm/RequestFactoryJreSuite.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Reduce the use of old event bus classes. The only public api that (issue1446819)
LGTM
http://gwt-code-reviews.appspot.com/1446819/diff/3008/user/src/com/google/gwt/event/shared/LegacyHandlerWrapper.java
File user/src/com/google/gwt/event/shared/LegacyHandlerWrapper.java
(right):
http://gwt-code-reviews.appspot.com/1446819/diff/3008/user/src/com/google/gwt/event/shared/LegacyHandlerWrapper.java#newcode22
user/src/com/google/gwt/event/shared/LegacyHandlerWrapper.java:22:
public class LegacyHandlerWrapper implements HandlerRegistration {
Mark this as @Deprecated?
http://gwt-code-reviews.appspot.com/1446819/diff/3008/user/src/com/google/gwt/place/shared/PlaceController.java
File user/src/com/google/gwt/place/shared/PlaceController.java (right):
http://gwt-code-reviews.appspot.com/1446819/diff/3008/user/src/com/google/gwt/place/shared/PlaceController.java#newcode75
user/src/com/google/gwt/place/shared/PlaceController.java:75: public
PlaceController(com.google.gwt.event.shared.EventBus eventBus) {
Javadoc with suggested replacements?
http://gwt-code-reviews.appspot.com/1446819/diff/3008/user/src/com/google/gwt/place/shared/PlaceHistoryHandler.java
File user/src/com/google/gwt/place/shared/PlaceHistoryHandler.java
(right):
http://gwt-code-reviews.appspot.com/1446819/diff/3008/user/src/com/google/gwt/place/shared/PlaceHistoryHandler.java#newcode121
user/src/com/google/gwt/place/shared/PlaceHistoryHandler.java:121:
@Deprecated
Javadoc with suggested replacement?
http://gwt-code-reviews.appspot.com/1446819/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add some test framework code for RequestFactory to support simple load testing. (issue1453814)
http://gwt-code-reviews.appspot.com/1453814/diff/1/user/src/com/google/web/bindery/requestfactory/vm/testing/UrlRequestTransport.java File user/src/com/google/web/bindery/requestfactory/vm/testing/UrlRequestTransport.java (right): http://gwt-code-reviews.appspot.com/1453814/diff/1/user/src/com/google/web/bindery/requestfactory/vm/testing/UrlRequestTransport.java#newcode22 user/src/com/google/web/bindery/requestfactory/vm/testing/UrlRequestTransport.java:22: import org.apache.commons.io.output.ByteArrayOutputStream; On 2011/06/15 13:01:26, tbroyer wrote: Did you mean java.io.ByteArrayOutputStream? I did. Good catch. http://gwt-code-reviews.appspot.com/1453814/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Ensure all ProxyAutoBeans not directly referenced from root object or via a shim can be garbage-... (issue1451819)
Reviewers: rjrjr, Message: Approach adapted from http://gwt-code-reviews.appspot.com/1401802 without having to deal with ClassLoader complications from mixing additional interfaces into Proxy (that seemed to cause weird problems with emma-enabled builds). This patch includes the changes in http://gwt-code-reviews.appspot.com/1453814/ and I'll update the issue once that code is submitted. The previous patch was insufficient because any circular references with wrapped objects (especially Lists and Iterators) would prevent the WeakMapping from ever clearing state. It only fixed the circularity problem for simple bean types. With this patch, I've run the test server against 8 looping instances of RequestFactoryJreSuite's main() method. The server and the clients were all run on the same machine with 32M heaps and this was observed to be stable over a thirty minute period. The heaps could be made smaller if it weren't for the Unicode tests having insanely nesting. Description: Ensure all ProxyAutoBeans not directly referenced from root object or via a shim can be garbage-collected. Issue 6193. Patch by: bobv Suggested by: t.broyer Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1451819/ Affected files: M user/src/com/google/gwt/core/client/impl/WeakMapping.java M user/src/com/google/web/bindery/autobean/shared/impl/AbstractAutoBean.java M user/src/com/google/web/bindery/autobean/vm/impl/JsonSplittable.java M user/src/com/google/web/bindery/autobean/vm/impl/ProxyAutoBean.java M user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java A user/src/com/google/web/bindery/requestfactory/vm/testing/UrlRequestTransport.java M user/super/com/google/gwt/core/translatable/com/google/gwt/core/client/impl/WeakMapping.java M user/test/com/google/web/bindery/requestfactory/server/RequestFactoryJreTest.java A user/test/com/google/web/bindery/requestfactory/server/RequestFactoryTestServer.java M user/test/com/google/web/bindery/requestfactory/vm/RequestFactoryJreSuite.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds hooks to Widget reduce MVP boilerplate and enshrines the app-wide EventBus. (issue1449817)
Just a look at the user/src classes so far.
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/DefaultEventBusProvider.java
File user/src/com/google/gwt/user/client/ui/DefaultEventBusProvider.java
(right):
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/DefaultEventBusProvider.java#newcode41
user/src/com/google/gwt/user/client/ui/DefaultEventBusProvider.java:41:
public static DefaultEventBusProvider instance() {
Why not also provide a static setInstance()?
Is there a ProviderEventBus interface this type could implement?
Sprinkling such a specific type name around an app seems a bit gross.
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/DefaultEventBusProvider.java#newcode68
user/src/com/google/gwt/user/client/ui/DefaultEventBusProvider.java:68:
public EventBus get() {
Without making get() and init() abstract, anyone providing a subclass
could forget to override one or the other. How about breaking this type
up in the way Scheduler and SchedulerImpl are arranged?
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/IsEventSource.java
File user/src/com/google/gwt/user/client/ui/IsEventSource.java (right):
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/IsEventSource.java#newcode28
user/src/com/google/gwt/user/client/ui/IsEventSource.java:28: * event
bus.
Why a String and not the general Object that Events can use as a source?
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/IsWidgetDriver.java
File user/src/com/google/gwt/user/client/ui/IsWidgetDriver.java (right):
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/IsWidgetDriver.java#newcode53
user/src/com/google/gwt/user/client/ui/IsWidgetDriver.java:53: public
interface IsWidgetDriver {
This type name implies there should be a single method
WidgetDriver asWidgetDriver();
WidgetDriver could then be an abstract base class to allow additional
lifecycle methods to be added in the future.
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/Widget.java
File user/src/com/google/gwt/user/client/ui/Widget.java (right):
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/Widget.java#newcode69
user/src/com/google/gwt/user/client/ui/Widget.java:69: private Command
updateDriver;
Sort fields?
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/Widget.java#newcode89
user/src/com/google/gwt/user/client/ui/Widget.java:89: public final H
extends EventHandler HandlerRegistration addBitlessDomHandler(final H
handler,
Why is the handler param final on these methods?
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/Widget.java#newcode580
user/src/com/google/gwt/user/client/ui/Widget.java:580: protected
EventBus requireEventBus() {
Instead of having two getEventBusMethods(), what about placing a boolean
flag on getEventBus() to say don't return null or at least delegating
from getEventBus() to getEventBus(boolean) so there's only one method
that actually makes EventBus objects pop into existence?
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/web/bindery/event/shared/HandlerRegistration.java
File
user/src/com/google/web/bindery/event/shared/HandlerRegistration.java
(right):
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/web/bindery/event/shared/HandlerRegistration.java#newcode37
user/src/com/google/web/bindery/event/shared/HandlerRegistration.java:37:
public class Null implements HandlerRegistration {
Do you ever need more than one instance of this type?
HandlerRegistration.NULL might be a better choice.
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/web/bindery/event/shared/ResettableEventBus.java
File
user/src/com/google/web/bindery/event/shared/ResettableEventBus.java
(right):
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/web/bindery/event/shared/ResettableEventBus.java#newcode37
user/src/com/google/web/bindery/event/shared/ResettableEventBus.java:37:
public ResettableEventBus(EventBus wrappedBus, String name) {
If the eventSourceName is turned into an Object eventSourceTag, it would
allow users to use enums or other Objects for dispatch instead of
descending into String-matching hell.
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/web/bindery/event/shared/ResettableEventBus.java#newcode77
user/src/com/google/web/bindery/event/shared/ResettableEventBus.java:77:
Extra whitespace.
http://gwt-code-reviews.appspot.com/1449817/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds hooks to Widget reduce MVP boilerplate and enshrines the app-wide EventBus. (issue1449817)
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/IsEventSource.java
File user/src/com/google/gwt/user/client/ui/IsEventSource.java (right):
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/IsEventSource.java#newcode28
user/src/com/google/gwt/user/client/ui/IsEventSource.java:28: * event
bus.
Perhaps the thing to do is provide a convenience setter for the string
case?
Object getEventSource()
void setEventSource(Object)
void setEventSourceName(String)
Would UiBinder not widen the string XML attribute value to an Object?
Having a setEventSourcName(String) seem redundant when the only way to
actually access it is through an Object-return method.
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/IsWidgetDriver.java
File user/src/com/google/gwt/user/client/ui/IsWidgetDriver.java (right):
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/gwt/user/client/ui/IsWidgetDriver.java#newcode53
user/src/com/google/gwt/user/client/ui/IsWidgetDriver.java:53: public
interface IsWidgetDriver {
DrivesWidgets SGTM.
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/web/bindery/event/shared/ResettableEventBus.java
File
user/src/com/google/web/bindery/event/shared/ResettableEventBus.java
(right):
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/web/bindery/event/shared/ResettableEventBus.java#newcode37
user/src/com/google/web/bindery/event/shared/ResettableEventBus.java:37:
public ResettableEventBus(EventBus wrappedBus, String name) {
On 2011/06/14 18:34:41, rjrjr wrote:
You okay with the setEventSource(Object) / setEventSourceName(String)
notion to
keep uibinder simple?
If setEventSourceName(String) is necessary to make UiBinder work and
just delegates over to setEventSource(), I think that's something that
can be lived with.
http://gwt-code-reviews.appspot.com/1449817/diff/4001/user/src/com/google/web/bindery/event/shared/ResettableEventBus.java#newcode55
user/src/com/google/web/bindery/event/shared/ResettableEventBus.java:55:
* Remove all handlers that have been added through this wrapper, and
neuter
On 2011/06/14 15:05:48, jlabanca wrote:
We should rename the method to neuter()
Many APIs use shutdown()
http://gwt-code-reviews.appspot.com/1449817/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Make Request.with() additive when used with different root objects. (issue1453813)
Reviewers: rjrjr, tbroyer, Message: Review requested. The path-processing code and creating client peers for domain objects are really orthogonal concerns, so I've split that code up. With this approach, re-resolving the properties on a proxy is no longer a special case. I re-used the tests from Thomas's original patch. Description: Make Request.with() additive when used with different root objects. Delay resolving invocation results until all results have been processed. Separate Resolver's path-processing code from client object creation code via Resolution type. Issue 6115. Tests copied from http://gwt-code-reviews.appspot.com/1377804 Patch by: bobv, tbroyer Review by: rjrjr, tbroyer Please review this at http://gwt-code-reviews.appspot.com/1453813/ Affected files: M user/src/com/google/web/bindery/requestfactory/server/Resolver.java M user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java M user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java M user/test/com/google/web/bindery/requestfactory/server/SimpleFoo.java M user/test/com/google/web/bindery/requestfactory/shared/SimpleFooRequest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make Request.with() additive when used with different root objects. (issue1453813)
http://gwt-code-reviews.appspot.com/1453813/diff/1/user/src/com/google/web/bindery/requestfactory/server/Resolver.java File user/src/com/google/web/bindery/requestfactory/server/Resolver.java (right): http://gwt-code-reviews.appspot.com/1453813/diff/1/user/src/com/google/web/bindery/requestfactory/server/Resolver.java#newcode191 user/src/com/google/web/bindery/requestfactory/server/Resolver.java:191: assert !(clientObject instanceof Resolution); On 2011/06/13 21:17:53, tbroyer wrote: Could it ever happen? Nope. Old code from an earlier draft. http://gwt-code-reviews.appspot.com/1453813/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add ServerFailure.getRequestContext(). (issue1451815)
Reviewers: rjrjr, jasonhall, Description: Add ServerFailure.getRequestContext(). Provide getters for Request - RequestContext - RequestFactory - RequestTransport. Add fakes for modified interfaces. Patch by: bobv Review by: rjrjr Suggested by: jasonhall Please review this at http://gwt-code-reviews.appspot.com/1451815/ Affected files: M user/src/com/google/web/bindery/requestfactory/shared/Request.java M user/src/com/google/web/bindery/requestfactory/shared/RequestContext.java M user/src/com/google/web/bindery/requestfactory/shared/RequestFactory.java M user/src/com/google/web/bindery/requestfactory/shared/RequestTransport.java M user/src/com/google/web/bindery/requestfactory/shared/ServerFailure.java M user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequest.java M user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java A user/src/com/google/web/bindery/requestfactory/shared/testing/FakeRequest.java M user/src/com/google/web/bindery/requestfactory/shared/testing/FakeRequestContext.java A user/src/com/google/web/bindery/requestfactory/shared/testing/FakeRequestFactory.java A user/src/com/google/web/bindery/requestfactory/shared/testing/FakeRequestTransport.java M user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add ServerFailure.getRequestContext(). (issue1451815)
Fixed lame comment and committed at r10316. http://gwt-code-reviews.appspot.com/1451815/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Suport polymorphic return values in RequestFactory. (issue1453811)
Both polymorphic return and parameter values are supported. Turns out that polymorphic parameters already worked. Committed at r10317. http://gwt-code-reviews.appspot.com/1453811/diff/1/user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/RequestFactoryModel.java File user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/RequestFactoryModel.java (right): http://gwt-code-reviews.appspot.com/1453811/diff/1/user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/RequestFactoryModel.java#newcode251 user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/RequestFactoryModel.java:251: poison(Unable to find extra type %s in TypeOracle, clazz.getCanonicalName()); On 2011/06/10 19:20:41, rjrjr wrote: TypeOracle is an implementation detail, odd to see it called out in a user facing error message. How about Unknown class %s in @ExtraTypes? Done. http://gwt-code-reviews.appspot.com/1453811/diff/1/user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/RequestFactoryModel.java#newcode289 user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/RequestFactoryModel.java:289: builder.setSuperProxyTyes(superTypes); On 2011/06/10 19:20:41, rjrjr wrote: Tyes Done. http://gwt-code-reviews.appspot.com/1453811/diff/1/user/test/com/google/web/bindery/requestfactory/server/RequestFactoryPolymorphicJreTest.java File user/test/com/google/web/bindery/requestfactory/server/RequestFactoryPolymorphicJreTest.java (right): http://gwt-code-reviews.appspot.com/1453811/diff/1/user/test/com/google/web/bindery/requestfactory/server/RequestFactoryPolymorphicJreTest.java#newcode62 user/test/com/google/web/bindery/requestfactory/server/RequestFactoryPolymorphicJreTest.java:62: * method declared to return Integer. On 2011/06/10 19:20:41, rjrjr wrote: Are such cases this subtle a fail in real life? Seems like this could be maddening to debug. Expanded the comment to explain why this normally won't happen. http://gwt-code-reviews.appspot.com/1453811/diff/1/user/test/com/google/web/bindery/requestfactory/shared/TestRequestFactory.java File user/test/com/google/web/bindery/requestfactory/shared/TestRequestFactory.java (right): http://gwt-code-reviews.appspot.com/1453811/diff/1/user/test/com/google/web/bindery/requestfactory/shared/TestRequestFactory.java#newcode19 user/test/com/google/web/bindery/requestfactory/shared/TestRequestFactory.java:19: * Creates TestFooPolymorphicRequest. On 2011/06/10 19:20:41, rjrjr wrote: Does this test serve any purpose any more? If so, could you spell out what it is? Done. http://gwt-code-reviews.appspot.com/1453811/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Clean up RequestFactoryInterfaceValidator and Deobfuscator to simplify (issue1455803)
Reviewers: rjrjr, Description: Clean up RequestFactoryInterfaceValidator and Deobfuscator to simplify ResolverServiceLayer. This allows the validator to be jettisoned in favor of a lighter-weight object. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1455803/ Affected files: M user/src/com/google/web/bindery/requestfactory/server/Deobfuscator.java M user/src/com/google/web/bindery/requestfactory/server/OperationData.java M user/src/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidator.java M user/src/com/google/web/bindery/requestfactory/server/ResolverServiceLayer.java M user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java M user/test/com/google/web/bindery/requestfactory/server/RequestFactoryPolymorphicJreTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Support RequestContext composition (issue 6234) and mix-in (issue 6035). (issue1447815)
Ready for another look. http://gwt-code-reviews.appspot.com/1447815/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] [google-web-toolkit] r10311 committed - Support RequestContext composition (issue 6234) and mix-in (issue 6035...
On Thu, Jun 9, 2011 at 6:13 PM, codesite-nore...@google.com wrote: Revision: 10311 Author:  b...@google.com Date:   Thu Jun  9 11:31:51 2011 Log:    Support RequestContext composition (issue 6234) and mix-in (issue 6035). Remove no-method-overloads restriction by using a full method descriptor to create the operation. Use obfuscated type and operation tokens to reduce payload and JS size (issue 5394). Without this change, the operation tokens can be excessively lengthly. The RequestFactory interface is used as the gwt.rpc analog. Add an annotation processor to generate an obfuscated type manifest and add it to the gwt-user project.  This is only necessary when using the JRE-compatible RequestFactory client code. Review at http://gwt-code-reviews.appspot.com/1447815 Patch by: bobv Review by: rjrjr Some release notes also added to the relevant issues: 6234: Several of the ServiceLayer.resolveX() method signatures have changed in this release. These changes were made in order to allow the use of obfuscated type and operation tokens to reduce payload and generated JS size and to allow the use of overloaded method names in RequestContext subtypes. Users who have written their own ServiceLayerDecorator subclasses that override any of the resolveX() methods will need to modify their code to conform to the new API. In general, the expected behavior of the resolveX() methods is unchanged. Users who need to retain compatibility with 2.3 and 2.4 RequestFactory server code can leave methods with the old signatures in place, removing any @Override annotations. 5394: Users who depend on RequestFactorySource must now compile their proxy interfaces with the RequestFactory annotation processor. This tool is bundled in the requestfactory-client.jar or available separately in requestfactory-apt.jar. For Java 6 users, javac will automatically detect the annotation processor. Eclipse users will need to enable annotation processing via Project properties -- Java Compiler -- Annotation Processing and add requestfactory-apt.jar to the list of jars in Java Compiler -- Annotation Processing -- Factory Path. Users can confirm that the annotation processor is installed by looking for a META-INF/requestFactory/typeTokens file to be generated in the compiler's output directory. This file must be packaged into the jar file that contains the compiled proxy classes. Additionally, the -Averbose=true flag can be passed to javac (or specified in the Annotation Processing configuration UI) to enable diagnostic output for the annotation processor. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Suport polymorphic return values in RequestFactory. (issue1453811)
Reviewers: rjrjr, Description: Suport polymorphic return values in RequestFactory. Issue 5367. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1453811/ Affected files: M user/src/com/google/web/bindery/requestfactory/gwt/rebind/RequestFactoryGenerator.java M user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/ContextMethod.java M user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/EntityProxyModel.java A user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/HasExtraTypes.java M user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/RequestFactoryModel.java M user/src/com/google/web/bindery/requestfactory/server/Deobfuscator.java M user/src/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidator.java M user/src/com/google/web/bindery/requestfactory/server/Resolver.java M user/src/com/google/web/bindery/requestfactory/shared/EntityProxy.java A user/src/com/google/web/bindery/requestfactory/shared/ExtraTypes.java M user/src/com/google/web/bindery/requestfactory/shared/ValueProxy.java M user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestContext.java M user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestFactory.java M user/src/com/google/web/bindery/requestfactory/vm/impl/TypeTokenResolver.java M user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryPolymorphicTest.java A user/test/com/google/web/bindery/requestfactory/server/RequestFactoryPolymorphicJreTest.java M user/test/com/google/web/bindery/requestfactory/shared/TestRequestFactory.java M user/test/com/google/web/bindery/requestfactory/vm/RequestFactoryJreSuite.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Support RequestContext composition (issue 6234) and mix-in (issue 6035). (issue1447815)
http://gwt-code-reviews.appspot.com/1447815/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Support RequestContext composition (issue 6234) and mix-in (issue 6035). (issue1447815)
To verify that the annotation processor is working in Eclipse, open up the Error Log view. You should see diagnostic messages from the processor. I'll turn those off before final submission. Since this patch updates the gwt-user project, you might need to shut Eclipse down first before patching. http://gwt-code-reviews.appspot.com/1447815/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Support is/has methods in Editor framework. (issue1443812)
Reviewers: rjrjr, Description: Support is/has methods in Editor framework. Issue 6040. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1443812/ Affected files: M user/src/com/google/gwt/editor/rebind/model/EditorModel.java M user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Creates a delegate for CompilationResult for use by third party linkers. (issue1451808)
LGTM http://gwt-code-reviews.appspot.com/1451808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add a way to disable image inlining on a per-image basis. (issue1451809)
Reviewers: rjrjr,
Description:
Add a way to disable image inlining on a per-image basis.
Patch by: bobv
Review by: rjrjr
Please review this at http://gwt-code-reviews.appspot.com/1451809/
Affected files:
M user/src/com/google/gwt/resources/client/ImageResource.java
M user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
M user/test/com/google/gwt/resources/client/ImageResourceTest.java
Index: user/src/com/google/gwt/resources/client/ImageResource.java
===
--- user/src/com/google/gwt/resources/client/ImageResource.java (revision
10245)
+++ user/src/com/google/gwt/resources/client/ImageResource.java (working
copy)
@@ -54,6 +54,14 @@
* ratio of the image will be maintained.
*/
int height() default -1;
+
+/**
+ * Set to {@code true} to require the ImageResource to be downloaded
as a
+ * separate resource at runtime. Specifically, this will disable the
use of
+ * {@code data:} URLs or other bundling optimizations for the image.
This
+ * can be used for infrequently-displayed images.
+ */
+boolean preventInlining() default false;
/**
* This option affects the image bundling optimization to allow the
image to
Index: user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
===
--- user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
(revision 10245)
+++ user/src/com/google/gwt/resources/rg/ImageResourceGenerator.java
(working copy)
@@ -191,9 +191,11 @@
* bytes should be associated with.
*/
static class BundleKey extends StringKey {
-private static String key(ResourceContext context,
-ImageResourceDeclaration image) {
- if (image.getRepeatStyle() == RepeatStyle.Both) {
+private static String key(ImageResourceDeclaration image, boolean
isExternal) {
+ if (isExternal) {
+return External: + image.get();
+ }
+ if (image.isPreventInlining() || image.getRepeatStyle() ==
RepeatStyle.Both) {
return Unbundled: + image.get();
}
return Arranged: + image.getRepeatStyle().toString();
@@ -201,13 +203,8 @@
private final RepeatStyle repeatStyle;
-public BundleKey(ImageResourceDeclaration image) {
- super(External: + image.get());
- this.repeatStyle = image.getRepeatStyle();
-}
-
-public BundleKey(ResourceContext context, ImageResourceDeclaration
image) {
- super(key(context, image));
+public BundleKey(ImageResourceDeclaration image, boolean isExternal) {
+ super(key(image, isExternal));
this.repeatStyle = image.getRepeatStyle();
}
@@ -314,7 +311,7 @@
String.class.getCanonicalName());
String contentsExpression = context.deploy(
- localized.getUrl(), null, false);
+ localized.getUrl(), null, image.isPreventInlining());
normalContentsFieldName = fields.define(stringType, externalImage,
contentsExpression, true, true);
@@ -326,7 +323,7 @@
byte[] rtlData = ImageBundleBuilder.toPng(logger, rect);
String rtlContentsUrlExpression = context.deploy(image.getName()
-+ _rtl.png, image/png, rtlData, false);
++ _rtl.png, image/png, rtlData, image.isPreventInlining());
rtlContentsFieldName =
fields.define(stringType, externalImage_rtl,
rtlContentsUrlExpression, true, true);
}
@@ -383,6 +380,10 @@
public boolean isFlipRtl() {
return options == null ? false : options.flipRtl();
+}
+
+public boolean isPreventInlining() {
+ return options == null ? false : options.preventInlining();
}
}
@@ -474,7 +475,7 @@
sw.println('' + name + \,);
ImageResourceDeclaration image = new ImageResourceDeclaration(method);
-DisplayedImage bundle = getImage(context, image);
+DisplayedImage bundle = getImage(image);
ImageRect rect = bundle.getImageRect(image);
assert rect != null : No ImageRect ever computed for + name;
@@ -542,10 +543,13 @@
LocalizedImage localizedImage;
ImageRect rect;
try {
- BundledImage bundledImage = (BundledImage) getImage(context, image);
+ BundledImage bundledImage = (BundledImage) getImage(image);
localizedImage = bundledImage.addImage(logger, context, image);
rect = bundledImage.getImageRect(image);
displayed = bundledImage;
+ if (image.isPreventInlining()) {
+cannotBundle = true;
+ }
} catch (CannotBundleImageException e) {
cannotBundle = true;
localizedImage = e.getLocalizedImage();
@@ -587,7 +591,7 @@
}
ExternalImage externalImage = new ExternalImage(image,
localizedImage,
rect);
- shared.externalImages.put(new BundleKey(image), externalImage);
+ shared.externalImages.put(new BundleKey(image, true), externalImage);
displayed
[gwt-contrib] Mass auto-format of RequestFactory code before landing a big patch. (issue1450811)
Reviewers: rjrjr, Description: Mass auto-format of RequestFactory code before landing a big patch. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1450811/ Affected files: M user/src/com/google/web/bindery/requestfactory/gwt/client/DefaultRequestTransport.java M user/src/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryLogHandler.java M user/src/com/google/web/bindery/requestfactory/gwt/client/package-info.java M user/src/com/google/web/bindery/requestfactory/gwt/rebind/RequestFactoryEditorDriverGenerator.java M user/src/com/google/web/bindery/requestfactory/gwt/rebind/RequestFactoryGenerator.java M user/src/com/google/web/bindery/requestfactory/gwt/ui/client/EntityProxyKeyProvider.java M user/src/com/google/web/bindery/requestfactory/gwt/ui/client/ProxyRenderer.java M user/src/com/google/web/bindery/requestfactory/gwt/ui/client/package-info.java M user/src/com/google/web/bindery/requestfactory/server/DefaultExceptionHandler.java M user/src/com/google/web/bindery/requestfactory/server/LocatorServiceLayer.java M user/src/com/google/web/bindery/requestfactory/server/Logging.java M user/src/com/google/web/bindery/requestfactory/server/ReflectiveServiceLayer.java M user/src/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidator.java M user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java M user/src/com/google/web/bindery/requestfactory/server/RequestFactoryServlet.java M user/src/com/google/web/bindery/requestfactory/server/RequestState.java M user/src/com/google/web/bindery/requestfactory/server/Resolver.java M user/src/com/google/web/bindery/requestfactory/server/ServiceLayer.java M user/src/com/google/web/bindery/requestfactory/server/ServiceLayerDecorator.java M user/src/com/google/web/bindery/requestfactory/shared/DefaultProxyStore.java M user/src/com/google/web/bindery/requestfactory/shared/EntityProxyChange.java M user/src/com/google/web/bindery/requestfactory/shared/LoggingRequest.java M user/src/com/google/web/bindery/requestfactory/shared/Request.java M user/src/com/google/web/bindery/requestfactory/shared/RequestFactory.java M user/src/com/google/web/bindery/requestfactory/shared/RequestTransport.java M user/src/com/google/web/bindery/requestfactory/shared/ServerFailure.java M user/src/com/google/web/bindery/requestfactory/shared/ValueLocator.java M user/src/com/google/web/bindery/requestfactory/shared/package-info.java M user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestFactory.java M user/src/com/google/web/bindery/requestfactory/vm/RequestFactorySource.java M user/src/com/google/web/bindery/requestfactory/vm/package-info.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Support RequestContext composition (issue 6234) and mix-in (issue 6035). (issue1447815)
Reviewers: rjrjr, Message: Follow-on changes: - Make ServiceLayerCache build itself reflectively; it's really obnoxious to update that type. - Fully separate the Deobfuscator API from RFIV to allow unused state to be jettisoned and remove the need for synchronization. - Add an annotation to allow the unobfuscated RequestFactory interface name to be removed in exchange for explicit registration of RF types on the server. Description: Support RequestContext composition (issue 6234) and mix-in (issue 6035). Remove no-method-overloads restriction by using a full method descriptor to create the operation. Use obfuscated type and operation tokens to reduce payload and JS size (issue 5394). Without this change, the operation tokens can be excessively lengthy. The RequestFactory interface is used as the gwt.rpc analog. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1447815/ Affected files: M user/src/com/google/web/bindery/requestfactory/gwt/rebind/RequestFactoryGenerator.java M user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/RequestFactoryModel.java M user/src/com/google/web/bindery/requestfactory/gwt/rebind/model/RequestMethod.java A user/src/com/google/web/bindery/requestfactory/server/FindServiceLayer.java M user/src/com/google/web/bindery/requestfactory/server/LocatorServiceLayer.java M user/src/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidator.java M user/src/com/google/web/bindery/requestfactory/server/ResolverServiceLayer.java M user/src/com/google/web/bindery/requestfactory/server/ServiceLayer.java M user/src/com/google/web/bindery/requestfactory/server/ServiceLayerCache.java M user/src/com/google/web/bindery/requestfactory/server/ServiceLayerDecorator.java M user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java A user/src/com/google/web/bindery/requestfactory/server/impl/OperationKey.java M user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java M user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestFactory.java M user/src/com/google/web/bindery/requestfactory/shared/impl/Constants.java M user/src/com/google/web/bindery/requestfactory/shared/messages/RequestMessage.java M user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestContext.java M user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestFactory.java M user/src/com/google/web/bindery/requestfactory/vm/RequestFactorySource.java M user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java M user/test/com/google/web/bindery/requestfactory/server/SimpleFoo.java M user/test/com/google/web/bindery/requestfactory/shared/ServiceInheritanceTest.java M user/test/com/google/web/bindery/requestfactory/shared/SimpleFooRequest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add a test to RequestFactoryInterfaceValidator that the findFoo() domain (issue1453803)
Reviewers: rjrjr,
http://gwt-code-reviews.appspot.com/1453803/diff/1/user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java
File
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java
(right):
http://gwt-code-reviews.appspot.com/1453803/diff/1/user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java#newcode281
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java:281:
interface SkipValidationContext extends RequestContext {
Re-sorted the file.
Description:
Add a test to RequestFactoryInterfaceValidator that the findFoo() domain
method is static.
http://code.google.com/p/google-web-toolkit/issues/detail?id=6428
Patch by: bobv
Review by: rjrjr
Reported by: zundel
Please review this at http://gwt-code-reviews.appspot.com/1453803/
Affected files:
M
user/src/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidator.java
M
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java
Index:
user/src/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidator.java
===
---
user/src/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidator.java
(revision 10245)
+++
user/src/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidator.java
(working copy)
@@ -1077,6 +1077,9 @@
+ cannot be used as the argument to %s(%s), print(keyType),
findMethod.getName(), print(findMethod.getArgumentTypes()[0]));
}
+ if (!findMethod.isDeclaredStatic()) {
+logger.poison(The %s method must be static, findMethodName);
+ }
} else {
logger.poison(There is no %s method in type %s that returns %2$s,
findMethodName, print(domainType));
Index:
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java
===
---
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java
(revision 10245)
+++
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java
(working copy)
@@ -62,12 +62,10 @@
return 0;
}
}
-
@ProxyFor(ClinitEntity.class)
interface ClinitEntityProxy extends EntityProxy {
Object OBJECT = new Object();
}
-
@Service(ClinitEntity.class)
interface ClinitRequestContext extends RequestContext {
Object OBJECT = new Object();
@@ -128,6 +126,30 @@
java.sql.Date getSqlDate();
}
+ /**
+ * Tests that the validator reports non-static finder methods.
+ */
+ static class EntityWithInstanceFind {
+public String getId() {
+ return null;
+}
+
+public int getVersion() {
+ return 0;
+}
+
+/**
+ * This method should be static.
+ */
+EntityWithInstanceFind findEntityWithInstanceFind(String key) {
+ return null;
+}
+ }
+
+ @ProxyFor(EntityWithInstanceFind.class)
+ interface EntityWithInstanceFindProxy extends EntityProxy {
+ }
+
class Foo {
}
@@ -246,21 +268,6 @@
@Service(Domain.class)
interface ServiceRequestMissingMethod extends RequestContext {
RequestInteger doesNotExist(int a);
- }
-
- @Service(Domain.class)
- interface SkipValidationContext extends RequestContext {
-@SkipInterfaceValidation
-RequestInteger doesNotExist(int a);
-
-@SkipInterfaceValidation
-RequestLong foo(int a);
- }
-
- @Service(Domain.class)
- interface SkipValidationProxy extends ValueProxy {
-@SkipInterfaceValidation
-boolean doesNotExist();
}
@Service(Domain.class)
@@ -270,6 +277,21 @@
DomainProxyMissingAnnotation getDomainProxyMissingAnnotation();
}
+ @Service(Domain.class)
+ interface SkipValidationContext extends RequestContext {
+@SkipInterfaceValidation
+RequestInteger doesNotExist(int a);
+
+@SkipInterfaceValidation
+RequestLong foo(int a);
+ }
+
+ @Service(Domain.class)
+ interface SkipValidationProxy extends ValueProxy {
+@SkipInterfaceValidation
+boolean doesNotExist();
+ }
+
@ProxyFor(Domain.class)
@ProxyForName(Domain)
@Service(Domain.class)
@@ -294,8 +316,7 @@
static class Value {
}
- static class VisibleErrorContext extends
- RequestFactoryInterfaceValidator.ErrorContext {
+ static class VisibleErrorContext extends
RequestFactoryInterfaceValidator.ErrorContext {
final ListString logs;
public VisibleErrorContext(Logger logger) {
@@ -365,6 +386,12 @@
assertTrue(v.isPoisoned());
}
+ public void testFindMustBeStatic() {
+v.validateEntityProxy(EntityWithInstanceFindProxy.class.getName());
+assertTrue(v.isPoisoned());
+assertTrue(errors.logs.contains(The findEntityWithInstanceFind method
must
[gwt-contrib] Use identity semantics when canonicalizing JsonSplittable instances. (issue1451805)
Reviewers: rjrjr,
Description:
Use identity semantics when canonicalizing JsonSplittable instances.
This is necessary to support Android, where the org.json arrays appear
to have value-based equality.
http://code.google.com/p/google-web-toolkit/issues/detail?id=6390
Patch by: bobv
Review by: rjrjr
Please review this at http://gwt-code-reviews.appspot.com/1451805/
Affected files:
M user/src/com/google/web/bindery/autobean/vm/impl/JsonSplittable.java
Index: user/src/com/google/web/bindery/autobean/vm/impl/JsonSplittable.java
===
--- user/src/com/google/web/bindery/autobean/vm/impl/JsonSplittable.java
(revision 10251)
+++ user/src/com/google/web/bindery/autobean/vm/impl/JsonSplittable.java
(working copy)
@@ -15,6 +15,7 @@
*/
package com.google.web.bindery.autobean.vm.impl;
+import com.google.gwt.core.client.impl.WeakMapping;
import com.google.web.bindery.autobean.shared.Splittable;
import com.google.web.bindery.autobean.shared.impl.HasSplittable;
import com.google.web.bindery.autobean.shared.impl.StringQuoter;
@@ -23,27 +24,17 @@
import org.json.JSONException;
import org.json.JSONObject;
-import java.lang.ref.Reference;
-import java.lang.ref.WeakReference;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import java.util.WeakHashMap;
/**
* Uses the org.json packages to slice and dice request payloads.
*/
public class JsonSplittable implements Splittable, HasSplittable {
-
- /**
- * Ensures that the same JsonSplittable will be returned for a given
backing
- * JSONObject.
- */
- private static final MapObject, ReferenceJsonSplittable canonical =
- new WeakHashMapObject, ReferenceJsonSplittable();
public static JsonSplittable create() {
return new JsonSplittable(new JSONObject());
@@ -306,13 +297,19 @@
if (JSONObject.NULL.equals(object)) {
return null;
}
-ReferenceJsonSplittable ref = canonical.get(object);
-JsonSplittable seen = ref == null ? null : ref.get();
+/*
+ * Maintain a 1:1 mapping between object instances and JsonSplittables.
+ * Doing this with a WeakHashMap doesn't work on Android, since its
org.json
+ * arrays appear to have value-based equality.
+ */
+JsonSplittable seen = (JsonSplittable) WeakMapping.get(object,
JsonSplittable.class.getName());
if (seen == null) {
if (object instanceof JSONObject) {
seen = new JsonSplittable((JSONObject) object);
+WeakMapping.set(object, JsonSplittable.class.getName(), seen);
} else if (object instanceof JSONArray) {
seen = new JsonSplittable((JSONArray) object);
+WeakMapping.set(object, JsonSplittable.class.getName(), seen);
} else if (object instanceof String) {
seen = new JsonSplittable(object.toString());
} else if (object instanceof Number) {
@@ -322,7 +319,6 @@
} else {
throw new RuntimeException(Unhandled type + object.getClass());
}
- canonical.put(object, new WeakReferenceJsonSplittable(seen));
}
return seen;
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add RequestContext.find() to support chained requests. (issue1448806)
The base interface for RequestFactory service endpoints. Add disclaimer explaining that this interface (and the others) are normally implemented by generated code, and are subject to incompatible updates? Done. Also added a FakeRequestContext base type in a testing subpackage. And should log an item on the issue tracker with the appropriate breaking change label. http://code.google.com/p/google-web-toolkit/issues/detail?id=6413 Ha! Do you have other sneaky literals like this lying around? Calls to the RequestData constructor are usually only in generated code. http://gwt-code-reviews.appspot.com/1448806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: Creating a default Locator for RequestFactoryServlet
On Thu, May 26, 2011 at 2:23 PM, Thomas Broyer t.bro...@gmail.com wrote: How about simply using a ServiceLayerDecorator that overrides resolveLocator, delegating to super.resolveLocator and, if it returns null then return the default locator instead? +1 The ServiceLayer design is intended to cut down on the number of configuration points that have to be built into the stock RequestFactory code. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add BatchedRequestScope utility class to aggregate all requests made within a single tick of the... (issue1449804)
Reviewers: rjrjr, Description: Add BatchedRequestScope utility class to aggregate all requests made within a single tick of the event loop. Add FanoutReceiver utility class. Remove redundant tests from RequestFactoryGwtJreSuite. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1449804/ Affected files: A user/src/com/google/web/bindery/requestfactory/gwt/client/BatchedRequestScope.java A user/src/com/google/web/bindery/requestfactory/shared/FanoutReceiver.java M user/src/com/google/web/bindery/requestfactory/shared/RequestContext.java M user/test/com/google/web/bindery/requestfactory/gwt/RequestFactoryGwtJreSuite.java M user/test/com/google/web/bindery/requestfactory/gwt/RequestFactorySuite.java A user/test/com/google/web/bindery/requestfactory/gwt/client/BatchedRequestScopeTest.java M user/test/com/google/web/bindery/requestfactory/vm/RequestFactoryJreSuite.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add BatchedRequestScope utility class to aggregate all requests made within a single tick of the... (issue1449804)
http://gwt-code-reviews.appspot.com/1449804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add RequestContext.find() to support chained requests. (issue1448806)
Reviewers: rjrjr,
Message:
I don't think RequestFactory.find() should be deprecated. It's a
convenient starting point. If you want to convert existing calls to
find() to operate in a chained manner, the Request.to() method returns
the underlying RequestContext, which will now have its own find()
method.
http://gwt-code-reviews.appspot.com/1448806/diff/1/user/src/com/google/web/bindery/requestfactory/shared/impl/FindRequest.java
File
user/src/com/google/web/bindery/requestfactory/shared/impl/FindRequest.java
(left):
http://gwt-code-reviews.appspot.com/1448806/diff/1/user/src/com/google/web/bindery/requestfactory/shared/impl/FindRequest.java#oldcode30
user/src/com/google/web/bindery/requestfactory/shared/impl/FindRequest.java:30:
public interface FindRequest extends RequestContext {
This type hasn't been used for a while.
http://gwt-code-reviews.appspot.com/1448806/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/FindServiceTest.java
File
user/test/com/google/web/bindery/requestfactory/gwt/client/FindServiceTest.java
(right):
http://gwt-code-reviews.appspot.com/1448806/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/FindServiceTest.java#newcode55
user/test/com/google/web/bindery/requestfactory/gwt/client/FindServiceTest.java:55:
public void testChainedFind() {
The addition of this method is the only real change, the rest is
reformatting.
Description:
Add RequestContext.find() to support chained requests.
Delete unused FindRequest type.
Patch by: bobv
Review by: rjrjr
Please review this at http://gwt-code-reviews.appspot.com/1448806/
Affected files:
M
user/src/com/google/web/bindery/requestfactory/shared/RequestContext.java
M
user/src/com/google/web/bindery/requestfactory/shared/RequestFactory.java
M
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
M
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestFactory.java
D
user/src/com/google/web/bindery/requestfactory/shared/impl/FindRequest.java
M
user/test/com/google/web/bindery/requestfactory/gwt/client/FindServiceTest.java
M
user/test/com/google/web/bindery/requestfactory/server/RequestFactoryInterfaceValidatorTest.java
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Re-implement runAsync to improve code size. (issue1442807)
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java
File
dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java#newcode65
dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java:65:
String location = runAsync.getName();
location vs. getName(). Has the concept changed?
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java
File
dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java#newcode55
dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java:55:
public static final String ASYNC_MAGIC_METHOD = runAsync;
Unused?
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java
File dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java (right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java#newcode40
dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java:40: * Based on
either explicit class literal, or the jsni name of the containing
The upside is that the actual value doesn't matter except that it's
unique across the compilation?
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java#newcode1247
dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java:1247:
func.setArtificiallyRescued(true);
:-)
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java
File dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java#newcode86
dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java:86:
return;
On 2011/05/20 22:47:15, scottb wrote:
Some fragments are now so small, they don't contain any functions. :)
Isn't that pointless? Shouldn't these non-fragments get pruned?
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
File dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java#newcode198
dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java:198:
static String getImplicitName(JMethod method) {
getQualifiedJsniName() instead?
http://gwt-code-reviews.appspot.com/1442807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] RequestFactoryServlet
Does it makes sense to add the ServletContext into RequestFactoryServlet inside GWT proper? If so, I can submit a patch for you. That sounds like a very clean approach. When you submit the patch, please create an issue in the GWT issue tracker with the URL of the code review. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix AutoBean VM memory leak due to circular references between the ProxyAutoBean, ShimHandler, a... (issue1448803)
Reviewers: tbroyer, rjrjr,
Message:
This is a simpler version of
http://gwt-code-reviews.appspot.com/1401802, which just adds a
WeakReference from the ProxyAutoBean to the shim. I tested this by
allocating new AutoBean instances in a tight loop and observing a
constant Java heap size well below the -Xmx value.
Description:
Fix AutoBean VM memory leak due to circular references between the
ProxyAutoBean, ShimHandler, and WeakReference.
Issue 6193.
Patch by: bobv
Review by: tbroyer, rjrjr
Please review this at http://gwt-code-reviews.appspot.com/1448803/
Affected files:
M user/src/com/google/web/bindery/autobean/vm/impl/ProxyAutoBean.java
Index: user/src/com/google/web/bindery/autobean/vm/impl/ProxyAutoBean.java
===
--- user/src/com/google/web/bindery/autobean/vm/impl/ProxyAutoBean.java
(revision 10198)
+++ user/src/com/google/web/bindery/autobean/vm/impl/ProxyAutoBean.java
(working copy)
@@ -15,14 +15,15 @@
*/
package com.google.web.bindery.autobean.vm.impl;
+import com.google.gwt.core.client.impl.WeakMapping;
import com.google.web.bindery.autobean.shared.AutoBean;
import com.google.web.bindery.autobean.shared.AutoBeanFactory;
import com.google.web.bindery.autobean.shared.AutoBeanUtils;
import com.google.web.bindery.autobean.shared.AutoBeanVisitor;
import com.google.web.bindery.autobean.shared.impl.AbstractAutoBean;
import com.google.web.bindery.autobean.vm.Configuration;
-import com.google.gwt.core.client.impl.WeakMapping;
-
+
+import java.lang.ref.WeakReference;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
@@ -66,7 +67,7 @@
Class?... extraInterfaces) {
Class?[] intfs;
if (extraInterfaces == null) {
- intfs = new Class?[]{intf};
+ intfs = new Class?[] {intf};
} else {
intfs = new Class?[extraInterfaces.length + 1];
intfs[0] = intf;
@@ -116,7 +117,25 @@
private final ClassT beanType;
private final Configuration configuration;
private final Data data;
- private final T shim;
+ /**
+ * Because the shim and the ProxyAutoBean are related through
WeakMapping, we
+ * need to ensure that the ProxyAutoBean doesn't artificially extend the
+ * lifetime of the shim. If there are no external references to the
shim, it's
+ * ok if it's deallocated, since it has no interesting state.
+ *
+ * pre
+ *
+ * ---
+ * | ProxyAutoBean ||Shim|
+ * | | --+-bean |
+ * | shim-+---X---||
+ * |___| ^
+ * ^ |
+ * | X
+ * |__ WeakMapping ___|
+ * /pre
+ */
+ private WeakReferenceT shim;
// These constructors mirror the generated constructors.
@SuppressWarnings(unchecked)
@@ -125,7 +144,6 @@
this.beanType = (ClassT) beanType;
this.configuration = configuration;
this.data = calculateData(beanType);
-this.shim = createShim();
}
@SuppressWarnings(unchecked)
@@ -135,12 +153,16 @@
this.beanType = (ClassT) beanType;
this.configuration = configuration;
this.data = calculateData(beanType);
-this.shim = createShim();
}
@Override
public T as() {
-return shim;
+T toReturn = shim == null ? null : shim.get();
+if (toReturn == null) {
+ toReturn = createShim();
+ shim = new WeakReferenceT(toReturn);
+}
+return toReturn;
}
public Configuration getConfiguration() {
@@ -241,7 +263,7 @@
Object value;
try {
getter.setAccessible(true);
-value = getter.invoke(shim);
+value = getter.invoke(as());
} catch (IllegalArgumentException e) {
throw new RuntimeException(e);
} catch (IllegalAccessException e) {
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix inconsistent use of finishTest() in RequestFactoryTest. (issue1421808)
Reviewers: zundel,
Description:
Fix inconsistent use of finishTest() in RequestFactoryTest.
Patch by: bobv
Review by: zundel
Please review this at http://gwt-code-reviews.appspot.com/1421808/
Affected files:
M
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
Index:
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
===
---
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
(revision 10094)
+++
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
(working copy)
@@ -573,7 +573,7 @@
ctx.receiveEnum(OnlyUsedByRequestContextMethod.FOO).fire(new
ReceiverVoid() {
@Override
public void onSuccess(Void response) {
-finishTest();
+finishTestAndReset();
}
});
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] HasRequestContext and 2.3.0rc1
On Wed, Apr 27, 2011 at 10:56 PM, Patrick Julien pjul...@gmail.com wrote:
How does this actually work?
class MyEditor implements HasRequestConextFoo {
// HasRequestContext extends the Editor interface
}
If I implement HasRequestContextT anywhere, the containing request
factory editor driver that is generated no longer compiles, saying
that there is a missing constructor
Can you attach the log output? What is the type declaration for the
offending Editor class?
--
Bob Vawter
Google Web Toolkit Team
--
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] 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(SetConstraintViolation?
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: Add RequestContext.append() to allow actions across different domain service types to be combine... (issue1423805)
Which to me leaves the question: is our usage described at [1] an intended use case, that's here to stay? or simply a fortunate unspecified behavior, and we should change our code to this new append() feature? For now, I'm going to call it a fortunate unspecified behavior. At some point in the future when I can get time allocated to add robust polymorphism support to RequestFactory, that behavior might go away if it helps promote uniformity between proxy-entity and context-code type hierarchy mappings. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Documentation review requested: RequestFactoryMovingParts
http://code.google.com/p/google-web-toolkit/wiki/RequestFactoryMovingParts The RFMP doc is targeted at potential contributors or those with non-trivial integration schemes. For those who have been using RequestFactory, what implementation details would you like to have more documentation on? -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Switch RequestFactory to use the real ConstraintViolation instead of the hacky Violation interface. (issue1422809)
Reviewers: rjrjr, Message: Request requested. http://gwt-code-reviews.appspot.com/1422809/diff/1/user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java File user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java (right): http://gwt-code-reviews.appspot.com/1422809/diff/1/user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java#newcode526 user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java:526: // Construct an ID that represents domainObject Interesting change starts here. Description: Switch RequestFactory to use the real ConstraintViolation instead of the hacky Violation interface. Deprecate Violation and Receiver.onViolation(). Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1422809/ Affected files: M samples/dynatablerf/src/com/google/gwt/sample/dynatablerf/client/PersonEditorWorkflow.java M user/src/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryEditorDriver.java M user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java M user/src/com/google/web/bindery/requestfactory/gwt/client/testing/MockRequestFactoryEditorDriver.java M user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java M user/src/com/google/web/bindery/requestfactory/server/SimpleRequestProcessor.java M user/src/com/google/web/bindery/requestfactory/shared/Receiver.java M user/src/com/google/web/bindery/requestfactory/shared/Violation.java M user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequest.java M user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java M user/src/com/google/web/bindery/requestfactory/shared/messages/ViolationMessage.java M user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java M user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryExceptionPropagationTest.java M user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java M user/test/com/google/web/bindery/requestfactory/gwt/client/ui/EditorTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Add RequestContext.append() to allow actions across different domain service types to be combine... (issue1423805)
Reviewers: rjrjr, Description: Add RequestContext.append() to allow actions across different domain service types to be combined in a single HTTP request. Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1423805/ Affected files: M user/src/com/google/web/bindery/requestfactory/shared/RequestContext.java M user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java M user/src/com/google/web/bindery/requestfactory/shared/impl/BaseProxyCategory.java M user/src/com/google/web/bindery/requestfactory/shared/impl/Constants.java M user/src/com/google/web/bindery/requestfactory/vm/InProcessRequestContext.java M user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java M user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTestBase.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Move single jso logic from BasicWebModeCompiler to JavaToJavaScriptCompiler. (issue1428802)
LGTM http://gwt-code-reviews.appspot.com/1428802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add RequestContext.append() to allow actions across different domain service types to be combine... (issue1423805)
r10052
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
File
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java
(right):
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java#newcode103
user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java:103:
* an invocation argument.
On 2011/04/21 19:36:38, rjrjr wrote:
Don't they also land here via create?
Done.
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
File
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
(right):
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode239
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:239:
assertSame(foo2, c3.edit(foo2));
On 2011/04/21 19:36:38, rjrjr wrote:
should you test a create from c3 as well? I could imagine there being
upstream /
downstream issues for non-neighbors.
Done.
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode243
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:243:
fail(Should have thrown IllegalStateException);
On 2011/04/21 19:36:38, rjrjr wrote:
because c3 has already been appended?
Done.
http://gwt-code-reviews.appspot.com/1423805/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode262
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:262:
c3.fire(new ReceiverVoid() {
On 2011/04/21 19:36:38, rjrjr wrote:
what happens if you fire c1 or c2 instead? Should that be tested?
Done.
http://gwt-code-reviews.appspot.com/1423805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Switch RequestFactory to use the real ConstraintViolation instead of the hacky Violation interface. (issue1422809)
r10053
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java
File
user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java
(right):
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java#newcode52
user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java:52:
@SuppressWarnings(deprecation)
Every field and method in this class, save getUserDataObject(), shows a
deprecation warning.
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/src/com/google/web/bindery/requestfactory/shared/Receiver.java
File user/src/com/google/web/bindery/requestfactory/shared/Receiver.java
(right):
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/src/com/google/web/bindery/requestfactory/shared/Receiver.java#newcode76
user/src/com/google/web/bindery/requestfactory/shared/Receiver.java:76:
* @param errors a Set of {@link Violation} instances
On 2011/04/21 18:49:32, rjrjr wrote:
a set of what instances?
Done.
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/src/com/google/web/bindery/requestfactory/shared/messages/ViolationMessage.java
File
user/src/com/google/web/bindery/requestfactory/shared/messages/ViolationMessage.java
(right):
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/src/com/google/web/bindery/requestfactory/shared/messages/ViolationMessage.java#newcode28
user/src/com/google/web/bindery/requestfactory/shared/messages/ViolationMessage.java:28:
String TEMPATE = T;
On 2011/04/21 18:49:32, rjrjr wrote:
tempLate
Done.
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java
File user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java
(right):
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java#newcode505
user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java:505:
@SuppressWarnings(deprecation)
On 2011/04/21 18:49:32, rjrjr wrote:
can you isolate the deprecated part to a separate method?
Done.
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryExceptionPropagationTest.java
File
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryExceptionPropagationTest.java
(right):
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryExceptionPropagationTest.java#newcode62
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryExceptionPropagationTest.java:62:
public void
onViolation(Setcom.google.web.bindery.requestfactory.shared.Violation
errors) {
On 2011/04/21 18:49:32, rjrjr wrote:
Should add a counter for the onConstraintViolation as well. Then let
it call
through to super to maintain coverage of the compatibility code.
Or, if that's out of scope for this test, just convert to the new
method?
Done.
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
File
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java
(right):
http://gwt-code-reviews.appspot.com/1422809/diff/1/user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java#newcode55
user/test/com/google/web/bindery/requestfactory/gwt/client/RequestFactoryTest.java:55:
public class RequestFactoryTest extends RequestFactoryTestBase {
Just copied any tests on onViolation() into a matching
onConstraintViolation() that forwards to the default
Receiver.onConstraintViolation().
http://gwt-code-reviews.appspot.com/1422809/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: RequestFactoryJarExtractor intermittently throws exceptions when running (issue1425805)
http://gwt-code-reviews.appspot.com/1425805/diff/3002/user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java File user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java (right): http://gwt-code-reviews.appspot.com/1425805/diff/3002/user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java#newcode750 user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java:750: private boolean executionFailed = false; Place in alphabetical order. http://gwt-code-reviews.appspot.com/1425805/diff/3002/user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java#newcode760 user/src/com/google/web/bindery/requestfactory/server/RequestFactoryJarExtractor.java:760: int numThreads = Math.min(MAX_THREADS, Runtime.getRuntime().availableProcessors()); Why limit to 4? http://gwt-code-reviews.appspot.com/1425805/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove JAnnotation from GWT AST. (issue1420803)
LGTM. http://gwt-code-reviews.appspot.com/1420803/diff/31/dev/core/src/com/google/gwt/dev/jjs/ast/JAnnotation.java File dev/core/src/com/google/gwt/dev/jjs/ast/JAnnotation.java (left): http://gwt-code-reviews.appspot.com/1420803/diff/31/dev/core/src/com/google/gwt/dev/jjs/ast/JAnnotation.java#oldcode30 dev/core/src/com/google/gwt/dev/jjs/ast/JAnnotation.java:30: * Represents a java annotation. JAnnotation was written with the assumption that other annotation-controlled compiler passes would be added, such as @ShouldNotBeInWebMode or @MustInline. None of that work ever happened, so ArtificialRescue would up being the only user. http://gwt-code-reviews.appspot.com/1420803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Make EventBus backward compatible. (issue1423803)
LGTM http://gwt-code-reviews.appspot.com/1423803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Introduces SkipInterfaceValidation annotation. (issue1338807)
Committed with minor modifications at r10022. http://gwt-code-reviews.appspot.com/1338807/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Finishes the job of making EventBus backward compatible, (issue1425804)
LGTM.
http://gwt-code-reviews.appspot.com/1425804/diff/1/user/src/com/google/gwt/event/shared/EventBus.java
File user/src/com/google/gwt/event/shared/EventBus.java (right):
http://gwt-code-reviews.appspot.com/1425804/diff/1/user/src/com/google/gwt/event/shared/EventBus.java#newcode28
user/src/com/google/gwt/event/shared/EventBus.java:28: public H
com.google.web.bindery.event.shared.HandlerRegistration
addHandler(Event.TypeH type, H handler) {
Should the HandlerRegistration in this package have some kind of do not
use for new code warning placed on its javadoc?
Is there a pattern for soft deprecation where we don't want to cause
deprecation warning spam, but don't want to encourage new uses of the
type? Would some kind of empty NewUsesAreBadIdea tag interface help
with this?
http://gwt-code-reviews.appspot.com/1425804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Serialization of Final Fields in RPC (issue1380807)
http://gwt-code-reviews.appspot.com/1380807/diff/11003/user/src/com/google/gwt/user/rebind/rpc/Shared.java
File user/src/com/google/gwt/user/rebind/rpc/Shared.java (right):
http://gwt-code-reviews.appspot.com/1380807/diff/11003/user/src/com/google/gwt/user/rebind/rpc/Shared.java#newcode56
user/src/com/google/gwt/user/rebind/rpc/Shared.java:56: private static
SerializeFinalFieldsOptions serializeFinalFieldsValue;
See comment in previous revision. Static fields in Generator types can
cause flaky behavior.
http://gwt-code-reviews.appspot.com/1380807/diff/11003/user/src/com/google/gwt/user/rebind/rpc/Shared.java#newcode100
user/src/com/google/gwt/user/rebind/rpc/Shared.java:100: propertyOracle,
RPC_PROP_SERIALIZE_FINAL_FIELDS, FALSE).toUpperCase();
You can use Enum.valueOf() instead of the if block below.
http://gwt-code-reviews.appspot.com/1380807/diff/11003/user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java
File user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/11003/user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java#newcode56
user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java:56:
private FinalFieldsTestServiceAsync finalFieldsTestService;
Member sort order. Fields should be defined before methods.
http://gwt-code-reviews.appspot.com/1380807/diff/11003/user/test/com/google/gwt/user/client/rpc/FinalFieldsTestService.java
File
user/test/com/google/gwt/user/client/rpc/FinalFieldsTestService.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/11003/user/test/com/google/gwt/user/client/rpc/FinalFieldsTestService.java#newcode27
user/test/com/google/gwt/user/client/rpc/FinalFieldsTestService.java:27:
public class FinalFieldsNode implements IsSerializable {
Use Serializable instead of IsSerializable.
http://gwt-code-reviews.appspot.com/1380807/diff/11003/user/test/com/google/gwt/user/server/rpc/FinalFieldsTestServiceImpl.java
File
user/test/com/google/gwt/user/server/rpc/FinalFieldsTestServiceImpl.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/11003/user/test/com/google/gwt/user/server/rpc/FinalFieldsTestServiceImpl.java#newcode28
user/test/com/google/gwt/user/server/rpc/FinalFieldsTestServiceImpl.java:28:
public FinalFieldsNode transferObject(FinalFieldsNode node) {
Check the incoming values in node to make sure that final fields are
being set properly when sent by the client.
http://gwt-code-reviews.appspot.com/1380807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Re: Adds method to customise ServiceLocator instantiation in ServiceLayerDecorator (issue1427801)
On Mon, Apr 18, 2011 at 1:26 PM, Ray Ryan rj...@google.com wrote: This looks like a job for…bobv! Will check this out tonight. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: * Soft permutations fail to collapse. Collapse 'derived' properties chain when 'parent' properti... (issue1424803)
This code needs comments. I'm having a hard time figuring out what it
does.
http://gwt-code-reviews.appspot.com/1424803/diff/1/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java
File dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java
(right):
http://gwt-code-reviews.appspot.com/1424803/diff/1/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java#newcode45
dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java:45: *
Collapses derived properties.
Expand this comment to roughly describe the process implemented for
future maintainers.
http://gwt-code-reviews.appspot.com/1424803/diff/1/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java#newcode52
dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java:52: //
find collapsed properties
For each input property, determine if it has a collapsed-value
equivalence set containing the associated input value.
http://gwt-code-reviews.appspot.com/1424803/diff/1/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java#newcode68
dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java:68:
MapString, ArrayListString dependencies = new TreeMapString,
ArrayListString();
Make this a MapBindingProperty, ListString. Add doc: A map of
binding properties to the names of the properties that must be evaluated
prior to computing the value of the key object.
http://gwt-code-reviews.appspot.com/1424803/diff/1/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java#newcode72
dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java:72: if
(deps.size() 0) {
!deps.isEmpty()
http://gwt-code-reviews.appspot.com/1424803/diff/1/dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java#newcode76
dev/core/src/com/google/gwt/dev/cfg/PropertyPermutations.java:76: for
(Map.EntryString, ArrayListString e : dependencies.entrySet()) {
What is this loop doing?
http://gwt-code-reviews.appspot.com/1424803/diff/1/user/src/com/google/gwt/user/tools/templates/sample/_srcFolder_/_moduleFolder_/_moduleShortName_.gwt.xmlsrc
File
user/src/com/google/gwt/user/tools/templates/sample/_srcFolder_/_moduleFolder_/_moduleShortName_.gwt.xmlsrc
(right):
http://gwt-code-reviews.appspot.com/1424803/diff/1/user/src/com/google/gwt/user/tools/templates/sample/_srcFolder_/_moduleFolder_/_moduleShortName_.gwt.xmlsrc#newcode26
user/src/com/google/gwt/user/tools/templates/sample/_srcFolder_/_moduleFolder_/_moduleShortName_.gwt.xmlsrc:26:
permutations into a single compiled unit. As a consequence, collapsed
units
into a single output file
http://gwt-code-reviews.appspot.com/1424803/diff/1/user/src/com/google/gwt/user/tools/templates/sample/_srcFolder_/_moduleFolder_/_moduleShortName_.gwt.xmlsrc#newcode39
user/src/com/google/gwt/user/tools/templates/sample/_srcFolder_/_moduleFolder_/_moduleShortName_.gwt.xmlsrc:39:
By default, GWT will collapse permutations of older and less used
browsers.
s/GWT/this template/
http://gwt-code-reviews.appspot.com/1424803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Move AutoBean package to com.google.web.bindery.autobean package. (issue1414803)
Updated the patch, remembering to move the client.impl code into autobean.gwt.client.impl. http://gwt-code-reviews.appspot.com/1414803/diff/1/user/src/com/google/web/bindery/autobean/vm/AutoBeanFactorySource.java File user/src/com/google/web/bindery/autobean/vm/AutoBeanFactorySource.java (right): http://gwt-code-reviews.appspot.com/1414803/diff/1/user/src/com/google/web/bindery/autobean/vm/AutoBeanFactorySource.java#newcode32 user/src/com/google/web/bindery/autobean/vm/AutoBeanFactorySource.java:32: * AutoBeanFactoyModel. On 2011/04/15 00:51:43, rjrjr wrote: Please put this is experimental disclaimer here and in the javadoc of the other vm packages, along with the package info Done. http://gwt-code-reviews.appspot.com/1414803/diff/1/user/src/com/google/web/bindery/autobean/vm/package-info.java File user/src/com/google/web/bindery/autobean/vm/package-info.java (right): http://gwt-code-reviews.appspot.com/1414803/diff/1/user/src/com/google/web/bindery/autobean/vm/package-info.java#newcode24 user/src/com/google/web/bindery/autobean/vm/package-info.java:24: * @see com.google.web.bindery.autobean.vm.AutoBeanFactoryMagic On 2011/04/15 00:51:43, rjrjr wrote: Magic is gone, right? Might want to grep for it. Done. http://gwt-code-reviews.appspot.com/1414803/diff/1/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java File user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java (right): http://gwt-code-reviews.appspot.com/1414803/diff/1/user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java#newcode47 user/src/com/google/web/bindery/requestfactory/gwt/client/impl/AbstractRequestFactoryEditorDriver.java:47: public abstract class AbstractRequestFactoryEditorDriverR, E extends EditorR It's in a gwt subpackage? It seems weird to put a RequestFactory dependency back in the main com.google.gwt namespace. http://gwt-code-reviews.appspot.com/1414803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Makes EventBus available outside of the gwt package, in (issue1394803)
LGTM http://gwt-code-reviews.appspot.com/1394803/diff/10001/user/src/com/google/gwt/event/shared/HandlerManager.java File user/src/com/google/gwt/event/shared/HandlerManager.java (right): http://gwt-code-reviews.appspot.com/1394803/diff/10001/user/src/com/google/gwt/event/shared/HandlerManager.java#newcode49 user/src/com/google/gwt/event/shared/HandlerManager.java:49: @com.google.web.bindery.event.shared.SimpleEventBus::doRemove(Lcom/google/web/bindery/event/shared/Event$Type;Ljava/lang/Object;Ljava/lang/Object;) Can you use the JSNI wildcard syntax here? http://gwt-code-reviews.appspot.com/1394803/diff/10001/user/test/com/google/web/bindery/event/shared/BarEvent.java File user/test/com/google/web/bindery/event/shared/BarEvent.java (right): http://gwt-code-reviews.appspot.com/1394803/diff/10001/user/test/com/google/web/bindery/event/shared/BarEvent.java#newcode17 user/test/com/google/web/bindery/event/shared/BarEvent.java:17: Extra blank line. http://gwt-code-reviews.appspot.com/1394803/diff/10001/user/test/com/google/web/bindery/event/shared/EventBusTestBase.java File user/test/com/google/web/bindery/event/shared/EventBusTestBase.java (right): http://gwt-code-reviews.appspot.com/1394803/diff/10001/user/test/com/google/web/bindery/event/shared/EventBusTestBase.java#newcode19 user/test/com/google/web/bindery/event/shared/EventBusTestBase.java:19: Extra blank lines. http://gwt-code-reviews.appspot.com/1394803/diff/10001/user/test/com/google/web/bindery/event/shared/EventSharedSuite.java File user/test/com/google/web/bindery/event/shared/EventSharedSuite.java (right): http://gwt-code-reviews.appspot.com/1394803/diff/10001/user/test/com/google/web/bindery/event/shared/EventSharedSuite.java#newcode29 user/test/com/google/web/bindery/event/shared/EventSharedSuite.java:29: suite.addTestSuite(com.google.web.bindery.event.shared.ResettableEventBusTest.class); Use import statements? http://gwt-code-reviews.appspot.com/1394803/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Move AutoBean package to com.google.web.bindery.autobean package. (issue1414803)
Reviewers: rjrjr, rice, Description: Move AutoBean package to com.google.web.bindery.autobean package. http://code.google.com/p/google-web-toolkit/issues/detail?id=6253 Patch by: bobv Review by: rjrjr Please review this at http://gwt-code-reviews.appspot.com/1414803/ Affected files: M tools/api-checker/config/gwt22_23userApi.conf D user/src/com/google/gwt/autobean/AutoBean.gwt.xml D user/src/com/google/gwt/autobean/client/impl/AbstractAutoBeanFactory.java D user/src/com/google/gwt/autobean/client/impl/ClientPropertyContext.java D user/src/com/google/gwt/autobean/client/impl/JsniCreatorMap.java D user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java D user/src/com/google/gwt/autobean/rebind/AutoBeanFactoryGenerator.java D user/src/com/google/gwt/autobean/rebind/model/AutoBeanFactoryMethod.java D user/src/com/google/gwt/autobean/rebind/model/AutoBeanFactoryModel.java D user/src/com/google/gwt/autobean/rebind/model/AutoBeanMethod.java D user/src/com/google/gwt/autobean/rebind/model/AutoBeanType.java D user/src/com/google/gwt/autobean/rebind/model/JBeanMethod.java D user/src/com/google/gwt/autobean/server/AutoBeanFactoryMagic.java D user/src/com/google/gwt/autobean/server/Configuration.java D user/src/com/google/gwt/autobean/server/impl/BeanMethod.java D user/src/com/google/gwt/autobean/server/impl/BeanPropertyContext.java D user/src/com/google/gwt/autobean/server/impl/FactoryHandler.java D user/src/com/google/gwt/autobean/server/impl/GetterPropertyContext.java D user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java D user/src/com/google/gwt/autobean/server/impl/MethodPropertyContext.java D user/src/com/google/gwt/autobean/server/impl/ProxyAutoBean.java D user/src/com/google/gwt/autobean/server/impl/ShimHandler.java D user/src/com/google/gwt/autobean/server/impl/SimpleBeanHandler.java D user/src/com/google/gwt/autobean/server/impl/TypeUtils.java D user/src/com/google/gwt/autobean/server/package-info.java D user/src/com/google/gwt/autobean/shared/AutoBean.java D user/src/com/google/gwt/autobean/shared/AutoBeanCodex.java D user/src/com/google/gwt/autobean/shared/AutoBeanFactory.java D user/src/com/google/gwt/autobean/shared/AutoBeanUtils.java D user/src/com/google/gwt/autobean/shared/AutoBeanVisitor.java D user/src/com/google/gwt/autobean/shared/Splittable.java D user/src/com/google/gwt/autobean/shared/ValueCodex.java D user/src/com/google/gwt/autobean/shared/ValueCodexHelper.java D user/src/com/google/gwt/autobean/shared/impl/AbstractAutoBean.java D user/src/com/google/gwt/autobean/shared/impl/AutoBeanCodexImpl.java D user/src/com/google/gwt/autobean/shared/impl/EnumMap.java D user/src/com/google/gwt/autobean/shared/impl/HasSplittable.java D user/src/com/google/gwt/autobean/shared/impl/SplittableComplexMap.java D user/src/com/google/gwt/autobean/shared/impl/SplittableList.java D user/src/com/google/gwt/autobean/shared/impl/SplittableSet.java D user/src/com/google/gwt/autobean/shared/impl/SplittableSimpleMap.java D user/src/com/google/gwt/autobean/shared/impl/StringQuoter.java D user/src/com/google/gwt/autobean/shared/package-info.java M user/src/com/google/gwt/editor/rebind/model/ModelUtils.java A user/src/com/google/web/bindery/autobean/AutoBean.gwt.xml A user/src/com/google/web/bindery/autobean/client/impl/AbstractAutoBeanFactory.java A user/src/com/google/web/bindery/autobean/client/impl/ClientPropertyContext.java A user/src/com/google/web/bindery/autobean/client/impl/JsniCreatorMap.java A user/src/com/google/web/bindery/autobean/client/impl/JsoSplittable.java A user/src/com/google/web/bindery/autobean/rebind/AutoBeanFactoryGenerator.java A user/src/com/google/web/bindery/autobean/rebind/model/AutoBeanFactoryMethod.java A user/src/com/google/web/bindery/autobean/rebind/model/AutoBeanFactoryModel.java A user/src/com/google/web/bindery/autobean/rebind/model/AutoBeanMethod.java A user/src/com/google/web/bindery/autobean/rebind/model/AutoBeanType.java A user/src/com/google/web/bindery/autobean/rebind/model/JBeanMethod.java A user/src/com/google/web/bindery/autobean/server/impl/BeanMethod.java A user/src/com/google/web/bindery/autobean/server/impl/BeanPropertyContext.java A user/src/com/google/web/bindery/autobean/server/impl/FactoryHandler.java A user/src/com/google/web/bindery/autobean/server/impl/GetterPropertyContext.java A user/src/com/google/web/bindery/autobean/server/impl/JsonSplittable.java A user/src/com/google/web/bindery/autobean/server/impl/MethodPropertyContext.java A user/src/com/google/web/bindery/autobean/server/impl/ProxyAutoBean.java A user/src/com/google/web/bindery/autobean/server/impl/ShimHandler.java A user/src/com/google/web/bindery/autobean/server/impl/SimpleBeanHandler.java A user/src/com/google/web/bindery/autobean/server/impl/TypeUtils.java A user/src/com/google/web/bindery/autobean/shared
[gwt-contrib] Re: update EntityProxyChange javadoc (issue1414801)
LGTM http://gwt-code-reviews.appspot.com/1414801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Serialization of Final Fields in RPC (issue1380807)
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java
File
user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java#newcode617
user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java:617:
}
Extra whitespace.
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java#newcode742
user/src/com/google/gwt/user/rebind/rpc/SerializableTypeOracleBuilder.java:742:
private final Shared.SerializeFinalFieldsOptions
shouldSerializeFinalFields;
Sort fields alphabetically unless they have an initialization
dependency. While you're at it, remove the blank lines within the field
declarations.
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/src/com/google/gwt/user/rebind/rpc/Shared.java
File user/src/com/google/gwt/user/rebind/rpc/Shared.java (right):
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/src/com/google/gwt/user/rebind/rpc/Shared.java#newcode56
user/src/com/google/gwt/user/rebind/rpc/Shared.java:56: private static
SerializeFinalFieldsOptions serializeFinalFieldsValue;
Don't store this in a static field, since it's dependent on the values
of a PropertyOracle.
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java
File
user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java#newcode279
user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java:279:
static boolean isNotStaticTransientOrFinal(Field field) {
Is this method still used?
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamReader.java
File
user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamReader.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamReader.java#newcode686
user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamReader.java:686:
!Modifier.isPublic(declField.getModifiers())) ||
Modifier.isFinal(declField.getModifiers());
Double-check formatting here.
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/test/com/google/gwt/user/RPCFinalFieldsTest.gwt.xml
File user/test/com/google/gwt/user/RPCFinalFieldsTest.gwt.xml (right):
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/test/com/google/gwt/user/RPCFinalFieldsTest.gwt.xml#newcode20
user/test/com/google/gwt/user/RPCFinalFieldsTest.gwt.xml:20:
set-property name='rpc.enforceTypeVersioning' value='true' /
What is this property for?
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java
File user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java#newcode22
user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java:22:
Extra blank lines here and in other test code.
http://gwt-code-reviews.appspot.com/1380807/diff/4001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java#newcode33
user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java:33:
FinalFieldsNode node = new FinalFieldsNode();
Is there a test anywhere of final field values being sent from the
client? This test uses the default constructor, which is also called by
the server code. Instead, shouldn't this pass non-default values to the
three-arg constructor to verify that the server code properly resets
final values?
http://gwt-code-reviews.appspot.com/1380807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Convert AutoBeans to use JSOs as their backing store with lazy reification of (issue1407802)
reify := make real
I found a couple of bugs running internal test suites that have been
fixed in the latest patch with a test added to the GWT code base.
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/rebind/AutoBeanFactoryGenerator.java
File
user/src/com/google/gwt/autobean/rebind/AutoBeanFactoryGenerator.java
(right):
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/rebind/AutoBeanFactoryGenerator.java#newcode235
user/src/com/google/gwt/autobean/rebind/AutoBeanFactoryGenerator.java:235:
sw.println(%s toReturn = getOrReify(\%s\);, castType,
method.getPropertyName());
On 2011/04/06 20:32:26, rice wrote:
two spaces after =
Done.
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/server/impl/FactoryHandler.java
File user/src/com/google/gwt/autobean/server/impl/FactoryHandler.java
(right):
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/server/impl/FactoryHandler.java#newcode77
user/src/com/google/gwt/autobean/server/impl/FactoryHandler.java:77:
On 2011/04/06 20:32:26, rice wrote:
Whitespace-only change
Done.
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java
File user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java
(left):
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java#oldcode65
user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java:65: *
Private equivalent of org.json.JSONObject.getNames(JSONObject)
On 2011/04/06 20:32:26, rice wrote:
Please restore this method and remove call to JSONObject.getNames
below (in
getPropertyKeys).
Done.
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java
File user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java
(right):
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java#newcode194
user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java:194:
String[] names = JSONObject.getNames(obj);
On 2011/04/06 20:32:26, rice wrote:
Call local getNames method (for Android compatibility)
Done.
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/server/impl/ProxyAutoBean.java
File user/src/com/google/gwt/autobean/server/impl/ProxyAutoBean.java
(right):
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/server/impl/ProxyAutoBean.java#newcode180
user/src/com/google/gwt/autobean/server/impl/ProxyAutoBean.java:180:
return null;
On 2011/04/06 20:32:26, rice wrote:
Does this need a doc comment (to indicate that it's a stub
implementation)?
Done.
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/shared/AutoBeanCodex.java
File user/src/com/google/gwt/autobean/shared/AutoBeanCodex.java (right):
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/shared/AutoBeanCodex.java#newcode27
user/src/com/google/gwt/autobean/shared/AutoBeanCodex.java:27: public
class AutoBeanCodex {
On 2011/04/06 20:32:26, rice wrote:
Do you mean 'codec'? (coder/decoder)
It's a pun.
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/shared/impl/SplittableSet.java
File user/src/com/google/gwt/autobean/shared/impl/SplittableSet.java
(right):
http://gwt-code-reviews.appspot.com/1407802/diff/1/user/src/com/google/gwt/autobean/shared/impl/SplittableSet.java#newcode26
user/src/com/google/gwt/autobean/shared/impl/SplittableSet.java:26: *
This type is optimized for the read-only case.
On 2011/04/06 20:32:26, rice wrote:
Might want to note that contains() is O(n)?
Done.
http://gwt-code-reviews.appspot.com/1407802/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Convert AutoBeans to use JSOs as their backing store with lazy reification of (issue1407802)
Re-rolling the patch with changes just to JsoSplittable and
SplittableTest to fix the Safari4 test failure and improve the
robustness of the type detection code.
http://gwt-code-reviews.appspot.com/1407802/diff/6001/user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java
File user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java
(right):
http://gwt-code-reviews.appspot.com/1407802/diff/6001/user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java#newcode61
user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java:61:
On 2011/04/07 19:05:04, cromwellian wrote:
As an aside, some optimizations in the GWT compiler seem to be able to
turn
if(referenceType != null) into if(referenceType). When this occurs,
if you
have the Splittable backed by directly by a non-null unwrapped false
value (,
0, or boolean false), such checks fail. This means comparing a
JsoSplittable to
null, or sticking it into an API that might do so (e.g. collection)
can have
unpredictable results. This is fine as long as usage of the type is
strictly
controlled by you.
I ran into this with the numeric 0 value. My workaround is to always
object-ify returned values in getRaw(). There's a test for this over in
SplittableTest.
http://gwt-code-reviews.appspot.com/1407802/diff/6001/user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java#newcode70
user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java:70:
private static native Splittable create0(String object) /*-{
On 2011/04/07 19:05:04, cromwellian wrote:
You can use Object(string) to wrap and object.valueOf() to unwrap.
This avoids the case of
( == false) - true
and even more obnoxiously
Object() == false - true
since the empty string is coerced to false.
http://gwt-code-reviews.appspot.com/1407802/diff/6001/user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java#newcode87
user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java:87:
public native double asNumber() /*-{
On 2011/04/07 19:05:04, cromwellian wrote:
+this also works except false/true won't be coerced to 0/1
That's ok. It's intended that you should check isFoo() before calling
asFoo() if you don't already know the type. You'll get an exception in
DevMode since the relevant typed field will be null in JsonSplittable.
http://gwt-code-reviews.appspot.com/1407802/diff/6001/user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java#newcode155
user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java:155:
public native boolean isIndexed() /*-{
On 2011/04/07 19:05:04, cromwellian wrote:
This can fail if the json payload ever comes from the host page, e.g.
via a
JSONP request, because $wnd.Array != Array. I don't know if you intend
to ever
support that (e.g. take a JsoSplittable passed to a public API method
for
decoding) A check that works regardless can be found here:
http://perfectionkills.com/instanceof-considered-harmful-or-how-to-write-a-robust-isarray/
I can refit all of the isFoo() methods using this trick, which should
work regardless of primitive vs. object vs. cross-frame.
http://gwt-code-reviews.appspot.com/1407802/diff/6001/user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java#newcode247
user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java:247:
private native String stringifyFast() /*-{
On 2011/04/07 19:05:04, cromwellian wrote:
Caution, if the JsoSplittable ever has hashCode() invoked on it (e.g.
it is put
into a collection), a field of $H will be present and get serialized.
Added a test for this.
http://gwt-code-reviews.appspot.com/1407802/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Reformat autobeans package to cut down on diff churn for lazy reification patch. (issue1407801)
Reviewers: rice, Description: Reformat autobeans package to cut down on diff churn for lazy reification patch. Patch by: bobv Review by: rice Please review this at http://gwt-code-reviews.appspot.com/1407801/ Affected files: M user/src/com/google/gwt/autobean/client/impl/AbstractAutoBeanFactory.java M user/src/com/google/gwt/autobean/client/impl/ClientPropertyContext.java M user/src/com/google/gwt/autobean/client/impl/JsniCreatorMap.java M user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java M user/src/com/google/gwt/autobean/rebind/AutoBeanFactoryGenerator.java M user/src/com/google/gwt/autobean/server/AutoBeanFactoryMagic.java M user/src/com/google/gwt/autobean/server/Configuration.java M user/src/com/google/gwt/autobean/shared/AutoBeanCodex.java M user/src/com/google/gwt/autobean/shared/AutoBeanUtils.java M user/src/com/google/gwt/autobean/shared/AutoBeanVisitor.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Change RequestFactoryMagic - RequestFactorySource in comment (issue1406801)
LGTM http://gwt-code-reviews.appspot.com/1406801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Convert AutoBeans to use JSOs as their backing store with lazy reification of (issue1407802)
Reviewers: rice, rjrjr, Description: Convert AutoBeans to use JSOs as their backing store with lazy reification of values. Use JsonSplittable in DevMode to avoid JSNI overhead. Patch by: bobv Review by: rice, rjrjr Please review this at http://gwt-code-reviews.appspot.com/1407802/ Affected files: M dev/core/src/com/google/gwt/dev/shell/CompilingClassLoader.java M tools/api-checker/config/gwt22_23userApi.conf M user/src/com/google/gwt/autobean/client/impl/AbstractAutoBeanFactory.java M user/src/com/google/gwt/autobean/client/impl/ClientPropertyContext.java M user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java M user/src/com/google/gwt/autobean/rebind/AutoBeanFactoryGenerator.java M user/src/com/google/gwt/autobean/server/impl/BeanMethod.java M user/src/com/google/gwt/autobean/server/impl/BeanPropertyContext.java M user/src/com/google/gwt/autobean/server/impl/FactoryHandler.java M user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java M user/src/com/google/gwt/autobean/server/impl/ProxyAutoBean.java M user/src/com/google/gwt/autobean/server/impl/ShimHandler.java M user/src/com/google/gwt/autobean/server/impl/SimpleBeanHandler.java M user/src/com/google/gwt/autobean/server/impl/TypeUtils.java M user/src/com/google/gwt/autobean/shared/AutoBean.java M user/src/com/google/gwt/autobean/shared/AutoBeanCodex.java M user/src/com/google/gwt/autobean/shared/AutoBeanUtils.java M user/src/com/google/gwt/autobean/shared/Splittable.java M user/src/com/google/gwt/autobean/shared/ValueCodex.java M user/src/com/google/gwt/autobean/shared/impl/AbstractAutoBean.java A user/src/com/google/gwt/autobean/shared/impl/AutoBeanCodexImpl.java M user/src/com/google/gwt/autobean/shared/impl/EnumMap.java A user/src/com/google/gwt/autobean/shared/impl/HasSplittable.java D user/src/com/google/gwt/autobean/shared/impl/LazySplittable.java A user/src/com/google/gwt/autobean/shared/impl/SplittableComplexMap.java A user/src/com/google/gwt/autobean/shared/impl/SplittableList.java A user/src/com/google/gwt/autobean/shared/impl/SplittableSet.java A user/src/com/google/gwt/autobean/shared/impl/SplittableSimpleMap.java M user/src/com/google/gwt/autobean/shared/impl/StringQuoter.java M user/src/com/google/gwt/core/client/JsonUtils.java M user/src/com/google/web/bindery/requestfactory/shared/impl/AbstractRequestContext.java M user/src/com/google/web/bindery/requestfactory/shared/impl/EntityCodex.java M user/super/com/google/gwt/autobean/super/com/google/gwt/autobean/shared/impl/StringQuoter.java M user/test/com/google/gwt/autobean/AutoBeanSuite.java M user/test/com/google/gwt/autobean/client/AutoBeanTest.java A user/test/com/google/gwt/autobean/server/SplittableJreTest.java M user/test/com/google/gwt/autobean/shared/AutoBeanCodexTest.java A user/test/com/google/gwt/autobean/shared/SplittableTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Move RequestFactory to com.google.web.bindery.requestfactory (issue1403802)
LVGTM http://gwt-code-reviews.appspot.com/1403802/diff/4105/user/src/com/google/web/bindery/requestfactory/vm/RequestFactorySource.java File user/src/com/google/web/bindery/requestfactory/vm/RequestFactorySource.java (right): http://gwt-code-reviews.appspot.com/1403802/diff/4105/user/src/com/google/web/bindery/requestfactory/vm/RequestFactorySource.java#newcode40 user/src/com/google/web/bindery/requestfactory/vm/RequestFactorySource.java:40: * @see InProcessRequestTransport Change to fully-qualified name since they're no longer in the same package. http://gwt-code-reviews.appspot.com/1403802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Issue 6193: Fix memory-leak in WeakMapping when the value holds a reference on the key (issue1401802)
This change would break the contract of the WeakMapping, since the value object isn't guaranteed to outlive the key object. A better way to fix AutoBeans is to only use WeakMapping for non-generated bean implementations, and have the shim implement a private interface that's queried by AutoBeanUtils.getAutoBean(). -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Log error instead of throwing when a generated unit cannot be transferred to a file. (issue1357804)
[+scottb, jbrosenberg] Jason or Scott, can you take a look at this? You have been working in this area more recently than I. Please review this at http://gwt-code-reviews.appspot.com/1357804/ -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Move com.google.gwt.requestfactory to com.google.requestfactory (issue1383808)
The move looks pretty straightforward. Just some thoughts that occurred
to me while looking at this:
- RequestFactoryMagic should be moved out of the testing sub-package.
- Are we happy with the client, shared, server package naming scheme?
- Should the GWT-specific code in the new packages, like
RequestFactoryEditorDriver, be in a gwt subpackage like
com.google.requestfactory.gwt?
- If there are more Android-specific changes that need to be made, it
might be worthwhile collecting those methods in a utility class.
These are all things that can be fined-tuned later, since doing them in
this CL would complicate the branch history unnecessarily.
http://gwt-code-reviews.appspot.com/1383808/diff/1/build.xml
File build.xml (right):
http://gwt-code-reviews.appspot.com/1383808/diff/1/build.xml#newcode196
build.xml:196: !-- Build a jar file containing a subset of
requestfactory --
In general, it seems like these targets should be in their own
requestfactory/build.xml along the lines of servlet/build.xml.
http://gwt-code-reviews.appspot.com/1383808/diff/1/build.xml#newcode226
build.xml:226: requestfactory-jar target=client-src/
client+src
http://gwt-code-reviews.appspot.com/1383808/diff/1/build.xml#newcode252
build.xml:252: /classpath
It should be possible to move these classpath elements into the first
macrodef without changing anything, since the test-only code isn't
reachable from the client seed classes.
http://gwt-code-reviews.appspot.com/1383808/diff/1/build.xml#newcode259
build.xml:259: target name=requestfactory-jre-test
depends=requestfactory-test+src description=Run
RequestFactoryJreSuite
This target should be added to the main test target.
http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/gwt/autobean/server/impl/BeanMethod.java
File user/src/com/google/gwt/autobean/server/impl/BeanMethod.java
(right):
http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/gwt/autobean/server/impl/BeanMethod.java#newcode208
user/src/com/google/gwt/autobean/server/impl/BeanMethod.java:208: //
since java.beans.Introspector is not available in Android 2.2
Switch to javadoc comment.
http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java
File user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java
(right):
http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java#newcode65
user/src/com/google/gwt/autobean/server/impl/JsonSplittable.java:65: //
since that method is not available in Android 2.2
Javadoc comment.
http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/requestfactory/server/RequestFactoryJarExtractor.java
File
user/src/com/google/requestfactory/server/RequestFactoryJarExtractor.java
(right):
http://gwt-code-reviews.appspot.com/1383808/diff/1/user/src/com/google/requestfactory/server/RequestFactoryJarExtractor.java#newcode466
user/src/com/google/requestfactory/server/RequestFactoryJarExtractor.java:466:
for (Type t : Type.getArgumentTypes(desc)) {
This can be
numArgs += t.getSize()
instead.
http://gwt-code-reviews.appspot.com/1383808/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Issue 1054 (issue1380807)
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/src/com/google/gwt/user/rebind/rpc/FieldSerializerCreator.java
File user/src/com/google/gwt/user/rebind/rpc/FieldSerializerCreator.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/src/com/google/gwt/user/rebind/rpc/FieldSerializerCreator.java#newcode469
user/src/com/google/gwt/user/rebind/rpc/FieldSerializerCreator.java:469:
if (true.equals(Shared.shouldSerializeFinalFields())) {
Use a enum instead of string comparisons.
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java
File
user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java#newcode276
user/src/com/google/gwt/user/server/rpc/impl/SerializabilityUtil.java:276:
!field.getDeclaringClass().toString().contains(java.lang.Enum);
The use of toString() for any purpose other than debugging is an
anti-pattern.
!field.getDeclaringClass().equals(Enum.class)
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java
File user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java#newcode2
user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java:2: *
Copyright 2010 Google Inc.
Update copyright dates in new files.
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java#newcode35
user/test/com/google/gwt/user/client/rpc/FinalFieldsTest.java:35:
service.transfer_object(node, new AsyncCallback() {
Don't use a raw type for these callbacks.
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTestFalseNoWarn.java
File
user/test/com/google/gwt/user/client/rpc/FinalFieldsTestFalseNoWarn.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTestFalseNoWarn.java#newcode39
user/test/com/google/gwt/user/client/rpc/FinalFieldsTestFalseNoWarn.java:39:
public void onSuccess(Object result) {
Verify the values of the unserialized final fields in the object.
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTestService.java
File
user/test/com/google/gwt/user/client/rpc/FinalFieldsTestService.java
(right):
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTestService.java#newcode20
user/test/com/google/gwt/user/client/rpc/FinalFieldsTestService.java:20:
* TODO: document me.
Do so.
http://gwt-code-reviews.appspot.com/1380807/diff/1001/user/test/com/google/gwt/user/client/rpc/FinalFieldsTestService.java#newcode46
user/test/com/google/gwt/user/client/rpc/FinalFieldsTestService.java:46:
FinalFieldsNode transfer_object(FinalFieldsNode node);
Use camel-cased method names.
http://gwt-code-reviews.appspot.com/1380807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Move com.google.gwt.requestfactory to com.google.requestfactory (issue1383808)
Hollow out the to-be-deleted classes to avoid the massive duplication of code going on in this patch. The old types should extend the new types and where that isn't possible, the old type should be rewritten as a delegate. Anything in the rebind or **/impl packages can be a straight move. They're not public types. http://gwt-code-reviews.appspot.com/1383808/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] RFC: GWT-compatible protocol buffer implementation
Here's the plan: http://code.google.com/p/google-web-toolkit/wiki/ProtocolBuffers If you are using protocol buffers in your backend, what features or concerns are important to you in extending pb's into your GWT client code? -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fixing issue 5807 on server side, new issue due to rietveld problems (issue1384806)
LGTM w/ nits.
http://gwt-code-reviews.appspot.com/1384806/diff/2001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java
File
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java
(right):
http://gwt-code-reviews.appspot.com/1384806/diff/2001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java#newcode96
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java:96:
// implementations in the tests
Use a block comment for multi-line comments to allow eclipse to re-flow
the text.
http://gwt-code-reviews.appspot.com/1384806/diff/2001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java#newcode132
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java:132:
public void testInvokeInheritedMethod() {
Sort methods.
http://gwt-code-reviews.appspot.com/1384806/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Fix issue 5807 on server side. Previously reviewed at 1370803. (issue1384802)
Add the base test class to RequestFactorySuite.
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/src/com/google/gwt/requestfactory/server/ServiceLayer.java
File user/src/com/google/gwt/requestfactory/server/ServiceLayer.java
(right):
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/src/com/google/gwt/requestfactory/server/ServiceLayer.java#newcode385
user/src/com/google/gwt/requestfactory/server/ServiceLayer.java:385: }
Fix whitespace diff.
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/server/ServiceInheritanceJreTest.java
File
user/test/com/google/gwt/requestfactory/server/ServiceInheritanceJreTest.java
(right):
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/server/ServiceInheritanceJreTest.java#newcode2
user/test/com/google/gwt/requestfactory/server/ServiceInheritanceJreTest.java:2:
* Copyright 2010 Google Inc.
Copyright date.
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java
File
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java
(right):
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java#newcode2
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java:2:
* Copyright 2010 Google Inc.
Date.
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java#newcode36
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java:36:
System.out.println(clazz);
Remove the println. Also, verify the input class is what you expect.
http://gwt-code-reviews.appspot.com/1384802/diff/4001/user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java#newcode94
user/test/com/google/gwt/requestfactory/shared/ServiceInheritanceTest.java:94:
public void testInvokeMethodOnSubclass() {
Add a second test to ensure that the method in the base class can still
be invoked.
http://gwt-code-reviews.appspot.com/1384802/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Promotes Gin's AsyncProvider to GWT, along with a more general (issue1387801)
LGTM.
http://gwt-code-reviews.appspot.com/1387801/diff/5/user/src/com/google/gwt/core/client/AsyncProvider.java
File user/src/com/google/gwt/core/client/AsyncProvider.java (right):
http://gwt-code-reviews.appspot.com/1387801/diff/5/user/src/com/google/gwt/core/client/AsyncProvider.java#newcode21
user/src/com/google/gwt/core/client/AsyncProvider.java:21: * via {@link
AsyncCallback}. For example, the instance might be
via an {@link
http://gwt-code-reviews.appspot.com/1387801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Issue 6092: Inconsistent use of classloaders in RequestFactory (issue1371803)
How about http://gwt-code-reviews.appspot.com/1374804 instead to allow developer control over the classloader instead? -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
