[gwt-contrib] Re: Elemental + build scripts initial import. (issue1728806)

[cromwellian]

Thomas, I went ahead and landed this because we need to start testing
2.5 for release and this patch is unwieldly large. I will follow up with
much smaller patches that fix the various issues. :)


http://gwt-code-reviews.appspot.com/1728806/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Extracted constant strings to the constructor, that allow translation to be provided from the ou... (issue1739803)

[jat]

http://gwt-code-reviews.appspot.com/1739803/diff/8001/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
File user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
(right):

http://gwt-code-reviews.appspot.com/1739803/diff/8001/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java#newcode86
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:86: *
Constant for labeling the simple pager navigational {@link ImageButton}s
On 2012/06/14 03:38:37, jlabanca wrote:

Add a comment that the default Messages only provide English

translations.

So, if you want the users to supply their own translations, then you
need to have them pass an instance of this interface in rather than
calling GWT.create on it (that could work, but it will make integration
with their translation system much harder).

Another option would be to get this translated to all the languages GWT
supports, which Google might be willing to do, but it won't be fast (it
is possible similar strings already exist in Google products that could
be used easily).

http://gwt-code-reviews.appspot.com/1739803/diff/8001/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java#newcode705
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:705:
private static final Messages MESSAGES =
GWT.create(Messages.class);
On 2012/06/14 03:38:37, jlabanca wrote:

My GWT.create() foo is failing me.  How does one go about adding

translations

again?


You shouldn't need to do anything if you aren't actually wanting to
supply other translations, as it will fall back to the ones in the
annotations.

http://gwt-code-reviews.appspot.com/1739803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Use Node.contains in IE9/Webkit for isOrHasChild (issue1725808)

[jlabanca]

I missed the changes to the other files.  This makes the standard impl
simpler and makes all versions of IE use the fixed IE-version.  I'll
review it closely and submit tomorrow.  Thanks for the patch.

http://gwt-code-reviews.appspot.com/1725808/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Extracted constant strings to the constructor, that allow translation to be provided from the ou... (issue1739803)

[jlabanca]

http://gwt-code-reviews.appspot.com/1739803/diff/8001/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
File user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java
(right):

http://gwt-code-reviews.appspot.com/1739803/diff/8001/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java#newcode86
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:86: *
Constant for labeling the simple pager navigational {@link ImageButton}s
Add a comment that the default Messages only provide English
translations.

http://gwt-code-reviews.appspot.com/1739803/diff/8001/user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java#newcode705
user/src/com/google/gwt/user/cellview/client/CellTreeNodeView.java:705:
private static final Messages MESSAGES =
GWT.create(Messages.class);
My GWT.create() foo is failing me.  How does one go about adding
translations again?

http://gwt-code-reviews.appspot.com/1739803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Include json-1.5.jar in gwt-dev.jar. JSON is needed by the Closure (issue1732804)

[skybrian]

Reviewers: cromwellian,

Description:
Include json-1.5.jar in gwt-dev.jar. JSON is needed by the Closure
compiler and also to generate source maps.

Fixes issue 7397


Please review this at http://gwt-code-reviews.appspot.com/1732804/

Affected files:
  M dev/build.xml
  M dev/codeserver/build.xml


Index: dev/build.xml
===
--- dev/build.xml   (revision 11054)
+++ dev/build.xml   (working copy)
@@ -40,7 +40,6 @@
 location="${gwt.tools.lib}/selenium/selenium-java-client-driver.jar" />

 
 location="${gwt.tools.lib}/w3c/flute/flute-1.3-gg2.jar" />
-location="${gwt.tools}/redist/json/r2_20080312/json-1.5.jar" />
 location="${gwt.tools.lib}/hibernate/validator/hibernate-validator-4.1.0.Final.jar"  
/>
 location="${gwt.tools.lib}/javax/validation/validation-api-1.0.0.GA.jar" />
 location="${gwt.build.lib}/gwt-dev-${build.host.platform}.jar" />

@@ -113,6 +112,9 @@
   
   
   
+
+
+  
 
 
   
@@ -179,6 +181,7 @@
   src="${gwt.tools.lib}/sun/swingworker/swing-worker-1.1.jar" />
   src="${gwt.tools.lib}/guava/guava-10.0.1/guava-10.0.1-rebased.jar" />
   src="${gwt.tools.lib}/jscomp/r1649/compiler-rebased.jar" />
+  src="${gwt.tools.redist}/json/r2_20080312/json-1.5.jar" />

 
   
 
Index: dev/codeserver/build.xml
===
--- dev/codeserver/build.xml(revision 11054)
+++ dev/codeserver/build.xml(working copy)
@@ -57,11 +57,6 @@

   


-
-

--  
location="${gwt.tools}/redist/json/r2_20080312/json-1.5.jar" />

-
 location="${gwt.build.lib}/gwt-user.jar" />
 location="${gwt.root}/samples/hello/src" />


@@ -69,7 +64,6 @@
   
 
 
-
 
   
   


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Comment out an invalid test in TreeMapTest. (issue1735805)

[jat]

On 2012/06/14 01:33:21, skybrian wrote:

LGTM



It seems like we should also change the web implementation to behave

like JDK 7.

But that can wait.


Unless we are going to upgrade the rest of the JRE to match JDK 7, I
disagree.

More likely, this behavior should be considered undefined and allow
either the current or new behaviors in the test.

Still, I am fine with this as an expedient.

http://gwt-code-reviews.appspot.com/1735805/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Comment out an invalid test in TreeMapTest. (issue1735805)

[skybrian]

LGTM

It seems like we should also change the web implementation to behave
like JDK 7. But that can wait.


http://gwt-code-reviews.appspot.com/1735805/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Bundle org.json in gwt-dev and javax.validation into gwt-user, unbundle from requestfactory-* (issue1731804)

[skybrian]

I'm getting failing tests because JSON is gone from gwt-user.jar and the
requestfactory jars. I could add the dependency internal to google, but
I think we might still be trying to do too much at once - this is
looking more like churn than an actual improvement.

To fix the compiler and close issue 7397, we only need to add json (of
any version) to gwt-dev.jar. So I think I'm going to commit a tiny
change that just does that. We can defer the other stuff until we have a
better plan.


https://gwt-code-reviews.appspot.com/1731804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Comment out an invalid test in TreeMapTest. (issue1735805)

[acleung]

Reviewers: skybrian,

Description:
Comment out an invalid test in TreeMapTest.


Please review this at http://gwt-code-reviews.appspot.com/1735805/

Affected files:
  M user/test/com/google/gwt/emultest/java/util/TreeMapTest.java


Index: user/test/com/google/gwt/emultest/java/util/TreeMapTest.java
===
--- user/test/com/google/gwt/emultest/java/util/TreeMapTest.java	(revision  
11042)
+++ user/test/com/google/gwt/emultest/java/util/TreeMapTest.java	(working  
copy)

@@ -1513,6 +1513,11 @@
 // The _throwsUnsupportedOperationException version of this test will
 // verify that the method is not supported.
 if (isRemoveSupported) {
+
+  // TODO(acleung): Post JDK7, map.put(null) will actually throw a NPE.
+  // Lets disable this for now. Once we no longer test on JDK6, we can
+  // add this back and always assert an NPE.
+  /*
   Map map;
   map = createMap();
   // test remove null key with map containing a single null key
@@ -1523,6 +1528,7 @@
   } catch (NullPointerException e) {
 assertFalse(useNullKey());
   }
+  */

   map = createMap();
   // test remove null key with map containing a single non-null key


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Introduce -XfragmentCount to replace -XfragmentMerge (issue1739804)

[cromwellian]

LGTM

http://gwt-code-reviews.appspot.com/1739804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Introduce -XfragmentCount to replace -XfragmentMerge (issue1739804)

[acleung]

Reviewers: cromwellian,

Description:
Introduce -XfragmentCount to replace -XfragmentMerge


Please review this at http://gwt-code-reviews.appspot.com/1739804/

Affected files:
  M dev/core/src/com/google/gwt/dev/PrecompileTaskOptionsImpl.java
  M dev/core/src/com/google/gwt/dev/jjs/JJSOptions.java
  M dev/core/src/com/google/gwt/dev/jjs/JJSOptionsImpl.java
  M dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
  A dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerFragmentCount.java
  M dev/core/src/com/google/gwt/dev/util/arg/ArgHandlerFragmentMerge.java
  A dev/core/src/com/google/gwt/dev/util/arg/OptionFragmentCount.java
  M dev/core/src/com/google/gwt/dev/util/arg/OptionFragmentsMerge.java


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Bundle org.json in gwt-dev and javax.validation into gwt-user, unbundle from requestfactory-* (issue1731804)

[t . broyer]

On 2012/06/13 21:45:00, skybrian wrote:

Okay, seems fine for now. I'm going to commit this.


FYI, I just removed all occurrences of json-1.5.jar, so we consistently
use json.jar everywhere.

https://gwt-code-reviews.appspot.com/1731804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Elemental + build scripts initial import. (issue1728806)

[t . broyer]

On 2012/06/13 23:33:46, cromwellian wrote:

Thomas,
   I'm a little knee deep in I/O slide stuff at the moment, so maybe

you can

sanity check my thinking here. Let me describe what used to be

happening in the

Json stuff and what I was trying to change it to before 2.5, but

probably didn't

finish.



Essentially, in ProdMode I wanted to run all unboxed values. This is
theoretically possible, but can't be made to work without 2 compiler

changes:


1) String is allowed as a JSO, so cast checks to cast String to

JsonString fail

if -XdisableCastChecking is not on



2) JsonNumber and JsonBoolean, backed by primitives in prodmode, can

fail in

conditional checks against != null/null.



To fix these, I started boxing values on fetch, but kept everything

stored raw

so it could be consumed by other JSO code unchanged.



However I noticed the boxing/deboxing code produces significant bloat

from the

getters, because they are inlined and much more frequent.



So a few weeks ago I decided to migrate to a strategy of 'box on store

and stay

boxed', so Json.parse() and put/set calls box the primitives. They are

stay

boxed unless one of the coercion functions is used.



For DevMode, constructing Json* objects gives you JRE implementations

for speed.

However, because a DevMode function can still call a JSNI function

which returns

a pure interface (e.g. JsonObject) both versions can be active, and

therefore,

the code must be defensive for DevMode. I haven't tried it, but I'm

guessing

DevMode handles boxed Number and Boolean properly.


FYI, JsoSplittable in AutoBeans is unfortunately @GwtScriptOnly, which
forces you to serialize and deserialize a JSO in DevMode instead of just
casting it to a Splittable.
So having the Js implementation work in DevMode alongside the "optimized
for the JVM" Jre implementation is great!


Ultimately, for ProdMode, maybe in a 2.5.1 release, I want to modify

the

compiler to completely eliminate any boxing at all.


Unless you're going to move your internal apps to this version of
elemental, how about keeping the non-optimized "box on get" in 2.5.0,
given it'll be "fixed" in 2.5.1?
For such an experimental feature, having compatibility with JSOs
(retrieved from JSNI, or a JsonpRequestBuilder for instance) seems more
important than optimized output; so you can start to code with Elemental
in 2.5, and have it optimized with 2.5.1 then.



http://gwt-code-reviews.appspot.com/1728806/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix SimpleViolation so that ConstraintViolations with paths that don't map to any editors are st... (issue1727807)

[skybrian]

LGTM. I can understand the code now and I'm basically okay with it; the
rest is nitpicks.



http://gwt-code-reviews.appspot.com/1727807/diff/15003/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java
File user/src/com/google/gwt/editor/client/impl/SimpleViolation.java
(right):

http://gwt-code-reviews.appspot.com/1727807/diff/15003/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java#newcode134
user/src/com/google/gwt/editor/client/impl/SimpleViolation.java:134:
found = true;
Nit: you could use "break" here and get rid of the "found" variable, but
I suppose that's a matter of preference.

