[gwt-contrib] Re: This change adds couple of things: (issue1251801)
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12008
File
user/src/com/google/gwt/user/server/rpc/XsrfProtectedServiceServlet.java
(right):
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12008#newcode120
user/src/com/google/gwt/user/server/rpc/XsrfProtectedServiceServlet.java:120:
private boolean isCookieValueValid(String cookieValue) {
On 2011/01/11 01:48:15, meder wrote:
On 2011/01/11 00:32:23, xtof wrote:
> I'm not too fond of how this method performs two different kinds of
functions,
> depending on how the class is configured:
> If the class is configured to use a session cookie, this method
seems to check
> that the token value is as expected, whereas if no sessionCookieName
is set,
it
> just checks for well-formedness of the cookie.
>
> Either way, I'm a bit confused as to how this works...
This method only checks if the value itself is sane code on line 107
will
compare the two values.
Got it...
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12009
File
user/src/com/google/gwt/user/server/rpc/XsrfTokenServiceServlet.java
(right):
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12009#newcode192
user/src/com/google/gwt/user/server/rpc/XsrfTokenServiceServlet.java:192:
setCookieAndExpireOldCookies(newXsrfCookie);
On 2011/01/11 01:48:15, meder wrote:
On 2011/01/11 00:37:06, xtof wrote:
> If I understand this correctly, the cookie is set both when the xsrf
token is
a
> random value, _and_ when the token is generated off of a
SessionCookie. In
the
> latter case, I don't understand why it's necessary to set the XSRF
cookie, and
> if it's not necessary I think it should be avoided.
I thought that this would make it easier for apps to work with token,
app would
have to only issue getNewXsrfToken() once and subsequently simply read
the value
from the cookie and construct XsrfToken that way.
I see. Well typically I'd think a GWT client would just call the
getNewXsrfToken rpc and hang on to the token in client state; I'm not
sure if it needs to be in a cookie. Come to think of it, if we do
provide infrastructure code that stores values in cookies, we should
make it configurable with respect to path of the cookie and 'secure'
attribute. Which in turn seems to introduce a fair bit of complication.
Plus, if a developer really wants to store the value in a cookie, they
can do so.
http://gwt-code-reviews.appspot.com/1251801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Extend GWTTestCase to support HTTP requests to remote server and moduleBaseURL mismatch (issue1043801)
patch set 4: sync to latest and convert tabs http://gwt-code-reviews.appspot.com/1043801/diff/44001/45006 File user/src/com/google/gwt/junit/JUnitShell.java (right): http://gwt-code-reviews.appspot.com/1043801/diff/44001/45006#newcode195 user/src/com/google/gwt/junit/JUnitShell.java:195: "specified host and port as a fallback. Implies -startProxy"; will upload patch set 4 to address tabs http://gwt-code-reviews.appspot.com/1043801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Extend GWTTestCase to support HTTP requests to remote server and moduleBaseURL mismatch (issue1043801)
http://gwt-code-reviews.appspot.com/1043801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: [GWT Designer] Don't cache rebind results while design time (issue1275801)
Hi Alex, Don't laugh, but everyone here in Atlanta is stuck inside their homes due to a blizzard with 15cm snowfall. Anyway, I was going to suggest that you add Jason Rosenberg (jbrosenberg) and Toby Reyelts (tobyr) as reviewers, since they are working on Dev Mode caching revamp. http://gwt-code-reviews.appspot.com/1275801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: [GWT Designer] Use per ClassLoader ModuleDef caching (issue1274801)
http://gwt-code-reviews.appspot.com/1274801/diff/1/3
File dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java (right):
http://gwt-code-reviews.appspot.com/1274801/diff/1/3#newcode145
dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java:145: ModuleDef
moduleDef = (ModuleDef) ClassLoaderLocalMap.get(key, moduleName);
I'm sure a separate context per thread makes sense for GWT Designer, but
do we want to enforce this constraint in general? I'd prefer we add an
overload instead.
http://gwt-code-reviews.appspot.com/1274801/diff/1/2
File dev/core/src/com/google/gwt/dev/util/ClassLoaderLocalMap.java
(right):
http://gwt-code-reviews.appspot.com/1274801/diff/1/2#newcode1
dev/core/src/com/google/gwt/dev/util/ClassLoaderLocalMap.java:1: package
com.google.gwt.dev.util;
missing opensource header
http://gwt-code-reviews.appspot.com/1274801/diff/1/2#newcode17
dev/core/src/com/google/gwt/dev/util/ClassLoaderLocalMap.java:17: *
Based on http://java.dzone.com/articles/classloaderlocal-how-avoid
I always use an anchor tag for hrefs so they show up nicely in online
javadoc.
http://gwt-code-reviews.appspot.com/1274801/diff/1/2#newcode38
dev/core/src/com/google/gwt/dev/util/ClassLoaderLocalMap.java:38:
private static byte[] buildHolderByteCode(String holderClassName) {
I would appreciate a method comment here.
http://gwt-code-reviews.appspot.com/1274801/diff/1/2#newcode44
dev/core/src/com/google/gwt/dev/util/ClassLoaderLocalMap.java:44: {
I think this code is separated into blocks for readability. That is
nice, but stands out as a bit unusual in the GWT code base. Would extra
whitespace would accomplish the same purpose?
http://gwt-code-reviews.appspot.com/1274801/diff/1/2#newcode75
dev/core/src/com/google/gwt/dev/util/ClassLoaderLocalMap.java:75: public
static boolean containsKey(ClassLoader cl, Object key) {
GWT style is to sort methods in the file. First all public, then
default, then protected, and finally all private methods - each sorted
alphabetically.
http://code.google.com/webtoolkit/makinggwtbetter.html#codestyle
http://gwt-code-reviews.appspot.com/1274801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Extend GWTTestCase to support HTTP requests to remote server and moduleBaseURL mismatch (issue1043801)
http://gwt-code-reviews.appspot.com/1043801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Extend GWTTestCase to support HTTP requests to remote server and moduleBaseURL mismatch (issue1043801)
will upload new patch shortly.
Added tests and a sample app, using "violator pattern" to access private
members.
http://gwt-code-reviews.appspot.com/1043801/diff/1/8
File user/src/com/google/gwt/junit/JUnitShell.java (right):
http://gwt-code-reviews.appspot.com/1043801/diff/1/8#newcode1536
user/src/com/google/gwt/junit/JUnitShell.java:1536: // TODO: figure out
if it is possible to allow multiple modules.
Modified ProxyFilter to support multiple modules.
On 2011/01/01 03:03:44, jat wrote:
On 2010/11/09 19:31:37, kjin wrote:
> I'm not sure about it -- E2E tests run for the whole application, so
(to my
> limited experience) there's only one module which contains entry
point.
> OTOH, I'm experimenting with initializing Module Under Test in test
methods,
so
> this will be addressed in next pass.
A number of internal GWT apps use multiple modules, each with their
own entry
points.
http://gwt-code-reviews.appspot.com/1043801/diff/14001/15001
File dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java (right):
http://gwt-code-reviews.appspot.com/1043801/diff/14001/15001#newcode293
dev/core/src/com/google/gwt/dev/cfg/ModuleDef.java:293: public String[]
getAllSourceFiles() {
dunno -- not in my CL.
On 2011/01/01 03:03:44, jat wrote:
What is this used for?
http://gwt-code-reviews.appspot.com/1043801/diff/14001/15006
File user/src/com/google/gwt/junit/JUnitShell.java (right):
http://gwt-code-reviews.appspot.com/1043801/diff/14001/15006#newcode151
user/src/com/google/gwt/junit/JUnitShell.java:151: protected interface
JUnitShellOptions extends HostedModeOptions, OptionProxyTo {
On 2011/01/01 03:03:44, jat wrote:
>80 chars
Done.
http://gwt-code-reviews.appspot.com/1043801/diff/14001/15006#newcode192
user/src/com/google/gwt/junit/JUnitShell.java:192: return "Proxies
non-junithost XHRs to the specified host and port. Implies -startProxy";
On 2011/01/01 03:03:44, jat wrote:
>80 chars
Done.
http://gwt-code-reviews.appspot.com/1043801/diff/14001/15006#newcode317
user/src/com/google/gwt/junit/JUnitShell.java:317: return "Starts a
proxy which replaces syntheticModuleName with moduleName as fallback";
On 2011/01/01 03:03:44, jat wrote:
This doesn't seem a terribly accurate description. Also, what exactly
does this
do if a proxy server/host wasn't specified?
Done.
http://gwt-code-reviews.appspot.com/1043801/diff/14001/15006#newcode327
user/src/com/google/gwt/junit/JUnitShell.java:327: return true;
I think this option is only for experimental users, like -batch and
-precompile. I could be convinced either way.
On 2011/01/01 03:03:44, jat wrote:
Why does this need to be excluded from -help?
http://gwt-code-reviews.appspot.com/1043801/diff/14001/15006#newcode1278
user/src/com/google/gwt/junit/JUnitShell.java:1278:
options.setProxyToPort(getPort());
modified; hope it is clearer.
The proxy intercepts requests on getPort(), but the -proxyTo host:port
is arbitrary, and the default is getHost():getPort()
On 2011/01/01 03:03:44, jat wrote:
What will this accomplish? Won't the proxy be running on getPort()?
http://gwt-code-reviews.appspot.com/1043801/diff/14001/15006#newcode1337
user/src/com/google/gwt/junit/JUnitShell.java:1337: Linker l =
module.getActivePrimaryLinker().newInstance();
On 2011/01/01 03:03:44, jat wrote:
Use a longer name.
Done.
http://gwt-code-reviews.appspot.com/1043801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [GWT Designer] Don't cache rebind results while design time (issue1275801)
Reviewers: rdayal, scottb, zundel,
Description:
Newly generated type needed every time when user modifies UiBinder-based
UI using the Designer Editor (I also set rebindCache to null while
instantiating ShellModuleSpaceHost).
Please review this at http://gwt-code-reviews.appspot.com/1275801/show
Affected files:
dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java
### Eclipse Workspace Patch 1.0
#P trunk
Index: dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java
===
--- dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java
(revision 9519)
+++ dev/core/src/com/google/gwt/dev/shell/StandardRebindOracle.java
(working copy)
@@ -32,6 +32,7 @@
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger;
import com.google.gwt.dev.util.log.speedtracer.SpeedTracerLogger.Event;
+import java.beans.Beans;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
@@ -249,8 +250,10 @@
Rebinder rebinder = new Rebinder();
resultTypeName = rebinder.rebind(logger, typeName, artifactAcceptor);
- typeNameBindingMap.put(typeName, resultTypeName);
-
+ if (!Beans.isDesignTime()) {
+// don't cache while design time
+typeNameBindingMap.put(typeName, resultTypeName);
+ }
Messages.TRACE_TOPLEVEL_REBIND_RESULT.log(logger, resultTypeName,
null);
}
return resultTypeName;
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: This change adds couple of things: (issue1251801)
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12005
File
user/src/com/google/gwt/user/server/rpc/AbstractXsrfProtectedServiceServlet.java
(right):
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12005#newcode80
user/src/com/google/gwt/user/server/rpc/AbstractXsrfProtectedServiceServlet.java:80:
!method.equals(classMethods)) {
On 2011/01/11 00:32:23, xtof wrote:
Shouldn't this be
!method.equals(classMethod))
(not classMethod*s*)?
Seems like there's a test case missing for this?
Awesome catach! There was a test
AbstractXsrfProtectedServiceServletTest:120, but it had a bug too.
Fixed.
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12008
File
user/src/com/google/gwt/user/server/rpc/XsrfProtectedServiceServlet.java
(right):
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12008#newcode39
user/src/com/google/gwt/user/server/rpc/XsrfProtectedServiceServlet.java:39:
* To prevent blind XSRF cookie overwrite by an active HTTP
man-in-the-middle in
On 2011/01/11 00:32:23, xtof wrote:
I think it would be worth stating more clearly that using the default
XSRF
cookie is vulnerable to active attacks, and is not recommended. I'm
almost
inclined to suggest that we should perhaps not even provide that
default mode.
After all, pretty much any app that worries about XSRF also uses
authentication,
and the XSRF token can be derived off of a session cookie or similar.
I am open to the idea of removing default XSRF protection and only
supporting session cookie derived tokens.
John, what do you think?
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12008#newcode120
user/src/com/google/gwt/user/server/rpc/XsrfProtectedServiceServlet.java:120:
private boolean isCookieValueValid(String cookieValue) {
On 2011/01/11 00:32:23, xtof wrote:
I'm not too fond of how this method performs two different kinds of
functions,
depending on how the class is configured:
If the class is configured to use a session cookie, this method seems
to check
that the token value is as expected, whereas if no sessionCookieName
is set, it
just checks for well-formedness of the cookie.
Either way, I'm a bit confused as to how this works...
This method only checks if the value itself is sane code on line 107
will compare the two values.
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12009
File
user/src/com/google/gwt/user/server/rpc/XsrfTokenServiceServlet.java
(right):
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12009#newcode192
user/src/com/google/gwt/user/server/rpc/XsrfTokenServiceServlet.java:192:
setCookieAndExpireOldCookies(newXsrfCookie);
On 2011/01/11 00:37:06, xtof wrote:
If I understand this correctly, the cookie is set both when the xsrf
token is a
random value, _and_ when the token is generated off of a
SessionCookie. In the
latter case, I don't understand why it's necessary to set the XSRF
cookie, and
if it's not necessary I think it should be avoided.
I thought that this would make it easier for apps to work with token,
app would have to only issue getNewXsrfToken() once and subsequently
simply read the value from the cookie and construct XsrfToken that way.
http://gwt-code-reviews.appspot.com/1251801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds some documentation on TypeOracle and JType (issue1268801)
http://gwt-code-reviews.appspot.com/1268801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds some documentation on TypeOracle and JType (issue1268801)
Updated Patch
http://gwt-code-reviews.appspot.com/1268801/diff/1/2
File dev/core/src/com/google/gwt/core/ext/typeinfo/JType.java (right):
http://gwt-code-reviews.appspot.com/1268801/diff/1/2#newcode24
dev/core/src/com/google/gwt/core/ext/typeinfo/JType.java:24: * Returns
this type with no type annotations or type variables.
On 2011/01/07 22:03:28, scottb wrote:
This seems kind of unclear.
Updated.
Honestly, I'm at a loss as to how to describe the difference between a
JRawType and getErasedType. I can see how they are different in
implementation, but it looks to me like two different developers trying
to do something similar.
http://gwt-code-reviews.appspot.com/1268801/diff/1/2#newcode29
dev/core/src/com/google/gwt/core/ext/typeinfo/JType.java:29: * Returns
the equivalent of {...@link Class#getName()} if the type were loaded.
On 2011/01/07 22:26:45, tobyr wrote:
This isn't true as far as I know. I believe this always returns the
"field
descriptor" for a type. See JVMS 4.3. I think we should javadoc it as
such.
Adding a couple of examples may not hurt: String.class =>
"Ljava/lang/String;".
boolean.class => "Z"
Done.
http://gwt-code-reviews.appspot.com/1268801/diff/1/2#newcode44
dev/core/src/com/google/gwt/core/ext/typeinfo/JType.java:44: * > Java
Language Spec, Edition 2.
On 2011/01/07 22:03:28, scottb wrote:
Seems a negative reformat.
Done.
http://gwt-code-reviews.appspot.com/1268801/diff/1/2#newcode55
dev/core/src/com/google/gwt/core/ext/typeinfo/JType.java:55: * Returns
the name of this class without the package name.
On 2011/01/07 22:03:28, scottb wrote:
Some clarification is needed for what happens to nested types.
Based on reading the code - it appears the enclosing type names are
stripped.
http://gwt-code-reviews.appspot.com/1268801/diff/1/3
File dev/core/src/com/google/gwt/dev/javac/typemodel/TypeOracle.java
(right):
http://gwt-code-reviews.appspot.com/1268801/diff/1/3#newcode50
dev/core/src/com/google/gwt/dev/javac/typemodel/TypeOracle.java:50: *
comment metadata from the source.
On 2011/01/07 22:03:28, scottb wrote:
The last part is no longer true.
Done.
http://gwt-code-reviews.appspot.com/1268801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [GWT Designer] Use per ClassLoader ModuleDef caching (issue1274801)
Reviewers: rdayal, scottb, zundel, Description: In GWT Designer we need separated class loader for every project open in Designer. Please review this at http://gwt-code-reviews.appspot.com/1274801/show Affected files: dev/core/src/com/google/gwt/dev/cfg/ModuleDefLoader.java dev/core/src/com/google/gwt/dev/util/ClassLoaderLocalMap.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: This change adds couple of things: (issue1251801)
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12009 File user/src/com/google/gwt/user/server/rpc/XsrfTokenServiceServlet.java (right): http://gwt-code-reviews.appspot.com/1251801/diff/11001/12009#newcode192 user/src/com/google/gwt/user/server/rpc/XsrfTokenServiceServlet.java:192: setCookieAndExpireOldCookies(newXsrfCookie); If I understand this correctly, the cookie is set both when the xsrf token is a random value, _and_ when the token is generated off of a SessionCookie. In the latter case, I don't understand why it's necessary to set the XSRF cookie, and if it's not necessary I think it should be avoided. http://gwt-code-reviews.appspot.com/1251801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: This change adds couple of things: (issue1251801)
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12005
File
user/src/com/google/gwt/user/server/rpc/AbstractXsrfProtectedServiceServlet.java
(right):
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12005#newcode80
user/src/com/google/gwt/user/server/rpc/AbstractXsrfProtectedServiceServlet.java:80:
!method.equals(classMethods)) {
Shouldn't this be
!method.equals(classMethod))
(not classMethod*s*)?
Seems like there's a test case missing for this?
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12008
File
user/src/com/google/gwt/user/server/rpc/XsrfProtectedServiceServlet.java
(right):
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12008#newcode39
user/src/com/google/gwt/user/server/rpc/XsrfProtectedServiceServlet.java:39:
* To prevent blind XSRF cookie overwrite by an active HTTP
man-in-the-middle in
I think it would be worth stating more clearly that using the default
XSRF cookie is vulnerable to active attacks, and is not recommended.
I'm almost inclined to suggest that we should perhaps not even provide
that default mode. After all, pretty much any app that worries about
XSRF also uses authentication, and the XSRF token can be derived off of
a session cookie or similar.
http://gwt-code-reviews.appspot.com/1251801/diff/11001/12008#newcode120
user/src/com/google/gwt/user/server/rpc/XsrfProtectedServiceServlet.java:120:
private boolean isCookieValueValid(String cookieValue) {
I'm not too fond of how this method performs two different kinds of
functions, depending on how the class is configured:
If the class is configured to use a session cookie, this method seems to
check that the token value is as expected, whereas if no
sessionCookieName is set, it just checks for well-formedness of the
cookie.
Either way, I'm a bit confused as to how this works...
http://gwt-code-reviews.appspot.com/1251801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add support for mapping ConstraintViolation objects into SimpleBeanEditor. (issue1260801)
LGTM http://gwt-code-reviews.appspot.com/1260801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add support for mapping ConstraintViolation objects into SimpleBeanEditor. (issue1260801)
Reviewers: rjrjr, Nick Chalko,
Message:
Patch updated.
The whole SimpleViolation class exists to accomodate the interim
"Violation" interface provided by RequestFactory, since
ConstraintViolation in the client was in flux when we were designing the
RF API. The Violation API needs to be deprecated and eliminated in the
future. Once that's done, SimpleViolation can also go away.
http://gwt-code-reviews.appspot.com/1260801/diff/1/3
File user/src/com/google/gwt/editor/client/SimpleBeanEditorDriver.java
(right):
http://gwt-code-reviews.appspot.com/1260801/diff/1/3#newcode25
user/src/com/google/gwt/editor/client/SimpleBeanEditorDriver.java:25: *
{...@link EditorDelegate#subscribe()}.
On 2011/01/06 17:57:10, rjrjr wrote:
Add a note that this interface is implemented by generated code, and
that
incompatible changes may be made from time to time? I should do the
same to
UiBinder
Done.
http://gwt-code-reviews.appspot.com/1260801/diff/1/3#newcode85
user/src/com/google/gwt/editor/client/SimpleBeanEditorDriver.java:85: *
JSR 303 Validator. The violations will be converted into
On 2011/01/06 17:57:10, rjrjr wrote:
Let's try to get away from the JSR verbiage. Just "probably generated
by a
{...@link javax.validation.Validator}."
Done.
http://gwt-code-reviews.appspot.com/1260801/diff/1/5
File user/src/com/google/gwt/editor/client/impl/EditorDriverUtils.java
(right):
http://gwt-code-reviews.appspot.com/1260801/diff/1/5#newcode26
user/src/com/google/gwt/editor/client/impl/EditorDriverUtils.java:26:
public class EditorDriverUtils {
On 2011/01/06 17:57:10, rjrjr wrote:
How about changing this to abstract class SimpleViolation, rather than
using the
nested interface? Util classes always give me a misc-rash.
Done.
http://gwt-code-reviews.appspot.com/1260801/diff/1/5#newcode32
user/src/com/google/gwt/editor/client/impl/EditorDriverUtils.java:32:
public static class ConstraintViolationIterable implements
On 2011/01/06 17:57:10, rjrjr wrote:
And this doesn't need to be a public type. Could have a static method
SimpleViolation#iterableFromConstraintViolations(Iterable>)
Done.
http://gwt-code-reviews.appspot.com/1260801/diff/1/9
File
user/src/com/google/gwt/requestfactory/client/testing/MockRequestFactoryEditorDriver.java
(right):
http://gwt-code-reviews.appspot.com/1260801/diff/1/9#newcode149
user/src/com/google/gwt/requestfactory/client/testing/MockRequestFactoryEditorDriver.java:149:
// TODO Auto-generated method stub
On 2011/01/06 22:31:02, Nick Chalko wrote:
Remove TODO
Done.
Description:
Add support for mapping ConstraintViolation objects into
SimpleBeanEditor.
Patch by: bobv
Review by: rjrjr
Please review this at http://gwt-code-reviews.appspot.com/1260801/show
Affected files:
M user/src/com/google/gwt/editor/Editor.gwt.xml
M user/src/com/google/gwt/editor/client/SimpleBeanEditorDriver.java
M
user/src/com/google/gwt/editor/client/impl/AbstractSimpleBeanEditorDriver.java
A user/src/com/google/gwt/editor/client/impl/SimpleViolation.java
M
user/src/com/google/gwt/editor/client/testing/MockSimpleBeanEditorDriver.java
M
user/src/com/google/gwt/requestfactory/client/RequestFactoryEditorDriver.java
M
user/src/com/google/gwt/requestfactory/client/impl/AbstractRequestFactoryEditorDriver.java
M
user/src/com/google/gwt/requestfactory/client/testing/MockRequestFactoryEditorDriver.java
M user/test/com/google/gwt/editor/rebind/model/EditorModelTest.java
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Optimizations for server-side invocations of CustomFieldSerializers. (issue1273801)
http://gwt-code-reviews.appspot.com/1273801/diff/1/2
File user/src/com/google/gwt/user/client/rpc/CustomFieldSerializer.java
(right):
http://gwt-code-reviews.appspot.com/1273801/diff/1/2#newcode19
user/src/com/google/gwt/user/client/rpc/CustomFieldSerializer.java:19: *
An interface that may be implemented by class-based custom field
serializers which will reduce
80 chars, throughout the change
http://gwt-code-reviews.appspot.com/1273801/diff/1/2#newcode37
user/src/com/google/gwt/user/client/rpc/CustomFieldSerializer.java:37: }
Why only serialize? Shouldn't there be instantiateInstance and
deserializeInstance methods as well?
http://gwt-code-reviews.appspot.com/1273801/diff/1/3
File
user/src/com/google/gwt/user/client/rpc/core/java/lang/Boolean_CustomFieldSerializer.java
(right):
http://gwt-code-reviews.appspot.com/1273801/diff/1/3#newcode44
user/src/com/google/gwt/user/client/rpc/core/java/lang/Boolean_CustomFieldSerializer.java:44:
public void serializeInstance(final SerializationStreamWriter
streamWriter,
GWT style is to only use final on locals/parameters when it is required,
such as for reference in an anonymous inner class.
http://gwt-code-reviews.appspot.com/1273801/diff/1/6
File
user/src/com/google/gwt/user/client/rpc/core/java/lang/Double_CustomFieldSerializer.java
(right):
http://gwt-code-reviews.appspot.com/1273801/diff/1/6#newcode45
user/src/com/google/gwt/user/client/rpc/core/java/lang/Double_CustomFieldSerializer.java:45:
throws SerializationException {
Continuation lines should be indented +4 columns. You might want to
look in eclipse/README.txt to setup eclipse so formatting/etc matches
our style guide.
http://gwt-code-reviews.appspot.com/1273801/diff/1/33
File
user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamWriter.java
(right):
http://gwt-code-reviews.appspot.com/1273801/diff/1/33#newcode288
user/src/com/google/gwt/user/server/rpc/impl/ServerSerializationStreamWriter.java:288:
private static class CustomFieldSerializerHandle {
What is the benefit of having this class over just storing a
CustomFieldSerializer, since it only holds a CFS instance and it must
be initialized at creation?
http://gwt-code-reviews.appspot.com/1273801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adding support for setting column widths in CellTable, and for allowing CellTable to use fixed t... (issue1263801)
committed as r9513 http://gwt-code-reviews.appspot.com/1263801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: TreeTest got upset by Safari 3 too. (issue1271801)
lgtm. http://gwt-code-reviews.appspot.com/1271801/show -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
