Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@kangwang1988
Copy link
Contributor

@kangwang1988 kangwang1988 commented Jan 28, 2019

Provide public api to allow FlutterEngine related context to be destoryed so that FlutterEngine can be released when needed.

I've noticed that many developers are using
FlutterEngine *flutterEngine = [[FlutterEngine alloc] initWithName:@"io.flutter" project:nil];
and they may find that the engine can't be released. (See: flutter/flutter#27149)
In this case, allowHeadlessExecution is set YES and they can't release the engine manually.
This PR will clearly make the allowHeadlessExecution:YES logic available to all developers and allow them to destory the context for the engine so that they can release the engine.
It(destoryContext)'s also safe even called multiple times.

…ryed so that FlutterEngine can be released when needed.
@kangwang1988
Copy link
Contributor Author

@dnfield
Prior review issue resolved for this pr.

@dnfield
Copy link
Contributor

dnfield commented Feb 9, 2019

I'm still not quite clear on this being the right API. I'm reluctant to introduce a method that would make the engine unuseable that isn't just dealloc. I understand if you're using ARC you can't diretly call dealloc - could I see a test case that shows this is necessary with ARC even if we were to call something similar from dealloc?

@dnfield
Copy link
Contributor

dnfield commented Feb 9, 2019

Ahh I'm looking at this again and I think I see what you're getting at better now. Even when we initialize a plain FlutterViewController, the engine ends up holding it alive. This is because we don't have a good way to distinguish between a VC disappearing for a temporary state (like when using the camera plugin) or a more permanent one. Unfortunately, I think we will have to expose this API - we really do need a developer to tell us when the ViewController (or the engine) is done. There are just too many possibilities, and in the end iOS expects a presenting ViewController to manage the dismissal of its child - the child isn't really ever made aware explicitly that it's being dismissed via the framework.

@dnfield
Copy link
Contributor

dnfield commented Feb 9, 2019

LGTM with a couple small doc updates. Sorry for the delay and my slowness in understanding this!

@kangwang1988
Copy link
Contributor Author

@dnfield
Review issues resolved.

Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dnfield
Copy link
Contributor

dnfield commented Feb 9, 2019

Looks like there's a formatting lint that needs to be applied, but once that's done this can land. Thanks for the contribution!

@kangwang1988
Copy link
Contributor Author

@dnfield
All checks passed and I'm going to merge this PR.

@kangwang1988 kangwang1988 merged commit 90569e8 into flutter:master Feb 9, 2019
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 9, 2019
engine-flutter-autoroll added a commit to flutter/flutter that referenced this pull request Feb 9, 2019
flutter/engine@7c70240...90569e8

git log 7c70240..90569e8 --no-merges --oneline
90569e8 Provide public api to allow FlutterEngine related context to be destoryed (flutter/engine#7610)
693645e Revert "Add mock capability to PerformanceOverlayLayer (#7537)" (flutter/engine#7765)
d130f15 Add x bit to some python scripts (flutter/engine#7764)
2146cde Roll src/third_party/skia 706a7cd1e826..87461aad7285 (1 commits) (flutter/engine#7763)
2a648b9 Roll src/third_party/skia be39f713e530..706a7cd1e826 (4 commits) (flutter/engine#7761)
68396ae Throttle picture raster cache (flutter/engine#7759)
a6753b0 Roll src/third_party/dart 52f5e34dbf..174d6fec3d (12 commits)
f9252e7 allow specifying out directory root (flutter/engine#7753)
12d0b95 Don't call OnAnimatorNotifyIdle if a frame is scheduled (flutter/engine#7746)
5f3f3bd Add mock capability to PerformanceOverlayLayer (flutter/engine#7537)
7d97afd Roll src/third_party/skia 37064c1739f3..be39f713e530 (7 commits) (flutter/engine#7757)
3bfd265 Add flutter config to macOS targets (flutter/engine#7756)
4d3a112 Document GPUSurfaceGLDelegate methods and move it to its own file. (flutter/engine#7755)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
kangwang1988 pushed a commit to XianyuTech/flutter that referenced this pull request Feb 12, 2019
flutter/engine@7c70240...90569e8

git log 7c70240..90569e8 --no-merges --oneline
90569e8 Provide public api to allow FlutterEngine related context to be destoryed (flutter/engine#7610)
693645e Revert &flutter#34;Add mock capability to PerformanceOverlayLayer (flutter#7537)&flutter#34; (flutter/engine#7765)
d130f15 Add x bit to some python scripts (flutter/engine#7764)
2146cde Roll src/third_party/skia 706a7cd1e826..87461aad7285 (1 commits) (flutter/engine#7763)
2a648b9 Roll src/third_party/skia be39f713e530..706a7cd1e826 (4 commits) (flutter/engine#7761)
68396ae Throttle picture raster cache (flutter/engine#7759)
a6753b0 Roll src/third_party/dart 52f5e34dbf..174d6fec3d (12 commits)
f9252e7 allow specifying out directory root (flutter/engine#7753)
12d0b95 Don&flutter#39;t call OnAnimatorNotifyIdle if a frame is scheduled (flutter/engine#7746)
5f3f3bd Add mock capability to PerformanceOverlayLayer (flutter/engine#7537)
7d97afd Roll src/third_party/skia 37064c1739f3..be39f713e530 (7 commits) (flutter/engine#7757)
3bfd265 Add flutter config to macOS targets (flutter/engine#7756)
4d3a112 Document GPUSurfaceGLDelegate methods and move it to its own file. (flutter/engine#7755)

The AutoRoll server is located here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+/master/autoroll/README.md

If the roll is causing failures, please contact the current sheriff ([email protected]), and stop
the roller if necessary.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants