Unverified Commit 030dc3fa authored by Alexander Aprelev's avatar Alexander Aprelev Committed by GitHub

Don't send accept/reject if compilation never started. (#27319)

* Don't send accept/reject if compilation never started.

Ensure that we wait for reject's response since that is async operation.

Fixes https://github.com/flutter/flutter/issues/27120.

* Fix analysis errors

* Rename flag. Ensure we raise it on first compilation too.
parent cfe992d0
...@@ -346,6 +346,14 @@ class _CompileExpressionRequest extends _CompilationRequest { ...@@ -346,6 +346,14 @@ class _CompileExpressionRequest extends _CompilationRequest {
compiler._compileExpression(this); compiler._compileExpression(this);
} }
class _RejectRequest extends _CompilationRequest {
_RejectRequest(Completer<CompilerOutput> completer): super(completer);
@override
Future<CompilerOutput> _run(ResidentCompiler compiler) async =>
compiler._reject();
}
/// Wrapper around incremental frontend server compiler, that communicates with /// Wrapper around incremental frontend server compiler, that communicates with
/// server via stdin/stdout. /// server via stdin/stdout.
/// ///
...@@ -389,6 +397,7 @@ class ResidentCompiler { ...@@ -389,6 +397,7 @@ class ResidentCompiler {
String _initializeFromDill; String _initializeFromDill;
bool _unsafePackageSerialization; bool _unsafePackageSerialization;
final List<String> _experimentalFlags; final List<String> _experimentalFlags;
bool _compileRequestNeedsConfirmation = false;
final StreamController<_CompilationRequest> _controller; final StreamController<_CompilationRequest> _controller;
...@@ -428,6 +437,8 @@ class ResidentCompiler { ...@@ -428,6 +437,8 @@ class ResidentCompiler {
); );
} }
_compileRequestNeedsConfirmation = true;
if (_server == null) { if (_server == null) {
return _compile( return _compile(
_mapFilename(request.mainPath, packageUriMapper), _mapFilename(request.mainPath, packageUriMapper),
...@@ -576,14 +587,33 @@ class ResidentCompiler { ...@@ -576,14 +587,33 @@ class ResidentCompiler {
/// ///
/// Either [accept] or [reject] should be called after every [recompile] call. /// Either [accept] or [reject] should be called after every [recompile] call.
void accept() { void accept() {
_server.stdin.writeln('accept'); if (_compileRequestNeedsConfirmation) {
_server.stdin.writeln('accept');
}
_compileRequestNeedsConfirmation = false;
} }
/// Should be invoked when results of compilation are rejected by the client. /// Should be invoked when results of compilation are rejected by the client.
/// ///
/// Either [accept] or [reject] should be called after every [recompile] call. /// Either [accept] or [reject] should be called after every [recompile] call.
void reject() { Future<CompilerOutput> reject() {
if (!_controller.hasListener) {
_controller.stream.listen(_handleCompilationRequest);
}
final Completer<CompilerOutput> completer = Completer<CompilerOutput>();
_controller.add(_RejectRequest(completer));
return completer.future;
}
Future<CompilerOutput> _reject() {
if (!_compileRequestNeedsConfirmation) {
return Future<CompilerOutput>.value(null);
}
_stdoutHandler.reset();
_server.stdin.writeln('reject'); _server.stdin.writeln('reject');
_compileRequestNeedsConfirmation = false;
return _stdoutHandler.compilerOutput.future;
} }
/// Should be invoked when frontend server compiler should forget what was /// Should be invoked when frontend server compiler should forget what was
......
...@@ -428,11 +428,11 @@ class FlutterDevice { ...@@ -428,11 +428,11 @@ class FlutterDevice {
return report; return report;
} }
void updateReloadStatus(bool wasReloadSuccessful) { Future<void> updateReloadStatus(bool wasReloadSuccessful) async {
if (wasReloadSuccessful) if (wasReloadSuccessful)
generator?.accept(); generator?.accept();
else else
generator?.reject(); await generator?.reject();
} }
} }
......
...@@ -483,7 +483,7 @@ class HotRunner extends ResidentRunner { ...@@ -483,7 +483,7 @@ class HotRunner extends ResidentRunner {
if (!updatedDevFS.success) { if (!updatedDevFS.success) {
for (FlutterDevice device in flutterDevices) { for (FlutterDevice device in flutterDevices) {
if (device.generator != null) if (device.generator != null)
device.generator.reject(); await device.generator.reject();
} }
return OperationResult(1, 'DevFS synchronization failed'); return OperationResult(1, 'DevFS synchronization failed');
} }
...@@ -681,13 +681,13 @@ class HotRunner extends ResidentRunner { ...@@ -681,13 +681,13 @@ class HotRunner extends ResidentRunner {
final List<Future<Map<String, dynamic>>> reportFutures = device.reloadSources( final List<Future<Map<String, dynamic>>> reportFutures = device.reloadSources(
entryPath, pause: pause entryPath, pause: pause
); );
Future.wait(reportFutures).then((List<Map<String, dynamic>> reports) { // ignore: unawaited_futures Future.wait(reportFutures).then((List<Map<String, dynamic>> reports) async { // ignore: unawaited_futures
// TODO(aam): Investigate why we are validating only first reload report, // TODO(aam): Investigate why we are validating only first reload report,
// which seems to be current behavior // which seems to be current behavior
final Map<String, dynamic> firstReport = reports.first; final Map<String, dynamic> firstReport = reports.first;
// Don't print errors because they will be printed further down when // Don't print errors because they will be printed further down when
// `validateReloadReport` is called again. // `validateReloadReport` is called again.
device.updateReloadStatus(validateReloadReport(firstReport, printErrors: false)); await device.updateReloadStatus(validateReloadReport(firstReport, printErrors: false));
completer.complete(DeviceReloadReport(device, reports)); completer.complete(DeviceReloadReport(device, reports));
}); });
} }
......
...@@ -274,14 +274,28 @@ example:org-dartlang-app:///lib/ ...@@ -274,14 +274,28 @@ example:org-dartlang-app:///lib/
); );
expect(mockFrontendServerStdIn.getAndClear(), 'compile /path/to/main.dart\n'); expect(mockFrontendServerStdIn.getAndClear(), 'compile /path/to/main.dart\n');
// No accept or reject commands should be issued until we
// send recompile request.
await _accept(streamController, generator, mockFrontendServerStdIn, '');
await _reject(streamController, generator, mockFrontendServerStdIn, '', '');
await _recompile(streamController, generator, mockFrontendServerStdIn, await _recompile(streamController, generator, mockFrontendServerStdIn,
'result abc\nline1\nline2\nabc /path/to/main.dart.dill 0\n'); 'result abc\nline1\nline2\nabc /path/to/main.dart.dill 0\n');
await _accept(streamController, generator, mockFrontendServerStdIn, '^accept\\n\$');
await _recompile(streamController, generator, mockFrontendServerStdIn,
'result abc\nline1\nline2\nabc /path/to/main.dart.dill 0\n');
await _reject(streamController, generator, mockFrontendServerStdIn, 'result abc\nabc\n',
'^reject\\n\$');
verifyNoMoreInteractions(mockFrontendServerStdIn); verifyNoMoreInteractions(mockFrontendServerStdIn);
expect(mockFrontendServerStdIn.getAndClear(), isEmpty); expect(mockFrontendServerStdIn.getAndClear(), isEmpty);
expect(logger.errorText, equals( expect(logger.errorText, equals(
'\nCompiler message:\nline0\nline1\n' '\nCompiler message:\nline0\nline1\n'
'\nCompiler message:\nline1\nline2\n' '\nCompiler message:\nline1\nline2\n'
'\nCompiler message:\nline1\nline2\n'
)); ));
}, overrides: <Type, Generator>{ }, overrides: <Type, Generator>{
ProcessManager: () => mockProcessManager, ProcessManager: () => mockProcessManager,
...@@ -504,6 +518,34 @@ Future<void> _recompile(StreamController<List<int>> streamController, ...@@ -504,6 +518,34 @@ Future<void> _recompile(StreamController<List<int>> streamController,
mockFrontendServerStdIn._stdInWrites.clear(); mockFrontendServerStdIn._stdInWrites.clear();
} }
Future<void> _accept(StreamController<List<int>> streamController,
ResidentCompiler generator, MockStdIn mockFrontendServerStdIn,
String expected) async {
// Put content into the output stream after generator.recompile gets
// going few lines below, resets completer.
generator.accept();
final String commands = mockFrontendServerStdIn.getAndClear();
final RegExp re = RegExp(expected);
expect(commands, matches(re));
mockFrontendServerStdIn._stdInWrites.clear();
}
Future<void> _reject(StreamController<List<int>> streamController,
ResidentCompiler generator, MockStdIn mockFrontendServerStdIn,
String mockCompilerOutput, String expected) async {
// Put content into the output stream after generator.recompile gets
// going few lines below, resets completer.
scheduleMicrotask(() {
streamController.add(utf8.encode(mockCompilerOutput));
});
final CompilerOutput output = await generator.reject();
expect(output, isNull);
final String commands = mockFrontendServerStdIn.getAndClear();
final RegExp re = RegExp(expected);
expect(commands, matches(re));
mockFrontendServerStdIn._stdInWrites.clear();
}
class MockProcessManager extends Mock implements ProcessManager {} class MockProcessManager extends Mock implements ProcessManager {}
class MockProcess extends Mock implements Process {} class MockProcess extends Mock implements Process {}
class MockStream extends Mock implements Stream<List<int>> {} class MockStream extends Mock implements Stream<List<int>> {}
......
...@@ -462,7 +462,7 @@ class MockResidentCompiler extends BasicMock implements ResidentCompiler { ...@@ -462,7 +462,7 @@ class MockResidentCompiler extends BasicMock implements ResidentCompiler {
void accept() {} void accept() {}
@override @override
void reject() {} Future<CompilerOutput> reject() async { return null; }
@override @override
void reset() {} void reset() {}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment