Fix flutter/flutter#26660

[kangwang1988]

When the engine is a embedder engine(not passed by developers), the shell is destroyed on disappearing. However, when FlutterVC present a NativeVC and go back, the shell needs to be created again. Otherwise, it will crash due to null shell assert.

[dnfield]

I'm not sure this is the right fix - when I try it locally I don't get a crash anymore, but I also don't get back to the original viewcontroller....

[kangwang1988]

@dnfield
I think your last refactor to break the recycle reference between FlutterViewController and FlutterEngine works.
However, if it destroy the shell while the FlutterViewController remains, every assert or reference to the shell will fail(for example, touchBegan: or in this case), it will crash. Once it destroy the shell, it needs to recreate it when necessary.

[dnfield]

Something's broken, I agree. I'm trying to figure out how to fix this and not break the previous problem :)

[kangwang1988]

I'm not sure this is the right fix - when I try it locally I don't get a crash anymore, but I also don't get back to the original viewcontroller....

Are you saying https://github.com/KevinTheGray/flutterissues?
In flutterissues, it just present and will not go back.

- (BOOL)application:(UIApplication *)application
    didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {
  FlutterViewController* controller = (FlutterViewController*)self.window.rootViewController;
  [GeneratedPluginRegistrant registerWithRegistry:self];
  FlutterMethodChannel* launchChannel = [FlutterMethodChannel
                                          methodChannelWithName:@"launch"
                                          binaryMessenger:controller];
  [launchChannel setMethodCallHandler:^(FlutterMethodCall * _Nonnull call, FlutterResult  _Nonnull result) {
    [controller presentViewController:[[UIViewController alloc] init] animated:YES completion:nil];
  }];
  return [super application:application didFinishLaunchingWithOptions:launchOptions];
}

[kangwang1988]

See git@github.com:kangwang1988/flutterissues.git
Made it dismissable, however, I found another problem after FlutterVC present a native one and dismiss back.
Though the touch event can be passed from iOS to Flutter Side, the gesture detection will fail.

2019-01-19 11:55:41.731621+0800 Runner[4302:824042] flutter: KWLM:main.dart:main called!
2019-01-19 11:55:41.732276+0800 Runner[4302:824042] flutter: KWLM:main.dart:MyApp.build called!
2019-01-19 11:55:49.346673+0800 Runner[4302:824042] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:49.366707+0800 Runner[4302:824042] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:49.375902+0800 Runner[4302:824042] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:49.392570+0800 Runner[4302:824042] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:49.408176+0800 Runner[4302:824042] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:49.408365+0800 Runner[4302:824042] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:49.409053+0800 Runner[4302:824042] flutter: KWLM:main.dart:_MyHomePageState.build.onTap called!
2019-01-19 11:55:50.218721+0800 Runner[4302:824121] flutter: KWLM:main.dart:main called!
2019-01-19 11:55:50.219643+0800 Runner[4302:824121] flutter: KWLM:main.dart:MyApp.build called!
2019-01-19 11:55:52.746989+0800 Runner[4302:824121] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:52.778007+0800 Runner[4302:824121] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:52.794216+0800 Runner[4302:824121] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:52.810991+0800 Runner[4302:824121] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:52.827744+0800 Runner[4302:824121] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand called!
2019-01-19 11:55:52.835336+0800 Runner[4302:824121] flutter: KWLM:gestures:converter.dart:PointerEventConverter.expand call

But I do think this is another problem.

[dnfield]

Ahh I missed something in there.

I had a really hectic day today, but I'll take a closer look at this soon - sorry for the delay.

[kangwang1988]

Ahh I missed something in there.

I had a really hectic day today, but I'll take a closer look at this soon - sorry for the delay.

Never mind.
Take care.

[dnfield]

I'm pretty sure this will lead to unexpected loss of state. If we allow the engine to get torn down here, restarting it will prevent th ecrash but also restart the app from scratch.

[dnfield]

I appreciate this PR - it helped me understand the problem better for sure. However, I think to really get at this we should override the present methods on FlutterViewController to make sur ethe engine doesn't disappear in those scenarios. What do you think?

[kangwang1988]

@dnfield
I though it would make things more complicated if you want to detect if the engine should remain based on the viewDidDisappear logic. There are several cases which might cause the viewDidDisappear logic, right?
push, present, setViewControllers directly, added as a childViewController, etc.

Besides, I don't think it's a good idea to process the logic depending on the viewDidDisappear one, it would be weird as the state is lost, as you mentioned in my patch.

dealloc seems a better function to do this.
However ,there is a recycle retain.

Can we add an external api exposed to developers so that we can still break the recycle retain, and don't call it from the default intra logic?

[dnfield]

Can you take a look at #7544? I agree with you that overriding those methods is probably not the way to go - this is a little bit different.

[kangwang1988]

@dnfield
OK

[kangwang1988]

Close #26660 via #7544