Restore AndroidHarness activity, deprecate it#2774
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Android build configuration to manage stubs and introduces a deprecated AndroidHarness activity that wraps AndroidHarnessFragment via reflection. Feedback suggests improving encapsulation by making the fragment field a local variable in onCreate and replacing the reflection-based fragment attachment with type-safe calls by expanding the compile-time stubs for the Fragment API.
| protected ImageView splashImageView; | ||
| protected FrameLayout frameLayout; | ||
|
|
||
| private HarnessFragment fragment; |
There was a problem hiding this comment.
The fragment field is only used within the onCreate method and does not need to be a class member. Converting it to a local variable improves encapsulation and avoids keeping an unnecessary reference in the activity instance.
| fragment = attachFragment(); | ||
| fragment.setFinishOnAppStop(finishOnAppStop); |
There was a problem hiding this comment.
Since the fragment field is being removed from the class members, it should be declared as a local variable here.
| fragment = attachFragment(); | |
| fragment.setFinishOnAppStop(finishOnAppStop); | |
| HarnessFragment fragment = attachFragment(); | |
| fragment.setFinishOnAppStop(finishOnAppStop); |
| private HarnessFragment attachFragment() { | ||
| try { | ||
| Method getSupportFragmentManager = getClass().getMethod("getSupportFragmentManager"); | ||
| Object fragmentManager = getSupportFragmentManager.invoke(this); | ||
| Object existingFragment = fragmentManager.getClass() | ||
| .getMethod("findFragmentByTag", String.class) | ||
| .invoke(fragmentManager, HARNESS_FRAGMENT_TAG); | ||
| if (existingFragment instanceof HarnessFragment) { | ||
| return (HarnessFragment) existingFragment; | ||
| } | ||
|
|
||
| HarnessFragment newFragment = new HarnessFragment(); | ||
| Object transaction = fragmentManager.getClass().getMethod("beginTransaction").invoke(fragmentManager); | ||
| transaction = transaction.getClass() | ||
| .getMethod("replace", int.class, androidx.fragment.app.Fragment.class, String.class) | ||
| .invoke(transaction, android.R.id.content, newFragment, HARNESS_FRAGMENT_TAG); | ||
| transaction.getClass().getMethod("commit").invoke(transaction); | ||
| return newFragment; | ||
| } catch (ReflectiveOperationException exception) { | ||
| throw new IllegalStateException("Unable to attach AndroidHarnessFragment", exception); | ||
| } | ||
| } |
There was a problem hiding this comment.
The attachFragment method uses extensive reflection to interact with the Fragment API. Since the project already utilizes compile-time stubs for FragmentActivity and Fragment, it would be much cleaner and safer to add stubs for FragmentManager and FragmentTransaction as well. This would allow replacing the reflection with type-safe direct calls, improving maintainability and avoiding potential issues with code obfuscation (e.g., R8/ProGuard) that might rename these methods at runtime.
1 participant
Fixes #2765
Add a new AndroidHarness activity that emulates the old one, just wrapping a AndroidHarnessFragment.
Also changes the gradle build so the androidx stubs are treated as compile time only. A real jmonkey android application should provide its own androidx dependencies.