[iOS][Keyboard] Wait vsync on UI thread and update viewport inset to avoid jitter.

[luckysmg]

Fixes flutter/flutter#120555.

Video for after and before this patch:
videos.zip

• Keyboard janky animation on iOS flutter#120555

The technical problem is: if setViewportMetrics called before framework's vsync process callback (rebuild & layout & paint), will cause jitter keyboard animation.So this PR is to keep the time when updateViewportMetrics call to framework side legal.

Before

After

Pre-launch Checklist

• I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
• I read the [Tree Hygiene] wiki page, which explains my responsibilities.
• I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
• I listed at least one issue that this PR fixes in the description above.
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
• I updated/added relevant documentation (doc comments with ///).
• I signed the [CLA].
• All existing tests and new tests are passing.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[chinmaygarde]

Can you cross reference issues here and perhaps describe how to test this?

[dnfield]

Is there an issue describing what you're trying to fix here?

Can you add a test that fails without this change?

[luckysmg]

Sorry my bad. Here is the issue this PR try to fix.

• Keyboard janky animation on iOS flutter#120555

And The technical problem is: if setViewportMetrics called before framework's vsync process callback (rebuild & layout & paint), will cause jitter keyboard animation.So this PR is to keep the time when updateViewportMetrics call to framework side legal.

A previous naive solution is #41201.

And I m still figuring how to write the test for this PR... But seems very triky to write test......

[luckysmg]

I made the setupKeyboardVsyncClient more testable.

So If use old way, current code in setupKeyboardVsyncClient will be:

  auto callback = [keyboardAnimationCallback](
                        std::unique_ptr<flutter::FrameTimingsRecorder> recorder) {
    fml::TimeDelta frameInterval = recorder->GetVsyncTargetTime() - recorder->GetVsyncStartTime();
    fml::TimePoint keyboardAnimationTargetTime = recorder->GetVsyncTargetTime() + frameInterval;
    keyboardAnimationCallback(keyboardAnimationTargetTime);
  };
  _keyboardAnimationVSyncClient =
      [[VSyncClient alloc] initWithTaskRunner:shell.GetTaskRunners().GetPlatformTaskRunner()
                                     callback:callback];

And new test testKeyboardAnimationWillWaitUIThreadVsync will fails

And if use the code in the patch, the new test will pass.

[luckysmg]

How to test this patch:
Using Phone like iPhone 12
and test code to see if the keyboard animation is smooth.

Here is video zip can show the effect better
videos.zip

Demo code

void main() {
  runApp(const MyApp());
}


class MyApp extends StatelessWidget {
  const MyApp({Key? key}) : super(key: key);

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: DemoScreen(),
    );
  }
}

class DemoScreen extends StatefulWidget {
  const DemoScreen({Key? key}) : super(key: key);

  @override
  State<DemoScreen> createState() => _DemoScreenState();
}

class _DemoScreenState extends State<DemoScreen> with TickerProviderStateMixin {
  final TextEditingController _messageController = TextEditingController();
  final FocusNode _focusNode = FocusNode();
  late final _messageNotifier = ValueNotifier(_messageController.text.isEmpty);
  bool hasFocus = false;

