sockeqwe / mosby

A Model-View-Presenter / Model-View-Intent library for modern Android apps
http://hannesdorfmann.com/mosby/
Apache License 2.0
5.49k stars 841 forks source link

Change BackstackAccessor.isInBackStackAndroidX() implementation #338

Closed uOOOO closed 4 years ago

uOOOO commented 4 years ago

FragmentManager.toString() occurs NPE when mHost is null since androidx.fragment:fragment:1.2.0-beta01.

        java.lang.NullPointerException: Attempt to invoke virtual method 'java.lang.Class java.lang.Object.getClass()' on a null object reference
        at androidx.fragment.app.FragmentManager.toString(FragmentManager.java:1022)
        at java.lang.String.valueOf(String.java:2896)
        at java.lang.StringBuilder.append(StringBuilder.java:132)
        at androidx.fragment.app.Fragment.dump(Fragment.java:2640)
        at androidx.core.app.BackstackAccessor.isInBackStackAndroidX(BackstackAccessor.java:57)
        at androidx.core.app.BackstackAccessor.isFragmentOnBackStack(BackstackAccessor.java:46)
        at com.hannesdorfmann.mosby3.mvp.delegate.FragmentMvpDelegateImpl.retainPresenterInstance(FragmentMvpDelegateImpl.java:163)
        at com.hannesdorfmann.mosby3.mvp.delegate.FragmentMvpDelegateImpl.onDestroy(FragmentMvpDelegateImpl.java:285)
        at com.hannesdorfmann.mosby3.mvp.MvpFragment.onDestroy(MvpFragment.java:103)

So this PR uses reflection instead of Fragment.dump().

mseroczynski commented 4 years ago

@sockeqwe Please merge & release it

Also kindly please let me know if you're planning to stop supporting / deprecate Mosby, I'd really appreciate knowing if I'm in trouble.

sockeqwe commented 4 years ago

Thanks for your PR, however, I don't think that this will work:

 if (clazz != null && clazz.getName().startsWith("androidx")) {
                Method method = clazz.getDeclaredMethod("isInBackStack");
                method.setAccessible(true);
                return (boolean) method.invoke(fragment);
            }

as android has some internal white / black listing and would at runtime not allow to use reflection on androidx classes (usually)...

Did that work for you? Can you provide a working unit test?

uOOOO commented 4 years ago

I knew that but do the support libraries also have grey or blacklist APIs? Our app is working on Pie device without any problem. And I can run BackstackActivityWithChildFragmentsTest.fragmentsOnBackstack() test in utils-fragment-integration-test module successfully on Q emulator. Before I ran the test, I changed compileSdkVersion and targetSdkVersion to 29.

Testing started at 12:25 ...

12/09 12:25:24: Launching 'fragmentsOnBackstack()' on Pixel 2 API 29.
Running tests

$ adb shell am instrument -w -r   -e debug false -e class 'com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragmentsTest#fragmentsOnBackstack' com.hannesdorfmann.mosby3.utils.fragment.integrationtest.test/android.support.test.runner.AndroidJUnitRunner
Waiting for process to come online...

Started running tests
Connected to process 25669 on device 'emulator-5554'.
Capturing and displaying logcat messages from application. This behavior can be disabled in the "Logcat output" section of the "Debugger" settings page.
I/integrationtes: The ClassLoaderContext is a special shared library.
I/chatty: uid=10137(com.hannesdorfmann.mosby3.utils.fragment.integrationtest) identical 2 lines
I/integrationtes: The ClassLoaderContext is a special shared library.
W/integrationtes: Accessing hidden method Landroid/app/Instrumentation;->execStartActivity(Landroid/content/Context;Landroid/os/IBinder;Landroid/os/IBinder;Landroid/app/Activity;Landroid/content/Intent;ILandroid/os/Bundle;)Landroid/app/Instrumentation$ActivityResult; (greylist, linking, allowed)
I/MonitoringInstrumentation: Instrumentation Started!
I/MonitoringInstrumentation: Setting context classloader to 'dalvik.system.PathClassLoader[DexPathList[[zip file "/system/framework/android.test.runner.jar", zip file "/system/framework/android.test.mock.jar", zip file "/data/app/com.hannesdorfmann.mosby3.utils.fragment.integrationtest.test-E-kjV-TiTTDBkzvEFtE6kA==/base.apk", zip file "/data/app/com.hannesdorfmann.mosby3.utils.fragment.integrationtest-urV2dPluCuHBow_yO4zjDQ==/base.apk"],nativeLibraryDirectories=[/data/app/com.hannesdorfmann.mosby3.utils.fragment.integrationtest.test-E-kjV-TiTTDBkzvEFtE6kA==/lib/x86, /data/app/com.hannesdorfmann.mosby3.utils.fragment.integrationtest-urV2dPluCuHBow_yO4zjDQ==/lib/x86, /system/lib, /system/product/lib]]]', Original: 'dalvik.system.PathClassLoader[DexPathList[[zip file "/system/framework/android.test.runner.jar", zip file "/system/framework/android.test.mock.jar", zip file "/data/app/com.hannesdorfmann.mosby3.utils.fragment.integrationtest.test-E-kjV-TiTTDBkzvEFtE6kA==/base.apk", zip file "/data/app/com.hannesdorfmann.mosby3.utils.fragment.integrationtest-urV2dPluCuHBow_yO4zjDQ==/base.apk"],nativeLibraryDirectories=[/data/app/com.hannesdorfmann.mosby3.utils.fragment.integrationtest.test-E-kjV-TiTTDBkzvEFtE6kA==/lib/x86, /data/app/com.hannesdorfmann.mosby3.utils.fragment.integrationtest-urV2dPluCuHBow_yO4zjDQ==/lib/x86, /system/lib, /system/product/lib]]]'
I/MonitoringInstrumentation: No JSBridge.
D/InfraTrack: Tracking disabled due to lack of internet permissions
I/UsageTrackerFacilitator: Usage tracking disabled
D/TestExecutor: Adding listener android.support.test.internal.runner.listener.LogRunListener
    Adding listener android.support.test.internal.runner.listener.InstrumentationResultPrinter
    Adding listener android.support.test.internal.runner.listener.ActivityFinisherRunListener
