LGTM.
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() {"); I think terminated is fine. The only other possibility would be something paralleling onReadyState, like LoadStateEvent/LoadStateChangeHandler. On 2010/03/19 21:51:16, Lex wrote:
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#newcode79 dev/core/src/com/google/gwt/dev/jjs/impl/HandleCrossIslandReferences.java:79: public void endVisit(JsVars x, JsContext<JsStatement> ctx) { I don't think it's all that important. A comment would probably have the same effect, it just took me some extra thought when reviewing to convince myself it wasn't going to be a problem. I say leave it alone. On 2010/03/22 16:16:32, Lex wrote:
I haven't thought of a real simple way to do this. How important does
this
extra sanity checking look?
http://gwt-code-reviews.appspot.com/213801/diff/1/8#newcode174 dev/core/src/com/google/gwt/dev/jjs/impl/HandleCrossIslandReferences.java:174: } I agree, you're right, deleting the variable makes sense. On 2010/03/19 21:51:16, Lex wrote:
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 the issue has cropped up before, but I agree, we can easily document that GWT modules require a <head>. On 2010/03/19 21:51:16, Lex wrote:
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/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.