  @override
  Widget build(BuildContext context) {
    return GestureDetector(
      onTap: () {
        _focusNode.unfocus();
      },
      child: Scaffold(
        appBar: AppBar(
          title: const Text('Chat screen'),
        ),
        body: Column(
          children: <Widget>[
            const Spacer(),
            const Text(
              'keyboard animation',
              style: TextStyle(fontSize: 30),
            ),
            const Spacer(),
            Container(
              padding: EdgeInsets.only(
                bottom: MediaQuery.paddingOf(context).bottom + 10,
                top: 6.0,
                left: 15.0,
                right: 15.0,
              ),
              constraints: const BoxConstraints(
                minHeight: 36,
              ),
              child: Row(
                crossAxisAlignment: CrossAxisAlignment.end,
                children: [
                  Expanded(
                    child: DecoratedBox(
                      decoration: BoxDecoration(
                        color: Colors.white,
                        borderRadius: BorderRadius.circular(19),
                        border: Border.all(
                          width: 0.5,
                          color: const Color(0xffCFCFCF),
                        ),
                      ),
                      child: Row(
                        crossAxisAlignment: CrossAxisAlignment.end,
                        children: [
                          Container(
                            margin: const EdgeInsets.fromLTRB(7, 6, 7, 6),
                            decoration: const BoxDecoration(
                              color: Color(0xFF007AEA),
                              shape: BoxShape.circle,
                            ),
                            width: 24,
                            height: 24,
                            child: const Icon(
                              Icons.add,
                              color: Colors.white,
                            ),
                          ),
                          const SizedBox(width: 9.0),
                          Expanded(
                            child: Padding(
                              padding: const EdgeInsets.only(left: 0, right: 8),
                              child: TextFormField(
                                controller: _messageController,
                                minLines: 1,
                                textCapitalization: TextCapitalization.sentences,
                                focusNode: _focusNode,
                                decoration: const InputDecoration(
                                  border: InputBorder.none,
                                  isDense: true,
                                  hintText: "Message",
                                ),
                              ),
                            ),
                          ),
                          ValueListenableBuilder<bool>(
                            valueListenable: _messageNotifier,
                            builder: (ctx, value, child) {
                              if (value) {
                                return const Padding(
                                  padding: EdgeInsets.fromLTRB(7, 6, 7, 6),
                                  child: Icon(
                                    Icons.emoji_emotions_outlined,
                                    color: Color(0xFF9E9E9E),
                                  ),
                                );
                              }
                              return const SizedBox.shrink();
                            },
                          )
                        ],
                      ),
                    ),
                  ),
                  const SizedBox(
                    width: 8,
                  ),
                  Container(
                    height: 36,
                    width: 36,
                    decoration: const BoxDecoration(
                      color: Color(0xFF007AEA),
                      shape: BoxShape.circle,
                    ),
                    child: ValueListenableBuilder<bool>(
                      valueListenable: _messageNotifier,
                      builder: (ctx, value, child) {
                        return GestureDetector(
                          onTap: () {
                            _focusNode.unfocus();
                            hasFocus = false;
                          },
                          child: const Icon(
                            Icons.send,
                            color: Colors.white,
                          ),
                        );
                      },
                    ),
                  ),
                ],
              ),
            ),
          ],
        ),
      ),
    );
  }
}

[hellohuanlin]

Could you explain more about the solution in the PR description?

[luckysmg]

Could you explain more about the solution in the PR description?

Done

[flutter-dashboard]

Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change).

If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review.

Changes reported for pull request #42312 at sha adfb63a

[Peng-Qian]

I am eagerly awaiting the implementation of this improvement. Presently, the management of the iOS keyboard's upper widget has been a source of considerable frustration due to issues of jank, jitter, and lack of synchronized animation

[luckysmg]

I have changed the code for nits.
I didn't change the block to objc style because it will be passed in a C++ block and it will have some trouble when I change to objc style block.
And I add a getter for FlutterEngine to get UITaskRunner , but seems test is not happy for this. See https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8778773587495260785/+/u/test:_Tests_for_ios_debug_sim/stdout

*** NSForwarding: warning: method signature and compiler disagree on struct-return-edness of 'uiTaskRunner'.  Signature thinks it does not return a struct, and compiler thinks it does.

But I locally run the test and success without error 🤔

[luckysmg]

Still not sure why bot doesn't happy about the interface uiTaskRunner.... It is the same like other interfaces.....

*** NSForwarding: warning: method signature and compiler disagree on struct-return-edness of 'uiTaskRunner'.  Signature thinks it does not return a struct, and compiler thinks it does.

[luckysmg]

I think I have solved the issue. Because the test testHandleKeyboardNotification use OCMPartialMock to mock FlutterEngine and call functions like [engine uiTaskRunner] seems will have some issue. I have modfied the test. Just keep the test can pass with this patch and also is valid for test the old thing (updateViewportMetrics）. ^_^

[cyanglaz]

LGTM

[delfme]

@luckysmg @chinmaygarde thx so much for this 🎉