running screenshot tests on ios-safari unit tests (#20963)

* running screenshot tests on ios-safari unit tests

* fixing the golden_smoke tests. changes to the documentation

* addressing reviewer comments

* cropping footer from the simulator screenshot. addressing some reviewer comments

* use .dart_tools for recording the screenshots

* fix the usage of the method

* adding TODO's for missing documentation and not supported windows tests

* addressing comments

* changing to incremental counter for file names

* add comment to the counter

* fix anaylze issues

* using takescreenshot method from the iosSimulator.

* address reviewer comments

* fix the scaling issue. disable eronous test

* change the smoke file for top gap 282

* change the variable name for scale factor

lib/web_ui/dev/browser_lock.yaml

@@ -21,9 +21,32 @@ firefox:
 edge:
   launcher_version: '1.2.0.0'
 ios-safari:
+  # Make sure this version is the same version supported by LUCI macOS bots.
+  # XCode on these bots will be updated once a year, do not forget to update
+  # `heightOfHeader` during this time.
   majorVersion: 13
   minorVersion: 0
   device: 'iPhone 11'
+  # `xcrun simctl` command is used to take screenshots. It takes the screenshot
+  # of the entire simulator. Therefore we need to crop all the parts other than
+  # the browsers' content. This file must be in sync with the local and LUCI
+  # versions of macOS, iOS Simulator, and Xcode.
+  # `heightOfHeader` is the number of pixels taken by the phone's header menu
+  # and the browsers address bar.
+  # TODO: https://github.com/flutter/flutter/issues/65672
+  heightOfHeader: 282
+  # `heightOfFooter` is the number of pixels taken by the phone's navigation
+  # menu.
+  heightOfFooter: 250
+  # Most of the time tests use a portion of the screen to compare goldens
+  # instead of the entire screen. This area is reprented by a rectangle
+  # when taking screenshots. However the rectangle dimensions are in logical
+  # coordinates. In order to convert these coordinates to coordinates on the
+  # phone screeen we enlarge or shrink the area by applying a linear
+  # transformation by a scale_factor (a.k.a. we perform isotropic scaling).
+  # This value will be differ depending on the phone.
+  scaleFactor: 1.15
+
 ## geckodriver is used for testing Firefox Browser. It works with multiple
 ## Firefox Browser versions.
 ## See: https://github.com/mozilla/geckodriver/releases

lib/web_ui/dev/environment.dart

@@ -168,6 +168,13 @@ class Environment {
         'goldens',
       ));
 
