[gwt-contrib] [google-web-toolkit] r10202 committed - Enables on the persistent unit cache by default....
Revision: 10202 Author: gwt.mirror...@gmail.com Date: Mon May 23 07:26:25 2011 Log: Enables on the persistent unit cache by default. Review at http://gwt-code-reviews.appspot.com/1448801 http://code.google.com/p/google-web-toolkit/source/detail?r=10202 Modified: /trunk/dev/core/src/com/google/gwt/dev/javac/UnitCacheFactory.java === --- /trunk/dev/core/src/com/google/gwt/dev/javac/UnitCacheFactory.java Thu Apr 28 09:54:26 2011 +++ /trunk/dev/core/src/com/google/gwt/dev/javac/UnitCacheFactory.java Mon May 23 07:26:25 2011 @@ -29,7 +29,7 @@ * The API must be enabled explicitly for persistent caching to be live. */ private static final String configPropertyValue = System.getProperty(gwt.persistentunitcache, - false); + true); private static final boolean usePersistent = configPropertyValue.length() == 0 || Boolean.parseBoolean(configPropertyValue); private static UnitCache instance = null; -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Re-implement runAsync to improve code size. (issue1442807)
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java
File
dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java#newcode65
dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java:65:
String location = runAsync.getName();
location vs. getName(). Has the concept changed?
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java
File
dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java#newcode55
dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java:55:
public static final String ASYNC_MAGIC_METHOD = runAsync;
Unused?
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java
File dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java (right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java#newcode40
dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java:40: * Based on
either explicit class literal, or the jsni name of the containing
The upside is that the actual value doesn't matter except that it's
unique across the compilation?
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java#newcode1247
dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java:1247:
func.setArtificiallyRescued(true);
:-)
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java
File dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java#newcode86
dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java:86:
return;
On 2011/05/20 22:47:15, scottb wrote:
Some fragments are now so small, they don't contain any functions. :)
Isn't that pointless? Shouldn't these non-fragments get pruned?
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
File dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java#newcode198
dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java:198:
static String getImplicitName(JMethod method) {
getQualifiedJsniName() instead?
http://gwt-code-reviews.appspot.com/1442807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Re-implement runAsync to improve code size. (issue1442807)
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java
File
dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java#newcode65
dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java:65:
String location = runAsync.getName();
On 2011/05/23 14:42:41, bobv wrote:
location vs. getName(). Has the concept changed?
Since that string can either be an arbitrary class literal name or the
comiler generated location, probably the name 'location' needs to change
to 'name.
See ReplaceRunAsyncs.AsyncCreateVisitor
I don't know why, but I've seen a lot of code pass a class literal
option. Maybe passing the class literal is a legacy thing and should be
deprecated?
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
File dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java#newcode198
dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java:198:
static String getImplicitName(JMethod method) {
On 2011/05/23 14:42:41, bobv wrote:
getQualifiedJsniName() instead?
add this as a method on JMethod?
http://gwt-code-reviews.appspot.com/1442807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: SafeHtmlRenderer code gen for UiBinder. Picking up issue 1426803 (issue1442804)
PTAL
http://gwt-code-reviews.appspot.com/1442804/diff/7001/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java
File user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/7001/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java#newcode119
user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java:119:
public void resetWritten() {
On 2011/05/16 19:37:47, rjrjr wrote:
No.
Done.
http://gwt-code-reviews.appspot.com/1442804/diff/7001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
File user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/7001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1360
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1360:
fieldManager.rewriteGwtFieldsDeclaration(niceWriter,
uiOwnerType.getName());
On 2011/05/16 19:37:47, rjrjr wrote:
I called this out in the first review. What's up with this rewriting
stuff, and
resetting the written status in the field writers? It seems very
wrong.
Done.
http://gwt-code-reviews.appspot.com/1442804/diff/7001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1469
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1469: //
TODO(sbrubaker): Fix method signature
On 2011/05/16 19:37:47, rjrjr wrote:
Stephanie won't be doing these. Shouldn't these issues be fixed now?
Done.
http://gwt-code-reviews.appspot.com/1442804/diff/7001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1479
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1479: //
TODO(sbrubaker): Find a better way to remove asString
On 2011/05/16 19:37:47, rjrjr wrote:
ditto
Done.
http://gwt-code-reviews.appspot.com/1442804/diff/7001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java
File
user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/7001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java#newcode80
user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java:80:
fieldName.setHTML(template.html1().asString());,
On 2011/05/16 19:37:47, rjrjr wrote:
This is a very fundamental change of the test. You're no longer
verifying the
output of the parser.
Done. Tests fixed by modifying MockUiBinderWriter instead.
http://gwt-code-reviews.appspot.com/1442804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: SafeHtmlRenderer code gen for UiBinder. Picking up issue 1426803 (issue1442804)
http://gwt-code-reviews.appspot.com/1442804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Pruner runs only once. (issue1436802)
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
File dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
(right):
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode84
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:84: *
Marks as referenced any types, methods, and fields that are reachable.
On 2011/05/20 20:14:17, jbrosenberg wrote:
s/any the classes/any classes/
Done.
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode382
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:382:
Done, factored out the into a well-documented method,
rescueArgumentsIfParametersCanBeRead.
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode385
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:385:
if (program.staticImplFor(method) != null) {
On 2011/05/20 20:14:17, jbrosenberg wrote:
Pruner.CleanupRefsVisitor
Done.
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode772
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:772:
On 2011/05/20 20:14:17, jbrosenberg wrote:
Perhaps add a comment for this map, similar to the Schrodinger's
comment below
for membersLiveExceptForInstantiability. I'm a bit confused by the
name
argsLiveExceptForParamRead, how about argsToAcceptIfParamRead.
Done.
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java#newcode787
dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java:787:
*/
On 2011/05/20 20:14:17, jbrosenberg wrote:
How about membersToRescueIfInstantiable
Done.
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
File dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java (right):
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java#newcode421
dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java:421: *
devirtualizations. If the instance method has been pruned, then it's
On 2011/05/20 20:14:17, jbrosenberg wrote:
Here I think it's assumed saveCodeGenTypes == true = it's the final
pass.
Perhaps this should be made more clear, or the comment updated to
clarify.
Done.
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java#newcode570
dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java:570: // Retain the
original arguments, they will be evaluated for side effects.
Yeah, when I went and reread the spec, I discovered that arguments are
evaluated before NPE is checked. :/
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java#newcode600
dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java:600:
On 2011/05/20 20:14:17, jbrosenberg wrote:
Is it now certain that the Pruner should not need any subsequent
passes to
complete all possible pruning? Or is it most of the way there, and
this is
now fair to cut it off to save time?
It's not 100%, but it's close enough that not iterating on Pruner
doesn't add more passes to the outer optimization loop for the compiles
I've tried.
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java
File dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java (right):
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java#newcode145
dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java:145:
program.addEntryMethod(findMainMethod(program));
The test contains one of the few cases that still tickles through,
namely the transformation from x.foo() - null.nullMethod() where x is
eventually pruned. It seemed more expedient to realize the immediate
benefit and let someone else solve the last case later. It was tricky
enough that I would have needed to invest more time than I had on it.
We don't have a test for the 'false' case.
http://gwt-code-reviews.appspot.com/1436802/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Pruner runs only once. (issue1436802)
LGTM http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java File dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java (right): http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java#newcode145 dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java:145: program.addEntryMethod(findMainMethod(program)); Can you leave a TODO comment for the possible follow-on work there? http://gwt-code-reviews.appspot.com/1436802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Partial fix for buganizer issue 4466425. Created a method to shift the expression range indices ... (issue1451801)
Reviewers: zundel, Description: Partial fix for buganizer issue 4466425. Created a method to shift the expression range indices for the detailed SOYC reports after function clustering. Please review this at http://gwt-code-reviews.appspot.com/1451801/ Affected files: M dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java M dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] EventBus package(s) in future GWT releases
We're trying to make bindery the correct location, but we're a bit in mid-step. But you shouldn't have to have two instances. gwt...EventBus extends binder...EventBus. On Sun, May 22, 2011 at 10:29 AM, dd cafeb...@googlemail.com wrote: Hey g-men, with GWT 2.3 the autobean, event and requestfactory packages moved from com.google.gwt to com.google.web.bindery. The 'new' RequestFactory.initialize(EventBus) depends on com.google.web.bindery.event.shared.EventBus. In contrast the PlaceController.initialize(EventBus) depends on com.google.gwt.event.shared.EventBus. Using both EventBus packages leads to two @Singleton EventBus instances in my application. Is EventBus intended to exist in two Packages in future GWT releases or will there be one EventBus location? Thanks in advance (and for your *great* work) Daniel -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Re-implement runAsync to improve code size. (issue1442807)
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java
File
dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java
(left):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java#oldcode131
dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java:131:
} else {
Exactly, I'm just precomputing essentially the same value and stashing
it in the AST node for posterity.
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java
File
dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java#newcode65
dev/core/src/com/google/gwt/core/ext/soyc/impl/SplitPointRecorder.java:65:
String location = runAsync.getName();
On 2011/05/23 14:42:41, bobv wrote:
location vs. getName(). Has the concept changed?
Dunno, the mixed terminology predates my patch. Name is probably more
accurate, since it can be either the class literal that 'names' it, or
the generated name based on enclosing method (both before and after my
patch).
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java
File
dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java#newcode45
dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java:45:
public final MessageSend messageSend;
MessageSend is basically any method call, static, instance, generic,
whose target is not a constructor.
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java#newcode55
dev/core/src/com/google/gwt/dev/jdt/FindDeferredBindingSitesVisitor.java:55:
public static final String ASYNC_MAGIC_METHOD = runAsync;
On 2011/05/23 14:42:41, bobv wrote:
Unused?
Done. Whoops.
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(left):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#oldcode519
dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:519:
allRootTypes.add(FragmentLoaderCreator.ASYNC_FRAGMENT_LOADER);
Actually, I meant the that immediately preceeding line,
allRootTypes.addAll(JProgram.INDEX_TYPES_SET) makes the removed line
unnecessary, because
JProgram.INDEX_TYPES_SET.contains(AsyncFragmentLoader).
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode617
dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:617:
if (module != null options.isRunAsyncEnabled()) {
Yep.
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
File dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java (right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java#newcode842
dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java:842: public
ListJRunAsync getRunAsyncs() {
Thanks, I thought so too. :)
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java#newcode1015
dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java:1015: public void
setRunAsyncs(ListJRunAsync runAsyncs) {
That is a fantastic observation, and not one I thought about. I believe
my patch does not affect the answer to the question, but it seems like a
great idea to follow up on.
I think what you would want to do is in Pruner.execImpl(), let
traverseEntryMethods() tell you which split points are reachable, and
only traverse those runAsync points. Of course, the second traversal
will reach new points, so you'd have to do it iteratively. Finally,
you'd reduce the set of compiler split points to just the reachable set.
There would be cleanup work elsewhere with code that assumes that the
set is fixed, and that the split point numbers are fixed up front,
contiguous, etc.
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java
File dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java (right):
http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/jjs/ast/JRunAsync.java#newcode40
[gwt-contrib] Re: Partial fix for buganizer issue 4466425. Created a method to shift the expression range indices ... (issue1451801)
http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java (right): http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1046 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1046: infoMap = ((JsReportGenerationVisitor) v).getSourceInfoMap(); I'm wondering if this would be cleaner if we were to move the 'clusterer' into JsReportGenerationVisitorWithSizeBreakdown Then in the JsReportGenerationVisitor constructor you could stash away infoMap and override clusterer with this extra reordering work http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1057 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1057: JsFunctionClusterer clusterer = shouldn't this be declared inside the if() block? Anyway, I'm proposing to move it inside JsReportGenerationVisitorWithSizeBreakdown anyway. http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1088 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1088: * This method shifts the Range indices that surround each JavaScript expression after clustering comments should be 80 chars. If you setup your eclipse formatting as in trunk/eclipse/README.txt, you should be able to use the eclipse Source-Format tool to fix it. http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1097 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1097: private static MapRange, SourceInfo updateRangesAfterClustering(StatementRanges oldRanges, Method out of order. See trunk/eclipse/README.txt If you have eclipse setup, you can just use the Source-Sort Members... function to fix it. http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1107 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1107: statementShifts.put(new Range(originalStart, originalEnd), new Range(permutedStart, permutedEnd)); code links should be 100 chars. again, autoformat will just fix this. http://gwt-code-reviews.appspot.com/1451801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Partial fix for buganizer issue 4466425. Created a method to shift the expression range indices ... (issue1451801)
Comment re: clusterer below. Working on the formatting now... On Mon, May 23, 2011 at 2:07 PM, zun...@google.com wrote: http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java (right): http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1046 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1046: infoMap = ((JsReportGenerationVisitor) v).getSourceInfoMap(); I'm wondering if this would be cleaner if we were to move the 'clusterer' into JsReportGenerationVisitorWithSizeBreakdown Then in the JsReportGenerationVisitor constructor you could stash away infoMap and override clusterer with this extra reordering work http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1057 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1057: JsFunctionClusterer clusterer = shouldn't this be declared inside the if() block? Anyway, I'm proposing to move it inside JsReportGenerationVisitorWithSizeBreakdown anyway. The clusterer instance is used by the JsIEBlockTextTransformer constructor below that: dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1072: JsIEBlockTextTransformer ieXformer = new JsIEBlockTextTransformer(clusterer); So if it's declared inside the if() block, the scope won't work. Should the JsIEBlockTextTransformer stuff be moved inside JsReportGenerationVisitorWithSizeBreakdown, too, then? http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1088 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1088: * This method shifts the Range indices that surround each JavaScript expression after clustering comments should be 80 chars. If you setup your eclipse formatting as in trunk/eclipse/README.txt, you should be able to use the eclipse Source-Format tool to fix it. http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1097 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1097: private static MapRange, SourceInfo updateRangesAfterClustering(StatementRanges oldRanges, Method out of order. See trunk/eclipse/README.txt If you have eclipse setup, you can just use the Source-Sort Members... function to fix it. http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1107 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1107: statementShifts.put(new Range(originalStart, originalEnd), new Range(permutedStart, permutedEnd)); code links should be 100 chars. again, autoformat will just fix this. http://gwt-code-reviews.appspot.com/1451801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] RequestFactoryServlet
@Bobv
One thing about RequestFactoryServlet is that it is somewhat difficult to
setup DI from containers such as Spring. I've been working on getting Spring
wired into the request factory framework and there are a bunch of examples,
but they all are pretty ugly. I think I've found a fairly clean solution,
and it only involves a minimal change to RequestFactoryServlet to
accomplish.
If we added ServletContext to a ThreadLocal, the same way we add HttpRequest
and HttpResponse, then it becomes trivial to get the servlet context out of
the request factory.
Then creating your Spring locator can look like :
public class SpringServiceLocator implements ServiceLocator {
public Object getInstance(Class? arg0) {
ApplicationContext ctx =
WebApplicationContextUtils.getWebApplicationContext(SpringRequestFactoryServlet.getThreadLocalServletContext().get());
return ctx.getBean(arg0);
}
}
and with Spring 3, everything should roll together nicely (I'm in the middle
of working this out, but this should work).
I'm will implement this for my project. My question is, Does it makes sense
to add the ServletContext into RequestFactoryServlet inside GWT proper? If
so, I can submit a patch for you.
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: SafeHtmlRenderer code gen for UiBinder. Picking up issue 1426803 (issue1442804)
Nearly there, just a couple of nits. Thanks!
After this patch lands we're going to need to think harder about the
testing. The coverage introduced here is pretty light. E.g., we're not
reproducing any of the escaping checks (like the various
test*FunnyChars tests in UiBinderTest. I don't have an immediate idea,
but we should certainly avoid mass copy and paste of the tests and the
appropriate template fragments.
Could you post a sample of the generated code?
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java
File user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java#newcode123
user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java:123:
public void resetWritten() {
uncalled, right?
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
File user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode235
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:235: private
boolean isRenderer;
Should be final
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode861
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:861:
die(baseClass.getName() + must implement UiBinder or
SafeHtmlRenderer);
Are we making a comparable check for implements SafeHtmlRenderer?
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1464
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1464: //
TODO(sbrubaker): Fix method signature
Is this TODO still valid? I've lost track of what it refers to. If it
is, can you make it a bit more informative?
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java
File
user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java#newcode80
user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java:80:
fieldName.setHTML(\@mockToken-fieldName-Hello, I
bcaption/byou.\);,
is the fieldName string coming from a constant in ElementParserTester
or something? If so, can you open up its visibility and use it in these
tests, rather having it inlined all over the place?
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
File user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java#newcode73
user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java:73:
public void testSafeHtmlRendererText() {
Doesn't need to block this patch, but as a follow up could you break
this out into its own test class, perhaps SafeHtmlRendererTest?
SafeHtmlAsComponentsTest should serve as an example.
http://gwt-code-reviews.appspot.com/1442804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: 1. Assert, at runtime, that GWT is running in Standards Mode (i.e. with an appropriate DOCTYPE d... (issue1422816)
DocumentModeAsserter is asserting that the Document.compatMode does not equal BackCompat, which corresponds to quirks mode. You should be able to use any variation of the strict mode doctype and still pass the asserter, as long as the Document.compatMode is CSS1Compat. If your app is in quirks mode, we tell you to use !DOCTYPE html, but the strict doctype would work as well. Also, you can disable the warning by extending the allowed doctypes property to include BackCompat. http://gwt-code-reviews.appspot.com/1422816/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Pruner runs only once. (issue1436802)
http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java File dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java (right): http://gwt-code-reviews.appspot.com/1436802/diff/4001/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java#newcode145 dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java:145: program.addEntryMethod(findMainMethod(program)); On 2011/05/23 17:26:46, jbrosenberg wrote: Can you leave a TODO comment for the possible follow-on work there? Done. http://gwt-code-reviews.appspot.com/1436802/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] RequestFactoryServlet
Does it makes sense to add the ServletContext into RequestFactoryServlet inside GWT proper? If so, I can submit a patch for you. That sounds like a very clean approach. When you submit the patch, please create an issue in the GWT issue tracker with the URL of the code review. -- Bob Vawter Google Web Toolkit Team -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Adds a ui:safehtml tag to UiBinder (issue1422812)
http://gwt-code-reviews.appspot.com/1422812/diff/7001/user/test/com/google/gwt/uibinder/elementparsers/ElementParserTester.java File user/test/com/google/gwt/uibinder/elementparsers/ElementParserTester.java (right): http://gwt-code-reviews.appspot.com/1422812/diff/7001/user/test/com/google/gwt/uibinder/elementparsers/ElementParserTester.java#newcode72 user/test/com/google/gwt/uibinder/elementparsers/ElementParserTester.java:72: static final String BINDER_URI = urn:ui:com.google.gwt.uibinder; Change the value back to binderUi. Tests that need to verify the uri can refer to this constant, make it public if need be. http://gwt-code-reviews.appspot.com/1422812/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: SafeHtmlRenderer code gen for UiBinder. Picking up issue 1426803 (issue1442804)
http://gwt-code-reviews.appspot.com/1442804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: SafeHtmlRenderer code gen for UiBinder. Picking up issue 1426803 (issue1442804)
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java
File user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java#newcode123
user/src/com/google/gwt/uibinder/rebind/AbstractFieldWriter.java:123:
public void resetWritten() {
On 2011/05/23 18:25:23, rjrjr wrote:
uncalled, right?
Done.
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
File user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode235
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:235: private
boolean isRenderer;
On 2011/05/23 18:25:23, rjrjr wrote:
Should be final
Done.
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode861
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:861:
die(baseClass.getName() + must implement UiBinder or
SafeHtmlRenderer);
On 2011/05/23 18:25:23, rjrjr wrote:
Are we making a comparable check for implements SafeHtmlRenderer?
Actually, this is the wrong place for this check. It should happen in
the constructor, where isRenderer is determined.
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java#newcode1464
user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java:1464: //
TODO(sbrubaker): Fix method signature
On 2011/05/23 18:25:23, rjrjr wrote:
Is this TODO still valid? I've lost track of what it refers to. If it
is, can
you make it a bit more informative?
Done. Confirmed with sbrubaker that the TODO was stale.
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java
File
user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java#newcode80
user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java:80:
fieldName.setHTML(\@mockToken-fieldName-Hello, I
bcaption/byou.\);,
On 2011/05/23 18:25:23, rjrjr wrote:
is the fieldName string coming from a constant in
ElementParserTester or
something? If so, can you open up its visibility and use it in these
tests,
rather having it inlined all over the place?
Done for the lines changed in this Issue.
The string comes from ElementParserTester#FIELD_NAME. It is hard-coded
all over the place, though (at least 45/278 tests in UiBinderJreSuite).
May I finish surfacing it in a separate Issue?
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
File user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java
(right):
http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java#newcode73
user/test/com/google/gwt/uibinder/test/client/UiBinderTest.java:73:
public void testSafeHtmlRendererText() {
On 2011/05/23 18:25:23, rjrjr wrote:
Doesn't need to block this patch, but as a follow up could you break
this out
into its own test class, perhaps SafeHtmlRendererTest?
SafeHtmlAsComponentsTest
should serve as an example.
SGTM.
http://gwt-code-reviews.appspot.com/1442804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: SafeHtmlRenderer code gen for UiBinder. Picking up issue 1426803 (issue1442804)
LGTM Woot! http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java File user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java (right): http://gwt-code-reviews.appspot.com/1442804/diff/10001/user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java#newcode80 user/test/com/google/gwt/uibinder/elementparsers/DialogBoxParserTest.java:80: fieldName.setHTML(\@mockToken-fieldName-Hello, I bcaption/byou.\);, Absolutely, and sorry for the mess. On 2011/05/23 19:52:21, rchandia wrote: On 2011/05/23 18:25:23, rjrjr wrote: is the fieldName string coming from a constant in ElementParserTester or something? If so, can you open up its visibility and use it in these tests, rather having it inlined all over the place? Done for the lines changed in this Issue. The string comes from ElementParserTester#FIELD_NAME. It is hard-coded all over the place, though (at least 45/278 tests in UiBinderJreSuite). May I finish surfacing it in a separate Issue? http://gwt-code-reviews.appspot.com/1442804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Attachable-IsRenderable rename step 1: Renaming the base interface. (issue1447806)
LGTM http://gwt-code-reviews.appspot.com/1447806/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10203 committed - Pruner runs only once....
Revision: 10203
Author: sco...@google.com
Date: Mon May 23 09:42:46 2011
Log: Pruner runs only once.
I know this feels a little hacky, but I tracked down most of the sources of
Pruner jitter that's causing it to run more than once.
1) Instance fields needed to be treated like method and put into limbo
until their enclosing type is instantiable. This shouldn't be too
controversial.
2) A big source of jitter is the fact that CleanupRefsVisitor removes
non-side-effect causing arguments when the invoked parameter is pruned.
Essentially, I am doing the same conceptual thing for arguments to methods
as we are for methods and fields-- unless the argument expressions have
side effects, they are not considered rescued until the target parameter is
rescued.
http://gwt-code-reviews.appspot.com/1436802/
http://code.google.com/p/google-web-toolkit/source/detail?r=10203
Modified:
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/Pruner.java
/trunk/dev/core/test/com/google/gwt/dev/jjs/impl/PrunerTest.java
===
---
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
Thu May 5 06:03:58 2011
+++
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
Mon May 23 09:42:46 2011
@@ -55,6 +55,7 @@
import com.google.gwt.dev.js.ast.JsName;
import com.google.gwt.dev.js.ast.JsNameRef;
import com.google.gwt.dev.js.ast.JsVisitor;
+import com.google.gwt.dev.util.collect.Lists;
import java.util.ArrayList;
import java.util.HashMap;
@@ -81,8 +82,8 @@
/**
* Marks as referenced any types, methods, and fields that are
reachable.
- * Also marks as instantiable any the classes and interfaces that can
- * possibly be instantiated.
+ * Also marks as instantiable any classes and interfaces that can
possibly
+ * be instantiated.
*
* TODO(later): make RescueVisitor use less stack?
*/
@@ -229,8 +230,7 @@
rescue(intfType, false, isInstantiated);
}
- rescueMethodsIfInstantiable(type);
-
+ rescueMembersIfInstantiable(type);
return false;
}
@@ -271,14 +271,23 @@
public boolean visit(JFieldRef ref, Context ctx) {
JField target = ref.getField();
- // JLS 12.4.1: references to static, non-final, or
- // non-compile-time-constant fields rescue the enclosing class.
- // JDT already folds in compile-time constants as literals, so we
must
- // rescue the enclosing types for any static fields that make it
here.
+ /*
+ * JLS 12.4.1: references to static, non-final, or
+ * non-compile-time-constant fields rescue the enclosing class. JDT
+ * already folds in compile-time constants as literals, so we must
rescue
+ * the enclosing types for any static fields that make it here.
+ */
if (target.isStatic()) {
rescue(target.getEnclosingType(), true, false);
}
- rescue(target);
+ if (target.isStatic() ||
instantiatedTypes.contains(target.getEnclosingType())) {
+rescue(target);
+ } else {
+// It's a field whose class is not instantiable
+if (!liveFieldsAndMethods.contains(target)) {
+ membersToRescueIfTypeIsInstantiated.add(target);
+}
+ }
return true;
}
@@ -306,7 +315,7 @@
accept(it);
}
- rescueMethodsIfInstantiable(type);
+ rescueMembersIfInstantiable(type);
return false;
}
@@ -363,11 +372,21 @@
} else {
// It's a virtual method whose class is not instantiable
if (!liveFieldsAndMethods.contains(method)) {
- methodsLiveExceptForInstantiability.add(method);
+ membersToRescueIfTypeIsInstantiated.add(method);
}
}
- return true;
+ if (argsToRescueIfParameterRead == null || method.canBePolymorphic()
+ || call instanceof JsniMethodRef) {
+return true;
+ }
+
+ if (program.staticImplFor(method) != null) {
+// CleanUpRefsVisitor does not prune these params, must rescue.
+return true;
+ }
+
+ return rescueArgumentsIfParametersCanBeRead(call, method);
}
@Override
@@ -524,7 +543,7 @@
if (method != null) {
if (!liveFieldsAndMethods.contains(method)) {
liveFieldsAndMethods.add(method);
- methodsLiveExceptForInstantiability.remove(method);
+ membersToRescueIfTypeIsInstantiated.remove(method);
if (dependencyRecorder != null) {
curMethodStack.add(method);
dependencyRecorder.methodIsLiveBecause(method, curMethodStack);
@@ -589,6 +608,7 @@
private void rescue(JVariable var) {
if (var != null) {
if (liveFieldsAndMethods.add(var)) {
+ membersToRescueIfTypeIsInstantiated.remove(var);
if (isStaticFieldInitializedToLiteral(var)) {
/*
[gwt-contrib] Re: Re-implement runAsync to improve code size. (issue1442807)
LGTM http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/js/JsUnusedFunctionRemover.java File dev/core/src/com/google/gwt/dev/js/JsUnusedFunctionRemover.java (left): http://gwt-code-reviews.appspot.com/1442807/diff/4001/dev/core/src/com/google/gwt/dev/js/JsUnusedFunctionRemover.java#oldcode118 dev/core/src/com/google/gwt/dev/js/JsUnusedFunctionRemover.java:118: (new JsNameRefVisitor()).accept(program); no, not worth it. http://gwt-code-reviews.appspot.com/1442807/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] gwtc classpath
Hello, I run into a case when I want to have different classpath for running gwtc (through com.google.gwt.dev.Compiler class) from classpath containing source code that gwtc would compile. Let me give you a specific scenario. My fork of gwt is using scala-library.jar of some version A for implementing internals of gwtc. Now application that gwtc should compile uses different version B of scala-library.jar that is binary incompatible with A. Is it possible to run gwtc so it uses A for it's own execution but uses B for JDT execution, populating TypeOracle, etc.? One thing that's worth mentioning: I'm talking here about compiled classes in classpath because they are used to populate TypeOracle. Source code (used for constructing GWT AST nodes) is in one copy corresponding to version B. -- Grzegorz Kossakowski -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Public (steph...@gmail.com): (issue1450802)
Reviewers: jlabanca,
Description:
Public (steph...@gmail.com):
Adds two protected static methods to EventBus that expose otherwise
inaccessible methods on Event (setSource and dispatch) to subclasses
of EventBus. Allows alternative EventBus implementations without
muddying the public API events show to handlers.
Tweaks the original patch (1443804) by making SimpleEventBus use the
new methods, and updating javadoc and a method name.
Please review this at http://gwt-code-reviews.appspot.com/1450802/
Affected files:
M user/src/com/google/web/bindery/event/shared/Event.java
M user/src/com/google/web/bindery/event/shared/EventBus.java
M user/src/com/google/web/bindery/event/shared/SimpleEventBus.java
Index: user/src/com/google/web/bindery/event/shared/Event.java
===
--- user/src/com/google/web/bindery/event/shared/Event.java (revision 10204)
+++ user/src/com/google/web/bindery/event/shared/Event.java (working copy)
@@ -106,9 +106,10 @@
/**
* Implemented by subclasses to to invoke their handlers in a type safe
* manner. Intended to be called by {@link EventBus#fireEvent(Event)} or
- * {@link EventBus#fireEventFromSource(Event, Object)}.
+ * {@link EventBus#fireEventFromSource(Event, Object)}.
*
* @param handler handler
+ * @see EventBus#dispatchEvent(Event, Object)
*/
protected abstract void dispatch(H handler);
@@ -118,6 +119,7 @@
*
* @param source the source of this event.
* @see EventBus#fireEventFromSource(Event, Object)
+ * @see EventBus#setSourceOfEvent(Event, Object)
*/
protected void setSource(Object source) {
this.source = source;
Index: user/src/com/google/web/bindery/event/shared/EventBus.java
===
--- user/src/com/google/web/bindery/event/shared/EventBus.java (revision
10204)
+++ user/src/com/google/web/bindery/event/shared/EventBus.java (working
copy)
@@ -29,6 +29,26 @@
* @see com.google.web.bindery.event.shared.testing.CountingEventBus
*/
public abstract class EventBus {
+
+ /**
+ * Invokes {@code event.dispatch} with {@code handler}.
+ * p
+ * Protected to allow EventBus implementations in different packages to
+ * dispatch events even though the {@code event.dispatch} method is
protected.
+ */
+ protected static H void dispatchEvent(EventH event, H handler) {
+event.dispatch(handler);
+ }
+
+ /**
+ * Sets {@code source} as the source of {@code event}.
+ * p
+ * Protected to allow EventBus implementations in different packages to
set an
+ * event source even though the {@code event.setSource} method is
protected.
+ */
+ protected static void setSourceOfEvent(Event? event, Object source) {
+event.setSource(source);
+ }
/**
* Adds an unfiltered handler to receive events of this type from all
sources.
@@ -70,7 +90,7 @@
* from executing.
*
* @throws UmbrellaException wrapping exceptions thrown by handlers
- *
+ *
* @param event the event to fire
*/
public abstract void fireEvent(Event? event);
Index: user/src/com/google/web/bindery/event/shared/SimpleEventBus.java
===
--- user/src/com/google/web/bindery/event/shared/SimpleEventBus.java
(revision 10204)
+++ user/src/com/google/web/bindery/event/shared/SimpleEventBus.java
(working copy)
@@ -178,7 +178,7 @@
firingDepth++;
if (source != null) {
-event.setSource(source);
+setSourceOfEvent(event, source);
}
ListH handlers = getDispatchList(event.getAssociatedType(),
source);
@@ -190,7 +190,7 @@
H handler = isReverseOrder ? it.previous() : it.next();
try {
- event.dispatch(handler);
+ dispatchEvent(event, handler);
} catch (Throwable e) {
if (causes == null) {
causes = new HashSetThrowable();
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Facilitate implementations of EventBus in other packages (issue1443804)
Tweaks in http://gwt-code-reviews.appspot.com/1450802 http://gwt-code-reviews.appspot.com/1443804/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Issue 6153
Hi GWT Devs, I've recently come across Issue 6153 (http://code.google.com/p/google-web-toolkit/issues/detail?id=6153) which affects the released version of GWT 2.3. Any chance of a patch or a release 2.3.1 to fix this? Automatic error reporting integrated with the editor framework is probably my favourite feature of GWT... it is shame to see it broken. Cheers, -Tim -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] Re: Partial fix for buganizer issue 4466425. Created a method to shift the expression range indices ... (issue1451801)
http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java (right): http://gwt-code-reviews.appspot.com/1451801/diff/1/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java#newcode1057 dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1057: JsFunctionClusterer clusterer = On 2011/05/23 18:07:01, zundel wrote: shouldn't this be declared inside the if() block? Anyway, I'm proposing to move it inside JsReportGenerationVisitorWithSizeBreakdown anyway. The clusterer instance is used by the JsIEBlockTextTransformer constructor below that: dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java:1072: JsIEBlockTextTransformer ieXformer = new JsIEBlockTextTransformer(clusterer); donnelly says: So if it's declared inside the if() block, the scope won't work. Should the JsIEBlockTextTransformer stuff be moved inside JsReportGenerationVisitorWithSizeBreakdown, too, then? zundel: That's a possibility. Is the transform going to be the same before and after the IEExformer change? In that case you could do something general like: v.runTransform(new JsClusterer()); v.runTransform(new JsIEBlockTextTransformer()); v.getJs() v.getSizeBreakdown(); v.getStatementRanges() http://gwt-code-reviews.appspot.com/1451801/ -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] gwtc classpath
Hi Grzegorz, You mentioned the compiled classes in the class path are used to populate the type orace. For the most part, the type oracle types are compiled from source. Types that don't have source are then checked to see if they are binary annotations and then a type oracle reference is entered. Could it be true that scala-library.jar contains both libraries to use with the GWT compiler and some sort of runtime environment (sources) to compile with your app? If so, maybe the solution is to split scala-library.jar into the equivalent of 'dev' and 'user' components, such that 'user' is not needed for running the compiler and 'dev' is not needed for the app to link against. On Mon, May 23, 2011 at 6:29 PM, Grzegorz Kossakowski grzegorz.kossakow...@gmail.com wrote: Hello, I run into a case when I want to have different classpath for running gwtc (through com.google.gwt.dev.Compiler class) from classpath containing source code that gwtc would compile. Let me give you a specific scenario. My fork of gwt is using scala-library.jar of some version A for implementing internals of gwtc. Now application that gwtc should compile uses different version B of scala-library.jar that is binary incompatible with A. Is it possible to run gwtc so it uses A for it's own execution but uses B for JDT execution, populating TypeOracle, etc.? One thing that's worth mentioning: I'm talking here about compiled classes in classpath because they are used to populate TypeOracle. Source code (used for constructing GWT AST nodes) is in one copy corresponding to version B. -- Grzegorz Kossakowski -- http://groups.google.com/group/Google-Web-Toolkit-Contributors -- Eric Z. Ayers Google Web Toolkit, Atlanta, GA USA -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10204 committed - - Modify the default HTML template to indicate that quirks mode is not...
Revision: 10204
Author: fre...@google.com
Date: Mon May 23 11:13:45 2011
Log: - Modify the default HTML template to indicate that quirks mode
is not supported
- Cleanup UserAgentGenerator.java, for consistency with
DocumentModeGenerator.java
- Add two new configuration properties:
1. document.compatMode, to enumerate the permitted values
{'CSS1Compat', 'BackCompat'} for
$doc.compatMode, by default:
set-configuration-property name=document.compatMode value=CSS1Compat/
2. document.compatMode.severity, to control the runtime check: IGNORE (no
check), WARN (Development
Mode warning only), ERROR (runtime error). Current default:
!-- The default value will be changed to 'ERROR' in a future release --
set-configuration-property name=document.compatMode.severity
value=WARN /
Fixes issues: 6086, 6306
Review at http://gwt-code-reviews.appspot.com/1422816
Review by: jlaba...@google.com
http://code.google.com/p/google-web-toolkit/source/detail?r=10204
Added:
/trunk/user/src/com/google/gwt/user/DocumentMode.gwt.xml
/trunk/user/src/com/google/gwt/user/client/DocumentModeAsserter.java
/trunk/user/src/com/google/gwt/user/rebind/DocumentModeGenerator.java
Modified:
/trunk/user/src/com/google/gwt/user/User.gwt.xml
/trunk/user/src/com/google/gwt/user/rebind/UserAgentGenerator.java
/trunk/user/src/com/google/gwt/user/tools/templates/sample/_warFolder_/_moduleShortName_.htmlsrc
===
--- /dev/null
+++ /trunk/user/src/com/google/gwt/user/DocumentMode.gwt.xml Mon May 23
11:13:45 2011
@@ -0,0 +1,53 @@
+!--
--
+!-- Copyright 2011 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 --
+!-- 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. License for the specific language governing permissions
and --
+!-- limitations under the
License. --
+
+!-- Defines the user.agent property and its provider
function. --
+!--
--
+!-- This module is typically inherited via
com.google.gwt.user.User--
+!--
--
+module
+
+ !--
+Enumerate one or more valid browser rendering modes, to be compared
with
+the runtime value of $doc.compatMode
+ - Specify 'CSS1Compat' for Standards Mode
+ - Specify 'BackCompat' for Quirks Mode
+
+ See http://en.wikipedia.org/wiki/Quirks_mode
+ --
+ define-configuration-property name=document.compatMode
+is-multi-valued=true /
+ !-- GWT only supports Standards Mode; Quirks Mode is deprecated --
+ set-configuration-property name=document.compatMode
value=CSS1Compat/
+
+ !--
+Determine the severity of the runtime $doc.compatMode check:
+ - Specify 'ERROR' to receive an error message at runtime
+ - Specify 'WARN' to receive a warning in Development Mode
+ - Specify 'IGNORE' for no runtime check
+ --
+ define-configuration-property name=document.compatMode.severity
+is-multi-valued=false /
+ !-- The default value will be changed to 'ERROR' in a future release --
+ set-configuration-property name=document.compatMode.severity
+value=WARN /
+
+ !-- Asserts a valid browser rendering mode at runtime --
+ entry-point class=com.google.gwt.user.client.DocumentModeAsserter /
+
+ generate-with class=com.google.gwt.user.rebind.DocumentModeGenerator
+when-type-assignable
class=com.google.gwt.user.client.DocumentModeAsserter.DocumentModeProperty
/
+ /generate-with
+
+/module
===
--- /dev/null
+++ /trunk/user/src/com/google/gwt/user/client/DocumentModeAsserter.java
Mon May 23 11:13:45 2011
@@ -0,0 +1,137 @@
+/*
+ * Copyright 2011 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
[gwt-contrib] Adds a ServletContext ThreadLocal to RequestFactoryServlet issue 6393 (issue1448804)
Reviewers: bobv, google-web-toolkit-contributors_googlegroups.com,
Description:
Adds a ThreadLocalServletContext which will make it easier to
facilitate getting beans from Spring.
Please review this at http://gwt-code-reviews.appspot.com/1448804/
Affected files:
user/src/com/google/web/bindery/requestfactory/server/RequestFactoryServlet.java
Index:
user/src/com/google/web/bindery/requestfactory/server/RequestFactoryServlet.java
===
---
user/src/com/google/web/bindery/requestfactory/server/RequestFactoryServlet.java
(revision 10204)
+++
user/src/com/google/web/bindery/requestfactory/server/RequestFactoryServlet.java
(working copy)
@@ -23,6 +23,7 @@
import java.util.logging.Level;
import java.util.logging.Logger;
+import javax.servlet.ServletContext;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@@ -45,6 +46,7 @@
*/
private static final ThreadLocalHttpServletRequest perThreadRequest =
new ThreadLocalHttpServletRequest();
private static final ThreadLocalHttpServletResponse perThreadResponse
= new ThreadLocalHttpServletResponse();
+ private static final ThreadLocalServletContext perThreadContext = new
ThreadLocalServletContext();
/**
* Returns the thread-local {@link HttpServletRequest}.
@@ -64,6 +66,15 @@
return perThreadResponse.get();
}
+ /**
+ * Returns the thread-local {@link ServletContext}
+ *
+ * @return the {@link ServletContext} associated with this servlet
+ */
+ public static ServletContext getThreadLocalServletContext() {
+return perThreadContext.get();
+ }
+
private final SimpleRequestProcessor processor;
/**
@@ -104,6 +115,7 @@
perThreadRequest.set(request);
perThreadResponse.set(response);
+perThreadContext.set(getServletContext());
// No new code should be placed outside of this try block.
try {
@@ -132,6 +144,7 @@
} finally {
perThreadRequest.set(null);
perThreadResponse.set(null);
+ perThreadContext.set(null);
}
}
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] RequestFactoryServlet
Created the issue as well and posted a link to the review. Thanks! http://gwt-code-reviews.appspot.com/1448804/ http://code.google.com/p/google-web-toolkit/issues/list?cursor=6393updated=6393ts=1306206518 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
Re: [gwt-contrib] RequestFactoryServlet
Oh and I did test this to make sure it would with Spring. If you're interested here is a link to my test project. https://github.com/larsenje/SpringRequestFactoryExample -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
[gwt-contrib] [google-web-toolkit] r10205 committed - Attachable-IsRenderable rename step 1: Renaming the base interface....
Revision: 10205
Author: rdcas...@google.com
Date: Mon May 23 17:23:56 2011
Log: Attachable-IsRenderable rename step 1: Renaming the base
interface.
Review at http://gwt-code-reviews.appspot.com/1447806
http://code.google.com/p/google-web-toolkit/source/detail?r=10205
Added:
/trunk/user/src/com/google/gwt/uibinder/elementparsers/IsRenderableInterpreter.java
/trunk/user/src/com/google/gwt/user/client/ui/IsRenderable.java
Deleted:
/trunk/user/src/com/google/gwt/uibinder/elementparsers/AttachableInterpreter.java
/trunk/user/src/com/google/gwt/user/client/ui/Attachable.java
Modified:
/trunk/user/src/com/google/gwt/uibinder/elementparsers/AttachableHTMLPanelParser.java
/trunk/user/src/com/google/gwt/uibinder/rebind/UiBinderWriter.java
/trunk/user/src/com/google/gwt/user/client/ui/AttachableComposite.java
/trunk/user/src/com/google/gwt/user/client/ui/AttachableHTMLPanel.java
===
--- /dev/null
+++
/trunk/user/src/com/google/gwt/uibinder/elementparsers/IsRenderableInterpreter.java
Mon May 23 17:23:56 2011
@@ -0,0 +1,79 @@
+/*
+ * Copyright 2011 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.uibinder.elementparsers;
+
+import com.google.gwt.core.ext.UnableToCompleteException;
+import com.google.gwt.uibinder.rebind.FieldManager;
+import com.google.gwt.uibinder.rebind.FieldWriter;
+import com.google.gwt.uibinder.rebind.UiBinderWriter;
+import com.google.gwt.uibinder.rebind.XMLElement;
+
+/**
+ * Used by {@link AttachableHTMLPanelParser} to interpret renderable
elements.
+ * Declares the appropriate {@link IsRenderable}, and returns the correct
HTML
+ * to be inlined in the AttachableHTMLPanel.
+ */
+class IsRenderableInterpreter implements XMLElement.InterpreterString {
+
+ private final String fieldName;
+
+ private final UiBinderWriter uiWriter;
+
+ public IsRenderableInterpreter(String fieldName, UiBinderWriter writer) {
+this.fieldName = fieldName;
+this.uiWriter = writer;
+assert writer.useLazyWidgetBuilders();
+ }
+
+ public String interpretElement(XMLElement elem)
+ throws UnableToCompleteException {
+if (!uiWriter.isRenderableElement(elem)) {
+ return null;
+}
+
+String idHolder = uiWriter.declareDomIdHolder();
+FieldManager fieldManager = uiWriter.getFieldManager();
+FieldWriter fieldWriter = fieldManager.require(fieldName);
+
+FieldWriter childFieldWriter =
uiWriter.parseElementToFieldWriter(elem);
+
+String elementPointer = idHolder + Element;
+fieldWriter.addAttachStatement(
+com.google.gwt.user.client.Element %s = +
+com.google.gwt.dom.client.Document.get().getElementById(%s).cast();,
+elementPointer, fieldManager.convertFieldToGetter(idHolder));
+fieldWriter.addAttachStatement(
+%s.wrapElement(%s);,
+fieldManager.convertFieldToGetter(childFieldWriter.getName()),
+elementPointer);
+
+// Some operations are more efficient when the Widget isn't attached to
+// the document. Perform them here.
+fieldWriter.addDetachStatement(
+%s.performDetachedInitialization();,
+fieldManager.convertFieldToGetter(childFieldWriter.getName()));
+
+fieldWriter.addDetachStatement(
+%s.logicalAdd(%s);,
+fieldManager.convertFieldToGetter(fieldName),
+fieldManager.convertFieldToGetter(childFieldWriter.getName()));
+
+// TODO(rdcastro): use the render() call that receives the
SafeHtmlBuilder
+String elementHtml =
fieldManager.convertFieldToGetter(childFieldWriter.getName()) + .render(
++ fieldManager.convertFieldToGetter(idHolder) + );
+return uiWriter.tokenForSafeHtmlExpression(elementHtml);
+ }
+}
===
--- /dev/null
+++ /trunk/user/src/com/google/gwt/user/client/ui/IsRenderable.java Mon May
23 17:23:56 2011
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2011 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
