Thanks, Bob!  What do you think about the error messages and about the
isLoaded method?

I'll post an updated patch in just a minute.


http://gwt-code-reviews.appspot.com/102801/diff/1/43
File dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
(right):

http://gwt-code-reviews.appspot.com/102801/diff/1/43#newcode189
Line 189: error("Multiple runASync calls are named " + name);
Done.  Here is an example trace:

    Replacing GWT.runAsync with island loader calls
       [ERROR] Multiple runAsync calls are named
com.google.gwt.sample.showcase.client.content.other.CwAnimation
          [ERROR] One call is in
com.google.gwt.sample.showcase.client.content.panels.CwAbsolutePanel.asyncOnInitialize
(file:/Users/spoon/gwt/git/samples/showcase/src/com/google/gwt/sample/showcase/client/content/panels/CwAbsolutePanel.java:201)
          [ERROR] One call is in
com.google.gwt.sample.showcase.client.content.other.CwAnimation.asyncOnInitialize
(file:/Users/spoon/gwt/git/samples/showcase/src/com/google/gwt/sample/showcase/client/content/other/CwAnimation.java:247)
       [ERROR] No runAsync call is named
com.google.gwt.sample.showcase.client.content.panels.CwAbsolutePanel

http://gwt-code-reviews.appspot.com/102801/diff/1/43#newcode223
Line 223: private static String nameFromClassLiteral(JClassLiteral
classLiteral) {
It's called twice.  Once in the new code, in ReplaceRunAsyncResources,
and once in the preexisting AsyncCreateVisitor.

http://gwt-code-reviews.appspot.com/102801/diff/1/43#newcode243
Line 243: private void execImpl() throws UnableToCompleteException {
This code runs before the AST is fully set up.  The errors generated are
just like for errors found by findEntryPoints() or
CodeSplitter.pickInitialLoadSequence().  In particular, there is no ICE
here.  See the above error log trace for an example of what it looks
like to users.

What's really lacking here is detection during development mode.  I
thought about getting into that, but there are some problems.  Most
notably, it fundamentally requires a whole-program scan, so it's one
more challenge to being lazy in development mode about what code is
loaded.  On the flip side, the benefit is marginal in this case, because
anyone tinkering with prefetching needs to be using production mode.

Do you think this is a blocking issue?  We can certainly add a test
earlier on, but it will take a little more development.

http://gwt-code-reviews.appspot.com/102801/diff/1/3
File
samples/showcase/src/com/google/gwt/sample/showcase/client/ContentWidget.java
(right):

http://gwt-code-reviews.appspot.com/102801/diff/1/3#newcode124
Line 124: private static <T> List<T> list(T... elems) {
On 2009/11/12 16:19:53, bobv wrote:
> Replace this with Arrays.asList()

Done.

http://gwt-code-reviews.appspot.com/102801/diff/1/39
File
samples/showcase/src/com/google/gwt/sample/showcase/client/content/lists/CwMenuBar.java
(right):

http://gwt-code-reviews.appspot.com/102801/diff/1/39#newcode35
Line 35: @ShowcaseStyle( {
Yes.  What should be done, though?  Other committers are removing the
"value = " bits.

http://gwt-code-reviews.appspot.com/102801/diff/1/45
File user/src/com/google/gwt/core/client/prefetch/Prefetcher.java
(right):

http://gwt-code-reviews.appspot.com/102801/diff/1/45#newcode33
Line 33: public static void prefetch(Iterable<? extends
PrefetchableResource> resources) {
I'm not sure what you mean?  Prefetcher.prefetch/start/stop is the API
on the design wave.

http://gwt-code-reviews.appspot.com/102801/diff/1/45#newcode51
Line 51: throw new RuntimeException("Unknown resource type: "
On 2009/11/12 16:19:53, bobv wrote:
> IllegalArgumentException

Done.

http://gwt-code-reviews.appspot.com/102801/diff/1/47
File user/src/com/google/gwt/core/client/prefetch/RunAsyncCode.java
(right):

http://gwt-code-reviews.appspot.com/102801/diff/1/47#newcode57
Line 57: public boolean isLoaded() {
I thought it would be good there, too, but there was some debate on what
the superinterface should have, exactly.  I put it here on the thinking
that that question will be clearer once we have at least one more
PrefetchableResource implemented.

http://gwt-code-reviews.appspot.com/102801/diff/1/44
File
user/test/com/google/gwt/core/client/impl/AsyncFragmentLoaderTest.java
(right):

http://gwt-code-reviews.appspot.com/102801/diff/1/44#newcode146
Line 146: private static List<Integer> list(int... elems) {
On 2009/11/12 16:19:53, bobv wrote:
> Arrays.asList() instead.

Done.

http://gwt-code-reviews.appspot.com/102801

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

Reply via email to