+  /// Path to the screenshots taken by iOS simulator.
+  io.Directory get webUiSimulatorScreenshotsDirectory =>
+      io.Directory(pathlib.join(
+        webUiDartToolDir.path,
+        'ios_screenshots',
+      ));
+
   /// Path to the script that clones the Flutter repo.
   io.File get cloneFlutterScript => io.File(pathlib.join(
         engineToolsDir.path,

lib/web_ui/dev/safari_installation.dart

@@ -7,9 +7,11 @@ import 'dart:async';
 import 'dart:io' as io;
 
 import 'package:args/args.dart';
+import 'package:simulators/simulator_manager.dart';
 import 'package:yaml/yaml.dart';
 
 import 'common.dart';
+import 'utils.dart';
 
 class SafariArgParser extends BrowserArgParser {
   static final SafariArgParser _singletonInstance = SafariArgParser._();
@@ -74,6 +76,48 @@ class IosSafariArgParser extends BrowserArgParser {
 
   IosSafariArgParser._();
 
+  /// Returns [IosSimulator] if the [Platform] is `macOS` and simulator
+  /// is started.
+  ///
+  /// Throws an [StateError] if these two conditions are not met.
+  IosSimulator get iosSimulator => io.Platform.isMacOS
+      ? (_iosSimulator != null
+          ? _iosSimulator
+          : throw StateError('iosSimulator not started. Please first call '
+              'initIOSSimulator method'))
+      : throw StateError('iOS Simulator is only avaliable on macOS machines.');
+
+  IosSimulator _iosSimulator;
+
+  /// Inializes and boots an [IosSimulator] using the [iosMajorVersion],
+  /// [iosMinorVersion] and [iosDevice] arguments.
+  Future<void> initIosSimulator() async {
+    if (_iosSimulator != null) {
+      throw StateError('_iosSimulator can only be initialized once');
+    }
+    final IosSimulatorManager iosSimulatorManager = IosSimulatorManager();
+    try {
+      _iosSimulator = await iosSimulatorManager.getSimulator(
+        iosMajorVersion,
+        iosMinorVersion,
+        iosDevice,
+      );
+    } catch (e) {
+      throw Exception('Error getting requested simulator. Try running '
+          '`felt create` command first before running the tests. Exception: '
+          '$e');
+    }
+
+    if (!_iosSimulator.booted) {
+      await _iosSimulator.boot();
+      print('INFO: Simulator ${_iosSimulator.id} booted.');
+      cleanupCallbacks.add(() async {
+        await _iosSimulator.shutdown();
+        print('INFO: Simulator ${_iosSimulator.id} shutdown.');
+      });
+    }
+  }
+
   @override
   void populateOptions(ArgParser argParser) {
     final YamlMap browserLock = BrowserLock.instance.configuration;

lib/web_ui/dev/screenshot_manager.dart

+// Copyright 2013 The Flutter Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// @dart = 2.6
+import 'dart:io' as io;
+import 'dart:convert';
+import 'dart:math';
+
+import 'package:image/image.dart';
+import 'package:path/path.dart' as path;
+import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'
+    as wip;
+import 'package:yaml/yaml.dart';
+
+import 'common.dart';
+import 'environment.dart';
+import 'safari_installation.dart';
+import 'utils.dart';
+
+/// [ScreenshotManager] implementation for Chrome.
+///
+/// This manager can be used for both macOS and Linux.
+// TODO: https://github.com/flutter/flutter/issues/65673
+class ChromeScreenshotManager extends ScreenshotManager {
+  String get filenameSuffix => '';
+
+  /// Capture a screenshot of the web content.
+  ///
+  /// Uses Webkit Inspection Protocol server's `captureScreenshot` API.
+  ///
+  /// [region] is used to decide which part of the web content will be used in
+  /// test image. It includes starting coordinate x,y as well as height and
+  /// width of the area to capture.
+  Future<Image> capture(Rectangle region) async {
+    final wip.ChromeConnection chromeConnection =
+        wip.ChromeConnection('localhost', kDevtoolsPort);
+    final wip.ChromeTab chromeTab = await chromeConnection.getTab(
+        (wip.ChromeTab chromeTab) => chromeTab.url.contains('localhost'));
+    final wip.WipConnection wipConnection = await chromeTab.connect();
+
+    Map<String, dynamic> captureScreenshotParameters = null;
+    if (region != null) {
+      captureScreenshotParameters = <String, dynamic>{
+        'format': 'png',
+        'clip': <String, dynamic>{
+          'x': region.left,
+          'y': region.top,
+          'width': region.width,
+          'height': region.height,
+          'scale':
+              // This is NOT the DPI of the page, instead it's the "zoom level".
+              1,
+        },
+      };
+    }
+
+    // Setting hardware-independent screen parameters:
+    // https://chromedevtools.github.io/devtools-protocol/tot/Emulation
+    await wipConnection
+        .sendCommand('Emulation.setDeviceMetricsOverride', <String, dynamic>{
+      'width': kMaxScreenshotWidth,
+      'height': kMaxScreenshotHeight,
+      'deviceScaleFactor': 1,
+      'mobile': false,
+    });
+    final wip.WipResponse response = await wipConnection.sendCommand(
+        'Page.captureScreenshot', captureScreenshotParameters);
+
+    final Image screenshot =
+        decodePng(base64.decode(response.result['data'] as String));
+
+    return screenshot;
+  }
+}
+
+/// [ScreenshotManager] implementation for Safari.
+///
+/// This manager will only be created/used for macOS.
+class IosSafariScreenshotManager extends ScreenshotManager {
+  String get filenameSuffix => '.iOS_Safari';
+
+  IosSafariScreenshotManager() {
+    final YamlMap browserLock = BrowserLock.instance.configuration;
+    _heightOfHeader = browserLock['ios-safari']['heightOfHeader'] as int;
+    _heightOfFooter = browserLock['ios-safari']['heightOfFooter'] as int;
+    _scaleFactor = browserLock['ios-safari']['scaleFactor'] as double;
+
+    /// Create the directory to use for taking screenshots, if it does not
+    /// exists.
+    if (!environment.webUiSimulatorScreenshotsDirectory.existsSync()) {
+      environment.webUiSimulatorScreenshotsDirectory.createSync();
+    }
+    // Temporary directories are deleted in the clenaup phase of after `felt`
+    // runs the tests.
+    temporaryDirectories.add(environment.webUiSimulatorScreenshotsDirectory);
+  }
+
+  /// This scale factor is used to enlarge/shrink the screenshot region
+  /// sent from the tests.
+  /// For more details see [_scaleScreenshotRegion(region)].
+  double _scaleFactor;
+
+  /// Height of the part to crop from the top of the image.
+  ///
+  /// `xcrun simctl` command takes the screenshot of the entire simulator. We
+  /// are cropping top bit from screenshot, otherwise due to the clock on top of
+  /// the screen, the screenshot will differ between each run.
+  /// Note that this gap can change per phone and per iOS version. For more
+  /// details refer to `browser_lock.yaml` file.
+  int _heightOfHeader;
+
+  /// Height of the part to crop from the bottom of the image.
+  ///
+  /// This area is the footer navigation bar of the phone, it is not the area
+  /// used by tests (which is inside the browser).
+  int _heightOfFooter;
+
+  /// Used as a suffix for the temporary file names used for screenshots.
+  int _fileNameCounter = 0;
+
+  /// Capture a screenshot of entire simulator.
+  ///
+  /// Example screenshot with dimensions: W x H.
+  ///
+  ///  <----------  W ------------->
+  ///  _____________________________
+  /// | Phone Top bar (clock etc.)  |   Ʌ
+  /// |_____________________________|   |
+  /// | Broswer search bar          |   |
+  /// |_____________________________|   |
+  /// | Web page content            |   |
+  /// |                             |   |
+  /// |                             |
+  /// |                             |   H
+  /// |                             |
+  /// |                             |   |
+  /// |                             |   |
+  /// |                             |   |
+  /// |                             |   |
+  /// |_____________________________|   |
+  /// | Phone footer bar            |   |
+  /// |_____________________________|   V
+  ///
+  /// After taking the screenshot, the image is cropped as heigh as
+  /// [_heightOfHeader] and [_heightOfFooter] from the top and bottom parts
+  /// consecutively. Hence web content has the dimensions:
+  ///
+  /// W x (H - [_heightOfHeader] - [_heightOfFooter])
+  ///
+  /// [region] is used to decide which part of the web content will be used in
+  /// test image. It includes starting coordinate x,y as well as height and
+  /// width of the area to capture.
+  ///
+  /// Uses simulator tool `xcrun simctl`'s 'screenshot' command.
+  Future<Image> capture(Rectangle region) async {
+    final String filename = 'screenshot${_fileNameCounter}.png';
+    _fileNameCounter++;
+
+    await IosSafariArgParser.instance.iosSimulator.takeScreenshot(
+        filename, environment.webUiSimulatorScreenshotsDirectory);
+
+    final io.File file = io.File(path.join(
+        environment.webUiSimulatorScreenshotsDirectory.path, filename));
+    List<int> imageBytes;
+    if (!file.existsSync()) {
+      throw Exception('Failed to read the screenshot '
+          'screenshot${_fileNameCounter}.png.');
+    }
+    imageBytes = await file.readAsBytes();
+    file.deleteSync();
+
+    final Image screenshot = decodePng(imageBytes);
+    // Create an image with no footer and header. The _heightOfHeader,
+    // _heightOfFooter values are already in real coordinates therefore
+    // they don't need to be scaled.
+    final Image content = copyCrop(
+      screenshot,
+      0,
+      _heightOfHeader,
+      screenshot.width,
+      screenshot.height - _heightOfFooter - _heightOfHeader,
+    );
+
+    if (region == null) {
+      return content;
+    } else {
+      final Rectangle scaledRegion = _scaleScreenshotRegion(region);
+      return copyCrop(
+        content,
+        scaledRegion.left.toInt(),
+        scaledRegion.top.toInt(),
+        scaledRegion.width.toInt(),
+        scaledRegion.height.toInt(),
+      );
+    }
+  }
+
+  /// Perform a linear transform on the screenshot region to convert its
+  /// dimensions from linear coordinated to coordinated on the phone screen.
+  /// This uniform/isotropic scaling is done using [_scaleFactor].
+  Rectangle _scaleScreenshotRegion(Rectangle region) {
+    return Rectangle(
+      region.left * _scaleFactor,
+      region.top * _scaleFactor,
+      region.width * _scaleFactor,
+      region.height * _scaleFactor,
+    );
+  }
+}
+
+const String _kBrowserChrome = 'chrome';
+const String _kBrowserIOSSafari = 'ios-safari';
+
+typedef ScreenshotManagerFactory = ScreenshotManager Function();
+
+/// Abstract class for taking screenshots in one of the browsers.
+abstract class ScreenshotManager {
+  static final Map<String, ScreenshotManagerFactory> _browserFactories =
+      <String, ScreenshotManagerFactory>{
+    _kBrowserChrome: () => ChromeScreenshotManager(),
+    _kBrowserIOSSafari: () => IosSafariScreenshotManager(),
+  };
+
+  static bool isBrowserSupported(String browser) =>
+      _browserFactories.containsKey(browser);
+
+  static ScreenshotManager choose(String browser) {
+    if (isBrowserSupported(browser)) {
+      return _browserFactories[browser]();
+    }
+    throw StateError('Screenshot tests are only supported on Chrome and on '
+        'iOS Safari');
+  }
+
+  /// Capture a screenshot.
+  ///
+  /// Please read the details for the implementing classes.
+  Future<Image> capture(Rectangle region);
+
+  /// Suffix to be added to the end of the filename.
+  ///
+  /// Example file names:
+  /// - Chrome, no-suffix: backdrop_filter_clip_moved.actual.png
+  /// - iOS Safari: backdrop_filter_clip_moved.actual.iOS_Safari.png
+  String get filenameSuffix;
+}

lib/web_ui/dev/test_platform.dart

@@ -7,6 +7,7 @@ import 'dart:async';
 import 'dart:convert';
 import 'dart:io';
 import 'dart:isolate';
+import 'dart:math';
 
 import 'package:async/async.dart';
 import 'package:http_multi_server/http_multi_server.dart';
@@ -34,13 +35,12 @@ import 'package:test_core/src/runner/environment.dart'; // ignore: implementatio
 
 import 'package:test_core/src/util/io.dart'; // ignore: implementation_imports
 import 'package:test_core/src/runner/configuration.dart'; // ignore: implementation_imports
-import 'package:webkit_inspection_protocol/webkit_inspection_protocol.dart'
-    as wip;
 
 import 'browser.dart';
 import 'common.dart';
 import 'environment.dart' as env;
 import 'goldens.dart';
+import 'screenshot_manager.dart';
 import 'supported_browsers.dart';
 
 class BrowserPlatform extends PlatformPlugin {
@@ -97,6 +97,11 @@ class BrowserPlatform extends PlatformPlugin {
   /// The HTTP client to use when caching JS files in `pub serve`.
   final HttpClient _http;
 
+  /// Handles taking screenshots during tests.
+  ///
+  /// Implementation will differ depending on the browser.
+  ScreenshotManager _screenshotManager;
+
   /// Whether [close] has been called.
   bool get _closed => _closeMemo.hasRun;
 
@@ -124,9 +129,10 @@ class BrowserPlatform extends PlatformPlugin {
               serveFilesOutsidePath:
                   config.suiteDefaults.precompiledPath != null))
           .add(_wrapperHandler);
-      // Screenshot tests are only enabled in chrome for now.
-      if (name == 'chrome') {
+      // Screenshot tests are only enabled in Chrome and Safari iOS for now.
+      if (browserName == 'chrome' || browserName == 'ios-safari') {
         cascade = cascade.add(_screeshotHandler);
+        _screenshotManager = ScreenshotManager.choose(browserName);
       }
     }
 
@@ -141,8 +147,9 @@ class BrowserPlatform extends PlatformPlugin {
   }
 
   Future<shelf.Response> _screeshotHandler(shelf.Request request) async {
-    if (browserName != 'chrome') {
-      throw Exception('Screenshots tests are only available in Chrome.');
+    if (browserName != 'chrome' && browserName != 'ios-safari') {
+      throw Exception('Screenshots tests are only available in Chrome '
+          'and in Safari-iOS.');
     }
 
     if (!request.requestedUri.path.endsWith('/screenshot')) {
@@ -178,6 +185,9 @@ class BrowserPlatform extends PlatformPlugin {
       write = true;
     }
 
+    filename =
+        filename.replaceAll('.png', '${_screenshotManager.filenameSuffix}.png');
+
     String goldensDirectory;
     if (filename.startsWith('__local__')) {
       filename = filename.substring('__local__/'.length);
@@ -208,42 +218,15 @@ To automatically create this file call matchGoldenFile('$filename', write: true)
 ''';
     }
 
-    final wip.ChromeConnection chromeConnection =
-        wip.ChromeConnection('localhost', kDevtoolsPort);
-    final wip.ChromeTab chromeTab = await chromeConnection.getTab(
-        (wip.ChromeTab chromeTab) => chromeTab.url.contains('localhost'));
-    final wip.WipConnection wipConnection = await chromeTab.connect();
-
-    Map<String, dynamic> captureScreenshotParameters = null;
-    if (region != null) {
-      captureScreenshotParameters = <String, dynamic>{
-        'format': 'png',
-        'clip': <String, dynamic>{
-          'x': region['x'],
-          'y': region['y'],
-          'width': region['width'],
-          'height': region['height'],
-          'scale':
-              1, // This is NOT the DPI of the page, instead it's the "zoom level".
-        },
-      };
-    }
-
-    // Setting hardware-independent screen parameters:
-    // https://chromedevtools.github.io/devtools-protocol/tot/Emulation
-    await wipConnection
-        .sendCommand('Emulation.setDeviceMetricsOverride', <String, dynamic>{
-      'width': kMaxScreenshotWidth,
-      'height': kMaxScreenshotHeight,
-      'deviceScaleFactor': 1,
-      'mobile': false,
-    });
-    final wip.WipResponse response = await wipConnection.sendCommand(
-        'Page.captureScreenshot', captureScreenshotParameters);
+    final Rectangle regionAsRectange = Rectangle(
+      region['x'] as num,
+      region['y'] as num,
+      region['width'] as num,
+      region['height'] as num,
+    );
 
-    // Compare screenshots
-    final Image screenshot =
-        decodePng(base64.decode(response.result['data'] as String));
+    // Take screenshot.
+    final Image screenshot = await _screenshotManager.capture(regionAsRectange);
 
     if (write) {
       // Don't even bother with the comparison, just write and return
@@ -257,6 +240,7 @@ To automatically create this file call matchGoldenFile('$filename', write: true)
       }
     }
 
+    // Compare screenshots.
     ImageDiff diff = ImageDiff(
       golden: decodeNamedImage(file.readAsBytesSync(), filename),
       other: screenshot,

lib/web_ui/dev/test_runner.dart

@@ -15,7 +15,6 @@ import 'package:test_core/src/runner/hack_register_platform.dart'
 import 'package:test_api/src/backend/runtime.dart'; // ignore: implementation_imports
 import 'package:test_core/src/executable.dart'
     as test; // ignore: implementation_imports
-import 'package:simulators/simulator_manager.dart';
 
 import 'common.dart';
 import 'environment.dart';
@@ -178,28 +177,8 @@ class TestCommand extends Command<bool> with ArgUtils {
 
     // In order to run iOS Safari unit tests we need to make sure iOS Simulator
     // is booted.
-    if (browser == 'ios-safari') {
-      final IosSimulatorManager iosSimulatorManager = IosSimulatorManager();
-      IosSimulator iosSimulator;
-      try {
-        iosSimulator = await iosSimulatorManager.getSimulator(
-            IosSafariArgParser.instance.iosMajorVersion,
-            IosSafariArgParser.instance.iosMinorVersion,
-            IosSafariArgParser.instance.iosDevice);
-      } catch (e) {
-        throw Exception('Error getting requested simulator. Try running '
-            '`felt create` command first before running the tests. exception: '
-            '$e');
-      }
-
-      if (!iosSimulator.booted) {
-        await iosSimulator.boot();
-        print('INFO: Simulator ${iosSimulator.id} booted.');
-        cleanupCallbacks.add(() async {
-          await iosSimulator.shutdown();
-          print('INFO: Simulator ${iosSimulator.id} shutdown.');
-        });
-      }
+    if (isSafariIOS) {
+      await IosSafariArgParser.instance.initIosSimulator();
     }
 
     await _buildTargets();
@@ -351,6 +330,9 @@ class TestCommand extends Command<bool> with ArgUtils {
   /// Whether [browser] is set to "safari".
   bool get isSafariOnMacOS => browser == 'safari' && io.Platform.isMacOS;
 
+  /// Whether [browser] is set to "ios-safari".
+  bool get isSafariIOS => browser == 'ios-safari' && io.Platform.isMacOS;
+
   /// Due to lack of resources Chrome integration tests only run on Linux on
   /// LUCI.
   ///
@@ -410,10 +392,12 @@ class TestCommand extends Command<bool> with ArgUtils {
       'test',
     ));
 
-    // Screenshot tests and smoke tests only run on: "Chrome locally" or
-    // "Chrome on a Linux bot". We can remove the Linux bot restriction after:
+    // Screenshot tests and smoke tests only run on: "Chrome/iOS Safari locally"
+    // or "Chrome on a Linux bot". We can remove the Linux bot restriction
+    // after solving the git issue faced on macOS and Windows bots:
     // TODO: https://github.com/flutter/flutter/issues/63710
-    if ((isChrome && isLuci && io.Platform.isLinux) || (isChrome && !isLuci)) {
+    if ((isChrome && isLuci && io.Platform.isLinux) ||
+        ((isChrome || isSafariIOS) && !isLuci)) {
       // Separate screenshot tests from unit-tests. Screenshot tests must run
       // one at a time. Otherwise, they will end up screenshotting each other.
       // This is not an issue for unit-tests.

lib/web_ui/dev/utils.dart

@@ -28,8 +28,7 @@ class FilePath {
 
   @override
   bool operator ==(Object other) {
-    return other is FilePath
-        && other._absolutePath == _absolutePath;
+    return other is FilePath && other._absolutePath == _absolutePath;
   }
 
   @override
@@ -216,7 +215,9 @@ void cleanup() async {
   // Delete temporary directories.
   if (temporaryDirectories.length > 0) {
     for (io.Directory directory in temporaryDirectories) {
-      directory.deleteSync(recursive: true);
+      if (!directory.existsSync()) {
+        directory.deleteSync(recursive: true);
+      }
     }
   }
 

lib/web_ui/pubspec.yaml

@@ -29,7 +29,7 @@ dev_dependencies:
     git:
       url: git://github.com/flutter/web_installers.git
       path: packages/simulators/
-      ref: 1da6bb8df222f0d124e737e8abc20d68ddb7ea43
+      ref: 4a9643e55e9bd127f4e4e3cb20e562ed5d5b9aa7
   web_driver_installer:
     git:
       url: git://github.com/flutter/web_installers.git

lib/web_ui/test/golden_tests/engine/recording_canvas_golden_test.dart

@@ -313,7 +313,9 @@ void testMain() async {
       const Rect.fromLTRB(textLeft, textTop, textLeft + widthConstraint, 21.0),
     );
     await _checkScreenshot(rc, 'draw_paragraph');
-  });
+  },  // TODO: https://github.com/flutter/flutter/issues/65789
+      skip: browserEngine == BrowserEngine.webkit &&
+          operatingSystem == OperatingSystem.iOs);
 
   test('Computes paint bounds for multi-line draw paragraph', () async {
     final RecordingCanvas rc = RecordingCanvas(screenRect);
@@ -330,7 +332,9 @@ void testMain() async {
       const Rect.fromLTRB(textLeft, textTop, textLeft + widthConstraint, 35.0),
     );
     await _checkScreenshot(rc, 'draw_paragraph_multi_line');
-  });
+  },  // TODO: https://github.com/flutter/flutter/issues/65789
+      skip: browserEngine == BrowserEngine.webkit &&
+          operatingSystem == OperatingSystem.iOs);
 
   test('Should exclude painting outside simple clipRect', () async {
     // One clipped line.