-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
fix(ios): proper disposal and recreation of iOS native views #9879
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
fix(ios): proper disposal and recreation of iOS native views #9879
Conversation
…view was transformed too early.
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla. |
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla. |
@cla-bot check |
The cla-bot has been summoned, and re-checked this pull request! |
PR Checklist
What is the current behavior?
Native iOS views are not disposed and cannot be recreated when their view instances are re-added to the view tree.
What is the new behavior?
Native View Disposal
This PR was initially created for the sole purpose of destroying old native views (until more problems showed up in the process).
This is based on Martin's @farfromrefug PR #9671 which was reverted because of few problems.
@rigor789 If I'm not mistaken, the most common error case was this one:
After a bit of research, I concluded the root of this cause is
_tearDownUI
calls being called before the view gets unloaded.So, what are the cases for a view to get unloaded too late when it's already destroyed?
One case is view controllers which have a
viewDidDisappear
method that is called once owner view is disposed. This method will callcallUnloaded
which will attempt to unload view and all its children in view tree. Let's keep in mind that if a view is disposed by means other thanremoveChild
, it's still in view hierarchy.One primary suspect for this crime is method
destroyNode
inView
class. I'm not familiar with NS Angular, but its renderer seems to wrap this method inside anotherdestroyNode
. One of errors mentioned in #9671 has also occured in an Angular app.I couldn't locate any other error-prone cases and I'm not completely sure all errors have occured because of the flaw in that certain method, but for start I added a missing
callUnloaded
call insidedestroyNode
.If there are more causes that trigger this issue, I believe we can track them upon further app usage and solve them immediately, since we know what can cause this now. Core has got a dozen of
_tearDownUI
calls that are supposed to get called after view is unloaded but who knows!Just as @farfromrefug pointed out, perhaps we should make sure those two calls are done in proper order in the future, like adding
callUnloaded
inside_tearDownUI
for example which needs some refactoring.So, all these details refer to https://github.com/NativeScript/NativeScript/pull/9879/files#diff-86ce61f77399a980c83c5a14ee2ac54b8c672d375d0280597c217eb17811a076, the change for disposing native views for iOS.
Re-using iOS view instance (non-reusable native view)
What if one wants to reuse the same NS instance even though its native view is not reusable? Upon re-adding an already removed view instance to the view tree, it should generate a new native view. This leads us to new problems which I'll use an example to describe below.
I'll use a non-reusable animated drawer view of mine embedded in
RootLayout
, in a plain NS app. This is a good example since there is a constant use ofinsertChild
andremoveChild
methods, and view will also get transformed it gets hidden to the left side of the screen.In short, attempting to open drawer will result in calling
insertChild
, while trying to close it will result in callingremoveChild
.This is how my drawer originally looks like when opening it for first time:

If it's removed and re-added to view tree in current NS core version, it will look like this:

It looks like this because images undergo a cleanup on disposal since NS 8.2. As view is not currently disposed properly, native view will not be created anew and this results in images not being reloaded.
Now, the following screenshot displays the result of using changes from PR #9671 alone.

View layout seems completely messed up and that is so confusing. This problem occured because view instances keep cache of last used native frame.
And here comes the addition of
disposeNativeView
in the iOS version ofView
class: https://github.com/NativeScript/NativeScript/pull/9879/files#diff-025d8125f2c2388468769424324a18b7a896921f7b44f720db8014eb653b5f84R67This comes with really good results:

Yet, it seems few children are not aligned properly, and there's no good explanation.
I concluded that the cause of this is the native view
translateX
transform occuring too early. That transform messes up view position in native window, resulting in padding and inset miscalculations.That is why we should prevent transforms from happening when layout is still invalid, and apply them once view content layout is ready.
See here: https://github.com/NativeScript/NativeScript/pull/9879/files#diff-025d8125f2c2388468769424324a18b7a896921f7b44f720db8014eb653b5f84R132
The final result:

The proper disposal of native views works quite well with the few recent additions of GC inside core.
About reuse, I believe this is a really good step for re-using iOS views even when their native instances are not reusable. This has always been possible in android, but it seems we lacked the proper view cleanup to make this work in iOS and that created a bit of confusion.
This PR is a possible candidate for fixing the iOS part of #9820, and its view cleanup is a good solution for my issue described in #9875