Thanks, Ray!  I'll update the patch on Monday.

I share your unease about the code increase due to stringifying.  Like
you suggest, I was thinking this would be a simple approach to start
with that we can optimize over time.  Do you have any ideas that would
be pretty simple?

If not, we might try the current thing and then optimize over time.  One
wild trick I have heard people suggest is to put the code in a /*...*/
comment and try to fish it back out.  However, that gets into possibly
browser-specific behavior, not to mention needing more development
time....


http://gwt-code-reviews.appspot.com/213801/diff/1/7
File dev/core/src/com/google/gwt/dev/jjs/impl/FragmentLoaderCreator.java
(right):

http://gwt-code-reviews.appspot.com/213801/diff/1/7#newcode164
dev/core/src/com/google/gwt/dev/jjs/impl/FragmentLoaderCreator.java:164:
srcWriter.println("  new AsyncFragmentLoader.LoadFinishedHandler() {");
What shall we call it?  LoadTerminatedHandler, maybe?

Most of the time there was definitely an error, but in one case it is
ambiguous.  Specifically, IE's script tag has a callback that informs
you that the download has stopped, but it doesn't tell you whether it
stopped successfully or not.

http://gwt-code-reviews.appspot.com/213801/diff/1/8
File
dev/core/src/com/google/gwt/dev/jjs/impl/HandleCrossIslandReferences.java
(right):

http://gwt-code-reviews.appspot.com/213801/diff/1/8#newcode50
dev/core/src/com/google/gwt/dev/jjs/impl/HandleCrossIslandReferences.java:50:
*/
No.

I guess I'll change it to HandleCrossFragmentReferences to be
consistent.  This is an implementation class, after all, so it may as
well use implementation terminology.

http://gwt-code-reviews.appspot.com/213801/diff/1/8#newcode79
dev/core/src/com/google/gwt/dev/jjs/impl/HandleCrossIslandReferences.java:79:
public void endVisit(JsVars x, JsContext<JsStatement> ctx) {
It will end up in the reads set, too.  Like you say, though, they won't
have any cross-island references.  The reads and writes will all be on
the same island.

It would be possible to check that all predefined variables are at top
scope.  That sounds like an assertion, and a perfectly reasonable one to
include.  The only thing is, I blanking on a convenient way to implement
that assertion.  Neither JsVar nor JsContext provides a convenient way
to check whether the node is at top scope.  Hmm.

http://gwt-code-reviews.appspot.com/213801/diff/1/8#newcode174
dev/core/src/com/google/gwt/dev/jjs/impl/HandleCrossIslandReferences.java:174:
}
Yes, that's possible.  It should be safe, though.  Let's see if you
agree.

Suppose the variable is named foo.  All accesses to it will be rewritten
as jslink.foo.  Thus, leaving the var in place would be a noop.  It will
never be used.

Instead, jslink.foo will be read and written.  If it is written before
it is read, then it acts just like a variable.  If it is read before it
is written, it will return undefined, also just like a variable.  So I
believe it is equivalent to the original "var foo".

Unless you see something that breaks with the rewrite, I'll update the
header comment to be more accurate.  "If not, then leave it alone" is
misleading.

http://gwt-code-reviews.appspot.com/213801/diff/1/16
File
user/src/com/google/gwt/core/client/impl/CrossSiteLoadingStrategy.java
(right):

http://gwt-code-reviews.appspot.com/213801/diff/1/16#newcode62
user/src/com/google/gwt/core/client/impl/CrossSiteLoadingStrategy.java:62:
private static native JavaScriptObject createScriptTag(String url) /*-{
I believe you're right, but do we need to worry about it?  This code all
runs after the </html> has been seen, and it would seem like a minor
restriction if people have to add a <head> tag to use GWT.

http://gwt-code-reviews.appspot.com/213801/diff/1/16#newcode114
user/src/com/google/gwt/core/client/impl/CrossSiteLoadingStrategy.java:114:
__gwtModuleFunction['runAsyncCallback'+fragment] = callback;
Yes.  That was an oversight.

http://gwt-code-reviews.appspot.com/213801/show

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

To unsubscribe from this group, send email to 
google-web-toolkit-contributors+unsubscribegooglegroups.com or reply to this email with 
the words "REMOVE ME" as the subject.

Reply via email to