Schedule a frame after markNeedsPaint is called

[angelosilvestre]

Schedule a frame after markNeedsPaint is called

Currently, we are calling markNeedsPaint directly when the follower layer transform changes.

This is causing some super_editor golden tests to fail due to a crash in the captureImage method called by matchesGoldenFile. This is the test failure output:

══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
The following assertion was thrown running a test:
'package:flutter_test/src/_matchers_io.dart': Failed assertion: line 29 pos 10:
'!renderObject.debugNeedsPaint': is not true.

When the exception was thrown, this was the stack:
#2      captureImage (package:flutter_test/src/_matchers_io.dart:29:10)
#3      MatchesGoldenFile.matchAsync (package:flutter_test/src/_matchers_io.dart:96:21)
#4      MatchesGoldenFileWithPixelAllowance.matchAsync (file:///var/super_editor/super_editor/test_goldens/test_tools_goldens.dart:164:26)
#5      _expect (package:matcher/src/expect/expect.dart:109:26)
#6      expectLater (package:matcher/src/expect/expect.dart:73:5)
#7      expectLater (package:flutter_test/src/widget_tester.dart:495:25)
#8      _testParagraphSelection.<anonymous closure> (file:///var/super_editor/super_editor/test_goldens/editor/mobile/mobile_selection_test.dart:833:11)
<asynchronous suspension>
#9      testGoldensOnAndroid.<anonymous closure> (file:///var/super_editor/super_editor/test_goldens/test_tools_goldens.dart:20:7)
<asynchronous suspension>
#10     testGoldens.<anonymous closure>.body (package:golden_toolkit/src/testing_tools.dart:167:11)
<asynchronous suspension>
#11     testGoldens.<anonymous closure> (package:golden_toolkit/src/testing_tools.dart:177:9)
<asynchronous suspension>
#12     testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:168:15)
<asynchronous suspension>
#13     TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1013:5)
<asynchronous suspension>
<asynchronous suspension>
(elided 3 frames from class _AssertionError and package:stack_trace)

The test description was:
  single tap text (on Android)
════════════════════════════════════════════════════════════════════════════════════════════════════
00:01 +0 -5: SuperEditor mobile selection Android single tap text (on Android) [E]                                                                                                                                                                                                                                      
  Test failed. See exception logs above.
  The test description was: single tap text (on Android)

This is the method that causes the crash:

Future<ui.Image> captureImage(Element element) {
  assert(element.renderObject != null);
  RenderObject renderObject = element.renderObject!;
  while (!renderObject.isRepaintBoundary) {
    renderObject = renderObject.parent!;
  }
  assert(!renderObject.debugNeedsPaint);  // crashes here
  final OffsetLayer layer = renderObject.debugLayer! as OffsetLayer;
  return layer.toImage(renderObject.paintBounds);
}

An example of a failing test:

testGoldensOnAndroid("with caret change colors", (tester) async {
    final testContext = await tester //
        .createDocument() //
        .fromMarkdown("This is some text to select.") //
        .useAppTheme(ThemeData(primaryColor: Colors.red)) //
        .pump();
    final nodeId = testContext.findEditContext().document.nodes.first.id;

    await tester.placeCaretInParagraph(nodeId, 15);

    await expectLater(
      find.byType(MaterialApp),
      matchesGoldenFile("goldens/supereditor_android_collapsed_handle_color.png"),
    );
  });

As an experiment, I tried to add await tester.pumpAndSettle(); in the test before the expectLater, but the exception still happens.

For some reason, the crash doesn't happen on mac.

This PR overrides markNeedsPaint to schedule a new frame.

[matthew-carroll]

I think I prefer the other proposed change, because that change makes it clear what the problem is - there might not be another frame scheduled so we'll end up with a dangling dirty state.

Also, instead of defining a new method, consider overriding markNeedsPaint if we always want to schedule a frame in such a case.

[angelosilvestre]

@matthew-carroll Updated.

[matthew-carroll]

This generally looks good, but can we get a bit more specific with the comment? I think most readers will wonder how it's possible that we mark ourselves dirty and yet a frame isn't scheduled on our behalf.

[angelosilvestre]

Updated. Do you think is it clear enough?

[matthew-carroll]

@angelosilvestre to remind myself of the status here, we ended up with you doing further investigation into the root cause, right?

[angelosilvestre]

@matthew-carroll Yeah, I still need to figure out how to write a failing test using follow_the_leader only.

[angelosilvestre]

@matthew-carroll As discussed, I modified this PR to only schedule a new frame when running in a test on Linux.

[matthew-carroll]

LGTM