Thanks, Scott!
http://gwt-code-reviews.appspot.com/875801/diff/1/4
File dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
(right):
http://gwt-code-reviews.appspot.com/875801/diff/1/4#newcode172
dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java:172: */
I'll add
Reviewers: rjrjr,
Description:
Removes @Override annotations on methods implementing interfaces.
They don't compile with a Java 1.5 compiler.
Review by: rj...@google.com
Please review this at http://gwt-code-reviews.appspot.com/884801/show
Affected files:
M
Thanks for flagging this problem, Ray. Can you review this tiny patch to
fix it?
http://gwt-code-reviews.appspot.com/884801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: unnurg,
Message:
This is a small change to the xsiframe linker. These hoops are a legacy
from the xs linker that is no longer needed. Since xsiframe has an
iframe, the code can be directly installed instead of being passed
around in a string.
Description:
The cross-site iframe linker
LGTM
http://gwt-code-reviews.appspot.com/839802/diff/3001/4010
File user/src/com/google/gwt/core/CrossSiteIframeLinker.gwt.xml (right):
http://gwt-code-reviews.appspot.com/839802/diff/3001/4010#newcode18
user/src/com/google/gwt/core/CrossSiteIframeLinker.gwt.xml:18:
This file could be dropped
Reviewers: cromwellian,
Description:
Gets the code splitter to be effective even in draft compile mode.
Review by: cromwell...@google.com
Please review this at http://gwt-code-reviews.appspot.com/875801/show
Affected files:
M
This is ready for review.
http://gwt-code-reviews.appspot.com/875801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/853801/diff/4001/5008
File user/test/com/google/gwt/dev/StrictModeTest.java (right):
http://gwt-code-reviews.appspot.com/853801/diff/4001/5008#newcode45
user/test/com/google/gwt/dev/StrictModeTest.java:45: private File
outDir;
The comment needs updating.
I
http://gwt-code-reviews.appspot.com/853801/diff/4001/5008
File user/test/com/google/gwt/dev/StrictModeTest.java (right):
http://gwt-code-reviews.appspot.com/853801/diff/4001/5008#newcode45
user/test/com/google/gwt/dev/StrictModeTest.java:45: private File
outDir;
You're right. I'll delete it. I
http://gwt-code-reviews.appspot.com/853801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Those two changes are made now. It's ready for another round of review.
http://gwt-code-reviews.appspot.com/853801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: aizatsky,
Description:
Adds source infos in the test AST nodes in ConstantsAssumptionTest.
Review by: aizat...@google.com
Please review this at http://gwt-code-reviews.appspot.com/831804/show
Affected files:
M
Can you review this tiny patch, Mike? Without this patch, there are
assertion failures.
http://gwt-code-reviews.appspot.com/831804/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thanks!
http://gwt-code-reviews.appspot.com/831804/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: scottb,
Description:
Adds a -strict option to the GWT compiler. If this option is specified,
then the compile will fail if any of the input files are bad.
Review by: sco...@google.com
Please review this at http://gwt-code-reviews.appspot.com/853801/show
Affected files:
M
I don't know anything about the *current* stats support in RPC.
Judging from the revision logs, it looks like Bob might have added that
system. Added to the CC. -Lex
On Tue, Aug 24, 2010 at 5:29 PM, Pascal Muetschard
pmuetschard...@google.com pmuetschard%2...@google.com wrote:
Could I
LGTM
http://gwt-code-reviews.appspot.com/784801/diff/3002/7001
File dev/core/src/com/google/gwt/core/ext/linker/impl/hosted.js (right):
http://gwt-code-reviews.appspot.com/784801/diff/3002/7001#newcode4
dev/core/src/com/google/gwt/core/ext/linker/impl/hosted.js:4: var
$moduleName =
LGTM
http://gwt-code-reviews.appspot.com/779801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Ray, does the patch look good to you? -Lex
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thanks, you two! I'll do a little more testing after the updates and
then submit it.
http://gwt-code-reviews.appspot.com/726802/diff/1/3
File dev/core/src/com/google/gwt/core/linker/CrossSiteIframeTemplate.js
(right):
http://gwt-code-reviews.appspot.com/726802/diff/1/3#newcode88
LGTM
http://gwt-code-reviews.appspot.com/699804/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: jgw,
Description:
Adds a new CrossSiteIframeLinker. This linker works cross-site,
because it uses a script tag to download code instead of XHR. However,
like the iframe linker, it still uses an iframe to hold all the
installed code.
Review by: j...@google.com
Please review this at
This is ready for review.
It's almost the same as this change:
http://gwt-code-reviews.appspot.com/726802
The only difference is that this patch makes a new linker rather than
updating the XS linker in place. The contents of the new linker are the
same as what was in the previous patch.
Err, make that the following issue:
http://gwt-code-reviews.appspot.com/674802
http://gwt-code-reviews.appspot.com/726802/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
I don't have a strong opinion about it. They can be non-final, with simply
no particular effort to truly make them extensible.
I think it might be possible to move the template JS files to
GWT-translated code with extension points managed through rebinding and
overriding. Until then, making
http://gwt-code-reviews.appspot.com/669801/diff/20001/21006
File dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java
(right):
http://gwt-code-reviews.appspot.com/669801/diff/20001/21006#newcode62
dev/core/src/com/google/gwt/dev/jjs/impl/JsFunctionClusterer.java:62: if
On Wed, Jul 28, 2010 at 6:15 PM, avassalo...@google.com wrote:
Oh, sorry. I made this comment somewhere else. The problem is the
endStatements() method doesn't use the regex to recognize the other
declaration style.
Ah, yes! Well at the least this code should be moved to a subroutine. I
On Mon, Jul 26, 2010 at 6:56 PM, John Tamplin j...@google.com wrote:
Well, we do know there will be other linkers, and if there aren't extension
points defined they will be done via cut-and-paste, which is what led to the
current state we are in.
No question that extension points are useful.
On Mon, Jul 26, 2010 at 12:37 PM, Scott Blum sco...@google.com wrote:
SGTM as far as process.
Is the new linker designed to curtail extension, or to sanely encourage it?
The existing primary linkers ended up getting extended in brittle ways.
That's a good point. Let's make it a final class
Reviewers: jgw,
Description:
Modifies the cross-site linker to put its code in an iframe
rather than a wrapper function.
Review by: j...@google.com
Please review this at http://gwt-code-reviews.appspot.com/674802/show
Affected files:
M dev/core/src/com/google/gwt/core/linker/XSLinker.java
Reviewers: robertvawter,
Description:
Wrap GWT.runAsync() entry points with $entry .
Please review this at http://gwt-code-reviews.appspot.com/706801/show
Affected files:
M dev/core/src/com/google/gwt/dev/jjs/impl/FragmentExtractor.java
M
This is ready for review.
http://gwt-code-reviews.appspot.com/706801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thanks!
http://gwt-code-reviews.appspot.com/706801/diff/1/2
File dev/core/src/com/google/gwt/dev/jjs/impl/FragmentExtractor.java
(right):
http://gwt-code-reviews.appspot.com/706801/diff/1/2#newcode640
dev/core/src/com/google/gwt/dev/jjs/impl/FragmentExtractor.java:640:
call.setQualifier(new
LGTM
http://gwt-code-reviews.appspot.com/707801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/702801/diff/7001/8006
File dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java
(right):
http://gwt-code-reviews.appspot.com/702801/diff/7001/8006#newcode163
dev/core/src/com/google/gwt/dev/jjs/impl/ExpressionAnalyzer.java:163:
canThrowException =
This is a pretty amazing first contribution! Also, the feature is
certainly desirable.
However, more work needs to be done to support that feature while also
handling all of the odd edge cases. For example, here are a few issues
to consider:
1. The GWT compiler uses MapString,String to
http://gwt-code-reviews.appspot.com/678802/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/678802/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/682801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Good find. Just one nit.
http://gwt-code-reviews.appspot.com/677801/diff/1/2
File dev/core/src/com/google/gwt/dev/jjs/ast/Context.java (right):
http://gwt-code-reviews.appspot.com/677801/diff/1/2#newcode33
dev/core/src/com/google/gwt/dev/jjs/ast/Context.java:33: boolean
isLvalue();
Cool, that
Sure, LGTM.
http://gwt-code-reviews.appspot.com/677801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM. It makes the test case a better example of how to use the
facility.
http://gwt-code-reviews.appspot.com/679801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/669801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thanks, Alex! Just nits:
- Let's attribute the original author(s) in the change description
- Let's put the edit-distance classes in a subpackage of
com.google.gwt.dev.util. Perhaps simply
com.google.gwt.dev.util.editdistance ?
http://gwt-code-reviews.appspot.com/669801/diff/1/7
File
On Mon, Jun 28, 2010 at 3:57 PM, Freeland Abbott fabb...@google.com wrote:
So, how breaking are we willing to be to correct that?
My knee-jerk reaction is that we don't want to do a lot of breaking just to
tidy up the definition of gwt-servlet. The only benefit is to reduce the
size of the
I think everyone is saying the same thing. Use Miroslav's suggested ASM tool
to get a first cut, and then bake the resulting whitelist into the ant
files. Then, little by little, refactor things so that the whitelist can
shrink, until all that's left is **/shared/** and **/server/** .
I'd only
LGTM
http://gwt-code-reviews.appspot.com/640803/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/631801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thank you for the minified example. I'll follow up on the issue.
http://code.google.com/p/google-web-toolkit/issues/detail?id=4423
http://gwt-code-reviews.appspot.com/575801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On Fri, Jun 4, 2010 at 5:13 AM, David david.no...@gmail.com wrote:
Less maintenance on the async, declarative transaction management,
undo, batching, less web.xml tweeking, ... there are many reasons why
we also use a command pattern.
With the system as it is, I believe your best bet is to
I believe type parameters work fine. Can you give an example that
doesn't work? I just tried modifying EnumTest to verify that at least in
simple cases, it works.
The way this works relies on Java generics having been defined to have
compatibility with pre-generic Java. You can always erase a
On Thu, Jun 3, 2010 at 12:56 PM, Ray Ryan rj...@google.com wrote:
No argument. And since we've never, ever managed to actually delete a
deprecated class so far as I know, the issue may not come up for a while…
There are some counterexamples. For example:
Thanks for the extra detail. I can verify that the given interface does
not translate correctly. The surprising thing is that this problem was
supposed to be fixed in JDT 3.4.2, which is exactly the version GWT is
using:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=243820
Maybe we aren't
LGTM
http://gwt-code-reviews.appspot.com/586801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM, though extending the comment as indicated would be nice.
http://gwt-code-reviews.appspot.com/588801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/588801/diff/1/2
File dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java (right):
http://gwt-code-reviews.appspot.com/588801/diff/1/2#newcode64
dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java:64: private
JDeclaredType enclosingType;
This
LGTM
http://gwt-code-reviews.appspot.com/587801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On Tue, Jun 1, 2010 at 11:56 AM, Ray Ryan rj...@google.com wrote:
Yup. Is the fix to make it use the resource oracle?
To play it safe:
First check resource oracle.
Next check the context class loader, as in Marko's email.
Then check wherever it looks now (the system class loader?).
Lex
--
On Tue, Jun 1, 2010 at 2:35 PM, Marko Vuksanovic
markovuksano...@gmail.comwrote:
Class Loaders are checked in parent to child direction - so if you try to
fetch a resource from a context class loader, system class loader is the
first that will be checked and only after resource is not found
LGTM
http://gwt-code-reviews.appspot.com/547801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM with nits. No need to rereview if you like the suggested changes.
These changes will make SOYC a lot easier to understand.
http://gwt-code-reviews.appspot.com/566801/diff/1/3
File dev/core/src/com/google/gwt/soyc/MakeTopLevelHtmlForPerm.java
(right):
Kathrin, it sounds like you have more time than Ray to look at SOYC
problems. Can you review this patch?
http://gwt-code-reviews.appspot.com/545801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thanks!
http://gwt-code-reviews.appspot.com/545801/diff/1/3
File dev/core/test/com/google/gwt/core/linker/SoycReportLinkerTest.java
(right):
http://gwt-code-reviews.appspot.com/545801/diff/1/3#newcode80
dev/core/test/com/google/gwt/core/linker/SoycReportLinkerTest.java:80: }
From face to face
Thanks for walking me through how all this works!
http://gwt-code-reviews.appspot.com/543801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM. I am used to GWT turning up oddities about JavaScript. This patch
does something less common: showing a surprise about Java!
http://gwt-code-reviews.appspot.com/553801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Let's see if the build file can be simplified some. The rest looks good.
http://gwt-code-reviews.appspot.com/547801/diff/1/4
File /user/build.xml (right):
http://gwt-code-reviews.appspot.com/547801/diff/1/4#newcode598
/user/build.xml:598: property name=compile.tests.complete
value=true/
This
Very nice. LGTM.
http://gwt-code-reviews.appspot.com/546801/diff/2001/3001
File
dev/core/src/com/google/gwt/core/ext/linker/impl/StandardLinkerContext.java
(right):
http://gwt-code-reviews.appspot.com/546801/diff/2001/3001#newcode422
The new data-flow optimizer is choking on the code. Some sort of
uncommon syntax in the code is breaking one of its assertions.
Let me poke around a little to see what's up. Filed as Issue 4957:
http://code.google.com/p/google-web-toolkit/issues/detail?id=4957
Lex
--
You received this
Reviewers: cromwellian,
Description:
When SoycReportLinker looks for report files, it now tolerates
missing soycReport from the beginning of the emitted
artifact paths.
Please review this at http://gwt-code-reviews.appspot.com/545801/show
Affected files:
M
Ray,
Can you review this, whenever your I/O involvement gives you time to do
so?
http://gwt-code-reviews.appspot.com/545801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thanks!
http://gwt-code-reviews.appspot.com/544801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: jgw,
Description:
Have IE's DOMImpl invoke its event dispatching function via a
round-trip through the window object. That way, the only reference
from DOM objects into GWT's code is on the window object, a
place IE is careful to clean up when the browser navigates to
a new page. This
Reviewers: jgw, jlabanca,
Description:
Fixes a memory leak on IE with the cross-site fragment loading strategy.
Now the callbacks on script tags are removed when they are invoked.
Please review this at http://gwt-code-reviews.appspot.com/544801/show
Affected files:
M
This is ready for review.
http://gwt-code-reviews.appspot.com/543801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
This is ready for review. It's a tiny patch, and either of you would
seem appropriate to me.
Joel, this is a simpler memory leak than the one in the other patch I
sent you.
John, you recently reviewed a similar change to this class having to do
with handling on-load and on-error events.
LVGTM.
Scott, you wrote OptimizerTestBase. What do you think about the divide
between JJSTestBase and OptimizerTestBase? JJS is for arbitrary tests
of JJS passes, while Optimizer is for ones that optimize one method into
another. Optimizer now has a stock Result class that can be reused for
Sorry, lost track of this patch. LGTM.
http://gwt-code-reviews.appspot.com/280801/diff/1/5
File user/super/com/google/gwt/emul/java/lang/Enum.java (right):
http://gwt-code-reviews.appspot.com/280801/diff/1/5#newcode32
user/super/com/google/gwt/emul/java/lang/Enum.java:32: if
(enumValueOfFunc
On Wed, May 5, 2010 at 2:15 PM, Ray Ryan rj...@google.com wrote:
You should be able to throw subclasses of RuntimeException, no?
For example, UnsupportedOperationException. -Lex
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On Mon, May 3, 2010 at 7:36 AM, Marko Vuksanovic
markovuksano...@gmail.comwrote:
I solved the problem... This had nothing to do with the GWT. The
problem was with adding a folder to java class path dynamically.
Although at first I thought I had done it correctly, it turned out
that that's a
Reviewers: mmendez,
Description:
Undo the collapse-all-permutations in JUnit.gwt.xml for now.
Review by: mmen...@google.com
Please review this at http://gwt-code-reviews.appspot.com/457801/show
Affected files:
M user/src/com/google/gwt/junit/JUnit.gwt.xml
Index:
On Tue, May 4, 2010 at 12:25 PM, Marko Vuksanovic markovuksano...@gmail.com
wrote:
Hi Lex,
The first solution seems interesting... could you please provide a code
snippet just to get me started...
Did you mean something like
Thread.currentThread().setContextClassLoader(urlClassloader);
LGTM
http://gwt-code-reviews.appspot.com/436801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/435801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
I tricked myself into thinking fallthrough work, because fallthrough
does happen from a statement to the following case: node. However, the
case: node turns into a conditional, so it's important to skip that
conditional if there's a fallthrough.
I'm surprised this hasn't bitten more
This is a great change! It simplifies the setup code for constant
analysis, and it means that the common case of a top assumption doesn't
require an explicit entry in the map.
Some small changes are requested in the implementation.
http://gwt-code-reviews.appspot.com/436801/diff/1/2
File
LGTM
Let's not quibble about the exact numbers right now. There was a lot of
discussion about them, but I can't remember what the decision was. The
important thing now is that the test passes. If someone can argue the
number should be different, let's file that as a separate bug.
Reviewers: scottb,
Description:
Don't devirtualize calls from AsyncLoader123.runAsync to
AsyncLoader123.runCallbacks. Doing so spoils code
splitting for runAsync calls that are only reachable
via other runAsync calls.
Review by: sco...@google.com
Please review this at
This kind of patch is awfully tricky. Adding a JRunAsync node to the AST
would make it a lot more straightforward.
I found it very hard to make a test case with the current factoring.
What we could do is divide CodeSplitter into two parts, one that does
the code mapping and one that divides up
Thanks!
http://gwt-code-reviews.appspot.com/399803/diff/1/2
File dev/core/src/com/google/gwt/dev/jjs/impl/FragmentLoaderCreator.java
(right):
http://gwt-code-reviews.appspot.com/399803/diff/1/2#newcode112
dev/core/src/com/google/gwt/dev/jjs/impl/FragmentLoaderCreator.java:112:
Committed at r7925.
http://gwt-code-reviews.appspot.com/353801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM.
http://gwt-code-reviews.appspot.com/354801/diff/1/2
File
dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java
(left):
http://gwt-code-reviews.appspot.com/354801/diff/1/2#oldcode426
dev/core/src/com/google/gwt/dev/jjs/impl/CompoundAssignmentNormalizer.java:426:
Sorry for the delay. Managing the latest GWT snapshot branch has taken
all my time.
This is a sweet optimization overall.
I believe two little things should be modified. The one about equality
can wait, if you'd prefer, but it seems like an easy patch that will
make it much more effective.
http://gwt-code-reviews.appspot.com/405801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/354801/diff/1/6
File dev/core/test/com/google/gwt/dev/jjs/impl/TempLocalVisitorTest.java
(right):
http://gwt-code-reviews.appspot.com/354801/diff/1/6#newcode1
dev/core/test/com/google/gwt/dev/jjs/impl/TempLocalVisitorTest.java:1:
// Copyright 2009 Google Inc.
Thanks, Mike! Description updated.
http://gwt-code-reviews.appspot.com/405801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: scottb, aizatsky,
Description:
The gflow constant analyzer assumes that parameters
could initially hold anything, rather than assuming
they haven't been initialized.
Please review this at http://gwt-code-reviews.appspot.com/405801/show
Affected files:
M
@Mike: does the gflow change look reasonable?
@Scott: the new test case fundamentally needs that some method
parameters be in scope of the test snippet. Does the approach in this
patch look reasonable?
http://gwt-code-reviews.appspot.com/405801/show
--
LGTM.
An additional way to make such a test more reliable is to choose
floating point numbers that are exactly representable, such as 1.5 and
1.25, and thus don't depend on single- versus double-precision.
http://gwt-code-reviews.appspot.com/312804/show
--
LGTM
http://gwt-code-reviews.appspot.com/318802/diff/1/2
File dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java
(right):
http://gwt-code-reviews.appspot.com/318802/diff/1/2#newcode619
dev/core/src/com/google/gwt/dev/jjs/impl/gflow/cfg/CfgBuilder.java:619:
CfgSwitchGotoNode
LGTM
http://gwt-code-reviews.appspot.com/298804/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
To unsubscribe, reply using remove me as the subject.
1 - 100 of 511 matches
Mail list logo
