Add debugging info to browser controller.

tools/testing/dart/browser_controller.dart

 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 library browser;
 
 import "dart:async";
 import "dart:core";
 import "dart:io";
 
 import 'android.dart';
@@ skipping 108 matching lines @@
     return browserTerminationFuture;
   }
 
   /**
    * Start the browser using the supplied argument.
    * This sets up the error handling and usage logging.
    */
   Future<bool> startBrowser(String command, List<String> arguments) {
     return Process.start(command, arguments).then((startedProcess) {
       process = startedProcess;
+      Completer stdoutDone = new Completer();
+      Completer stderrDone = new Completer();
+
       process.stdout.transform(new StringDecoder()).listen((data) {
         _addStdout(data);
       }, onError: (error) {
         // This should _never_ happen, but we really want this in the log
         // if it actually does due to dart:io or vm bug.
         _logEvent("An error occured in the process stdout handling: $error");
+      }, onDone: () {
+        stdoutDone.complete(true);
       });
 
       process.stderr.transform(new StringDecoder()).listen((data) {
         _addStderr(data);
       }, onError: (error) {
         // This should _never_ happen, but we really want this in the log
         // if it actually does due to dart:io or vm bug.
         _logEvent("An error occured in the process stderr handling: $error");
+      },  onDone: () {
+        stderrDone.complete(true);
       });
 
       process.exitCode.then((exitCode) {
         _logEvent("Browser closed with exitcode $exitCode");
-        if (_processClosed != null) _processClosed();
-        if (_cleanup != null) _cleanup();
-        if (onClose != null) onClose(exitCode);
+        Future.wait([stdoutDone.future, stderrDone.future]).then((_) {
+          if (_processClosed != null) _processClosed();
+          if (_cleanup != null) _cleanup();
+          if (onClose != null) onClose(exitCode);
+        });
       });
       return true;
     }).catchError((error) {
       _logEvent("Running $command $arguments failed with $error");
       return false;
     });
   }
 
   /**
    * Get any stdout that the browser wrote during execution.
@@ skipping 302 matching lines @@
  * The interface is rather simple. After starting the runner tests
  * are simply added to the queue and a the supplied callbacks are called
  * whenever a test completes.
  */
 class BrowserTestRunner {
   String local_ip;
   String browserName;
   int maxNumBrowsers;
   // Used to send back logs from the browser (start, stop etc)
   Function logger;
+  int browserIdCount = 0;
 
   bool underTermination = false;
 
   List<BrowserTest> testQueue = new List<BrowserTest>();
   Map<String, BrowserTestingStatus> browserStatus =
       new Map<String, BrowserTestingStatus>();
 
   var adbDeviceMapping = new Map<String, AdbDevice>();
   // This cache is used to guarantee that we never see double reporting.
   // If we do we need to provide developers with this information.
@@ skipping 51 matching lines @@
             browser.id = id;
           }
           browsersCompleter.complete(browsers);
         } else {
           throw new StateError("No android devices found.");
         }
       });
     } else {
       var browsers = [];
       for (int i = 0; i < maxNumBrowsers; i++) {
-        var id = "BROWSER$i";
+        var id = "BROWSER$browserIdCount";
+        browserIdCount++;
         var browser = getInstance();
         browsers.add(browser);
         // We store this in case we need to kill the browser.
         browser.id = id;
       }
       browsersCompleter.complete(browsers);
     }
     return browsersCompleter.future;
   }
 
   var timedOut = [];
 
   void handleResults(String browserId, String output, int testId) {
     var status = browserStatus[browserId];
+    DebugLogger.info("Handling result for browser ${browserId}");
     if (testCache.containsKey(testId)) {
       doubleReportingTests.add(testId);
       return;
     }
 
     if (status.timeout) {
       // We don't do anything, this browser is currently being killed and
       // replaced.
     } else if (status.currentTest != null) {
       status.currentTest.timeoutTimer.cancel();
@@ skipping 15 matching lines @@
       print(output);
       terminate().then((_) {
         exit(1);
       });
     }
   }
 
   void handleTimeout(BrowserTestingStatus status) {
     // We simply kill the browser and starts up a new one!
     // We could be smarter here, but it does not seems like it is worth it.
+    DebugLogger.info("Handling timeout for browser ${status.browser.id}");
     status.timeout = true;
     timedOut.add(status.currentTest.url);
     var id = status.browser.id;
     status.browser.close().then((closed) {
       if (!closed) {
         // Very bad, we could not kill the browser.
         print("could not kill browser $id");
         return;
       }
       // We don't want to start a new browser if we are terminating.
       if (underTermination) return;
-
       var browser;
+      var new_id = id;
       if (browserName == 'chromeOnAndroid') {
         browser = new AndroidChrome(adbDeviceMapping[id]);
       } else {
+        browserStatus.remove(id);
         browser = getInstance();
+        new_id = "BROWSER$browserIdCount";
+        browserIdCount++;
+        browser.id = new_id;

[kustermann, 2013/06/12 12:52:09]

Please move this 2 lines down after the else branch to set this in the android
case as well.

[ricow1, 2013/06/12 12:54:30]

Done.

+        browserStatus[browser.id] = new BrowserTestingStatus(browser);
       }
-      browser.start(testingServer.getDriverUrl(id)).then((success) {
+      browser.start(testingServer.getDriverUrl(new_id)).then((success) {
         // We may have started terminating in the mean time.
         if (underTermination) {
           browser.close().then((success) {
             // We should never hit this, print it out.
             if (!success) {
               print("Could not kill browser ($id) started due to timeout");
             }
           });
           return;
         }
         if (success) {
-          browser.id = id;
-          status.browser = browser;
-          status.timeout = false;
+          browserStatus[browser.id] = new BrowserTestingStatus(browser);
         } else {
           // TODO(ricow): Handle this better.
           print("This is bad, should never happen, could not start browser");
           exit(1);
         }
       });
     });
 
     status.currentTest.doneCallback("TIMEOUT");
     status.currentTest = null;
   }
 
   BrowserTest getNextTest(String browserId) {
     if (testQueue.isEmpty) return null;
     var status = browserStatus[browserId];
     if (status == null) return null;
+    DebugLogger.info("Handling getNext for browser ${browserId}"
+                     " timeout status: ${status.timeout}");
+
     // We are currently terminating this browser, don't start a new test.
     if (status.timeout) return null;
     BrowserTest test = testQueue.removeLast();
     if (status.currentTest == null) {
       status.currentTest = test;
     } else {
       // TODO(ricow): Handle this better.
       print("This is bad, should never happen, getNextTest all full");
+      print("This happened for browser $browserId");
       print("Old test was: ${status.currentTest.url}");
       print("Timed out tests:");
       for (var v in timedOut) {
         print("  $v");
       }
       exit(1);
     }
     Timer timer = new Timer(new Duration(seconds: test.timeout),
                             () { handleTimeout(status); });
     status.currentTest.timeoutTimer = timer;
@@ skipping 253 matching lines @@
 </head>
   <body onload="startTesting()">
     Dart test driver, number of tests: <div id="number"></div>
     <iframe id="embedded_iframe"></iframe>
   </body>
 </html>
 """;
     return driverContent;
   }
 }