delay taking screenshot to allow GPU thread to render the frame (#12896)

* delay taking screenshot to allow GPU thread to render the frame

* address comments

dev/devicelab/lib/tasks/integration_ui.dart

@@ -24,6 +24,7 @@ Future<TaskResult> runEndToEndTests() async {
     const List<String> entryPoints = const <String>[
       'lib/keyboard_resize.dart',
       'lib/driver.dart',
+      'lib/screenshot.dart',
     ];
 
     for (final String entryPoint in entryPoints) {

dev/integration_tests/ui/lib/screenshot.dart

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter/material.dart';
+import 'package:flutter_driver/driver_extension.dart';
+
+/// This sample application creates a hard to render frame, causing the
+/// driver script to race the GPU thread. If the driver script wins the
+/// race, it will screenshot the previous frame. If the GPU thread wins
+/// it, it will screenshot the latest frame.
+void main() {
+  enableFlutterDriverExtension();
+
+  runApp(new Toggler());
+}
+
+class Toggler extends StatefulWidget {
+  @override
+  State<Toggler> createState() => new TogglerState();
+}
+
+class TogglerState extends State<Toggler> {
+  bool _visible = false;
+
+  @override
+  Widget build(BuildContext context) {
+    return new MaterialApp(
+      home: new Scaffold(
+        appBar: new AppBar(
+          title: const Text('FlutterDriver test'),
+        ),
+        body: new Material(
+          child: new Column(
+            children: <Widget>[
+              new FlatButton(
+                key: const ValueKey<String>('toggle'),
+                child: const Text('Toggle visibility'),
+                onPressed: () {
+                  setState(() {
+                    _visible = !_visible;
+                  });
+                },
+              ),
+              new Expanded(
+                child: new ListView(
+                  children: _buildRows(_visible ? 10 : 0),
+                ),
+              ),
+            ],
+          ),
+        ),
+      ),
+    );
+  }
+}
+
+List<Widget> _buildRows(int count) {
+  return new List<Widget>.generate(count, (int i) {
+    return new Row(
+      children: _buildCells(i / count),
+    );
+  });
+}
+
+/// Builds cells that are known to take time to render causing a delay on the
+/// GPU thread.
+List<Widget> _buildCells(double epsilon) {
+  return new List<Widget>.generate(15, (int i) {
+    return new Expanded(
+      child: new Material(
+        // A magic color that the test will be looking for on the screenshot.
+        color: const Color(0xffff0102),
+        borderRadius: new BorderRadius.all(new Radius.circular(i.toDouble() + epsilon)),
+        elevation: 5.0,
+        child: const SizedBox(height: 10.0, width: 10.0),
+      ),
+    );
+  });
+}

dev/integration_tests/ui/pubspec.yaml

@@ -2,6 +2,7 @@ name: integration_ui
 description: Flutter non-plugin UI integration tests.
 
 dependencies:
+  image: 1.1.29
   flutter:
     sdk: flutter
   flutter_driver:

dev/integration_tests/ui/test_driver/screenshot_test.dart

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_driver/flutter_driver.dart';
+import 'package:image/image.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group('FlutterDriver', () {
+    FlutterDriver driver;
+
+    setUpAll(() async {
+      driver = await FlutterDriver.connect();
+    });
+
+    tearDownAll(() async {
+      await driver.close();
+    });
+
+    test('should take screenshot', () async {
+      final SerializableFinder toggleBtn = find.byValueKey('toggle');
+      // Cards use a magic background color that we look for in the screenshots.
+      final Matcher cardsAreVisible = contains(0xff0201ff);
+      await driver.waitFor(toggleBtn);
+
+      bool cardsShouldBeVisible = false;
+      Image imageBefore = decodePng(await driver.screenshot());
+      for (int i = 0; i < 10; i += 1) {
+        await driver.tap(toggleBtn);
+        cardsShouldBeVisible = !cardsShouldBeVisible;
+        final Image imageAfter = decodePng(await driver.screenshot());
+
+        if (cardsShouldBeVisible) {
+          expect(imageBefore.data, isNot(cardsAreVisible));
+          expect(imageAfter.data, cardsAreVisible);
+        } else {
+          expect(imageBefore.data, cardsAreVisible);
+          expect(imageAfter.data, isNot(cardsAreVisible));
+        }
+
+        imageBefore = imageAfter;
+      }
+    }, timeout: const Timeout(const Duration(minutes: 2)));
+  });
+}

packages/flutter_driver/lib/src/driver.dart

@@ -435,6 +435,42 @@ class FlutterDriver {
   /// Take a screenshot.  The image will be returned as a PNG.
   Future<List<int>> screenshot({ Duration timeout }) async {
     timeout ??= _kLongTimeout;
+
+    // HACK: this artificial delay here is to deal with a race between the
+    //       driver script and the GPU thread. The issue is that driver API
+    //       synchronizes with the framework based on transient callbacks, which
+    //       are out of sync with the GPU thread. Here's the timeline of events
+    //       in ASCII art:
+    //
+    //       -------------------------------------------------------------------
+    //       Before this change:
+    //       -------------------------------------------------------------------
+    //       UI    : <-- build -->
+    //       GPU   :               <-- rasterize -->
+    //       Gap   :              | random |
+    //       Driver:                        <-- screenshot -->
+    //
+    //       In the diagram above, the gap is the time between the last driver
+    //       action taken, such as a `tap()`, and the subsequent call to
+    //       `screenshot()`. The gap is random because it is determined by the
+    //       unpredictable network communication between the driver process and
+    //       the application. If this gap is too short, the screenshot is taken
+    //       before the GPU thread is done rasterizing the frame, so the
+    //       screenshot of the previous frame is taken, which is wrong.
+    //
+    //       -------------------------------------------------------------------
+    //       After this change:
+    //       -------------------------------------------------------------------
+    //       UI    : <-- build -->
+    //       GPU   :               <-- rasterize -->
+    //       Gap   :              |    2 seconds or more   |
+    //       Driver:                                        <-- screenshot -->
+    //
+    //       The two-second gap should be long enough for the GPU thread to
+    //       finish rasterizing the frame, but not longer than necessary to keep
+    //       driver tests as fast a possible.
+    await new Future<Null>.delayed(const Duration(seconds: 2));
+
     final Map<String, dynamic> result = await _peer.sendRequest('_flutter.screenshot').timeout(timeout);
     return BASE64.decode(result['screenshot']);
   }