Unverified Commit d13cd884 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

[flutter_tools] remove VmService screenshot for native devices. (#135462)

* This is completely broken on the Impeller renderer, see: https://github.com/flutter/flutter/issues/135052
* Even on the Skia renderer, this gives a software rasterized screenshot which will absolutely look different from a native rendering screenshot.

I plan to remove this functionality from the engine.
parent e5d3b704
......@@ -18,7 +18,6 @@ const String _kType = 'type';
const String _kVmServiceUrl = 'vm-service-url';
const String _kDeviceType = 'device';
const String _kSkiaType = 'skia';
const String _kRasterizerType = 'rasterizer';
class ScreenshotCommand extends FlutterCommand {
ScreenshotCommand({required this.fs}) {
......@@ -33,7 +32,7 @@ class ScreenshotCommand extends FlutterCommand {
aliases: <String>[ 'observatory-url' ], // for historical reasons
valueHelp: 'URI',
help: 'The VM Service URL to which to connect.\n'
'This is required when "--$_kType" is "$_kSkiaType" or "$_kRasterizerType".\n'
'This is required when "--$_kType" is "$_kSkiaType".\n'
'To find the VM service URL, use "flutter run" and look for '
'"A Dart VM Service ... is available at" in the output.',
);
......@@ -41,13 +40,12 @@ class ScreenshotCommand extends FlutterCommand {
_kType,
valueHelp: 'type',
help: 'The type of screenshot to retrieve.',
allowed: const <String>[_kDeviceType, _kSkiaType, _kRasterizerType],
allowed: const <String>[_kDeviceType, _kSkiaType],
allowedHelp: const <String, String>{
_kDeviceType: "Delegate to the device's native screenshot capabilities. This "
'screenshots the entire screen currently being displayed (including content '
'not rendered by Flutter, like the device status bar).',
_kSkiaType: 'Render the Flutter app as a Skia picture. Requires "--$_kVmServiceUrl".',
_kRasterizerType: 'Render the Flutter app using the rasterizer. Requires "--$_kVmServiceUrl."',
},
defaultsTo: _kDeviceType,
);
......@@ -116,8 +114,6 @@ class ScreenshotCommand extends FlutterCommand {
await runScreenshot(outputFile);
case _kSkiaType:
success = await runSkia(outputFile);
case _kRasterizerType:
success = await runRasterizer(outputFile);
}
return success ? FlutterCommandResult.success()
......@@ -173,30 +169,6 @@ class ScreenshotCommand extends FlutterCommand {
return true;
}
Future<bool> runRasterizer(File? outputFile) async {
final Uri vmServiceUrl = Uri.parse(stringArg(_kVmServiceUrl)!);
final FlutterVmService vmService = await connectToVmService(vmServiceUrl, logger: globals.logger);
final vm_service.Response? response = await vmService.screenshot();
if (response == null) {
globals.printError(
'The screenshot request failed, probably because the device was '
'disconnected',
);
return false;
}
outputFile ??= globals.fsUtils.getUniqueFile(
fs.currentDirectory,
'flutter',
'png',
);
final IOSink sink = outputFile.openWrite();
sink.add(base64.decode(response.json?['screenshot'] as String));
await sink.close();
_showOutputFileInfo(outputFile);
ensureOutputIsNotJsonRpcError(outputFile);
return true;
}
static void checkOutput(File outputFile, FileSystem fs) {
if (!fs.file(outputFile.path).existsSync()) {
throwToolExit(
......
......@@ -1017,17 +1017,17 @@ abstract class ResidentHandlers {
}
Future<bool> _takeVmServiceScreenshot(FlutterDevice device, File outputFile) async {
final bool isWebDevice = device.targetPlatform == TargetPlatform.web_javascript;
if (device.targetPlatform != TargetPlatform.web_javascript) {
return false;
}
assert(supportsServiceProtocol);
return _toggleDebugBanner(device, () async {
final vm_service.Response? response = isWebDevice
? await device.vmService!.callMethodWrapper('ext.dwds.screenshot')
: await device.vmService!.screenshot();
final vm_service.Response? response = await device.vmService!.callMethodWrapper('ext.dwds.screenshot');
if (response == null) {
throw Exception('Failed to take screenshot');
}
final String data = response.json![isWebDevice ? 'data' : 'screenshot'] as String;
final String data = response.json!['data'] as String;
outputFile.writeAsBytesSync(base64.decode(data));
});
}
......
......@@ -28,7 +28,6 @@ const String kFlushUIThreadTasksMethod = '_flutter.flushUIThreadTasks';
const String kRunInViewMethod = '_flutter.runInView';
const String kListViewsMethod = '_flutter.listViews';
const String kScreenshotSkpMethod = '_flutter.screenshotSkp';
const String kScreenshotMethod = '_flutter.screenshot';
const String kRenderFrameWithRasterStatsMethod = '_flutter.renderFrameWithRasterStats';
const String kReloadAssetFonts = '_flutter.reloadAssetFonts';
......@@ -1054,10 +1053,6 @@ class FlutterVmService {
);
}
Future<vm_service.Response?> screenshot() {
return _checkedCallServiceExtension(kScreenshotMethod);
}
Future<vm_service.Response?> screenshotSkp() {
return _checkedCallServiceExtension(kScreenshotSkpMethod);
}
......
......@@ -31,11 +31,6 @@ void main() {
.run(<String>['screenshot', '--type=skia', '--vm-service-url=http://localhost:8181']),
throwsA(isException.having((Exception exception) => exception.toString(), 'message', contains('dummy'))),
);
await expectLater(() => createTestCommandRunner(ScreenshotCommand(fs: MemoryFileSystem.test()))
.run(<String>['screenshot', '--type=rasterizer', '--vm-service-url=http://localhost:8181']),
throwsA(isException.having((Exception exception) => exception.toString(), 'message', contains('dummy'))),
);
});
......@@ -44,11 +39,6 @@ void main() {
.run(<String>['screenshot', '--type=skia']),
throwsToolExit(message: 'VM Service URI must be specified for screenshot type skia')
);
await expectLater(() => createTestCommandRunner(ScreenshotCommand(fs: MemoryFileSystem.test()))
.run(<String>['screenshot', '--type=rasterizer',]),
throwsToolExit(message: 'VM Service URI must be specified for screenshot type rasterizer'),
);
});
testUsingContext('device screenshots require device', () async {
......
......@@ -1015,38 +1015,14 @@ void main() {
expect(logger.statusText, contains('Screenshot written to flutter_01.png (0kB)'));
});
testWithoutContext('s, can take screenshot on debug device that does not support screenshot', () async {
testWithoutContext('s, will not take screenshot on non-web device without screenshot tooling support', () async {
final BufferLogger logger = BufferLogger.test();
final FileSystem fileSystem = MemoryFileSystem.test();
final TerminalHandler terminalHandler = setUpTerminalHandler(<FakeVmServiceRequest>[
listViews,
FakeVmServiceRequest(
method: 'ext.flutter.debugAllowBanner',
args: <String, Object?>{
'isolateId': fakeUnpausedIsolate.id,
'enabled': 'false',
},
),
FakeVmServiceRequest(
method: '_flutter.screenshot',
args: <String, Object>{},
jsonResponse: <String, Object>{
'screenshot': base64.encode(<int>[1, 2, 3, 4]),
},
),
FakeVmServiceRequest(
method: 'ext.flutter.debugAllowBanner',
args: <String, Object?>{
'isolateId': fakeUnpausedIsolate.id,
'enabled': 'true',
},
),
], logger: logger, fileSystem: fileSystem);
final TerminalHandler terminalHandler = setUpTerminalHandler(<FakeVmServiceRequest>[], logger: logger, fileSystem: fileSystem);
await terminalHandler.processTerminalInput('s');
expect(logger.statusText, contains('Screenshot written to flutter_01.png (0kB)'));
expect(fileSystem.currentDirectory.childFile('flutter_01.png').readAsBytesSync(), <int>[1, 2, 3, 4]);
expect(logger.statusText, isNot(contains('Screenshot written to')));
});
testWithoutContext('s, can take screenshot on debug web device that does not support screenshot', () async {
......@@ -1133,66 +1109,6 @@ void main() {
expect(fileSystem.currentDirectory.childFile('flutter_01.png'), isNot(exists));
});
testWithoutContext('s, bails taking screenshot on debug device if debugAllowBanner throws RpcError', () async {
final BufferLogger logger = BufferLogger.test();
final FileSystem fileSystem = MemoryFileSystem.test();
final TerminalHandler terminalHandler = setUpTerminalHandler(
<FakeVmServiceRequest>[
listViews,
FakeVmServiceRequest(
method: 'ext.flutter.debugAllowBanner',
args: <String, Object?>{
'isolateId': fakeUnpausedIsolate.id,
'enabled': 'false',
},
// Failed response,
errorCode: RPCErrorCodes.kInternalError,
),
],
logger: logger,
fileSystem: fileSystem,
);
await terminalHandler.processTerminalInput('s');
expect(logger.errorText, contains('Error'));
});
testWithoutContext('s, bails taking screenshot on debug device if flutter.screenshot throws RpcError, restoring banner', () async {
final BufferLogger logger = BufferLogger.test();
final FileSystem fileSystem = MemoryFileSystem.test();
final TerminalHandler terminalHandler = setUpTerminalHandler(
<FakeVmServiceRequest>[
listViews,
FakeVmServiceRequest(
method: 'ext.flutter.debugAllowBanner',
args: <String, Object?>{
'isolateId': fakeUnpausedIsolate.id,
'enabled': 'false',
},
),
const FakeVmServiceRequest(
method: '_flutter.screenshot',
// Failed response,
errorCode: RPCErrorCodes.kInternalError,
),
FakeVmServiceRequest(
method: 'ext.flutter.debugAllowBanner',
args: <String, Object?>{
'isolateId': fakeUnpausedIsolate.id,
'enabled': 'true',
},
),
],
logger: logger,
fileSystem: fileSystem,
);
await terminalHandler.processTerminalInput('s');
expect(logger.errorText, contains('Error'));
});
testWithoutContext('s, bails taking screenshot on debug device if dwds.screenshot throws RpcError, restoring banner', () async {
final BufferLogger logger = BufferLogger.test();
final FileSystem fileSystem = MemoryFileSystem.test();
......
......@@ -442,10 +442,6 @@ void main() {
method: kListViewsMethod,
errorCode: RPCErrorCodes.kServiceDisappeared,
),
const FakeVmServiceRequest(
method: kScreenshotMethod,
errorCode: RPCErrorCodes.kServiceDisappeared,
),
const FakeVmServiceRequest(
method: kScreenshotSkpMethod,
errorCode: RPCErrorCodes.kServiceDisappeared,
......@@ -480,9 +476,6 @@ void main() {
final List<FlutterView> views = await fakeVmServiceHost.vmService.getFlutterViews();
expect(views, isEmpty);
final vm_service.Response? screenshot = await fakeVmServiceHost.vmService.screenshot();
expect(screenshot, isNull);
final vm_service.Response? screenshotSkp = await fakeVmServiceHost.vmService.screenshotSkp();
expect(screenshotSkp, isNull);
......
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