http://gwt-code-reviews.appspot.com/1727807/diff/15003/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java#newcode144
user/src/com/google/gwt/editor/client/impl/SimpleViolation.java:144:
throw new IllegalStateException(originalAbsolutePath);
Hmm... Thomas said to take out the message, but I don't see much point
given the amount of bloat we already have at this level. Maybe keep it
short: "No editor: " +

http://gwt-code-reviews.appspot.com/1727807/diff/15003/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java#newcode171
user/src/com/google/gwt/editor/client/impl/SimpleViolation.java:171:
boolean isOriginal = addlPath.isEmpty();
Nit: inlining isOriginal seems a bit better.

http://gwt-code-reviews.appspot.com/1727807/diff/15003/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java#newcode199
user/src/com/google/gwt/editor/client/impl/SimpleViolation.java:199: //
No EditorDelegate to attach it to, so fake the source
I don't know what you mean by "fake the source"

http://gwt-code-reviews.appspot.com/1727807/diff/15003/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java#newcode211
user/src/com/google/gwt/editor/client/impl/SimpleViolation.java:211:
private static String getParentParent(String absolutePath) {
"getParentPath"?

http://gwt-code-reviews.appspot.com/1727807/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Update POM versions to 2.5.0-rc1 (issue1734804)

[t . broyer]

LGTM

http://gwt-code-reviews.appspot.com/1734804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Elemental + build scripts initial import. (issue1728806)

[cromwellian]

Thomas,
  I'm a little knee deep in I/O slide stuff at the moment, so maybe you
can sanity check my thinking here. Let me describe what used to be
happening in the Json stuff and what I was trying to change it to before
2.5, but probably didn't finish.

Essentially, in ProdMode I wanted to run all unboxed values. This is
theoretically possible, but can't be made to work without 2 compiler
changes:

1) String is allowed as a JSO, so cast checks to cast String to
JsonString fail if -XdisableCastChecking is not on

2) JsonNumber and JsonBoolean, backed by primitives in prodmode, can
fail in conditional checks against != null/null.

To fix these, I started boxing values on fetch, but kept everything
stored raw so it could be consumed by other JSO code unchanged.

However I noticed the boxing/deboxing code produces significant bloat
from the getters, because they are inlined and much more frequent.

So a few weeks ago I decided to migrate to a strategy of 'box on store
and stay boxed', so Json.parse() and put/set calls box the primitives.
They are stay boxed unless one of the coercion functions is used.

For DevMode, constructing Json* objects gives you JRE implementations
for speed. However, because a DevMode function can still call a JSNI
function which returns a pure interface (e.g. JsonObject) both versions
can be active, and therefore, the code must be defensive for DevMode. I
haven't tried it, but I'm guessing DevMode handles boxed Number and
Boolean properly.

Ultimately, for ProdMode, maybe in a 2.5.1 release, I want to modify the
compiler to completely eliminate any boxing at all.




