Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(657)

Unified Diff: lib/src/runner/browser/phantom_js.dart

Issue 1175163003: Factor out some common logic from the launchers. (Closed) Base URL: git@github.com:dart-lang/test@master
Patch Set: fix test Created 5 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « lib/src/runner/browser/internet_explorer.dart ('k') | lib/src/runner/browser/safari.dart » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: lib/src/runner/browser/phantom_js.dart
diff --git a/lib/src/runner/browser/phantom_js.dart b/lib/src/runner/browser/phantom_js.dart
index 06247008d05b0f94a8299dd94ab0a71b96bec941..9776d9b4c7f3372f9ff409b749ae3306a910b819 100644
--- a/lib/src/runner/browser/phantom_js.dart
+++ b/lib/src/runner/browser/phantom_js.dart
@@ -5,15 +5,12 @@
library test.runner.browser.phantom_js;
import 'dart:async';
-import 'dart:convert';
import 'dart:io';
import 'package:path/path.dart' as p;
-import 'package:stack_trace/stack_trace.dart';
import '../../util/exit_codes.dart' as exit_codes;
import '../../util/io.dart';
-import '../../utils.dart';
import '../application_exception.dart';
import 'browser.dart';
@@ -39,77 +36,42 @@ page.open(system.args[1], function(status) {
/// A class for running an instance of PhantomJS.
///
/// Any errors starting or running the process are reported through [onExit].
-class PhantomJS implements Browser {
- /// The underlying process.
- Process _process;
+class PhantomJS extends Browser {
+ final name = "PhantomJS";
- Future get onExit => _onExitCompleter.future;
- final _onExitCompleter = new Completer();
-
- /// A future that completes when the browser process has started.
- ///
- /// This is used to ensure that [close] works regardless of when it's called.
- Future get _onProcessStarted => _onProcessStartedCompleter.future;
- final _onProcessStartedCompleter = new Completer();
+ PhantomJS(url, {String executable})
+ : super(() => _startBrowser(url, executable));
/// Starts a new instance of PhantomJS open to the given [url], which may be a
/// [Uri] or a [String].
///
/// If [executable] is passed, it's used as the PhantomJS executable.
/// Otherwise the default executable name for the current OS will be used.
- PhantomJS(url, {String executable}) {
+ static Future<Process> _startBrowser(url, [String executable]) async {
if (executable == null) {
executable = Platform.isWindows ? "phantomjs.exe" : "phantomjs";
}
- // Don't return a Future here because there's no need for the caller to wait
- // for the process to actually start. They should just wait for the HTTP
- // request instead.
- invoke(() async {
- try {
- var exitCode = await withTempDir((dir) async {
- var script = p.join(dir, "script.js");
- new File(script).writeAsStringSync(_script);
-
- var process = await Process.start(
- executable, [script, url.toString()]);
-
- // PhantomJS synchronously emits standard output, which means that if we
- // don't drain its stdout stream it can deadlock.
- process.stdout.listen((_) {});
-
- _process = process;
- _onProcessStartedCompleter.complete();
-
- return await _process.exitCode;
- });
-
- if (exitCode == exit_codes.protocol) {
- throw new ApplicationException(
- "Only PhantomJS version 2.0.0 or greater is supported");
- }
-
- if (exitCode != 0) {
- var error = await UTF8.decodeStream(_process.stderr);
- throw new ApplicationException(
- "PhantomJS failed with exit code $exitCode:\n$error");
- }
-
- _onExitCompleter.complete();
- } catch (error, stackTrace) {
- if (stackTrace == null) stackTrace = new Trace.current();
- _onExitCompleter.completeError(
- new ApplicationException(
- "Failed to start PhantomJS: ${getErrorMessage(error)}."),
- stackTrace);
+ var dir = createTempDir();
+ var script = p.join(dir, "script.js");
+ new File(script).writeAsStringSync(_script);
+
+ var process = await Process.start(
+ executable, [script, url.toString()]);
+
+ // PhantomJS synchronously emits standard output, which means that if we
+ // don't drain its stdout stream it can deadlock.
+ process.stdout.listen((_) {});
+
+ process.exitCode.then((exitCode) {
+ new Directory(dir).deleteSync(recursive: true);
+
+ if (exitCode == exit_codes.protocol) {
+ throw new ApplicationException(
+ "Only PhantomJS version 2.0.0 or greater is supported");
}
});
- }
-
- Future close() {
- _onProcessStarted.then((_) => _process.kill());
- // Swallow exceptions. The user should explicitly use [onExit] for these.
- return onExit.catchError((_) {});
+ return process;
}
}
« no previous file with comments | « lib/src/runner/browser/internet_explorer.dart ('k') | lib/src/runner/browser/safari.dart » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698