LGTM
http://gwt-code-reviews.appspot.com/1879804/
--
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM. General potentially unrelated question, what about the case
static class Covariant extends Itemint[]
Is this handled properly?
http://gwt-code-reviews.appspot.com/1874803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/11/29 03:35:49, skybrian wrote:
LGTM. No API/Source breakages this time around?
http://gwt-code-reviews.appspot.com/1872803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/11/06 22:28:54, Nick Chalko wrote:
LGTM
http://gwt-code-reviews.appspot.com/1865803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
You could also return a Live Map implementation so that the Map always
reflects the most up to date values even if getParameterMap() isn't
called. The same caching logic would apply.
On 2012/10/31 20:19:01, perneto wrote:
One option is to have an extra method like encodeForHtmlContext() where
you only pay the tax in that circumstance.
http://gwt-code-reviews.appspot.com/1853803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1851804/diff/3001/plugins/xpcom/JavaObject.cpp
File plugins/xpcom/JavaObject.cpp (right):
http://gwt-code-reviews.appspot.com/1851804/diff/3001/plugins/xpcom/JavaObject.cpp#newcode151
plugins/xpcom/JavaObject.cpp:151: return JavaObject::getProperty(ctx,
LGTM
http://gwt-code-reviews.appspot.com/1850804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
On 2012/09/29 03:44:59, skybrian wrote:
http://gwt-code-reviews.appspot.com/1844803/diff/1/plugins/npapi/Makefile
File plugins/npapi/Makefile (right):
http://gwt-code-reviews.appspot.com/1844803/diff/1/plugins/npapi/Makefile#newcode17
plugins/npapi/Makefile:17: ARCH=x86
On 2012/09/29
Hey Thomas, great comments, thanks. I will update the patch shortly with
fixes. One issue is that V8 doesn't seem to be as great at GC as I
thought, and I'm going to have to speak to the Chrome team. I thought
originally that anything not reachable from GC roots would ultimately be
collected,
Reviewers: skybrian,
Description:
Add experimental GWT.unloadModule() function.
Enable via set-property name=gwt.unloadEnabled value=true/
Invoke via window.__gwt_activeModules[moduleName].unloadModule().
Attempts to do the following:
1) Cleanup all outstanding non-primitive properties from
On 2012/09/09 05:06:43, jtamplin wrote:
LGTM
LGTM+1. Thomas you are a machine.
https://gwt-code-reviews.appspot.com/1825803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1817803/diff/1/plugins/xpcom/JavaObject.cpp
File plugins/xpcom/JavaObject.cpp (right):
http://gwt-code-reviews.appspot.com/1817803/diff/1/plugins/xpcom/JavaObject.cpp#newcode385
plugins/xpcom/JavaObject.cpp:385: (JSVAL_IS_PRIMITIVE(argv[1])
LGTM
http://gwt-code-reviews.appspot.com/1800807/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1797803/diff/1/dev/core/src/com/google/gwt/dev/js/BaselineCoverageGatherer.java
File dev/core/src/com/google/gwt/dev/js/BaselineCoverageGatherer.java
(right):
LGTM
http://gwt-code-reviews.appspot.com/1793803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
lgtm
http://gwt-code-reviews.appspot.com/1794804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thomas, I went ahead and landed this because we need to start testing
2.5 for release and this patch is unwieldly large. I will follow up with
much smaller patches that fix the various issues. :)
http://gwt-code-reviews.appspot.com/1728806/
--
On 2012/06/15 00:48:51, skybrian wrote:
LGTM
http://gwt-code-reviews.appspot.com/1740803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1740803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1731805/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Thomas,
I'm a little knee deep in I/O slide stuff at the moment, so maybe you
can sanity check my thinking here. Let me describe what used to be
happening in the Json stuff and what I was trying to change it to before
2.5, but probably didn't finish.
Essentially, in ProdMode I wanted to run
LGTM
http://gwt-code-reviews.appspot.com/1739804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: skybrian,
Description:
Elemental + build scripts initial import.
Review by: skybr...@google.com
Please review this at http://gwt-code-reviews.appspot.com/1728806/
Affected files:
A elemental/META-INF/MANIFEST.MF
A elemental/README
A elemental/STOP.EXPERIMENTAL
A
LGTM
http://gwt-code-reviews.appspot.com/1730804/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/java/com/google/gwt/dev/codeserver/Options.java
File dev/codeserver/java/com/google/gwt/dev/codeserver/Options.java
(right):
LGTM, I didn't review the Java files because I assume they are mostly
unchanged since the last time.
http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/BUILD
File dev/codeserver/BUILD (right):
http://gwt-code-reviews.appspot.com/1727804/diff/1/dev/codeserver/BUILD#newcode1
LGTM
http://gwt-code-reviews.appspot.com/1726803/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/CodeSplitter2.java
File dev/core/src/com/google/gwt/dev/jjs/impl/CodeSplitter2.java
(right):
http://gwt-code-reviews.appspot.com/1709803/diff/1/user/src/com/google/gwt/dom/client/Style.java
File user/src/com/google/gwt/dom/client/Style.java (right):
http://gwt-code-reviews.appspot.com/1709803/diff/1/user/src/com/google/gwt/dom/client/Style.java#newcode1764
lgtm
http://gwt-code-reviews.appspot.com/1677803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1677803/diff/4001/user/src/com/google/gwt/core/server/ServerGwtBridge.java
File user/src/com/google/gwt/core/server/ServerGwtBridge.java (right):
LGTM, but I wonder if we should hide it behind a flag, e.g.
if(Boolean.parseBoolean(getProperty(gwt.diskcache.compress, false)))
{ // compression code }
else { // no compression }
http://gwt-code-reviews.appspot.com/1672803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM, although I wonder how much effect it has on build time. I can't
imagine anyone ever wanting crappier icons, but if it say, doubled build
time, we can always add an option like set-configuration-property
name=imageBundle.draftQuality value=true/ or something.
LGTM.
On 2012/03/15 20:12:33, stephenh wrote:
Working on a test...hold on.
Okay, I couldn't figure how to write a GWTTestCase for this because
the
formatter applies to the text the user currently has selected. I
didn't see
anything in the existing RichTextAreaText that covered
LGTM, although you may want to add a !-- -- doc section in
CompilerParams prior to the property to explain what it is doing (turn
off symbol map, and therefore it disables StackTraceDeobfuscation in the
logging API)
http://gwt-code-reviews.appspot.com/1652803/
--
LGTM
http://gwt-code-reviews.appspot.com/1654803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
On 2012/03/02 10:39:39, acleung wrote:
Are you missing the module files (.gwt.xml) that define this new
property?
http://gwt-code-reviews.appspot.com/1652803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
I typically put these in CompilerParameters.gwt.xml or Core.gwt.xml. For
example, the sourceMap switch is in CompilerParameters.gwt.xml
On 2012/03/02 23:40:31, acleung wrote:
On 2012/03/02 22:46:28, rdayal wrote:
On Fri Mar 02 13:09:00 GMT-500 2012, mailto:cromwell...@google.com
wrote:
LGTM
http://gwt-code-reviews.appspot.com/1639803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1639803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1631803/diff/1/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
File dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java (right):
http://gwt-code-reviews.appspot.com/1631803/diff/1/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java#newcode449
http://gwt-code-reviews.appspot.com/1618807/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
(right):
LGTM. Sorry, as an IntelliJ user, Eclipse often escapes my mind. :)
http://gwt-code-reviews.appspot.com/1606803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1594803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: danielwh_google.com,
Description:
Upgrade GWT's htmlunit dependencies.
Please review this at http://gwt-code-reviews.appspot.com/1590804/
Affected files:
M dev/build.xml
M dev/core/src/com/google/gwt/dev/shell/HtmlUnitSessionHandler.java
M eclipse/dev/.classpath
M
);
+
+ // TODO(cromwellian) handle all types
+ if (arg1 instanceof JsNumberLiteral arg2 instanceof
JsNumberLiteral) {
+ double num1 = ((JsNumberLiteral) arg1).getValue();
+ double num2 = ((JsNumberLiteral) arg2).getValue();
+ boolean result = false;
+ switch(op
LGTM
http://gwt-code-reviews.appspot.com/1590803/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaScriptAST.java
(right):
Bizarre.
LGTM.
http://gwt-code-reviews.appspot.com/1559803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1558803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: unnurg,
Description:
Add SourceMap support to GWT
See http://code.google.com/p/google-web-toolkit/wiki/SourceMaps or
details.
Please review this at http://gwt-code-reviews.appspot.com/1558803/
Affected files:
M dev/build.xml
M
http://gwt-code-reviews.appspot.com/1544803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1544803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Reviewers: rjrjr,
Description:
When -XdisableClassMetadata is used, Class.getName() can return
Class$SseedNumber as a class name. However, there are other modes
where it can return Class$obfuscated function name. In some rare
cases, these two could collide of if an obfuscated name of a class
lgtm
http://gwt-code-reviews.appspot.com/1532803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1532803/diff/1/user/src/com/google/web/bindery/event/shared/UmbrellaException.java
File user/src/com/google/web/bindery/event/shared/UmbrellaException.java
(right):
http://gwt-code-reviews.appspot.com/1513803/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/CodeSplitter.java
File dev/core/src/com/google/gwt/dev/jjs/impl/CodeSplitter.java (right):
Reviewers: jbrosenberg,
Description:
Re-roll r10435
Changes logic to assign seedIds even to non-instantiable types so that
Foo.class.getName() will return something sensible when
-XdisableClassMetadata
is used.
Please review this at http://gwt-code-reviews.appspot.com/1474804/
Affected
http://gwt-code-reviews.appspot.com/1447821/diff/6053/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
(right):
http://gwt-code-reviews.appspot.com/1447821/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM, needs test?
http://gwt-code-reviews.appspot.com/1467815/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1447821/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
Should we really be fixing the issue here, or should we just fix it up
in GenerateJavaScriptAST or JsNamer? There are lots of locations in the
GWT compiler where methods and fields are synthesized, so I would bet
that there are other collisions like this lurking, just not being
tripped.
Another
LGTM
http://gwt-code-reviews.appspot.com/1470802/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
I made the fixes and submitted it. Since most people are headed out of
the office for the holiday, I need to get it in early to see if I need
to revert it before everyone is gone. :)
http://gwt-code-reviews.appspot.com/1447821/diff/9006/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/1447821/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1454806/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1454806/diff/1/dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java
File dev/core/src/com/google/gwt/dev/js/JsDuplicateFunctionRemover.java
(right):
LGTM. You're right, you can't do it statically because the array type
can be from a cast.
http://gwt-code-reviews.appspot.com/1470801/diff/1003/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Array.java
File
http://gwt-code-reviews.appspot.com/1447821/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1447821/diff/3001/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(right):
LGTM
http://gwt-code-reviews.appspot.com/1470801/diff/1/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java
File user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java
(right):
http://gwt-code-reviews.appspot.com/1470801/diff/1/user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java
File user/test/com/google/gwt/dev/jjs/test/SingleJsoImplTest.java
(right):
On 2011/06/27 15:09:23, zundel wrote:
On 2011/06/08 23:26:58, cromwellian wrote:
Can you help me understand what the hoist fucntions are about?
Normally, virtual methods are declared inline, e.g.
function Foo() {}
_ = Foo.prototype = new ...
_.myMethod = function myMethod(a,b) { code
Overall LGTM. I had to update CFA in my recent CL on class literal
optimization to treat an invocation of Object.getClass() as rescuing the
class literals of any instantiated types as well as to handle the new
Immortal CodeGenTypes.
We may have to revisit the UnifyAstVisitor, since it may need
Reviewers: scottb, jbrosenberg,
Description:
This patch substantially reduces the overhead of Java types in the
output by minimizing vtable setup and virtual class literal fetches.
Improvements are anywhere from 5% to 10% uncompressed obfuscated JS.
The patch includes the following changes:
1)
Reviewers: scottb, jbrosenberg,
Description:
Enhancement to JsDuplicateFunctionRemover to remove duplicate virtual
methods as well. That is, given
_.method1 = function(a) { body }
_.method2 = function(b) { body }
Hoist the duplicate virtual methods and replace with a named global
function.
http://gwt-code-reviews.appspot.com/1447821/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
General question: I'm curious why this couldn't be simpler, that is, why
does unification depend on control flow? Couldn't you unify everything
without respect to control flow, and then run a pruning pass afterwards
reusing the existing CFA code, or is doing it in an integrated pass an
attempt to
On 2011/06/03 23:06:36, jbrosenberg wrote:
What's the motivation for this?
http://gwt-code-reviews.appspot.com/1454801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM, I wonder how many more of these implicit upcasts are hiding :)
http://gwt-code-reviews.appspot.com/1449812/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1451801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1449801/diff/1/dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java
File
dev/core/src/com/google/gwt/dev/jjs/impl/ToStringGenerationVisitor.java
(left):
LGTM
http://gwt-code-reviews.appspot.com/1425813/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1427808/diff/1/dev/core/src/com/google/gwt/dev/jjs/ast/JGwtCreate.java
File dev/core/src/com/google/gwt/dev/jjs/ast/JGwtCreate.java (right):
Reviewers: scottb,
Description:
Fix cast normalizer to properly deal with multidimensional JSO arrays.
Please review this at http://gwt-code-reviews.appspot.com/1428803/
Affected files:
M dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java
M
http://gwt-code-reviews.appspot.com/1407802/diff/6001/user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java
File user/src/com/google/gwt/autobean/client/impl/JsoSplittable.java
(right):
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
File
dev/core/src/com/google/gwt/dev/jjs/impl/PostOptimizationCompoundAssignmentNormalizer.java
(right):
http://gwt-code-reviews.appspot.com/1389803/diff/1/dev/core/super/com/google/gwt/lang/LongLibBase.java
File dev/core/super/com/google/gwt/lang/LongLibBase.java (right):
http://gwt-code-reviews.appspot.com/1389803/diff/1/dev/core/super/com/google/gwt/lang/LongLibBase.java#newcode325
http://gwt-code-reviews.appspot.com/1385803/diff/2002/dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
File dev/core/src/com/google/gwt/dev/jjs/JavaToJavaScriptCompiler.java
(right):
Reviewers: scottb,
Description:
Operations like i += d where i is an int and d is a double are not
properly
truncated (narrowed) to the LHS type. This patch forces i += d to be
written as
i = i + d, and applies a narrowing cast as needed, e.g. i = (int)(i +
d);
Updated to fix String concat
http://gwt-code-reviews.appspot.com/1385803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1385803/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
LGTM
http://gwt-code-reviews.appspot.com/1386801/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
http://gwt-code-reviews.appspot.com/1351801/show
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
/Cast.java
(right):
http://gwt-code-reviews.appspot.com/1351801/diff/1/4#newcode92
dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Cast.java:92:
return src != null (isNonStringJavaObject(src) || isJavaString(src));
Reverted.
On 2011/02/09 01:53:46, cromwellian wrote:
On 2011
LGTM
http://gwt-code-reviews.appspot.com/1352804/diff/1/4
File dev/core/src/com/google/gwt/dev/jjs/SourceInfoCorrelation.java
(right):
http://gwt-code-reviews.appspot.com/1352804/diff/1/4#newcode68
dev/core/src/com/google/gwt/dev/jjs/SourceInfoCorrelation.java:68:
primaryCorrelations[index]
Reviewers: scottb,
Description:
Eliminates empty constructor seed functions by using 2 helper functions
to setup
anonymous closure versions. Something like this:
function fooSeed() {}
_ = fooSeed.prototype = FooConstructor.prototype = new superType();
_.castableTypeMap = { ... }
_.getClass =
LGTM
http://gwt-code-reviews.appspot.com/1345801/diff/2001/2004
File dev/core/src/com/google/gwt/core/ext/soyc/Member.java (left):
http://gwt-code-reviews.appspot.com/1345801/diff/2001/2004#oldcode44
dev/core/src/com/google/gwt/core/ext/soyc/Member.java:44:
Hypothetically, would the
LGTM. I don't its worth nuking the 'dims' field, I mean, feel free, but
I'm ok with leaving it as a transient cache.
http://gwt-code-reviews.appspot.com/1352803/diff/1/5
File dev/core/src/com/google/gwt/dev/jjs/impl/ArrayNormalizer.java
(right):
1 - 100 of 178 matches
Mail list logo