I/TestRunner: run started: 1 tests
I/TestRunner: started: fragmentsOnBackstack(com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragmentsTest)
I/MonitoringInstrumentation: Activities that are still in CREATED to STOPPED: 0
D/ActivityTestRule: Launching activity com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments
D/MonitoringInstrumentation: execStartActivity(context, ibinder, ibinder, activity, intent, int, bundle
W/ActivityThread: handleWindowVisibility: no activity for token android.os.BinderProxy@a1e3304
D/libEGL: Emulator has host GPU support, qemu.gles is set to 1.
W/libc: Unable to set property "qemu.gles" to "1": connection failed; errno=13 (Permission denied)
W/RenderThread: type=1400 audit(0.0:269): avc: denied { write } for name="property_service" dev="tmpfs" ino=8906 scontext=u:r:untrusted_app:s0:c137,c256,c512,c768 tcontext=u:object_r:property_socket:s0 tclass=sock_file permissive=0
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@7cd7b70 in: PRE_ON_CREATE
D/libEGL: loaded /vendor/lib/egl/libEGL_emulation.so
D/libEGL: loaded /vendor/lib/egl/libGLESv1_CM_emulation.so
D/libEGL: loaded /vendor/lib/egl/libGLESv2_emulation.so
W/integrationtes: Accessing hidden method Landroid/view/View;->computeFitSystemWindows(Landroid/graphics/Rect;Landroid/graphics/Rect;)Z (greylist, reflection, allowed)
W/integrationtes: Accessing hidden method Landroid/view/ViewGroup;->makeOptionalFitsSystemWindows()V (greylist, reflection, allowed)
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@7cd7b70 in: CREATED
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@7cd7b70 in: STARTED
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@7cd7b70 in: RESUMED
D/HostConnection: HostConnection::get() New Host Connection established 0xe0a32410, tid 25703
D/HostConnection: HostComposition ext ANDROID_EMU_CHECKSUM_HELPER_v1 ANDROID_EMU_native_sync_v2 ANDROID_EMU_native_sync_v3 ANDROID_EMU_dma_v1 ANDROID_EMU_direct_mem ANDROID_EMU_host_composition_v1 ANDROID_EMU_host_composition_v2 ANDROID_EMU_YUV420_888_to_NV21 ANDROID_EMU_YUV_Cache GL_OES_EGL_image_external_essl3 GL_OES_vertex_array_object GL_KHR_texture_compression_astc_ldr ANDROID_EMU_gles_max_version_3_0 
W/OpenGLRenderer: Failed to choose config with EGL_SWAP_BEHAVIOR_PRESERVED, retrying without...
D/eglCodecCommon: setVertexArrayObject: set vao to 0 (0) 0 0
D/EGL_emulation: eglCreateContext: 0xe0a1a300: maj 3 min 0 rcv 3
D/EGL_emulation: eglMakeCurrent: 0xe0a1a300: ver 3 0 (tinfo 0xe0a0f6b0)
W/Gralloc3: mapper 3.x is not supported
D/HostConnection: createUnique: call
D/HostConnection: HostConnection::get() New Host Connection established 0xe0a32550, tid 25703
D/HostConnection: HostComposition ext ANDROID_EMU_CHECKSUM_HELPER_v1 ANDROID_EMU_native_sync_v2 ANDROID_EMU_native_sync_v3 ANDROID_EMU_dma_v1 ANDROID_EMU_direct_mem ANDROID_EMU_host_composition_v1 ANDROID_EMU_host_composition_v2 ANDROID_EMU_YUV420_888_to_NV21 ANDROID_EMU_YUV_Cache GL_OES_EGL_image_external_essl3 GL_OES_vertex_array_object GL_KHR_texture_compression_astc_ldr ANDROID_EMU_gles_max_version_3_0 
D/eglCodecCommon: allocate: Ask for block of size 0x1000
D/eglCodecCommon: allocate: ioctl allocate returned offset 0x3ff807000 size 0x2000
D/EGL_emulation: eglMakeCurrent: 0xe0a1a300: ver 3 0 (tinfo 0xe0a0f6b0)
D/eglCodecCommon: setVertexArrayObject: set vao to 0 (0) 1 0
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@7cd7b70 in: PAUSED
D/EGL_emulation: eglMakeCurrent: 0xe0a1a300: ver 3 0 (tinfo 0xe0a0f6b0)
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@7cd7b70 in: STOPPED
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@7cd7b70 in: DESTROYED
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@ce73585 in: PRE_ON_CREATE
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@ce73585 in: CREATED
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@ce73585 in: STARTED
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@ce73585 in: RESUMED
D/EGL_emulation: eglMakeCurrent: 0xe0a1a300: ver 3 0 (tinfo 0xe0a0f6b0)
I/TestRunner: finished: fragmentsOnBackstack(com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragmentsTest)
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@ce73585 in: PAUSED
I/MonitoringInstrumentation: Activities that are still in CREATED to STOPPED: 1
I/TestRunner: run finished: 1 tests, 0 failed, 0 ignored
I/MonitoringInstrumentation: Unstopped activity count: 1
I/MonitoringInstrumentation: Activities that are still in CREATED to STOPPED: 1
D/EGL_emulation: eglMakeCurrent: 0xe0a1a300: ver 3 0 (tinfo 0xe0a0f6b0)
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@ce73585 in: STOPPED
D/LifecycleMonitor: Lifecycle status change: com.hannesdorfmann.mosby3.utils.fragment.integrationtest.backstack.BackstackActivityWithChildFragments@ce73585 in: DESTROYED
I/MonitoringInstrumentation: waitForActivitiesToComplete() took: 51ms

Tests ran to completion.
uOOOO commented 4 years ago

I added utils-fragment-androidx-integration-test module. To use jetifier, I had to create new module and copy 'utils-fragment.aar' to its libs directory not use 'utils-fragment' module directly. I think you can check whether reflection works or not.

tommus commented 4 years ago

Any update on this?

sockeqwe commented 4 years ago

I thought a bit more about it @uOOOO and have written down my thoughts in #339

Your feedback would be highly appreciated

mseroczynski commented 4 years ago

@sockeqwe Could you please release any temporary workaround version? It seems really, really critical in current circumstances. To be honest I don't really care how it works (if it works at least a bit better), I just badly need to buy myself some time to find a new solution.

uOOOO commented 4 years ago

I'm so sorry for the late reply. Actually, this is a workaround for fixing another workaround. So, yes, this is not a proper solution. Furthermore I don't like that approach(reflection) myself. :sweat: But I had to make it work again. Anyway, I agree with @sockeqwe's point that the root cause must be fixed by correct way.

mseroczynski commented 4 years ago

I guess it's not needed anymore since fragment-1.2.1 returned toString in it's previous form

uOOOO commented 4 years ago

Yep, Google fixed it.

Calling toString() on a FragmentManager instance no longer throws a NullPointerException when the Activity is already destroyed. (b/148189412) https://developer.android.com/jetpack/androidx/releases/fragment

huytower commented 4 years ago

Confirm that this issue was fixed by google by add these lines to build.gradle in app as Document :

implementation 'androidx.appcompat:appcompat:1.3.0-alpha01' implementation "androidx.fragment:fragment-ktx:1.3.0-alpha07"

https://developer.android.com/jetpack/androidx/releases/fragment