Simplify close functionality in the 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';
 import 'utils.dart';
 
 /** Class describing the interface for communicating with browsers. */
 abstract class Browser {
   StringBuffer _stdout = new StringBuffer();
   StringBuffer _stderr = new StringBuffer();
   StringBuffer _usageLog = new StringBuffer();
-  // This function is called when the process is closed.
-  Completer _processClosedCompleter = new Completer();
-  // This is called after the process is closed, after _processClosedCompleter
-  // has been called, but before onExit. Subclasses can use this to cleanup
-  // any browser specific resources (temp directories, profiles, etc)
-  // The function is expected to do it's work synchronously.
+  // This is called after the process is closed, before the done future
+  // is completed.
+  // Subclasses can use this to cleanup any browser specific resources
+  // (temp directories, profiles, etc). The function is expected to do
+  // it's work synchronously.
   Function _cleanup;
 
   /** The version of the browser - normally set when starting a browser */
   String version = "";
   /**
    * The underlying process - don't mess directly with this if you don't
    * know what you are doing (this is an interactive process that needs
    * special threatment to not leak).
    */
   Process process;
 
   Function logger;
 
   /**
    * Id of the browser
    */
   String id;
 
   /** Print everything (stdout, stderr, usageLog) whenever we add to it */
   bool debugPrint = false;
 
-  // This future will be lazily set when calling close() and will complete once
-  // the process did exit.
-  Future browserTerminationFuture;
+  // This future returns when the process exits. It is also the return value
+  // of close()
+  Future done;
 
   Browser();
 
   factory Browser.byName(String name) {
     if (name == 'ff' || name == 'firefox') {
       return new Firefox();
     } else if (name == 'chrome') {
       return new Chrome();
     } else if (name == 'safari') {
       return new Safari();
@@ skipping 25 matching lines @@
     _stdout.write(output);
   }
 
   void _addStderr(String output) {
     if (debugPrint) print("stderr: $output");
     _stderr.write(output);
   }
 
   Future close() {
     _logEvent("Close called on browser");
-    if (browserTerminationFuture == null) {
-      var completer = new Completer();
-      browserTerminationFuture = completer.future;
-
-      if (process != null) {
-        _processClosedCompleter.future.then((_) {
-          process = null;
-          completer.complete(true);
-          if (_cleanup != null) {
-            _cleanup();
-          }
-        });
-
-        if (process.kill(ProcessSignal.SIGKILL)) {
-          _logEvent("Successfully sent kill signal to process.");
-        } else {
-          _logEvent("Sending kill signal failed.");
-        }
+    if (process != null) {
+      if (process.kill(ProcessSignal.SIGKILL)) {
+        _logEvent("Successfully sent kill signal to process.");
       } else {
-        _logEvent("The process is already dead.");
-        completer.complete(true);
+        _logEvent("Sending kill signal failed.");
       }
+      return done;
+    } else {
+      _logEvent("The process is already dead.");
+      return new Future.immediate(true);
     }
-    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;
+      // Used to notify when exiting, and as a return value on calls to
+      // close().
+      var doneCompleter = new Completer();
+      done = doneCompleter.future;
+
       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");
         Future.wait([stdoutDone.future, stderrDone.future]).then((_) {
-          _processClosedCompleter.complete(exitCode);
+          process = null;
+          if (_cleanup != null) {
+            _cleanup();
+          }
+          doneCompleter.complete(exitCode);
         });
       });
       return true;
     }).catchError((error) {
       _logEvent("Running $command $arguments failed with $error");
       return false;
     });
   }
 
   /**
@@ skipping 487 matching lines @@
     }
   }
 
   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;
-      }
+    status.browser.close().then((_) {
       // 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";
@@ skipping 379 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;
   }
 }