Revert "Make Safari tests more robust."

tools/testing/dart/browser_controller.dart

Index: tools/testing/dart/browser_controller.dart
diff --git a/tools/testing/dart/browser_controller.dart b/tools/testing/dart/browser_controller.dart
index 7094120661ad9d45ee880021dce68a5be28e1d92..27765697ea3b74db45fe86f46381cd51199c2df6 100644
--- a/tools/testing/dart/browser_controller.dart
+++ b/tools/testing/dart/browser_controller.dart
@@ -14,9 +14,6 @@ import 'http_server.dart';
 import 'path.dart';
 import 'utils.dart';
 
-import 'reset_safari.dart' show
-    killAndResetSafari;
-
 class BrowserOutput {
   final StringBuffer stdout = new StringBuffer();
   final StringBuffer stderr = new StringBuffer();
@@ -56,10 +53,10 @@ abstract class Browser {
   String id;
 
   /**
-   * Reset the browser to a known configuration on start-up.
+   * Delete the browser specific caches on startup.
    * Browser specific implementations are free to ignore this.
    */
-  static bool resetBrowserConfiguration = false;
+  static bool deleteCache = false;
 
   /** Print everything (stdout, stderr, usageLog) whenever we add to it */
   bool debugPrint = false;
@@ -108,15 +105,6 @@ abstract class Browser {
     'ie10'
   ];
 
-  /// If [browserName] doesn't support Window.open, we use iframes instead.
-  static bool requiresIframe(String browserName) {
-    return !BROWSERS_WITH_WINDOW_SUPPORT.contains(browserName);
-  }
-
-  static bool requiresFocus(String browserName) {
-    return browserName == "safari";
-  }
-
   // TODO(kustermann): add standard support for chrome on android
   static bool supportedBrowser(String name) {
     return SUPPORTED_BROWSERS.contains(name);
@@ -281,13 +269,6 @@ abstract class Browser {
 
   /** Starts the browser loading the given url */
   Future<bool> start(String url);
-
-  /// Called when the driver page is requested, that is, when the browser first
-  /// contacts the test server. At this time, it's safe to assume that the
-  /// browser process has started and opened its first window.
-  ///
-  /// This is used by [Safari] to ensure the browser window has focus.
-  Future<Null> onDriverPageRequested() => new Future<Null>.value();
 }
 
 class Safari extends Browser {
@@ -297,48 +278,62 @@ class Safari extends Browser {
   static const String versionFile =
       "/Applications/Safari.app/Contents/version.plist";
 
-  static const String safariBundleLocation = "/Applications/Safari.app/";
+  /**
+   * Directories where safari stores state. We delete these if the deleteCache
+   * is set
+   */
+  static const List<String> CACHE_DIRECTORIES = const [
+    "Library/Caches/com.apple.Safari",
+    "Library/Safari",
+    "Library/Saved Application State/com.apple.Safari.savedState",
+    "Library/Caches/Metadata/Safari"
+  ];
 
-  // Clears the cache if the static resetBrowserConfiguration flag is set.
-  // Returns false if the command to actually clear the cache did not complete.
-  Future<bool> resetConfiguration() async {
-    if (!Browser.resetBrowserConfiguration) return true;
+  Future<bool> allowPopUps() {
+    var command = "defaults";
+    var args = [
+      "write",
+      "com.apple.safari",
+      "com.apple.Safari.ContentPageGroupIdentifier."
+          "WebKit2JavaScriptCanOpenWindowsAutomatically",
+      "1"
+    ];
+    return Process.run(command, args).then((result) {
+      if (result.exitCode != 0) {
+        _logEvent("Could not disable pop-up blocking for safari");
+        return false;
+      }
+      return true;
+    });
+  }
 
-    Completer completer = new Completer();
-    handleUncaughtError(error, StackTrace stackTrace) {
-      if (!completer.isCompleted) {
-        completer.completeError(error, stackTrace);
+  Future<bool> deleteIfExists(Iterator<String> paths) {
+    if (!paths.moveNext()) return new Future.value(true);
+    Directory directory = new Directory(paths.current);
+    return directory.exists().then((exists) {
+      if (exists) {
+        _logEvent("Deleting ${paths.current}");
+        return directory
+            .delete(recursive: true)
+            .then((_) => deleteIfExists(paths))
+            .catchError((error) {
+          _logEvent("Failure trying to delete ${paths.current}: $error");
+          return false;
+        });
       } else {
-        throw new AsyncError(error, stackTrace);
+        _logEvent("${paths.current} is not present");
+        return deleteIfExists(paths);
       }
-    }
-    Zone parent = Zone.current;
-    ZoneSpecification specification = new ZoneSpecification(
-        print: (Zone self, ZoneDelegate delegate, Zone zone, String line) {
-          delegate.run(parent, () {
-            _logEvent(line);
-          });
-        });
-    Future zoneWrapper() {
-      Uri safariUri = Uri.base.resolve(safariBundleLocation);
-      return new Future(() => killAndResetSafari(bundle: safariUri))
-          .then(completer.complete);
-    }
-
-    // We run killAndResetSafari in a Zone as opposed to running an external
-    // process. The Zone allows us to collect its output, and protect the rest
-    // of the test infrastructure against errors in it.
-    runZoned(
-        zoneWrapper, zoneSpecification: specification,
-        onError: handleUncaughtError);
+    });
+  }
 
-    try {
-      await completer.future;
-      return true;
-    } catch (error, st) {
-      _logEvent("Unable to reset Safari: $error$st");
-      return false;
-    }
+  // Clears the cache if the static deleteCache flag is set.
+  // Returns false if the command to actually clear the cache did not complete.
+  Future<bool> clearCache() {
+    if (!Browser.deleteCache) return new Future.value(true);
+    var home = Platform.environment['HOME'];
+    Iterator iterator = CACHE_DIRECTORIES.map((s) => "$home/$s").iterator;
+    return deleteIfExists(iterator);
   }
 
   Future<String> getVersion() {
@@ -374,58 +369,42 @@ class Safari extends Browser {
     });
   }
 
-  Future<Null> _createLaunchHTML(var path, var url) async {
+  void _createLaunchHTML(var path, var url) {
     var file = new File("${path}/launch.html");
-    var randomFile = await file.open(mode: FileMode.WRITE);
+    var randomFile = file.openSync(mode: FileMode.WRITE);
     var content = '<script language="JavaScript">location = "$url"</script>';
-    await randomFile.writeString(content);
-    await randomFile.close();
+    randomFile.writeStringSync(content);
+    randomFile.close();
   }
 
-  Future<bool> start(String url) async {
+  Future<bool> start(String url) {
     _logEvent("Starting Safari browser on: $url");
-    if (!await resetConfiguration()) {
-      _logEvent("Could not clear cache");
-      return false;
-    }
-    String version;
-    try {
-      version = await getVersion();
-    } catch (error) {
-      _logEvent("Running $_binary --version failed with $error");
-      return false;
-    }
-    _logEvent("Got version: $version");
-    Directory userDir;
-    try {
-      userDir = await Directory.systemTemp.createTemp();
-    } catch (error) {
-      _logEvent("Error creating temporary directory: $error");
-      return false;
-    }
-    _cleanup = () {
-      userDir.deleteSync(recursive: true);
-    };
-    try {
-      await _createLaunchHTML(userDir.path, url);
-    } catch (error) {
-      _logEvent("Error creating launch HTML: $error");
-      return false;
-    }
-    var args = [
-        "-d", "-i", "-m", "-s", "-u", _binary,
-        "${userDir.path}/launch.html"];
-    try {
-      return startBrowserProcess("/usr/bin/caffeinate", args);
-    } catch (error) {
-      _logEvent("Error starting browser process: $error");
-      return false;
-    }
-  }
-
-  Future<Null> onDriverPageRequested() async {
-    await Process.run("/usr/bin/osascript",
-        ['-e', 'tell application "Safari" to activate']);
+    return allowPopUps().then((success) {
+      if (!success) {
+        return false;
+      }
+      return clearCache().then((cleared) {
+        if (!cleared) {
+          _logEvent("Could not clear cache");
+          return false;
+        }
+        // Get the version and log that.
+        return getVersion().then((version) {
+          _logEvent("Got version: $version");
+          return Directory.systemTemp.createTemp().then((userDir) {
+            _cleanup = () {
+              userDir.deleteSync(recursive: true);
+            };
+            _createLaunchHTML(userDir.path, url);
+            var args = ["${userDir.path}/launch.html"];
+            return startBrowserProcess(_binary, args);
+          });
+        }).catchError((error) {
+          _logEvent("Running $_binary --version failed with $error");
+          return false;
+        });
+      });
+    });
   }
 
   String toString() => "Safari";
@@ -498,16 +477,16 @@ class Chrome extends Browser {
 class SafariMobileSimulator extends Safari {
   /**
    * Directories where safari simulator stores state. We delete these if the
-   * resetBrowserConfiguration is set
+   * deleteCache is set
    */
   static const List<String> CACHE_DIRECTORIES = const [
     "Library/Application Support/iPhone Simulator/7.1/Applications"
   ];
 
-  // Clears the cache if the static resetBrowserConfiguration flag is set.
+  // Clears the cache if the static deleteCache flag is set.
   // Returns false if the command to actually clear the cache did not complete.
-  Future<bool> resetConfiguration() {
-    if (!Browser.resetBrowserConfiguration) return new Future.value(true);
+  Future<bool> clearCache() {
+    if (!Browser.deleteCache) return new Future.value(true);
     var home = Platform.environment['HOME'];
     Iterator iterator = CACHE_DIRECTORIES.map((s) => "$home/$s").iterator;
     return deleteIfExists(iterator);
@@ -515,7 +494,7 @@ class SafariMobileSimulator extends Safari {
 
   Future<bool> start(String url) {
     _logEvent("Starting safari mobile simulator browser on: $url");
-    return resetConfiguration().then((success) {
+    return clearCache().then((success) {
       if (!success) {
         _logEvent("Could not clear cache, exiting");
         return false;
@@ -582,10 +561,9 @@ class IE extends Browser {
     });
   }
 
-  // Clears the recovery cache if the static resetBrowserConfiguration flag is
-  // set.
-  Future<bool> resetConfiguration() {
-    if (!Browser.resetBrowserConfiguration) return new Future.value(true);
+  // Clears the recovery cache if the static deleteCache flag is set.
+  Future<bool> clearCache() {
+    if (!Browser.deleteCache) return new Future.value(true);
     var localAppData = Platform.environment['LOCALAPPDATA'];
 
     Directory dir = new Directory("$localAppData\\Microsoft\\"
@@ -600,7 +578,7 @@ class IE extends Browser {
 
   Future<bool> start(String url) {
     _logEvent("Starting ie browser on: $url");
-    return resetConfiguration().then((_) => getVersion()).then((version) {
+    return clearCache().then((_) => getVersion()).then((version) {
       _logEvent("Got version: $version");
       return startBrowserProcess(_binary, [url]);
     });
@@ -898,12 +876,12 @@ class BrowserTestRunner {
   static const Duration MIN_NONEMPTY_QUEUE_TIME = const Duration(seconds: 1);
 
   final Map configuration;
-  final BrowserTestingServer testingServer;
+  BrowserTestingServer testingServer;
 
   final String localIp;
-  final String browserName;
-  final int maxNumBrowsers;
-  final bool checkedMode;
+  String browserName;
+  int maxNumBrowsers;
+  bool checkedMode;
   int numBrowsers = 0;
   // Used to send back logs from the browser (start, stop etc)
   Function logger;
@@ -950,23 +928,27 @@ class BrowserTestRunner {
     if (_currentStartingBrowserId == id) _currentStartingBrowserId = null;
   }
 
+  // If [browserName] doesn't support opening new windows, we use new iframes
+  // instead.
+  bool get useIframe =>
+      !Browser.BROWSERS_WITH_WINDOW_SUPPORT.contains(browserName);
+
+  /// The optional testingServer parameter allows callers to pass in
+  /// a testing server with different behavior than the  default
+  /// BrowserTestServer. The url handlers of the testingServer are
+  /// overwritten, so an existing handler can't be shared between instances.
   BrowserTestRunner(
-      Map configuration,
-      String localIp,
-      String browserName,
-      this.maxNumBrowsers)
-      : configuration = configuration,
-        localIp = localIp,
-        browserName = (browserName == 'ff') ? 'firefox' : browserName,
-        checkedMode = configuration['checked'],
-        testingServer = new BrowserTestingServer(
-            configuration, localIp,
-            Browser.requiresIframe(browserName),
-            Browser.requiresFocus(browserName)) {
-    testingServer.testRunner = this;
+      this.configuration, this.localIp, this.browserName, this.maxNumBrowsers,
+      {BrowserTestingServer this.testingServer}) {
+    checkedMode = configuration['checked'];
+    if (browserName == 'ff') browserName = 'firefox';
   }
 
   Future start() async {
+    if (testingServer == null) {
+      testingServer =
+          new BrowserTestingServer(configuration, localIp, useIframe);
+    }
     await testingServer.start();
     testingServer
       ..testDoneCallBack = handleResults
@@ -1303,9 +1285,6 @@ class BrowserTestingServer {
   ///                                   test
 
   final String localIp;
-  final bool useIframe;
-  final bool requiresFocus;
-  BrowserTestRunner testRunner;
 
   static const String driverPath = "/driver";
   static const String nextTestPath = "/next_test";
@@ -1318,14 +1297,14 @@ class BrowserTestingServer {
   var testCount = 0;
   var errorReportingServer;
   bool underTermination = false;
+  bool useIframe = false;
 
   Function testDoneCallBack;
   Function testStatusUpdateCallBack;
   Function testStartedCallBack;
   Function nextTestCallBack;
 
-  BrowserTestingServer(
-      this.configuration, this.localIp, this.useIframe, this.requiresFocus);
+  BrowserTestingServer(this.configuration, this.localIp, this.useIframe);
 
   Future start() {
     var test_driver_error_port = configuration['test_driver_error_port'];
@@ -1387,41 +1366,29 @@ class BrowserTestingServer {
       handleStarted(request, browserId(request, startedPath), testId(request));
     });
 
-    void sendPageHandler(HttpRequest request) {
-      // Do NOT make this method async. We need to call catchError below
-      // synchronously to avoid unhandled asynchronous errors.
-      noCache(request);
-      Future<String> textResponse;
-      if (request.uri.path.startsWith(driverPath)) {
-        textResponse = getDriverPage(browserId(request, driverPath));
-        request.response.headers.set('Content-Type', 'text/html');
-      } else if (request.uri.path.startsWith(nextTestPath)) {
-        textResponse = new Future<String>.value(
-            getNextTest(browserId(request, nextTestPath)));
-        request.response.headers.set('Content-Type', 'text/plain');
-      } else {
-        textResponse = new Future<String>.value("");
-      }
-      request.response.done.catchError((error) {
-        if (!underTermination) {
-          return textResponse.then((String text) {
-            print("URI ${request.uri}");
-            print("textResponse $textResponse");
-            throw "Error returning content to browser: $error";
+    makeSendPageHandler(String prefix) => (HttpRequest request) {
+          noCache(request);
+          var textResponse = "";
+          if (prefix == driverPath) {
+            textResponse = getDriverPage(browserId(request, prefix));
+            request.response.headers.set('Content-Type', 'text/html');
+          }
+          if (prefix == nextTestPath) {
+            textResponse = getNextTest(browserId(request, prefix));
+            request.response.headers.set('Content-Type', 'text/plain');
+          }
+          request.response.write(textResponse);
+          request.listen((_) {}, onDone: request.response.close);
+          request.response.done.catchError((error) {
+            if (!underTermination) {
+              print("URI ${request.uri}");
+              print("Textresponse $textResponse");
+              throw "Error returning content to browser: $error";
+            }
           });
-        }
-      });
-      textResponse.then((String text) async {
-        request.response.write(text);
-        await request.listen(null).asFuture();
-        // Ignoring the returned closure as it returns the 'done' future
-        // which alread has catchError installed above.
-        request.response.close();
-      });
-    }
-
-    server.addHandler(driverPath, sendPageHandler);
-    server.addHandler(nextTestPath, sendPageHandler);
+        };
+    server.addHandler(driverPath, makeSendPageHandler(driverPath));
+    server.addHandler(nextTestPath, makeSendPageHandler(nextTestPath));
   }
 
   void handleReport(HttpRequest request, String browserId, var testId,
@@ -1480,34 +1447,33 @@ class BrowserTestingServer {
     return "http://$localIp:$port/driver/$browserId";
   }
 
-  Future<String> getDriverPage(String browserId) async {
-    await testRunner.browserStatus[browserId].browser.onDriverPageRequested();
+  String getDriverPage(String browserId) {
     var errorReportingUrl =
         "http://$localIp:${errorReportingServer.port}/$browserId";
     String driverContent = """
 <!DOCTYPE html><html>
 <head>
-  <title>Driving page</title>
   <style>
-.big-notice {
-  background-color: red;
-  color: white;
-  font-weight: bold;
-  font-size: xx-large;
-  text-align: center;
-}
-.controller.box {
-  white-space: nowrap;
-  overflow: scroll;
-  height: 6em;
-}
-body {
-  font-family: sans-serif;
-}
-body div {
-  padding-top: 10px;
-}
+    body {
+      margin: 0;
+    }
+    .box {
+      overflow: hidden;
+      overflow-y: auto;
+      position: absolute;
+      left: 0;
+      right: 0;
+    }
+    .controller.box {
+      height: 75px;
+      top: 0;
+    }
+    .test.box {
+      top: 75px;
+      bottom: 0;
+    }
   </style>
+  <title>Driving page</title>
   <script type='text/javascript'>
     var STATUS_UPDATE_INTERVAL = 10000;
 
@@ -1637,8 +1603,7 @@ body div {
           embedded_iframe_div.removeChild(embedded_iframe);
           embedded_iframe = document.createElement('iframe');
           embedded_iframe.id = "embedded_iframe";
-          embedded_iframe.width='800px';
-          embedded_iframe.height='600px';
+          embedded_iframe.style="width:100%;height:100%";
           embedded_iframe_div.appendChild(embedded_iframe);
           embedded_iframe.src = url;
         } else {
@@ -1810,26 +1775,13 @@ body div {
   </script>
 </head>
   <body onload="startTesting()">
-
-    <div class='big-notice'>
-      Please keep this window in focus at all times.
-    </div>
-
-    <div>
-      Some browsers, Safari, in particular, may pause JavaScript when not
-      visible to conserve power consumption and CPU resources. In addition,
-      some tests of focus events will not work correctly if this window doesn't
-      have focus. It's also advisable to close any other programs that may open
-      modal dialogs, for example, Chrome with Calendar open.
-    </div>
-
     <div class="controller box">
     Dart test driver, number of tests: <span id="number"></span><br>
     Currently executing: <span id="currently_executing"></span><br>
     Unhandled error: <span id="unhandled_error"></span>
     </div>
     <div id="embedded_iframe_div" class="test box">
-      <iframe id="embedded_iframe"></iframe>
+      <iframe style="width:100%;height:100%;" id="embedded_iframe"></iframe>
     </div>
   </body>
 </html>