Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[iOS][Text Input] Avoid crash when its UIViewController.view is nil. #39768

Merged
merged 4 commits into from Feb 23, 2023

Conversation

luckysmg
Copy link
Contributor

@luckysmg luckysmg commented Feb 21, 2023

Fix:

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 and new tests are passing.

@luckysmg luckysmg self-assigned this Feb 21, 2023
@luckysmg luckysmg changed the title [iOS][Text Input] Avoid handling method calls when host UIViewController.view is nil. [iOS][Text Input] Avoid handling method calls when its UIViewController.view is nil. Feb 21, 2023
@luckysmg luckysmg requested review from LongCatIsLooong and hellohuanlin and removed request for LongCatIsLooong February 21, 2023 07:10
@LongCatIsLooong
Copy link
Contributor

Could you help me understand when this could happen? From the stacktrace here: flutter/flutter#106404 (comment), not handling

-[FlutterTextInputPlugin setTextInputClient:withConfiguration:]

could be disastrous (since the framework always assumes the connection is successfully established), unless the engine is about to be torn down. But if the engine is being deallocated we're not supposed to get any platform messages right?

@luckysmg
Copy link
Contributor Author

In add-to-app scene. We will share one engine with multiple ViewController, and we will set engine's view controller to nil manually sometimes. Any if some message's coming will trigger this crash....

@luckysmg
Copy link
Contributor Author

Here is the demo code to reproduce 100% with this crash.
in main.dart:

Dart code
import 'package:flutter/material.dart';

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

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  // This widget is the root of your application.
  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      title: 'Flutter Demo',
      theme: ThemeData(
        colorScheme: ColorScheme.fromSeed(seedColor: Colors.deepPurple),
        useMaterial3: true,
      ),
      home: const MyHomePage(title: 'Flutter Demo Home Page'),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {

  FocusNode node = FocusNode();

  @override
  void initState() {
    super.initState();
    Future.delayed(const Duration(seconds: 5),() {
      node.requestFocus();
    });
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        backgroundColor: Theme.of(context).colorScheme.inversePrimary,
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            const Text(
              'You have pushed the button this many times:',
            ),
            TextField(
              focusNode: node,
            ),
          ],
        ),
      ),
    );
  }
}

in FlutterAppDelegate.swift

swift code
import UIKit
import Flutter

@UIApplicationMain
@objc class AppDelegate: FlutterAppDelegate {
  override func application(
    _ application: UIApplication,
    didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?
  ) -> Bool {
    GeneratedPluginRegistrant.register(with: self)
      
      
      let vc = window.rootViewController as! FlutterViewController;
      DispatchQueue.main.asyncAfter(deadline: DispatchTime.now() + 3){
          let engine = vc.engine
          engine?.viewController = nil
      }
      
    return super.application(application, didFinishLaunchingWithOptions: launchOptions)
  }
}

It is very easy for developers to write code like this (delay some time request focus), if we detach the view controller from engine(This is very common in add-to-app scene), it will cause crash. I think we should do something to fallback, maybe not perfect, but at least, it will be better than crash directly ^_^

@luckysmg luckysmg changed the title [iOS][Text Input] Avoid handling method calls when its UIViewController.view is nil. [iOS][Text Input] Avoid crash when its UIViewController.view is nil. Feb 22, 2023
@luckysmg
Copy link
Contributor Author

I have modified the code. I think it's better than before. Plz re-review ^_^

@luckysmg luckysmg added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 23, 2023
@auto-submit auto-submit bot merged commit c457713 into flutter:main Feb 23, 2023
@luckysmg luckysmg deleted the fix_textInputPlugin branch February 23, 2023 02:47
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 23, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 23, 2023
…121303)

* 00f2a3861 Deprecate WindowPadding (flutter/engine#39775)

* 4c6c259ba Roll Skia from 335cabcf8b99 to 080897012390 (4 revisions) (flutter/engine#39802)

* e1c327750 Roll Skia from 080897012390 to f163f6ed5db0 (1 revision) (flutter/engine#39806)

* c4577135b [iOS][Text Input] Avoid crash when its UIViewController.view is nil. (flutter/engine#39768)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-ios
Projects
None yet
3 participants