Fix crash during composable attachment to android.app.Activity #613
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
android.app.Activity, which is the base class in the framework package, does not provide a lifecycle owner by default. ComposeView requires a lifecycle owner when attaching to a view and not finding one will result in a crash. android.app.Activty has not been updated for a long time and Android created AppCompatActivity/Fragment activity in the support package to allow updates to be compatible with old android devices. Android recommends using inheriting from AppCompatActivity or similar which provides a lifecycle owner by default (irrespective of whether compose is being used or not). There may be cases where apps use this legacy Activity in their apps and launching our Ui Service components on top of them will not work unless the ComposeView sees that there is some lifecycle owner. To fix this, attach a proxy lifecycle owner and related owners to the view of the activity just before attaching the ComposeView and detach it when the activity is destroyed. This will allow displaying out UI components like InAppMessages, Floating buttons on these legacy implementations. Since there is originally no observer attachable to the Activity, there are no components dependent on the events from this owner except for the compose view.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## feature/uiservice #613 +/- ##
=======================================================
- Coverage 81.52% 81.14% -0.37%
Complexity 2110 2110
=======================================================
Files 189 190 +1
Lines 8852 8899 +47
Branches 1108 1110 +2
=======================================================
+ Hits 7216 7221 +5
- Misses 1078 1120 +42
Partials 558 558
Flags with carried forward coverage won't be shown. Click here to find out more.
|
code/core/src/phone/java/com/adobe/marketing/mobile/services/ui/common/ActivityCompatOwner.kt
Outdated
Show resolved
Hide resolved
code/core/src/phone/java/com/adobe/marketing/mobile/services/ui/common/ActivityCompatOwner.kt
Outdated
Show resolved
Hide resolved
code/core/src/phone/java/com/adobe/marketing/mobile/services/ui/common/ActivityCompatOwner.kt
Outdated
Show resolved
Hide resolved
code/core/src/phone/java/com/adobe/marketing/mobile/services/ui/common/ActivityCompatOwner.kt
Outdated
Show resolved
Hide resolved
code/core/src/phone/java/com/adobe/marketing/mobile/services/ui/common/ActivityCompatOwner.kt
Outdated
Show resolved
Hide resolved
code/core/src/phone/java/com/adobe/marketing/mobile/services/ui/common/AEPPresentable.kt
Show resolved
Hide resolved
yangyansong-adbe
approved these changes
Feb 26, 2024
praveek
reviewed
Feb 27, 2024
| view?.apply { | ||
| setViewTreeLifecycleOwner(null) | ||
| setViewTreeViewModelStoreOwner(null) | ||
| setViewTreeSavedStateRegistryOwner(null) |
There was a problem hiding this comment.
Did you miss clearing setViewTreeOnBackPressedDispatcherOwner?
There was a problem hiding this comment.
The backPressedDispatcherOwner will be cleared automatically. ThesetViewTreeOnBackPressedDispatcherOwner does not accept null's.
praveek
approved these changes
Feb 27, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
ComposeView requires a lifecycle owner when attaching to a view and not finding one will result in a crash.
android.app.Activity, which is the base class in the framework package, does not provide a lifecycle owner by default.android.app.Activtyhas not been updated for a long time and Android createdAppCompatActivity/FragmentActivityin the support package to allow updates to be compatible with old android devices. Android recommends using inheriting fromAppCompatActivityor similar which provides a lifecycle owner by default (irrespective of whether compose is being used or not).There may be cases where apps still use this legacy Activity in their apps and launching our Ui Service components on top of them will not work unless the ComposeView sees that there is some lifecycle owner.
To fix this, attach a proxy lifecycle owner and related owners to the view of the activity just before attaching the ComposeView and detach it when the activity is destroyed. This will allow displaying out UI components like InAppMessages, Floating buttons on these legacy implementations. Since there is originally no observer attachable to the Activity, there are no components dependent on the events from this owner except for the compose view.
Related Issue
Motivation and Context
Fix crash during composable attachment to android.app.Activity
How Has This Been Tested?
android.app.Activitywith test app and also using Assurance 2.x which uses the same.. Unit tests need complicated logic due to the way underlying implementation of theView.find*Owner.Screenshots (if appropriate):
Types of changes
Checklist: