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

) * This is completely broken on the Impeller renderer, see: #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.
flutter · Sep 29, 2023 · d13cd88 · d13cd88
1 parent e5d3b70
commit d13cd88
Show file tree

Hide file tree

Showing 6 changed files with 10 additions and 144 deletions.
diff --git a/packages/flutter_tools/lib/src/commands/screenshot.dart b/packages/flutter_tools/lib/src/commands/screenshot.dart
@@ -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,21 +32,20 @@ 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.',
     );
     argParser.addOption(
       _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(

diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart
@@ -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));
     });
   }

diff --git a/packages/flutter_tools/lib/src/vmservice.dart b/packages/flutter_tools/lib/src/vmservice.dart
@@ -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);
   }

diff --git a/packages/flutter_tools/test/commands.shard/hermetic/screenshot_command_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/screenshot_command_test.dart
@@ -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 {

diff --git a/packages/flutter_tools/test/general.shard/terminal_handler_test.dart b/packages/flutter_tools/test/general.shard/terminal_handler_test.dart
@@ -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();

diff --git a/packages/flutter_tools/test/general.shard/vmservice_test.dart b/packages/flutter_tools/test/general.shard/vmservice_test.dart
@@ -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);