René's URL Explorer Experiment

Title: fix(ios): proper disposal and recreation of iOS native views by CatchABus · Pull Request #9879 · NativeScript/NativeScript · GitHub

Open Graph Title: fix(ios): proper disposal and recreation of iOS native views by CatchABus · Pull Request #9879 · NativeScript/NativeScript

X Title: fix(ios): proper disposal and recreation of iOS native views by CatchABus · Pull Request #9879 · NativeScript/NativeScript

Description: PR Checklist The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/tools/notes/CONTRIBUTING.md#commit-messages. There is an issue for the bug/feature this PR is for. To avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it. You have signed the CLA. All existing tests are passing: https://github.com/NativeScript/NativeScript/blob/master/tools/notes/DevelopmentWorkflow.md#running-unit-tests-application. Tests for the changes are included - https://github.com/NativeScript/NativeScript/blob/master/tools/notes/WritingUnitTests.md. 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: ***** Fatal JavaScript exception - application has been terminated. ***** NativeScript encountered a fatal error: Uncaught TypeError: Cannot set property 'delegate' of null at onUnloaded(file: src/webpack:/node_modules/@nativescript/core/ui/text-field/index.ios.js:114:0) at (file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:321:0) at callFunctionWithSuper(file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:312:0) at callUnloaded(file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:321:0) at unloadView(file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:466:0) at (file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:249:0) at eachChildView(file: src/webpack:/node_modules/@nativescript/core/ui/layouts/layout-base-common.js:101:0) at eachChild(file: src/webpack:/node_modules/@nativescript/core/ui/core/view/view-common.js:772:0) at onUnloaded(file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:248:0) at (file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:321:0) at callFunctionWithSuper(file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:312:0) at callUnloaded(file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:321:0) at unloadView(file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:466:0) at (file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:249:0) at eachChildView(file: src/webpack:/node_modules/@nativescript/core/ui/layouts/layout-base-common.js:101:0) at eachChild(file: src/webpack:/node_modules/@nativescript/core/ui/core/view/view-common.js:772:0) at onUnloaded(file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:248:0) at (file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:321:0) at callFunctionWithSuper(file: src/webpack:/node_modules/@nativescript/core/ui/core/view-base/index.js:312:0) at callUnloaded(file:///app/vend<…> 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 call callUnloaded 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 than removeChild, it's still in view hierarchy. One primary suspect for this crime is method destroyNode in View class. I'm not familiar with NS Angular, but its renderer seems to wrap this method inside another destroyNode. 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 inside destroyNode. 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 of insertChild and removeChild 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 calling removeChild. 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 of View class: https://github.com/NativeScript/NativeScript/pull/9879/files#diff-025d8125f2c2388468769424324a18b7a896921f7b44f720db8014eb653b5f84R67 This 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

Open Graph Description: PR Checklist The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/tools/notes/CONTRIBUTING.md#commit-messages. There is an issue for the bug/feature this...

X Description: PR Checklist The PR title follows our guidelines: https://github.com/NativeScript/NativeScript/blob/master/tools/notes/CONTRIBUTING.md#commit-messages. There is an issue for the bug/feature this...

Opengraph URL: https://github.com/NativeScript/NativeScript/pull/9879

X: @github

direct link

Domain: github.com

Links:

Viewport: width=device-width