http://gwt-code-reviews.appspot.com/1386806/diff/1/user/src/com/google/gwt/activity/client/RunAsyncActivity.java
File user/src/com/google/gwt/activity/client/RunAsyncActivity.java
(right):

http://gwt-code-reviews.appspot.com/1386806/diff/1/user/src/com/google/gwt/activity/client/RunAsyncActivity.java#newcode66
user/src/com/google/gwt/activity/client/RunAsyncActivity.java:66:
GWT.runAsync(new RunAsyncCallback() {
I'm not used to GWT.runAsync but won't this create a single split-point
for all provided activities, rather than a split-point for each of them
(as it would be the case with AsyncProvider)?
If that's the case, I'd rather remove that overload and mandate an
AsyncProvider.

http://gwt-code-reviews.appspot.com/1386806/diff/1/user/src/com/google/gwt/activity/client/RunAsyncActivity.java#newcode87
user/src/com/google/gwt/activity/client/RunAsyncActivity.java:87:
wrapped.start(panel, eventBus);
What if the activity is started, cancelled before it loads, and started
again?
As coded here, it'll start twice. I guess GWT.runAsync takes care of the
ongoing script load and doesn't try to load it twice; but that's not
enough here, because the "cancel" call will be lost, and actually, the
wrapped activity from the first onSuccess will be lost in "started"
state.
Moreover, AsyncProvider might return a new Activity instance each time
it's called, which makes it impossible to control the actually Activity
life-time from the ActivityMapper (you might return the same
RunAsyncActivity instance, but it doesn't mean you'll have a single
instance of the wrapped activity).

I once started coding such an AsyncActivityWrapper but never actually
tested it. I stored the panel and eventBus in fields and only
async-loaded the activity once to make sure the callback is only called
once; and I didn't attempt to async-load the activity if it was already
loaded, or loading. Something like:
  if (loaded()) {
    wrapped.start(panel,eventBus);
  } else {
    this.panel = panel;
    this.eventBus = eventBus;
    if (!loading()) { // only ever trigger a single async load
      asyncProvider.get(...
        public void onSuccess(Activity result) {
          wrapped = result;
          wrapped.start(RunAsyncActivity.this.panel,
              RunAsyncActivity.this.eventBus);
        }
        ...
      });
      loading = true;
    }

As I said, I never actually tested my code, so maybe it's not worth
it...

http://gwt-code-reviews.appspot.com/1386806/diff/1/user/src/com/google/gwt/activity/client/RunAsyncActivity.java#newcode100
user/src/com/google/gwt/activity/client/RunAsyncActivity.java:100:
wrapped.onCancel();
How about an 'else { cancelled = true; }' and in the onSuccess then only
call the activity's start() if !cancelled?

The same would have to be done in onStop() too (even though it shouldn't
ever happen, because the activity shouldn't stopped if it hasn't yet
"fully started" by calling the AcceptsOneWidget back)

(either a new boolean field or a specific value assigned to wrapped)

As coded here, if you cancel the activity before it's loaded, the
onSuccess will start it anyway and the onCancel will be lost.

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

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

Reply via email to