Add armor for exceptions thrown by Activity#onCancel Review at http://gwt-code-reviews.appspot.com/1446816 git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10272 8db76d5a-ed1c-0410-87a9-c151d255dfc7

diff --git a/user/src/com/google/gwt/activity/shared/ActivityManager.java b/user/src/com/google/gwt/activity/shared/ActivityManager.java
index 45f41b5..8cb9a33 100644
--- a/user/src/com/google/gwt/activity/shared/ActivityManager.java
+++ b/user/src/com/google/gwt/activity/shared/ActivityManager.java

@@ -95,13 +95,12 @@
    * be minimized by decent caching. Perenially slow activities might mitigate
    * this by providing a widget immediately, with some kind of "loading"
    * treatment.
-   * 
-   * @see com.google.gwt.place.shared.PlaceChangeEvent.Handler#onPlaceChange(PlaceChangeEvent)
    */
   public void onPlaceChange(PlaceChangeEvent event) {
     Activity nextActivity = getNextActivity(event);
 
     Throwable caughtOnStop = null;
+    Throwable caughtOnCancel = null;
     Throwable caughtOnStart = null;
 
     if (nextActivity == null) {
@@ -115,7 +114,7 @@
     if (startingNext) {
       // The place changed again before the new current activity showed its
       // widget
-      currentActivity.onCancel();
+      caughtOnCancel = tryStopOrCancel(false);
       currentActivity = NULL_ACTIVITY;
       startingNext = false;
     } else if (!currentActivity.equals(NULL_ACTIVITY)) {
@@ -126,46 +125,26 @@
        * them accidentally firing as a side effect of its tear down
        */
       stopperedEventBus.removeHandlers();
-
-      try {
-        currentActivity.onStop();
-      } catch (Throwable t) {
-        caughtOnStop = t;
-      } finally {
-        /*
-         * And kill them off again in case it was naughty and added new ones
-         * during onstop
-         */
-        stopperedEventBus.removeHandlers();
-      }
+      caughtOnStop = tryStopOrCancel(true);
     }
 
     currentActivity = nextActivity;
 
     if (currentActivity.equals(NULL_ACTIVITY)) {
       showWidget(null);
-      return;
+    } else {
+      startingNext = true;
+      caughtOnStart = tryStart();
     }
-
-    startingNext = true;
-
-    /*
-     * Now start the thing. Wrap the actual display with a per-call instance
-     * that protects the display from canceled or stopped activities, and which
-     * maintains our startingNext state.
-     */
-    try {
-      currentActivity.start(new ProtectedDisplay(currentActivity),
-          stopperedEventBus);
-    } catch (Throwable t) {
-      caughtOnStart = t;
-    }
-
-    if (caughtOnStart != null || caughtOnStop != null) {
+    
+    if (caughtOnStart != null || caughtOnCancel != null || caughtOnStop != null) {
       Set<Throwable> causes = new LinkedHashSet<Throwable>();
       if (caughtOnStop != null) {
         causes.add(caughtOnStop);
       }
+      if (caughtOnCancel != null) {
+        causes.add(caughtOnCancel);
+      }
       if (caughtOnStart != null) {
         causes.add(caughtOnStart);
       }
@@ -180,9 +159,7 @@
    * @see com.google.gwt.place.shared.PlaceChangeRequestEvent.Handler#onPlaceChangeRequest(PlaceChangeRequestEvent)
    */
   public void onPlaceChangeRequest(PlaceChangeRequestEvent event) {
-    if (!currentActivity.equals(NULL_ACTIVITY)) {
-      event.setWarning(currentActivity.mayStop());
-    }
+    event.setWarning(currentActivity.mayStop());
   }
 
   /**
@@ -204,6 +181,41 @@
     }
   }
 
+  public Throwable tryStart() {
+    Throwable caughtOnStart = null;
+    try {
+      /* Wrap the actual display with a per-call instance
+       * that protects the display from canceled or stopped activities, and which
+       * maintains our startingNext state.
+       */
+      currentActivity.start(new ProtectedDisplay(currentActivity),
+          stopperedEventBus);
+    } catch (Throwable t) {
+      caughtOnStart = t;
+    }
+    return caughtOnStart;
+  }
+
+  public Throwable tryStopOrCancel(boolean stop) {
+    Throwable caughtOnStop = null;
+    try {
+      if (stop) {
+        currentActivity.onStop();
+      } else {
+        currentActivity.onCancel();
+      }
+    } catch (Throwable t) {
+      caughtOnStop = t;
+    } finally {
+      /*
+       * Kill off the handlers again in case it was naughty and added new ones
+       * during onstop or oncancel
+       */
+      stopperedEventBus.removeHandlers();
+    }
+    return caughtOnStop;
+  }
+
   private Activity getNextActivity(PlaceChangeEvent event) {
     if (display == null) {
       /*

diff --git a/user/test/com/google/gwt/activity/shared/ActivityManagerTest.java b/user/test/com/google/gwt/activity/shared/ActivityManagerTest.java
index ece9341..62a4af8 100644
--- a/user/test/com/google/gwt/activity/shared/ActivityManagerTest.java
+++ b/user/test/com/google/gwt/activity/shared/ActivityManagerTest.java

@@ -305,18 +305,18 @@
     assertEquals(0, eventBus.getCount(PlaceChangeRequestEvent.TYPE));
   }
 
-  public void testExceptionsOnStopAndStart() {
-    activity1 = new SyncActivity(null) {
+  public void testExceptionsOnStartAndCancel() {
+    activity1 = new AsyncActivity(null) {
       @Override
       public void start(AcceptsOneWidget panel, EventBus eventBus) {
         super.start(panel, eventBus);
         bus.addHandler(Event.TYPE, new Handler());
       }
       @Override
-      public void onStop() {
-        super.onStop();
+      public void onCancel() {
+        super.onCancel();
         bus.addHandler(Event.TYPE, new Handler());
-        throw new UnsupportedOperationException("Auto-generated method stub");
+        throw new UnsupportedOperationException("Exception on cancel");
       }
     };
 
@@ -324,7 +324,7 @@
       @Override
       public void start(AcceptsOneWidget panel, EventBus eventBus) {
         super.start(panel, eventBus);
-        throw new UnsupportedOperationException("Auto-generated method stub");
+        throw new UnsupportedOperationException("Exception on start");
       }
     };
 
@@ -340,9 +340,56 @@
 
       fail("Expected exception");
     } catch (UmbrellaException e) {
-      // HandlerManager throws this one
+      // EventBus throws this one
       assertEquals(1, e.getCauses().size());
+      // And this is the one thrown by ActivityManager
+      UmbrellaException nested = (UmbrellaException) e.getCause();
+      assertEquals(2, nested.getCauses().size());
+    }
 
+    assertTrue(activity1.canceled);
+    assertNotNull(activity2.display);
+    assertEquals(0, eventBus.getCount(Event.TYPE));
+  }
+  
+  public void testExceptionsOnStopAndStart() {
+    activity1 = new SyncActivity(null) {
+      @Override
+      public void start(AcceptsOneWidget panel, EventBus eventBus) {
+        super.start(panel, eventBus);
+        bus.addHandler(Event.TYPE, new Handler());
+      }
+      @Override
+      public void onStop() {
+        super.onStop();
+        bus.addHandler(Event.TYPE, new Handler());
+        throw new UnsupportedOperationException("Exception on stop");
+      }
+    };
+
+    activity2 = new SyncActivity(null) {
+      @Override
+      public void start(AcceptsOneWidget panel, EventBus eventBus) {
+        super.start(panel, eventBus);
+        throw new UnsupportedOperationException("Exception on start");
+      }
+    };
+
+    manager.setDisplay(realDisplay);
+
+    try {
+      PlaceChangeEvent event = new PlaceChangeEvent(place1);
+      eventBus.fireEvent(event);
+      assertEquals(1, eventBus.getCount(Event.TYPE));
+
+      event = new PlaceChangeEvent(place2);
+      eventBus.fireEvent(event);
+
+      fail("Expected exception");
+    } catch (UmbrellaException e) {
+      // EventBus throws this one
+      assertEquals(1, e.getCauses().size());
+      // And this is the one thrown by ActivityManager
       UmbrellaException nested = (UmbrellaException) e.getCause();
       assertEquals(2, nested.getCauses().size());
     }