Fix a race condition in the GWTTestCase runner.

If a web browser was quick to report its first results to the GWTTestCase
servlet, RunStyle.launchModule() was sometimes called twice. This caused
an unexpected page reload, so there were more clients than expected
but the first one was gone, resulting in the test timing out.

The race is most likely when the first test block is small, for example
when the first test class is in a different module than the rest and
has few test methods.

The race was in notDone() where the code would check the number of clients
and then later check if it got enough test results back. If the first check
failed but the second one succeeded, notDone() could return false without
setting firstLaunch to false.

Fixed by replacing the firstLaunch variable with two variables that have
clearer purposes.

Change-Id: Id75762a714fd489f624734308017ac53cddf886f
Review-Link: https://gwt-review.googlesource.com/#/c/2150/

Review by: mdempsky@google.com

git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@11547 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/user/src/com/google/gwt/junit/JUnitShell.java b/user/src/com/google/gwt/junit/JUnitShell.java
index 1924078..3ae26a9 100644
--- a/user/src/com/google/gwt/junit/JUnitShell.java
+++ b/user/src/com/google/gwt/junit/JUnitShell.java
@@ -788,9 +788,14 @@
   private boolean developmentMode = true;
 
   /**
-   * If true, no launches have yet been successful.
+   * Used to make sure we don't start the runStyle more than once.
    */
-  private boolean firstLaunch = true;
+  private boolean runStyleStarted;
+
+  /**
+   * If true, we haven't started all the clients yet. (Used for manual mode.)
+   */
+  private boolean waitingForClients = true;
 
   /**
    * If true, the last attempt to launch failed.
@@ -945,17 +950,18 @@
   protected boolean notDone() {
     int activeClients = messageQueue.getNumClientsRetrievedTest(currentTestInfo);
     int expectedClients = messageQueue.getNumClients();
-    if (firstLaunch && runStyle instanceof RunStyleManual) {
+    if (runStyle instanceof RunStyleManual && waitingForClients) {
       String[] newClients = messageQueue.getNewClients();
       int printIndex = activeClients - newClients.length + 1;
       for (String newClient : newClients) {
         System.out.println(printIndex + " - " + newClient);
         ++printIndex;
       }
-      if (activeClients != expectedClients) {
+      if (activeClients < expectedClients) {
         // Wait forever for first contact; user-driven.
         return true;
       }
+      waitingForClients = false;
     }
 
     // Limit permutations after all clients have connected.
@@ -983,7 +989,6 @@
             "Too many clients: expected " + expectedClients + ", found "
                 + activeClients);
       }
-      firstLaunch = false;
 
       /*
        * It's now safe to release any reference to the last module since all
@@ -1311,7 +1316,7 @@
     compileStrategy.maybeAddTestBlockForCurrentTest(testCase, batchingStrategy);
 
     try {
-      if (firstLaunch) {
+      if (!runStyleStarted) {
         runStyle.launchModule(currentModule.getName());
       }
     } catch (UnableToCompleteException e) {
@@ -1319,6 +1324,7 @@
       testResult.addError(testCase, new JUnitFatalLaunchException(e));
       return;
     }
+    runStyleStarted = true;
 
     boolean mustRetry = mustRetry(numTries);
     // Wait for test to complete