http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonArray.java
File elemental/src/elemental/js/json/JsJsonArray.java (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonArray.java#newcode44
elemental/src/elemental/js/json/JsJsonArray.java:44: return this[index];
On 2012/06/13 15:36:39, tbroyer wrote:

Should box() the value.
(JsJsonObject box()es its value in get(String))


Hmm, that's wrong then. I use to box on get, but it turns out to produce
inlined-box code everywhere that is bloated, so I changed it to
box-on-store in most places. I need to make sure this is consistent.  It
turns out that in large apps, fetches are more frequent than
assignments, so the code is smaller.

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonFactory.java
File elemental/src/elemental/js/json/JsJsonFactory.java (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonFactory.java#newcode70
elemental/src/elemental/js/json/JsJsonFactory.java:70: return
Object(value);

See my other comment. Based on analysis of internal apps, I decided to
opt to keep the values stored as boxed values instead of boxing on get,
since it leads to more bloated code.

If you don't box a primitive in prodmode, you end up with code like this
failing:

JsonValue x = foo.get("y");
if (x != null) {
  // do something
}

If 'x' turns out to be unboxed boolean 'false' or unboxed numeric 0,
this null check fails because the compiler optimizes the if(x != null)
to if(x)

On 2012/06/13 15:36:39, tbroyer wrote:

Is that needed in prod mode? shouldn't JsJsonValue.box() be used here?



Actually, is that needed at all, given both JsJsonArray and

JsJsonObject 'box'

their values before returning them?
All that's needed is to box() the returned value, in case you parse a
'primitive' (e.g. parse("true"))


http://gwt-code-reviews.appspot.com/1728806/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Remove unused symbols in SymbolMaps. (fixed build breakage, it was delete some symbols that are ... (issue1731805)

[cromwellian]

LGTM

http://gwt-code-reviews.appspot.com/1731805/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Bundle org.json in gwt-dev and javax.validation into gwt-user, unbundle from requestfactory-* (issue1731804)

[skybrian]

Okay, seems fine for now. I'm going to commit this.


https://gwt-code-reviews.appspot.com/1731804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Bundle org.json in gwt-dev and javax.validation into gwt-user, unbundle from requestfactory-* (issue1731804)

[Rajeev Dayal]

On Wed, Jun 13, 2012 at 5:04 PM, Brian Slesinsky wrote:

> I don't think we support Java 1.5 anymore?
> http://code.google.com/p/google-web-toolkit/issues/detail?id=6790
>
> https://groups.google.com/forum/#!msg/google-web-toolkit/fATw0rL8lSE/xbxX5Hf8ozUJ
>
> I'm totally fine with dropping support for 1.5 altogether.
>

+1. I think support was dropped in GWT 2.4.


>
> - Brian
>
> On Wed, Jun 13, 2012 at 9:47 AM,   wrote:
> > I've removed the changes concerning javax.validation, so it's still
> > distributed as a separate JAR.
> >
> > org.json is now bundled in gwt-dev, no longer bundled into
> > requestfactory-* JARs, and distributed as a separate JAR in the SDK (so
> > that requestfactory users have the choice to user gwt-servlet-deps.jar
> > with both org.json and javax.validation, or either one or both the
> > separate JARs; Android users can just pick the javax.validation JAR, as
> > org.json is provided by the platform).
> >
> > I've also updated JSON to use the original, Java-1.6-compiled, JAR, as
> > GWT now requires Java6 anyway. That way, the JAR in the SDK is named
> > json.jar
> > I've kept the json-1.5.jar in gwt-servlet-deps.jar though, in case
> > people are still using Java 1.5 on the server-side.
> >
> > That means that for those people using Java 1.5 on their server, if they
> > didn't use gwt-servlet-deps.jar but relied on the org.json bundled into
> > requestfactory-server.jar, they'll have to download the json-1.5.jar
> > (either from the GWT SVN or elsewhere), or switch to using
> > gwt-serlvet-deps.jar.
> > Given that Java 1.5 has reached EOL, I don't really mind giving them a
> > bit more work (IFF they update GWT!)
> > I'm open to reverting to json-1.5.jar though.
> >
> > https://gwt-code-reviews.appspot.com/1731804/
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Bundle org.json in gwt-dev and javax.validation into gwt-user, unbundle from requestfactory-* (issue1731804)

[Brian Slesinsky]

I don't think we support Java 1.5 anymore?
http://code.google.com/p/google-web-toolkit/issues/detail?id=6790
https://groups.google.com/forum/#!msg/google-web-toolkit/fATw0rL8lSE/xbxX5Hf8ozUJ

I'm totally fine with dropping support for 1.5 altogether.

- Brian

On Wed, Jun 13, 2012 at 9:47 AM,   wrote:
> I've removed the changes concerning javax.validation, so it's still
> distributed as a separate JAR.
>
> org.json is now bundled in gwt-dev, no longer bundled into
> requestfactory-* JARs, and distributed as a separate JAR in the SDK (so
> that requestfactory users have the choice to user gwt-servlet-deps.jar
> with both org.json and javax.validation, or either one or both the
> separate JARs; Android users can just pick the javax.validation JAR, as
> org.json is provided by the platform).
>
> I've also updated JSON to use the original, Java-1.6-compiled, JAR, as
> GWT now requires Java6 anyway. That way, the JAR in the SDK is named
> json.jar
> I've kept the json-1.5.jar in gwt-servlet-deps.jar though, in case
> people are still using Java 1.5 on the server-side.
>
> That means that for those people using Java 1.5 on their server, if they
> didn't use gwt-servlet-deps.jar but relied on the org.json bundled into
> requestfactory-server.jar, they'll have to download the json-1.5.jar
> (either from the GWT SVN or elsewhere), or switch to using
> gwt-serlvet-deps.jar.
> Given that Java 1.5 has reached EOL, I don't really mind giving them a
> bit more work (IFF they update GWT!)
> I'm open to reverting to json-1.5.jar though.
>
> https://gwt-code-reviews.appspot.com/1731804/

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Elemental + build scripts initial import. (issue1728806)

[skybrian]

I mostly looked over the build process. (Since this is experimental, I
suppose it can all be done later.)


http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/META-INF/MANIFEST.MF
File elemental/META-INF/MANIFEST.MF (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/META-INF/MANIFEST.MF#newcode1
elemental/META-INF/MANIFEST.MF:1: Manifest-Version: 1.0
Do we need this file?

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/README
File elemental/README (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/README#newcode1
elemental/README:1: Elemental is a "to the metal" raw programming API
designed to keep up with fast changing
Maybe wrap to 80 columns? (Also in STOP.EXPERIMENTAL.)

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/README
File elemental/idl/README (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/README#newcode7
elemental/idl/README:7: 4) cp -r generated/src/elemental/*
../src/elemental/
These instructions are out of date since we're generating the code now.

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/css/generate-style.py
File elemental/idl/css/generate-style.py (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/css/generate-style.py#newcode1
elemental/idl/css/generate-style.py:1: #!/usr/bin/python2.4
Google style is to leave this out. (We're not using it anyway.)

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/css/generate-style.py#newcode11
elemental/idl/css/generate-style.py:11: __author__ =
'danila...@google.com (Daniel Danilatos)'
remove for open source release?

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/docs/README
File elemental/idl/docs/README (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/docs/README#newcode1
elemental/idl/docs/README:1: W3C documentation from MDN taken from the
Dart project's MDN scraper.
This directory is empty, so I assume the README is just a placeholder to
make sure it's here for generated files. Maybe say something about that?
(Otherwise, remove.)

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/elemental/elemental.idl
File elemental/idl/elemental/elemental.idl (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/elemental/elemental.idl#newcode3
elemental/idl/elemental/elemental.idl:3: * License TBD
Fix?

Also, add a pointer to a doc explaining the syntax used here.

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/elemental/elemental.idl#newcode25
elemental/idl/elemental/elemental.idl:25: // FIXME: thread datatbase to
all the clients that need it and probably
Which database? What does this mean?

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/elemental/elemental.idl#newcode163
elemental/idl/elemental/elemental.idl:163: // TODO(dstockwell): Define
these manually.
Does this TODO make sense for elemental?

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/elemental/elemental.idl#newcode216
elemental/idl/elemental/elemental.idl:216: // TODO(vsm): Define new
names for these (see b/4436830).
Seems to be left over from Dart?

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/elemental/elemental.idl#newcode349
elemental/idl/elemental/elemental.idl:349: /* suppressed, we don't
support covariant return, just cast to the clamped interface if needed
*/
still true in Java?

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/scripts/all_tests.py
File elemental/idl/scripts/all_tests.py (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/scripts/all_tests.py#newcode1
elemental/idl/scripts/all_tests.py:1: #!/usr/bin/python
At some point we should probably make BUILD and build.xml targets to run
these tests.

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/scripts/database.py
File elemental/idl/scripts/database.py (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/scripts/database.py#newcode4
elemental/idl/scripts/database.py:4: # BSD-style license that can be
found in the LICENSE file.
I guess we need the LICENSE file here?

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/scripts/database.py#newcode26
elemental/idl/scripts/database.py:26: FremontCut syntax, which is
derived from the Web IDL syntax and includes
Context? A Google search doesn't find any results for "FremontCut
syntax" outside the Dart code base.

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/scripts/idlparser.dart
File elemental/idl/scripts/idlparser.dart (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/scripts/idlparser.dart#newcode1
elemental/idl/scripts/idlparser.dart:1: // Copyright (c) 2011, the Dart
project authors.  Please see the AUTHORS file
Do we need this file? (And other dart files.)

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/idl/s

[gwt-contrib] Re: Update POM versions to 2.5.0-rc1 (issue1734804)

[rdayal]

LGTM.

http://gwt-code-reviews.appspot.com/1734804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Use Node.contains in IE9/Webkit for isOrHasChild (issue1725808)

[stephen . haberman]

Did you performance test to see if element.contains() is
faster than the old impl code?


No, no perf tests.

The old code in DOMImplStandardBase walked the DOM, so I thought it was
a pretty safe assumption that Webkit's .contains would be faster (either
from being a single JS call vs. many or, hopefully, by using some sort
of internal optimization).

I would be pretty surprised if element.contains is slower, but can still
put together some numbers if you'd like. Would you like me to look in to
this?


At this point, the new code is more complicated than the old code.


Well, DOMImplStandardBase is simpler. :-)

I don't think DOMImplIE9 is substantially more complex; it's just
calling/reusing some code from DOMImplTrident.

Granted, I also changed DOMImplTrident's logic, but that was because I
added more tests (that more than just elements, e.g. document and text
nodes) and fixed a unreported bug.

http://gwt-code-reviews.appspot.com/1725808/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Figuring out Maven and GWT versions

[Rodrigo Chandia]

Review at http://gwt-code-reviews.appspot.com/1734804

On Wednesday, June 13, 2012 2:32:28 PM UTC-4, Thomas Broyer wrote:
>
>
> On Wednesday, June 13, 2012 8:19:52 PM UTC+2, Rodrigo Chandia wrote:
>>
>> We have a few POMs through GWT which need its versions updated for the 
>> release. The question is what to use for version string. We have no process 
>> or infrastructure in place for snapshots so I'd like to keep that out of 
>> this picture.
>>
>> I propose to use "2.5.0.RCx" while we publish release candidates so that 
>> they match the branch(es) we will produce until the 2.5.0 release. If so, I 
>> will shortly (today) put a patch for review to change all references to GWT 
>> version in the POMs to "2.5.0.RC1". Soon after we cut the RC1 release we 
>> will publish it to Sonatype.
>>
>> Rinse and repeat for every RCx and the final 2.5.0 release.
>>
>> Thoughts, concerns?
>>
>
> The version should be 2.5.0-RC1 (or rc1), with a dash. But the 
> maven/push-gwt.sh script would tell you 2.5.0.RC1 is not acceptable ;-)
>  
>
>> PD: Let's discuss separately how or whether to publish SNAPSHOT versions 
>> based on nightly green builds.
>>
>
> That would be great, but we should probably delay this discussion a few 
> weeks ;-) 
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Update POM versions to 2.5.0-rc1 (issue1734804)

[rchandia]

Reviewers: rdayal, tbroyer,

Description:
Update POM versions to 2.5.0-rc1


Please review this at http://gwt-code-reviews.appspot.com/1734804/

Affected files:
  M samples/dynatablerf/pom.xml
  M samples/expenses/pom.xml
  M samples/mobilewebapp/pom.xml
  M samples/validation/pom.xml


Index: samples/dynatablerf/pom.xml
===
--- samples/dynatablerf/pom.xml (revision 11052)
+++ samples/dynatablerf/pom.xml (working copy)
@@ -13,7 +13,7 @@

   
 
-2.4.0
+2.5.0-rc1

 
 1.6
Index: samples/expenses/pom.xml
===
--- samples/expenses/pom.xml(revision 11052)
+++ samples/expenses/pom.xml(working copy)
@@ -8,7 +8,7 @@
   0.1.0.BUILD-SNAPSHOT
   expenses
   
-2.4.0
+2.5.0-rc1
 1.1.0.RELEASE
 3.0.3.RELEASE
 1.6.1
Index: samples/mobilewebapp/pom.xml
===
--- samples/mobilewebapp/pom.xml(revision 11052)
+++ samples/mobilewebapp/pom.xml(working copy)
@@ -11,7 +11,7 @@

   
 
-2.4.0
+2.5.0-rc1

 
 1.6
Index: samples/validation/pom.xml
===
--- samples/validation/pom.xml  (revision 11052)
+++ samples/validation/pom.xml  (working copy)
@@ -11,7 +11,7 @@

   
 
-2.4.0
+2.5.0-rc1

 
 1.6


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Use Node.contains in IE9/Webkit for isOrHasChild (issue1725808)

[jlabanca]

At this point, the new code is more complicated than the old code.  Is
it worth switching anymore?  Did you performance test to see if
element.contains() is faster than the old impl code?

http://gwt-code-reviews.appspot.com/1725808/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Make CodeSplitter2 outputs a reasonable SOYC. (issue1726803)

[Alan Leung]

I forgot the mention. The only "real change" between this and the CL Ray
reviewed was that I added a no-op dependency recorder in the unit test.

On Mon, Jun 11, 2012 at 5:23 PM,  wrote:

> I don't understand this code, but I wonder if there is any way to write
> a smoke test for soyc?
>
>
I left a TODO for now.


>
> http://gwt-code-reviews.**appspot.com/1726803/
>

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Remove unused symbols in SymbolMaps. (fixed build breakage, it was delete some symbols that are ... (issue1731805)

[acleung]

Reviewers: cromwellian,

Description:
Remove unused symbols in SymbolMaps. (fixed build breakage, it was
delete some symbols that are needed to deserialize RPC calls)


Please review this at http://gwt-code-reviews.appspot.com/1731805/

Affected files:
  M dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
  A dev/core/src/com/google/gwt/dev/jjs/impl/VerifySymbolMap.java


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Figuring out Maven and GWT versions

[Thomas Broyer]

On Wednesday, June 13, 2012 8:19:52 PM UTC+2, Rodrigo Chandia wrote:
>
> We have a few POMs through GWT which need its versions updated for the 
> release. The question is what to use for version string. We have no process 
> or infrastructure in place for snapshots so I'd like to keep that out of 
> this picture.
>
> I propose to use "2.5.0.RCx" while we publish release candidates so that 
> they match the branch(es) we will produce until the 2.5.0 release. If so, I 
> will shortly (today) put a patch for review to change all references to GWT 
> version in the POMs to "2.5.0.RC1". Soon after we cut the RC1 release we 
> will publish it to Sonatype.
>
> Rinse and repeat for every RCx and the final 2.5.0 release.
>
> Thoughts, concerns?
>

The version should be 2.5.0-RC1 (or rc1), with a dash. But the 
maven/push-gwt.sh script would tell you 2.5.0.RC1 is not acceptable ;-)
 

> PD: Let's discuss separately how or whether to publish SNAPSHOT versions 
> based on nightly green builds.
>

That would be great, but we should probably delay this discussion a few 
weeks ;-) 

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Figuring out Maven and GWT versions

[Rodrigo Chandia]

We have a few POMs through GWT which need its versions updated for the 
release. The question is what to use for version string. We have no process 
or infrastructure in place for snapshots so I'd like to keep that out of 
this picture.

I propose to use "2.5.0.RCx" while we publish release candidates so that 
they match the branch(es) we will produce until the 2.5.0 release. If so, I 
will shortly (today) put a patch for review to change all references to GWT 
version in the POMs to "2.5.0.RC1". Soon after we cut the RC1 release we 
will publish it to Sonatype.

Rinse and repeat for every RCx and the final 2.5.0 release.

Thoughts, concerns?

PD: Let's discuss separately how or whether to publish SNAPSHOT versions 
based on nightly green builds.

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Revert "GWT support for Chrome Frame" (issue1713803)

[rdayal]

On 2012/06/08 19:39:13, rdayal wrote:

On 2012/05/22 16:11:49, tbroyer wrote:



LGTM.


Committed a r11045.

https://gwt-code-reviews.appspot.com/1713803/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: ExternalTextResourceGenerator is locale/machine-dependent. (issue1716804)

[rdayal]

On 2012/05/25 23:01:27, jat wrote:

LGTM


Committed as r11044.

https://gwt-code-reviews.appspot.com/1716804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: MeniItem should use ScheduledCommand instead of Command (issue1726805)

[rchandia]

Submitted as r11052

http://gwt-code-reviews.appspot.com/1726805/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Bundle org.json in gwt-dev and javax.validation into gwt-user, unbundle from requestfactory-* (issue1731804)

[t . broyer]

I've removed the changes concerning javax.validation, so it's still
distributed as a separate JAR.

org.json is now bundled in gwt-dev, no longer bundled into
requestfactory-* JARs, and distributed as a separate JAR in the SDK (so
that requestfactory users have the choice to user gwt-servlet-deps.jar
with both org.json and javax.validation, or either one or both the
separate JARs; Android users can just pick the javax.validation JAR, as
org.json is provided by the platform).

I've also updated JSON to use the original, Java-1.6-compiled, JAR, as
GWT now requires Java6 anyway. That way, the JAR in the SDK is named
json.jar
I've kept the json-1.5.jar in gwt-servlet-deps.jar though, in case
people are still using Java 1.5 on the server-side.

That means that for those people using Java 1.5 on their server, if they
didn't use gwt-servlet-deps.jar but relied on the org.json bundled into
requestfactory-server.jar, they'll have to download the json-1.5.jar
(either from the GWT SVN or elsewhere), or switch to using
gwt-serlvet-deps.jar.
Given that Java 1.5 has reached EOL, I don't really mind giving them a
bit more work (IFF they update GWT!)
I'm open to reverting to json-1.5.jar though.

https://gwt-code-reviews.appspot.com/1731804/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Use UiRenderer (Uibinder for cells) in MobileWebApp sample. (issue1733805)

[rchandia]

On 2012/06/13 16:23:11, kromanovs wrote:

Well, you can add dependency to 2.5.0-SNAPSHOT if it's available in

the maven

central repo. If it's not - then I think it's not yet a time to push

this change

out.


GWT 2.5 is in the process of being released. I rather think it is time
to update the versions in the pom files across GWT trunk. Or not?



http://gwt-code-reviews.appspot.com/1733805/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Integrate Elemental to the build and deploy as Maven artifact (issue1727808)

[t . broyer]

Reviewers: skybrian, cromwellian,


https://gwt-code-reviews.appspot.com/1727808/diff/1/build.xml
File build.xml (left):

https://gwt-code-reviews.appspot.com/1727808/diff/1/build.xml#oldcode57
build.xml:57: 
See http://gwt-code-reviews.appspot.com/1729805/diff/1/build.xml for the
rationale.

Description:
Integrate Elemental to the build and deploy as Maven artifact


Please review this at https://gwt-code-reviews.appspot.com/1727808/

Affected files:
  M build.xml
  M distro-source/build.xml
  M maven/lib-gwt.sh
  A maven/poms/gwt/gwt-elemental/pom-template.xml


Index: build.xml
diff --git a/build.xml b/build.xml
index  
ad7b34a7e3c3851d8bb87f49a69f5d142c76ff31..b6a8f516c5db3303c3a5ad26ee91377d8bda8cab  
100755

--- a/build.xml
+++ b/build.xml
@@ -38,6 +38,7 @@
 
 
 
+
 
 
 
@@ -54,7 +55,6 @@
   

 
 
-
   

   

@@ -67,6 +67,11 @@
 
   

+  

+
+
+  
+
   

 
 
@@ -110,6 +115,7 @@
 
 
 
+
 
 
 
@@ -122,6 +128,7 @@
 
 
 
+
 
 
 
@@ -134,6 +141,7 @@
 
 
 
+
 
 
 
Index: distro-source/build.xml
diff --git a/distro-source/build.xml b/distro-source/build.xml
index  
791aef689c1fa7c3d9e220b18ae149ea1ce9d648..08b1147170dbe2e870abccc1267f94c28d051a03  
100755

--- a/distro-source/build.xml
+++ b/distro-source/build.xml
@@ -28,6 +28,7 @@
   prefix="${project.distname}" />
   prefix="${project.distname}" />
   prefix="${project.distname}" />
+  prefix="${project.distname}" />

   
   prefix="${project.distname}" />


Index: maven/lib-gwt.sh
diff --git a/maven/lib-gwt.sh b/maven/lib-gwt.sh
index  
c8f54124c7beaa8da412cf00ffdfbf6464dc5e03..a3f217a299124b7a33cc2c45cab1fef210478005  
100644

--- a/maven/lib-gwt.sh
+++ b/maven/lib-gwt.sh
@@ -93,7 +93,7 @@ function maven-gwt() {
 zip -d $GWT_EXTRACT_DIR/requestfactory-${i}.jar org/json/*
   done

-  for i in dev user servlet codeserver
+  for i in dev user servlet codeserver elemental
   do
 CUR_FILE=`ls $GWT_EXTRACT_DIR/gwt-${i}.jar`

@@ -122,7 +122,7 @@ function maven-gwt() {
   # push parent poms
   maven-deploy-file $mavenRepoUrl $mavenRepoId $pomDir/gwt/pom.xml  
$pomDir/gwt/pom.xml


-  for i in dev user servlet codeserver
+  for i in dev user servlet codeserver elemental
   do
 CUR_FILE=`ls $GWT_EXTRACT_DIR/gwt-${i}.jar`
 gwtPomFile=$pomDir/gwt/gwt-$i/pom.xml
Index: maven/poms/gwt/gwt-elemental/pom-template.xml
diff --git a/maven/poms/gwt/gwt-elemental/pom-template.xml  
b/maven/poms/gwt/gwt-elemental/pom-template.xml

new file mode 100644
index  
..cf94ed362079b4e99f56751ac0fcb051225f7afd

--- /dev/null
+++ b/maven/poms/gwt/gwt-elemental/pom-template.xml
@@ -0,0 +1,23 @@
+
+http://maven.apache.org/POM/4.0.0";
+xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+xsi:schemaLocation="http://maven.apache.org/POM/4.0.0  
http://maven.apache.org/maven-v4_0_0.xsd";>

+4.0.0
+
+com.google.gwt
+gwt
+${gwtVersion}
+
+com.google.gwt
+gwt-elemental
+jar
+${gwtVersion}
+
+
+  
+com.google.gwt
+gwt-user
+${gwtVersion}
+  
+
+


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Use UiRenderer (Uibinder for cells) in MobileWebApp sample. (issue1733805)

[rchandia]

Great change, but UiRenderer is not a part of GWT 2.4.0 release while

project's

pom file depend on it. Any comments?


That'd be a bug. I guess it should be 2.5.0, but that version has not
been released yet. What is the Maven way to do this?



http://gwt-code-reviews.appspot.com/1733805/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] MeniItem should use ScheduledCommand instead of Command (issue1726805)

[rchandia]

Reviewers: rdayal,

Description:
MeniItem should use ScheduledCommand instead of Command

Repost of 1698803
Thanks Patrick!
Patch by: tucker...@gmail.com


Please review this at http://gwt-code-reviews.appspot.com/1726805/

Affected files:
  M tools/api-checker/config/gwt24_25userApi.conf
  M user/src/com/google/gwt/user/client/ui/MenuBar.java
  M user/src/com/google/gwt/user/client/ui/MenuItem.java
  M user/src/com/google/gwt/user/client/ui/SuggestBox.java
  M user/test/com/google/gwt/user/client/ui/MenuBarTest.java
  M user/test/com/google/gwt/user/client/ui/MenuItemTest.java


--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Elemental + build scripts initial import. (issue1728806)

[t . broyer]

Only looked at elemental.json.* so far.
Looks like the Js implementations haven't been used much in DevMode
(which is not really surprising given the super-source implementation of
element.json.Json, which is @GwtScriptOnly, so only an explicit 'new
JsJsonFactory()', or casting a JSO to a JsonValue, would use it),
despite trying to support it.

BTW, elemental.js.json.* isn't on the SVN.


http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonArray.java
File elemental/src/elemental/js/json/JsJsonArray.java (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonArray.java#newcode44
elemental/src/elemental/js/json/JsJsonArray.java:44: return this[index];
Should box() the value.
(JsJsonObject box()es its value in get(String))

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonFactory.java
File elemental/src/elemental/js/json/JsJsonFactory.java (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonFactory.java#newcode67
elemental/src/elemental/js/json/JsJsonFactory.java:67: if (typeof value
=== 'object') {
Is this really necessary?
Object() of an object should return the object unmodified, right?

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonFactory.java#newcode70
elemental/src/elemental/js/json/JsJsonFactory.java:70: return
Object(value);
Is that needed in prod mode? shouldn't JsJsonValue.box() be used here?

Actually, is that needed at all, given both JsJsonArray and JsJsonObject
'box' their values before returning them?
All that's needed is to box() the returned value, in case you parse a
'primitive' (e.g. parse("true"))

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonValue.java
File elemental/src/elemental/js/json/JsJsonValue.java (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonValue.java#newcode44
elemental/src/elemental/js/json/JsJsonValue.java:44: return
Object.prototype.toString.apply(@elemental.js.json.JsJsonValue::debox(Lelemental/json/JsonValue;)(obj))
=== '[object Array]';
How about Array.isArray(@...debox(...)(obj)) ?
It's supported in Safari 5+ and iOS 4+ (Android 1.5+ and Chrome 5+)

(not sure there's a need to "debox" for an array though).

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonValue.java#newcode48
elemental/src/elemental/js/json/JsJsonValue.java:48: //
TODO(cromwellian): if this moves to GWT, we may have to support more
leniency
Looks like it just moved to GWT ;-)

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonValue.java#newcode59
elemental/src/elemental/js/json/JsJsonValue.java:59:
(!!@elemental.js.json.JsJsonValue::debox(Lelemental/json/JsonValue;)(this)).valueOf();
valueOf() should be called *before* the "!!":

(!!new Boolean(true)).valueOf()  -> false
(!!new Boolean(false)).valueOf() -> false
!!(new Boolean(true).valueOf())  -> true
!!(new Boolean(false).valueOf()) -> false

Because every JS object has a valueOf() method, that should Just Work™.

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonValue.java#newcode66
elemental/src/elemental/js/json/JsJsonValue.java:66:
(+@elemental.js.json.JsJsonValue::debox(Lelemental/json/JsonValue;)(this)).valueOf();
Same here: valueOf() should be called *before* the coercion with '+'.

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/js/json/JsJsonValue.java#newcode114
elemental/src/elemental/js/json/JsJsonValue.java:114: return
@elemental.js.json.JsJsonValue::debox(Lelemental/json/JsonValue;)(this);
Why use JSNI here?

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/json/impl/JreJsonFactory.java
File elemental/src/elemental/json/impl/JreJsonFactory.java (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/json/impl/JreJsonFactory.java#newcode30
elemental/src/elemental/json/impl/JreJsonFactory.java:30: *
Implementation of JsonFactory interface using org.json library.
"using org.json library" ?! ;-)

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/json/impl/JreJsonFactory.java#newcode61
elemental/src/elemental/json/impl/JreJsonFactory.java:61: // some
clients send in (json) expecting an eval is required
I don't see this in JsJsonFactory, and JSON.parse() chokes on the '('
(tested in Chrome 21)

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/json/impl/JreJsonString.java
File elemental/src/elemental/json/impl/JreJsonString.java (right):

http://gwt-code-reviews.appspot.com/1728806/diff/1/elemental/src/elemental/json/impl/JreJsonString.java#newcode70
elemental/src/elemental/json/impl/JreJsonString.java:70: return
getObject().equals(((JreJsonValue)value).getObject());
Is it me or all JreJsonValue

Re: [gwt-contrib] tests

[Stephen Haberman]

> 
>[junit] Exception in thread "pool-1-thread-569"
> >java.lang.NullPointerException [junit]  at
> >com.google.gwt.dev.util.DiskCache.transferToStream(DiskCache.java:187)
> >[junit] at
> >
> >  com.google.gwt.dev.util.DiskCacheToken.writeObject(DiskCacheToken.java:91)
> >
> 
> At the very least, that suggests there is a bug in the DiskCache code
> that lets an NPE leak out.  Do you have enough disk space?

Sorry for not replying sooner; I have ~25gb free now (down from 50gb
free a few days ago, but I had to install some IE VMs; crap they're
huge).

I would not be surprised if something terribly degenerate happened; I
have swap turned off on my machine, so whenever my ~12gb of RAM is
gone (which it was in this case), things start failing in really odd
ways. Chrome tabs crash, applications go wonky, etc.

I might poke at it a bit more but am probably not going to worry too
much about it.

- Stephen

-- 
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix SimpleViolation so that ConstraintViolations with paths that don't map to any editors are st... (issue1727807)

[t . broyer]

Sorry for not having spotted it in the previous patch set: the new
'private static' methods are not ordered alphabetically (yes, they're in
"logical" order, but the coding style for GWT mandates alphabetical
ordering...)

Otherwise good (I'd still have splitted the for() loop with the if/else
outside, but I'll Brian decide if it's worth it)

http://gwt-code-reviews.appspot.com/1727807/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors

[gwt-contrib] Re: Fix SimpleViolation so that ConstraintViolations with paths that don't map to any editors are st... (issue1727807)

[t . broyer]

I'll leave the final word to Brian.

Some minor comments below, but otherwise looks good.


http://gwt-code-reviews.appspot.com/1727807/diff/14001/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java
File user/src/com/google/gwt/editor/client/impl/SimpleViolation.java
(right):

http://gwt-code-reviews.appspot.com/1727807/diff/14001/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java#newcode141
user/src/com/google/gwt/editor/client/impl/SimpleViolation.java:141: //
a delegate.  If for some reason it doesn't match, then
"If for some reason it doesn't match, and we enter findParent with an
empty absolutePath, it will throw an exception."

I wonder if that case should be moved out of findParent though, so it's
close to that comment.

Or we could simply inline 'findParent' here.

http://gwt-code-reviews.appspot.com/1727807/diff/14001/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java#newcode176
user/src/com/google/gwt/editor/client/impl/SimpleViolation.java:176:
originalAbsolutePath.substring(absolutePath.length());
Could we compute this only once outside the loop?
Maybe even making 2 loops:
if (originalAbsolutePath.equals(absolutePath)) {
  for (...) {
delegate.recordError(...);
  }
} else {
  String addlPath = ...;
  for (...) {
delegate.recordError(...);
  }
}

http://gwt-code-reviews.appspot.com/1727807/diff/14001/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java#newcode210
user/src/com/google/gwt/editor/client/impl/SimpleViolation.java:210:
private static String findParent(String originalAbsolutePath,
Because it doesn't really "find" anything, I'd rather name it
'getParentPath'.

http://gwt-code-reviews.appspot.com/1727807/diff/14001/user/src/com/google/gwt/editor/client/impl/SimpleViolation.java#newcode214
user/src/com/google/gwt/editor/client/impl/SimpleViolation.java:214: "no
delegates or editors found for path: " + originalAbsolutePath);
Not sure we want this message in the compiled JS, given it's something
that should really never happen.
Maybe throw an exception without message, but add an assert just before
the throw, for when assertions are enabled (DevMode / SuperDevMode, or
"debug builds"):

   assert false : "no delegates or editors found...
   throw new IllegalStateException();

Please ignore if the compiler is smart enough to prune it already.

http://gwt-code-reviews.appspot.com/1727807/

--
http://groups.google.com/group/Google-Web-Toolkit-Contributors