Re: [gwt-contrib] JConstructor redesign
On Fri, Mar 5, 2010 at 9:35 PM, Ray Cromwell cromwell...@gmail.com wrote:
I have a prototype benchmark that can load your app N times using
WebDriver, injecting a __gwtStatsEvent function which records
load/parse times, I'll see if I can get a patch of it up.
That'd be awesome, thanks!
I wonder if DeRPC will be broken by my integer seeds patch.
For sure. :/
I need to find a way to have this patch preserve the big gains (12%
pre-gzip, 2% post) that my integer seeds patch gets. It looks look it
might be possible to have something like:
function Cons(a,b,..) { // code }
injectConstructorSeed(cons, seedId);
the injectConstructorSeed() function would essentially do
injectConstructorSeed(func, seedId) {
func.prototype = defineSeed(seedId);
}
hopefully, this would be obfuscated to something like:
function Cx(a,b) { ...}
dS(Cx, 10);
The merge is going to be painful, but I think worth it.
-Ray
I *think* it should be okay. Actually, injectConstructorSeed could take a
list of arguments, so that if you have 10 live constructors, you get:
injectConstructorSeed(seedId, c1, c2, c3, ...)
We'll have to special-case FragmentExtractor to split this across multiple
fragments, but I'm quasi-familiar with that code now. :)
So... you Lex going to start reviewing? :)
Scott
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Updates the IFRame and XS selection script templates to support
Reviewers: jgw,
Description:
Updates the IFRame and XS selection script templates to support
inlined selection scripts. There are two changes involved:
1. There is a baseUrl meta property that can be used to override
the choice of base URL.
2. Meta tags can be made to apply to only module MODULENAME by putting
MODULENAME:: at the beginning of the name attribute of the meta tag.
Review by: j...@google.com
Please review this at http://gwt-code-reviews.appspot.com/159810
Affected files:
M dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js
M dev/core/src/com/google/gwt/core/linker/XSTemplate.js
Index: dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js
===
--- dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js (revision
7685)
+++ dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js (working copy)
@@ -120,6 +120,10 @@
,markerId = __gwt_marker___MODULE_NAME__
,markerScript;
+if (base = metaProps['baseUrl']) {
+ return;
+}
+
$doc.write('script id=' + markerId + '/script');
markerScript = $doc.getElementById(markerId);
@@ -189,6 +193,12 @@
var meta = metas[i], name = meta.getAttribute('name'), content;
if (name) {
+name = name.replace('__MODULE_NAME__::', '');
+if (name.indexOf('::') = 0) {
+ // It's for a different module
+ continue;
+ }
+
if (name == 'gwt:property') {
content = meta.getAttribute('content');
if (content) {
@@ -347,6 +357,7 @@
// --- STRAIGHT-LINE CODE ---
// do it early for compile/browse rebasing
+ processMetas();
computeScriptBase();
var strongName;
@@ -361,7 +372,6 @@
strongName = ;
}
- processMetas();
// --- WINDOW ONLOAD HOOK ---
Index: dev/core/src/com/google/gwt/core/linker/XSTemplate.js
===
--- dev/core/src/com/google/gwt/core/linker/XSTemplate.js (revision 7685)
+++ dev/core/src/com/google/gwt/core/linker/XSTemplate.js (working copy)
@@ -104,6 +104,10 @@
,markerId = __gwt_marker___MODULE_NAME__
,markerScript;
+if (base = metaProps['baseUrl']) {
+ return;
+}
+
$doc.write('script id=' + markerId + '/script');
markerScript = $doc.getElementById(markerId);
@@ -173,7 +177,13 @@
var meta = metas[i], name = meta.getAttribute('name'), content;
if (name) {
-if (name == 'gwt:property') {
+name = name.replace('__MODULE_NAME__::', '');
+if (name.indexOf('::') = 0) {
+ // It's for a different module
+ continue;
+}
+
+ if (name == 'gwt:property') {
content = meta.getAttribute('content');
if (content) {
var value, eq = content.indexOf('=');
@@ -287,8 +297,8 @@
}
// do it early for compile/browse rebasing
+ processMetas();
computeScriptBase();
- processMetas();
// --- WINDOW ONLOAD HOOK ---
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Updates the IFRame and XS selection script templates to support
Joel, can you review this? http://gwt-code-reviews.appspot.com/159810/diff/1/3 File dev/core/src/com/google/gwt/core/linker/XSTemplate.js (left): http://gwt-code-reviews.appspot.com/159810/diff/1/3#oldcode291 Line 291: processMetas(); This ordering is necessary because the meta props now affect the base URL. I couldn't think of any way the other ordering would be necessary. http://gwt-code-reviews.appspot.com/159810 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Updates the IFRame and XS selection script templates to support
Just idle curiosity here, but why did you have to make the same change
twice? I know you're modifying selection scripts, and maybe that
means you need to violate DRY for some reason, but it strikes me as a
potential refactoring.
Ian
On Mon, Mar 8, 2010 at 9:23 AM, sp...@google.com wrote:
Reviewers: jgw,
Description:
Updates the IFRame and XS selection script templates to support
inlined selection scripts. There are two changes involved:
1. There is a baseUrl meta property that can be used to override
the choice of base URL.
2. Meta tags can be made to apply to only module MODULENAME by putting
MODULENAME:: at the beginning of the name attribute of the meta tag.
Review by: j...@google.com
Please review this at http://gwt-code-reviews.appspot.com/159810
Affected files:
M dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js
M dev/core/src/com/google/gwt/core/linker/XSTemplate.js
Index: dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js
===
--- dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js (revision
7685)
+++ dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js (working
copy)
@@ -120,6 +120,10 @@
,markerId = __gwt_marker___MODULE_NAME__
,markerScript;
+ if (base = metaProps['baseUrl']) {
+ return;
+ }
+
$doc.write('script id=' + markerId + '/script');
markerScript = $doc.getElementById(markerId);
@@ -189,6 +193,12 @@
var meta = metas[i], name = meta.getAttribute('name'), content;
if (name) {
+ name = name.replace('__MODULE_NAME__::', '');
+ if (name.indexOf('::') = 0) {
+ // It's for a different module
+ continue;
+ }
+
if (name == 'gwt:property') {
content = meta.getAttribute('content');
if (content) {
@@ -347,6 +357,7 @@
// --- STRAIGHT-LINE CODE ---
// do it early for compile/browse rebasing
+ processMetas();
computeScriptBase();
var strongName;
@@ -361,7 +372,6 @@
strongName = ;
}
- processMetas();
// --- WINDOW ONLOAD HOOK ---
Index: dev/core/src/com/google/gwt/core/linker/XSTemplate.js
===
--- dev/core/src/com/google/gwt/core/linker/XSTemplate.js (revision
7685)
+++ dev/core/src/com/google/gwt/core/linker/XSTemplate.js (working
copy)
@@ -104,6 +104,10 @@
,markerId = __gwt_marker___MODULE_NAME__
,markerScript;
+ if (base = metaProps['baseUrl']) {
+ return;
+ }
+
$doc.write('script id=' + markerId + '/script');
markerScript = $doc.getElementById(markerId);
@@ -173,7 +177,13 @@
var meta = metas[i], name = meta.getAttribute('name'), content;
if (name) {
- if (name == 'gwt:property') {
+ name = name.replace('__MODULE_NAME__::', '');
+ if (name.indexOf('::') = 0) {
+ // It's for a different module
+ continue;
+ }
+
+ if (name == 'gwt:property') {
content = meta.getAttribute('content');
if (content) {
var value, eq = content.indexOf('=');
@@ -287,8 +297,8 @@
}
// do it early for compile/browse rebasing
+ processMetas();
computeScriptBase();
- processMetas();
// --- WINDOW ONLOAD HOOK ---
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: JConstructor redesign
Mostly LGTM, except for the concern about JNewInstance.isEmpty(). It
looks like JNewInstance.isEmpty() should be computed within
RemoveEmptySuperCalls to be efficient, or for that matter, to reliably
terminate.
There are also some nits.
http://gwt-code-reviews.appspot.com/159803/diff/1/3
File dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java (right):
http://gwt-code-reviews.appspot.com/159803/diff/1/3#newcode31
Line 31: private boolean isEmpty = false;
Nice, that will be handy.
http://gwt-code-reviews.appspot.com/159803/diff/1/3#newcode38
Line 38: public JConstructor(SourceInfo info, JClassType enclosingType,
You probably meant this to be default access.
http://gwt-code-reviews.appspot.com/159803/diff/1/3#newcode64
Line 64: public boolean isEmpty() {
It's worth a comment that this method is currently expensive. It's also
worth a TODO to make it less expensive by only recomputing this at
places it could have possibly changed.
http://gwt-code-reviews.appspot.com/159803/diff/1/3#newcode80
Line 80: if (expr instanceof JMethodCall) {
Check for JNewInstance instead of JMethodCall? Then testing instanceof
JConstructor shouldn't be needed.
http://gwt-code-reviews.appspot.com/159803/diff/1/3#newcode84
Line 84: return isEmpty = ((JConstructor) target).isEmpty();
Can't this loop, if two constructors call each other? Maybe we need to
implement the TODO already, and move the computation to be done, say,
after pruning. It's easy to deal efficiently with cycles in the call
graph if we do (yet another :( ) global walk over all constructor
bodies.
It could also be rolled into your follow-up patch dealing with empty
constructors, if it starts looking like too much work.
http://gwt-code-reviews.appspot.com/159803/diff/1/4
File dev/core/src/com/google/gwt/dev/jjs/ast/JGwtCreate.java (right):
http://gwt-code-reviews.appspot.com/159803/diff/1/4#newcode38
Line 38: if (ctor instanceof JConstructor) {
That's ever so much nicer a way to check for constructor calls.
http://gwt-code-reviews.appspot.com/159803/diff/1/7
File dev/core/src/com/google/gwt/dev/jjs/ast/JNewInstance.java (right):
http://gwt-code-reviews.appspot.com/159803/diff/1/7#newcode65
Line 65: public boolean hasClinit() {
Looks great.
http://gwt-code-reviews.appspot.com/159803/diff/1/8
File dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java (right):
http://gwt-code-reviews.appspot.com/159803/diff/1/8#newcode993
Line 993: true, false);
Makes sense. Perhaps we should make this a real method that does
something and returns null. It seems like each non-trivial patch needs
a new special case for the null method and/or the null field.
http://gwt-code-reviews.appspot.com/159803/diff/1/25
File dev/core/src/com/google/gwt/dev/jjs/impl/RemoveEmptySuperCalls.java
(right):
http://gwt-code-reviews.appspot.com/159803/diff/1/25#newcode50
Line 50: ctx.replaceMe(multi.makeStatement());
It's not something that needs to happen now, but this three-way branch
would work really well as a method in Simplifier. The type signature
would be Simplifier.multi(ListJExpression exprs); . We keep
rewriting this logic because we don't want to have to wait until
DeadCodeElimination swings around to do the optimization.
http://gwt-code-reviews.appspot.com/159803
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] RunAsync code size improvements
Reviewers: Lex,
Message:
Here's an example of what the new loader output looks like:
package com.google.gwt.lang.asyncloaders;
import com.google.gwt.core.client.GWT;
import com.google.gwt.core.client.RunAsyncCallback;
import com.google.gwt.core.client.impl.AsyncFragmentLoader;
public class AsyncLoader1 {
// Callbacks that are pending
private static AsyncLoader1__Callback callbacksHead = null;
// The tail of the callbacks list
private static AsyncLoader1__Callback callbacksTail = null;
// A callback caller for this entry point
private static AsyncLoader1 instance = null;
public static void onLoad() {
instance = new AsyncLoader1();
AsyncFragmentLoader.BROWSER_LOADER.fragmentHasLoaded(1);
AsyncFragmentLoader.BROWSER_LOADER.logEventProgress(runCallbacks1,
begin);
instance.runCallbacks();
AsyncFragmentLoader.BROWSER_LOADER.logEventProgress(runCallbacks1,
end);
}
public static void runAsync(RunAsyncCallback callback) {
AsyncLoader1__Callback newCallback = new AsyncLoader1__Callback();
newCallback.callback = callback;
if (callbacksTail != null) {
callbacksTail.next = newCallback;
}
callbacksTail = newCallback;
if (callbacksHead == null) {
callbacksHead = newCallback;
}
if (instance != null) {
instance.runCallbacks();
return;
}
if (!AsyncFragmentLoader.BROWSER_LOADER.isLoading(1)) {
AsyncFragmentLoader.BROWSER_LOADER.inject(1,
new AsyncFragmentLoader.LoadErrorHandler() {
public void loadFailed(Throwable reason) {
runCallbackOnFailures(reason);
}
});
}
}
private static void runCallbackOnFailures(Throwable e) {
while (callbacksHead != null) {
callbacksHead.callback.onFailure(e);
callbacksHead = callbacksHead.next;
}
callbacksTail = null;
}
public void runCallbacks() {
while (callbacksHead != null) {
GWT.UncaughtExceptionHandler handler =
GWT.getUncaughtExceptionHandler();
AsyncLoader1__Callback next = callbacksHead;
callbacksHead = callbacksHead.next;
if (callbacksHead == null) {
callbacksTail = null;
}
if (handler == null) {
next.callback.onSuccess();
} else {
try {
next.callback.onSuccess();
} catch (Throwable e) {
handler.onUncaughtException(e);
}
}
}
}
}
http://gwt-code-reviews.appspot.com/159811/diff/1/2
File dev/core/src/com/google/gwt/dev/jjs/impl/FragmentLoaderCreator.java
(left):
http://gwt-code-reviews.appspot.com/159811/diff/1/2#oldcode114
Line 114: private void generateOnErrorMethod(PrintWriter srcWriter) {
I pulled this method because I couldn't see any callers. Was this still
needed?
http://gwt-code-reviews.appspot.com/159811/diff/1/4
File user/src/com/google/gwt/core/client/prefetch/Prefetcher.java
(right):
http://gwt-code-reviews.appspot.com/159811/diff/1/4#newcode33
Line 33: public static void prefetch(Collection? extends
PrefetchableResource resources) {
Wasn't sure about this change, but I thought I'd suggest it.
Technically, since the code below the GWT.isScript() is JS only (where
there's no range checking), we don't have to know the correct size up
front.
Description:
A few runAsync-related code size improvements.
1) Removes the clinit, AsyncLoader__Supers, and loading fields from
generated AsyncLoaders. PRETTY mode Showcase's initial fragment drops
from 500k to 480k. Probably less impressive in OBF. A tiny improvement
within the split fragments, too.
2) Removes the last couple uses of JRE collections from
AsyncFragmentLoader in favor of arrays.
3) Removes a missed logEventProgress Integer - int conversion that was
begun in an earlier commit.
Please review this at http://gwt-code-reviews.appspot.com/159811
Affected files:
M dev/core/src/com/google/gwt/dev/jjs/impl/FragmentLoaderCreator.java
M user/src/com/google/gwt/core/client/impl/AsyncFragmentLoader.java
M user/src/com/google/gwt/core/client/prefetch/Prefetcher.java
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: JConstructor redesign
http://gwt-code-reviews.appspot.com/159803/diff/1/3
File dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java (right):
http://gwt-code-reviews.appspot.com/159803/diff/1/3#newcode64
Line 64: public boolean isEmpty() {
Good call. Adding a method comment.
http://gwt-code-reviews.appspot.com/159803/diff/1/3#newcode76
Line 76: JStatement stmt = statements.get(0);
I'm adding a comment here:
// Only one statement. Check to see if it's an empty super() or this()
call.
http://gwt-code-reviews.appspot.com/159803/diff/1/3#newcode80
Line 80: if (expr instanceof JMethodCall) {
Actually, this code really needed extra commenting. What we're actually
looking for here is a super() or this() call, which is not a
JNewInstance. In fact, I'm updating this test to read:
if (expr instanceof JMethodCall !(expr instanceof JNewInstance)) {
http://gwt-code-reviews.appspot.com/159803/diff/1/3#newcode84
Line 84: return isEmpty = ((JConstructor) target).isEmpty();
Again, sorry for the confusion. Since this only applies to this/super
constructor chaining, it will definitely terminate relatively fast.
http://gwt-code-reviews.appspot.com/159803/diff/1/8
File dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java (right):
http://gwt-code-reviews.appspot.com/159803/diff/1/8#newcode993
Line 993: true, false);
Heh, I thought once about making a NullHolder class that had a
Java-declared nullMethod/nullField that were indexed. Only problem was,
it would end up getting visited by all the visitors and Bad Thing would
happen. So I gave up at the time.
http://gwt-code-reviews.appspot.com/159803/diff/1/25
File dev/core/src/com/google/gwt/dev/jjs/impl/RemoveEmptySuperCalls.java
(right):
http://gwt-code-reviews.appspot.com/159803/diff/1/25#newcode50
Line 50: ctx.replaceMe(multi.makeStatement());
Good idea. Lemme do this in a follow-on patch, though, because I
actually want to propose that Simplifier have a completely static API.
The only reason it's instance currently is for legacy reasons, accessing
primitive types through JProgram (which isn't needed anymore).
http://gwt-code-reviews.appspot.com/159803
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Updates the IFRame and XS selection script templates to support
LGTM http://gwt-code-reviews.appspot.com/159810/diff/1/2 File dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js (right): http://gwt-code-reviews.appspot.com/159810/diff/1/2#newcode200 Line 200: } Clever. I like it. http://gwt-code-reviews.appspot.com/159810/diff/1/3 File dev/core/src/com/google/gwt/core/linker/XSTemplate.js (left): http://gwt-code-reviews.appspot.com/159810/diff/1/3#oldcode291 Line 291: processMetas(); On 2010/03/08 17:25:52, Lex wrote: This ordering is necessary because the meta props now affect the base URL. I couldn't think of any way the other ordering would be necessary. I'm pretty sure this reordering is fine. http://gwt-code-reviews.appspot.com/159810 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Updates the IFRame and XS selection script templates to support
@Ian: I know, it's nasty. These things have gotten a bit out of hand, and we
have a big, fat TODO to come back and clean this up (as well as to bring
together various other linkers under a more coherent umbrella). But we
probably won't be able to get around to it for at least another quarter,
unfortunately.
On Mon, Mar 8, 2010 at 12:37 PM, Ian Petersen ispet...@gmail.com wrote:
Just idle curiosity here, but why did you have to make the same change
twice? I know you're modifying selection scripts, and maybe that
means you need to violate DRY for some reason, but it strikes me as a
potential refactoring.
Ian
On Mon, Mar 8, 2010 at 9:23 AM, sp...@google.com wrote:
Reviewers: jgw,
Description:
Updates the IFRame and XS selection script templates to support
inlined selection scripts. There are two changes involved:
1. There is a baseUrl meta property that can be used to override
the choice of base URL.
2. Meta tags can be made to apply to only module MODULENAME by putting
MODULENAME:: at the beginning of the name attribute of the meta tag.
Review by: j...@google.com
Please review this at http://gwt-code-reviews.appspot.com/159810
Affected files:
M dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js
M dev/core/src/com/google/gwt/core/linker/XSTemplate.js
Index: dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js
===
--- dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js (revision
7685)
+++ dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js (working
copy)
@@ -120,6 +120,10 @@
,markerId = __gwt_marker___MODULE_NAME__
,markerScript;
+if (base = metaProps['baseUrl']) {
+ return;
+}
+
$doc.write('script id=' + markerId + '/script');
markerScript = $doc.getElementById(markerId);
@@ -189,6 +193,12 @@
var meta = metas[i], name = meta.getAttribute('name'), content;
if (name) {
+name = name.replace('__MODULE_NAME__::', '');
+if (name.indexOf('::') = 0) {
+ // It's for a different module
+ continue;
+ }
+
if (name == 'gwt:property') {
content = meta.getAttribute('content');
if (content) {
@@ -347,6 +357,7 @@
// --- STRAIGHT-LINE CODE ---
// do it early for compile/browse rebasing
+ processMetas();
computeScriptBase();
var strongName;
@@ -361,7 +372,6 @@
strongName = ;
}
- processMetas();
// --- WINDOW ONLOAD HOOK ---
Index: dev/core/src/com/google/gwt/core/linker/XSTemplate.js
===
--- dev/core/src/com/google/gwt/core/linker/XSTemplate.js (revision
7685)
+++ dev/core/src/com/google/gwt/core/linker/XSTemplate.js (working
copy)
@@ -104,6 +104,10 @@
,markerId = __gwt_marker___MODULE_NAME__
,markerScript;
+if (base = metaProps['baseUrl']) {
+ return;
+}
+
$doc.write('script id=' + markerId + '/script');
markerScript = $doc.getElementById(markerId);
@@ -173,7 +177,13 @@
var meta = metas[i], name = meta.getAttribute('name'), content;
if (name) {
-if (name == 'gwt:property') {
+name = name.replace('__MODULE_NAME__::', '');
+if (name.indexOf('::') = 0) {
+ // It's for a different module
+ continue;
+}
+
+ if (name == 'gwt:property') {
content = meta.getAttribute('content');
if (content) {
var value, eq = content.indexOf('=');
@@ -287,8 +297,8 @@
}
// do it early for compile/browse rebasing
+ processMetas();
computeScriptBase();
- processMetas();
// --- WINDOW ONLOAD HOOK ---
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Remove caching of style sheets in IE6 (cache may be invalid when there are multiple modules)
I added a new patch that caches the lengths but not the actual style
sheet contents. In the worst case, if the lengths are wrong we will
just append in a suboptimal way, but not lose any data. Benchmarking
shows that the performance difference is minor -- still just 2-3 ms per
injection in my particular benchmark.
On 2010/03/05 17:19:51, jlabanca wrote:
http://gwt-code-reviews.appspot.com/159804/diff/1/3
File user/src/com/google/gwt/dom/client/StyleInjector.java (right):
http://gwt-code-reviews.appspot.com/159804/diff/1/3#newcode101
Line 101: private static native int getDocumentStyleSheetLength(int
idx) /*-{
I don't know for sure if reading cssText.length is slow, but I'm
worried that we
had a reason to cache the length originally. I agree that there is no
way to
cache the info because we can't control what happens outside of GWT,
so we
wouldn't even know if external code modified the style sheets.
Can you just try injecting 40 large style sheets and compare the
performance
before and after the change? At the very least, we should warn people
if its
going to get really slow.
http://gwt-code-reviews.appspot.com/159804
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Changes for crawling:
Reviewers: amitmanjhi, Description: Changes for crawling: - CrawlableHyperlink - client-side changes to Showcase sample Please review this at http://gwt-code-reviews.appspot.com/161801 Affected files: M samples/showcase/src/com/google/gwt/sample/showcase/client/Showcase.java M samples/showcase/war/Showcase.html A user/src/com/google/gwt/user/client/ui/CrawlableHyperlink.java A user/test/com/google/gwt/user/client/ui/CrawlableHyperlinkTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] supporting java.io.InputStream/Reader in GWT's JRE
Please don't laugh (at least not out loud). I've run into a situation where I really need to use a JavaCC generated parser on the client-side. The generated code is all simple Java, except for its use of java.io.InputStream and java.io.Reader. Is it possible to implement additional GWT JRE objects as maybe a GWT module, or through various .gwt.xml directives? Basically, could support for those base classes be added without modifying the GWT source directly? If yes, is there a starting documentation page somewhere on the intertubes? ISTM that translating java.io.InputStream would be stupid simple, and then I could roll my own lame implementation that uses a java.lang.String as the backing store. In my case, I just need to parse user-typed strings. I've got no need to tie the InputStream to an HTTP response, for example. java.io.Reader might be a bit more difficult since it references stuff in java.nio, but I could probably work around that (even if I have to manually/programmatically hack the JavaCC generated parser source at build time). Anyways, any advice on where to begin implementing an additional GWT JRE object will be greatly appreciated. eric -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] Updates the IFRame and XS selection script templates to support
On Mon, Mar 8, 2010 at 12:37 PM, Ian Petersen ispet...@gmail.com wrote: Just idle curiosity here, but why did you have to make the same change twice? I know you're modifying selection scripts, and maybe that means you need to violate DRY for some reason, but it strikes me as a potential refactoring. Agreed. It just grew like that over time. To address it, I was thinking that perhaps we could pull that code out into separate files, and then use replaceWith calls to insert the code, the same way the linkers currently fill in __MODULENAME__. -Lex -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] supporting java.io.InputStream/Reader in GWT's JRE
Look at the user/super/com/google/gwt/emul source for the GWT JRE emulation, e.g. http://code.google.com/p/google-web-toolkit/source/browse/trunk/user/super/com/google/gwt/emul/Emulation.gwt.xml super-source/ is what you want. It allows Web mode and Hosted Mode to see different source code. -Ray On Mon, Mar 8, 2010 at 12:54 PM, Eric B. Ridge eeb...@gmail.com wrote: Please don't laugh (at least not out loud). I've run into a situation where I really need to use a JavaCC generated parser on the client-side. The generated code is all simple Java, except for its use of java.io.InputStream and java.io.Reader. Is it possible to implement additional GWT JRE objects as maybe a GWT module, or through various .gwt.xml directives? Basically, could support for those base classes be added without modifying the GWT source directly? If yes, is there a starting documentation page somewhere on the intertubes? ISTM that translating java.io.InputStream would be stupid simple, and then I could roll my own lame implementation that uses a java.lang.String as the backing store. In my case, I just need to parse user-typed strings. I've got no need to tie the InputStream to an HTTP response, for example. java.io.Reader might be a bit more difficult since it references stuff in java.nio, but I could probably work around that (even if I have to manually/programmatically hack the JavaCC generated parser source at build time). Anyways, any advice on where to begin implementing an additional GWT JRE object will be greatly appreciated. eric -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: JConstructor redesign
http://gwt-code-reviews.appspot.com/159803/diff/1/3
File dev/core/src/com/google/gwt/dev/jjs/ast/JConstructor.java (right):
http://gwt-code-reviews.appspot.com/159803/diff/1/3#newcode80
Line 80: if (expr instanceof JMethodCall) {
I see. It looks good now.
http://gwt-code-reviews.appspot.com/159803/diff/1/25
File dev/core/src/com/google/gwt/dev/jjs/impl/RemoveEmptySuperCalls.java
(right):
http://gwt-code-reviews.appspot.com/159803/diff/1/25#newcode50
Line 50: ctx.replaceMe(multi.makeStatement());
That dependency will surely come right back. For example, it's an
oversight that Simplifier.cast does not check whether the cast is
trivial. More broadly, Simplifier should be able to construct any kind
of expression or statement node. This very patch includes a new node
type that can only be created via a JProgram object.
http://gwt-code-reviews.appspot.com/159803
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Give a better error message when RunAsyncCode.runAsyncCode is passed something
Reviewers: cromwellian, Description: Give a better error message when RunAsyncCode.runAsyncCode is passed something other than a class literal. Please review this at http://gwt-code-reviews.appspot.com/162801 Affected files: M dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java M dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r7686 committed - Updates the IFRame and XS selection script templates to support...
Revision: 7686
Author: sp...@google.com
Date: Mon Mar 8 12:02:36 2010
Log: Updates the IFRame and XS selection script templates to support
inlined selection scripts. There are two changes involved:
1. There is a baseUrl meta property that can be used to override
the choice of base URL.
2. Meta tags can be made to apply to only module MODULENAME by putting
MODULENAME:: at the beginning of the name attribute of the meta tag.
http://groups.google.com/group/google-web-toolkit-contributors/browse_thread/thread/f86a8e085457a7b5/1449181b1952c35a?lnk=raot
http://gwt-code-reviews.appspot.com/159810
Review by: j...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=7686
Modified:
/trunk/dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js
/trunk/dev/core/src/com/google/gwt/core/linker/XSTemplate.js
===
--- /trunk/dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js Wed
Oct 21 13:20:55 2009
+++ /trunk/dev/core/src/com/google/gwt/core/linker/IFrameTemplate.js Mon
Mar 8 12:02:36 2010
@@ -120,6 +120,10 @@
,markerId = __gwt_marker___MODULE_NAME__
,markerScript;
+if (base = metaProps['baseUrl']) {
+ return;
+}
+
$doc.write('script id=' + markerId + '/script');
markerScript = $doc.getElementById(markerId);
@@ -189,6 +193,12 @@
var meta = metas[i], name = meta.getAttribute('name'), content;
if (name) {
+name = name.replace('__MODULE_NAME__::', '');
+if (name.indexOf('::') = 0) {
+ // It's for a different module
+ continue;
+ }
+
if (name == 'gwt:property') {
content = meta.getAttribute('content');
if (content) {
@@ -347,6 +357,7 @@
// --- STRAIGHT-LINE CODE ---
// do it early for compile/browse rebasing
+ processMetas();
computeScriptBase();
var strongName;
@@ -361,7 +372,6 @@
strongName = ;
}
- processMetas();
// --- WINDOW ONLOAD HOOK ---
===
--- /trunk/dev/core/src/com/google/gwt/core/linker/XSTemplate.js Wed Oct 21
13:20:55 2009
+++ /trunk/dev/core/src/com/google/gwt/core/linker/XSTemplate.js Mon Mar 8
12:02:36 2010
@@ -104,6 +104,10 @@
,markerId = __gwt_marker___MODULE_NAME__
,markerScript;
+if (base = metaProps['baseUrl']) {
+ return;
+}
+
$doc.write('script id=' + markerId + '/script');
markerScript = $doc.getElementById(markerId);
@@ -173,7 +177,13 @@
var meta = metas[i], name = meta.getAttribute('name'), content;
if (name) {
-if (name == 'gwt:property') {
+name = name.replace('__MODULE_NAME__::', '');
+if (name.indexOf('::') = 0) {
+ // It's for a different module
+ continue;
+}
+
+ if (name == 'gwt:property') {
content = meta.getAttribute('content');
if (content) {
var value, eq = content.indexOf('=');
@@ -287,8 +297,8 @@
}
// do it early for compile/browse rebasing
- computeScriptBase();
processMetas();
+ computeScriptBase();
// --- WINDOW ONLOAD HOOK ---
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Give a better error message when RunAsyncCode.runAsyncCode is passed something
I left out a file. Creating a new issue. http://gwt-code-reviews.appspot.com/162801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Give a better error message when RunAsyncCode.runAsyncCode is passed something
Reviewers: cromwellian, Description: Give a better error message when RunAsyncCode.runAsyncCode is passed something other than a class literal. Please review this at http://gwt-code-reviews.appspot.com/161802 Affected files: M dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java M dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java A dev/core/test/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncsErrorMessagesTest.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Give a better error message when RunAsyncCode.runAsyncCode is passed something
Can you review this, Ray? In trunk, the compiler generates an internal compiler exception if runAsyncCode is called with something other than a class literal as an argument. This fixes that problem, and while at it, updates all the other error messages in ReplaceRunAsyncs to be more consisteent. http://gwt-code-reviews.appspot.com/161802 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Changes for crawling:
http://gwt-code-reviews.appspot.com/161801/diff/1/3 File samples/showcase/war/Showcase.html (right): http://gwt-code-reviews.appspot.com/161801/diff/1/3#newcode4 Line 4: script language='javascript' Any disadvantage to leaving the title here (even though you are setting it later)? At least, browsers that can't run JS can see the title. http://gwt-code-reviews.appspot.com/161801/diff/1/4 File user/src/com/google/gwt/user/client/ui/CrawlableHyperlink.java (right): http://gwt-code-reviews.appspot.com/161801/diff/1/4#newcode58 Line 58: } Why override these constructors if the override is not doing anything useful? Shouldn't over-riding just the setTargetHistoryToken method and having a more descriptive Javadoc for this class suffice? http://gwt-code-reviews.appspot.com/161801/diff/1/5 File user/test/com/google/gwt/user/client/ui/CrawlableHyperlinkTest.java (right): http://gwt-code-reviews.appspot.com/161801/diff/1/5#newcode28 Line 28: } change to com.google.gwt.user.DebugTest, as in HyperLinkTest. http://gwt-code-reviews.appspot.com/161801/diff/1/5#newcode36 Line 36: Arguments of assertEquals must be swapped. It is assertEquals(expected, actual). http://gwt-code-reviews.appspot.com/161801/diff/1/5#newcode41 Line 41: assertEquals(historyToken, !myToken); historyToken can be inlined everywhere. http://gwt-code-reviews.appspot.com/161801 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add support for creating Grid elements using UiBinder.
One more question - how to know which person to invite for a code review? On Mar 7, 11:11 am, Marko Vuksanovic markovuksano...@gmail.com wrote: I have created a patch that enables user to create Grid and its child elements using UiBinder (issue 4705 -http://code.google.com/p/google-web-toolkit/issues/detail?id=4705). Something like... I have also submitted the patch for public code review -http://gwt-code-reviews.appspot.com/154810. So if somebody from the gwt team could have a look Shortest code snippet which demonstrates use: g:Grid rows='3' columns='1' g:row g:customCell g:Labelfoo/g:Label /g:customCell /g:row g:row g:textCell foo /g:textCell /g:row g:row g:customCell divfoo/div /g:customCell /g:row /g:Grid -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: JConstructor redesign
http://gwt-code-reviews.appspot.com/159803/diff/1/25 File dev/core/src/com/google/gwt/dev/jjs/impl/RemoveEmptySuperCalls.java (right): http://gwt-code-reviews.appspot.com/159803/diff/1/25#newcode50 Line 50: ctx.replaceMe(multi.makeStatement()); Oh, I see your point. :( Yeah, that'd be a problem here, as there's no JProgram reference in sight. I'll drop in a TODO: move to Simplifier. http://gwt-code-reviews.appspot.com/159803 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Add support for creating Grid elements using UiBinder.
You don't, really. I'm the right person in this case, and you're right near the top of my todo list. Sorry I haven't gotten to it yet. On Mon, Mar 8, 2010 at 3:46 PM, Marko Vuksanovic markovuksano...@gmail.comwrote: One more question - how to know which person to invite for a code review? On Mar 7, 11:11 am, Marko Vuksanovic markovuksano...@gmail.com wrote: I have created a patch that enables user to create Grid and its child elements using UiBinder (issue 4705 - http://code.google.com/p/google-web-toolkit/issues/detail?id=4705). Something like... I have also submitted the patch for public code review -http://gwt-code-reviews.appspot.com/154810. So if somebody from the gwt team could have a look Shortest code snippet which demonstrates use: g:Grid rows='3' columns='1' g:row g:customCell g:Labelfoo/g:Label /g:customCell /g:row g:row g:textCell foo /g:textCell /g:row g:row g:customCell divfoo/div /g:customCell /g:row /g:Grid -- I wish this were a Wave -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r7687 committed - Initial code that uses org.json lib on the server side, passes actual ...
Revision: 7687
Author: gwt.mirror...@gmail.com
Date: Mon Mar 8 13:39:42 2010
Log: Initial code that uses org.json lib on the server side, passes actual
request
parameters, plus integration with server side domain objects.
Review at http://gwt-code-reviews.appspot.com/160809
Review by: rj...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=7687
Added:
/trunk/bikeshed/src/com/google/gwt/sample/expenses/shared/MethodName.java
/trunk/bikeshed/war/WEB-INF/lib/json.jar
Modified:
/trunk/bikeshed/.classpath
/trunk/bikeshed/src/com/google/gwt/sample/expenses/client/Expenses.java
/trunk/bikeshed/src/com/google/gwt/sample/expenses/client/Shell.java
/trunk/bikeshed/src/com/google/gwt/sample/expenses/client/Shell.ui.xml
/trunk/bikeshed/src/com/google/gwt/sample/expenses/domain/Storage.java
/trunk/bikeshed/src/com/google/gwt/sample/expenses/server/ExpensesDataServlet.java
/trunk/bikeshed/src/com/google/gwt/sample/expenses/shared/EmployeeRequests.java
/trunk/bikeshed/src/com/google/gwt/valuestore/shared/Path.java
/trunk/bikeshed/src/com/google/gwt/valuestore/shared/Property.java
===
--- /dev/null
+++
/trunk/bikeshed/src/com/google/gwt/sample/expenses/shared/MethodName.java
Mon Mar 8 13:39:42 2010
@@ -0,0 +1,24 @@
+/*
+ * Copyright 2010 Google Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the License); you may
not
+ * use this file except in compliance with the License. You may obtain a
copy of
+ * the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an AS IS BASIS,
WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
under
+ * the License.
+ */
+package com.google.gwt.sample.expenses.shared;
+
+/**
+ * Represents the MethodName.
+ */
+public enum MethodName {
+ FIND_ALL_EMPLOYEES,
+ FIND_EMPLOYEE,
+}
===
--- /dev/null
+++ /trunk/bikeshed/war/WEB-INF/lib/json.jarMon Mar 8 13:39:42 2010
Binary file, no diff available.
===
--- /trunk/bikeshed/.classpath Fri Mar 5 07:05:48 2010
+++ /trunk/bikeshed/.classpath Mon Mar 8 13:39:42 2010
@@ -7,5 +7,6 @@
classpathentry kind=con
path=org.eclipse.jdt.launching.JRE_CONTAINER/
classpathentry kind=lib path=war/WEB-INF/lib/gwt-servlet.jar/
classpathentry kind=con
path=org.eclipse.jdt.junit.JUNIT_CONTAINER/3/
+ classpathentry kind=lib path=war/WEB-INF/lib/json.jar/
classpathentry kind=output path=war/WEB-INF/classes/
/classpath
===
--- /trunk/bikeshed/src/com/google/gwt/sample/expenses/client/Expenses.java
Thu Mar 4 14:11:39 2010
+++ /trunk/bikeshed/src/com/google/gwt/sample/expenses/client/Expenses.java
Mon Mar 8 13:39:42 2010
@@ -17,17 +17,10 @@
import com.google.gwt.core.client.EntryPoint;
import com.google.gwt.core.client.GWT;
-import com.google.gwt.core.client.JsArray;
import com.google.gwt.event.dom.client.ChangeEvent;
import com.google.gwt.event.dom.client.ChangeHandler;
-import com.google.gwt.http.client.Request;
-import com.google.gwt.http.client.RequestBuilder;
-import com.google.gwt.http.client.RequestCallback;
-import com.google.gwt.http.client.RequestException;
-import com.google.gwt.http.client.Response;
import com.google.gwt.sample.expenses.shared.Employee;
import com.google.gwt.sample.expenses.shared.ExpenseRequestFactory;
-import com.google.gwt.user.client.Command;
import com.google.gwt.user.client.ui.HasValueList;
import com.google.gwt.user.client.ui.RootLayoutPanel;
import com.google.gwt.user.client.ui.TextBox;
@@ -57,37 +50,6 @@
final Shell shell = new Shell();
root.add(shell);
-Command refresh = new Command() {
- public void execute() {
-RequestBuilder builder = new RequestBuilder(RequestBuilder.GET,
-/expenses/data);
-builder.setCallback(new RequestCallback() {
-
- public void onError(Request request, Throwable exception) {
-shell.error.setInnerText(SERVER_ERROR);
- }
-
- public void onResponseReceived(Request request, Response
response) {
-if (200 == response.getStatusCode()) {
- String text = response.getText();
- JsArrayValuesImplEmployee valueArray =
ValuesImpl.arrayFromJson(text);
- shell.setValueList(valueArray);
-} else {
- shell.error.setInnerText(SERVER_ERROR + (
- + response.getStatusText() + ));
-}
- }
-});
-
-try {
- builder.send();
-} catch (RequestException e) {
- shell.error.setInnerText(SERVER_ERROR + ( + e.getMessage()
+ ));
-}
- }
-};
-
[gwt-contrib] Re: RequestFactory prototype: Initial code that uses org.json lib on the server side
Ray and I pair-programmed and commited this patch http://code.google.com/p/google-web-toolkit/source/detail?r=7687 Ray: please close this issue. http://gwt-code-reviews.appspot.com/160809/diff/1/9 File bikeshed/src/com/google/gwt/valuestore/shared/Property.java (right): http://gwt-code-reviews.appspot.com/160809/diff/1/9#newcode21 Line 21: * @param T Entity type On 2010/03/07 20:45:48, Ray Ryan wrote: Type of the property holder Done. http://gwt-code-reviews.appspot.com/160809 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Fix escaping
Reviewers: jat,
Description:
Fix the escaping of a String used in one of test methods of
JsonpRequestTest
Please review this at http://gwt-code-reviews.appspot.com/163801
Affected files:
user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java
Index: user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java
===
--- user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java (revision
7687)
+++ user/test/com/google/gwt/jsonp/client/JsonpRequestTest.java (working
copy)
@@ -184,7 +184,7 @@
public void testCancel() {
delayTestFinish(2000);
// setup a server request that will delay for 500ms
-JsonpRequestString req = jsonp.requestString(echoDelayed(A, 500),
+JsonpRequestString req = jsonp.requestString(echoDelayed('A', 500),
new AssertFailureCallbackString(A));
// cancel it before it comes back
req.cancel();
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
