diff --git a/.gitignore b/.gitignore index 59f615a51..731a79b4e 100644 --- a/.gitignore +++ b/.gitignore @@ -38,3 +38,7 @@ upload-plugin/build/pluginUnderTestMetadata/plugin-under-test-metadata.propertie upload-plugin/build/ app-native/.cxx/ .vscode/ + +# Content/feedback UI test runner artifacts (videos, logcat, verdicts) +.github/scripts/test_output/ +.github/scripts/__pycache__/ diff --git a/CHANGELOG.md b/CHANGELOG.md index f98f1fe76..c58ea896e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,13 @@ +## 26.1.3 +* Added gradle configuration cache support to upload symbols plugin. +* Improved user properties auto-save conditions to flush event queue with every user property call. + +* Mitigated `IncorrectContextUseViolation` StrictMode warnings from non-UI context use in display metrics and content overlay construction. +* Mitigated an issue where content overlays and feedback widgets blocked keyboard input on the host activity. +* Mitigated a memory retention issue where content overlays and feedback widgets were briefly held after closing. +* Mitigated a memory leak where the content overlay retained its initial host activity across activity transitions. +* Mitigated a memory leak where content overlays and feedback widgets remained referenced via lifecycle callbacks when the host activity finished. + ## 26.1.2 * Added `CountlyInitProvider` ContentProvider to register activity lifecycle callbacks before `Application.onCreate()`. This ensures the SDK captures the current activity in single-activity frameworks (Flutter, React Native) and apps with deferred initialization. * Added `CountlyConfig.setInitialActivity(Activity)` as an explicit way for wrapper SDKs to provide the host activity during initialization. @@ -172,6 +182,8 @@ * During an internal timer tick * Upon flushing the event queue +* Updated the internal request mechanism. Downgrading from this version is not recommended. + * Added support for array, List and JSONArray to all user given segmentations. They will support only mutable and ummutable versions of the primitive types. Which are: * String, Integer, int, Boolean, bool, Float, float, Double, double, Long, long * Keep in mind that float array will be converted to the double array by the JSONArray diff --git a/app/build.gradle b/app/build.gradle index 507064199..c249e0f0a 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -52,6 +52,12 @@ android { } } + buildFeatures { + // Required so `BuildConfig.COUNTLY_SERVER_URL` / `COUNTLY_APP_KEY` + // (declared below) get generated. AGP 8+ disables BuildConfig by default. + buildConfig true + } + defaultConfig { applicationId "ly.count.android.demo" minSdk 21 @@ -59,6 +65,11 @@ android { versionCode 1 versionName "1.0" testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" + + buildConfigField "String", "COUNTLY_SERVER_URL", + "\"${System.getenv('COUNTLY_SERVER_URL') ?: project.findProperty('countlyServerUrl') ?: 'https://your.server.ly'}\"" + buildConfigField "String", "COUNTLY_APP_KEY", + "\"${System.getenv('COUNTLY_APP_KEY') ?: project.findProperty('countlyAppKey') ?: 'YOUR_APP_KEY'}\"" } buildTypes { debug { diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index c12a5034f..d1a77f1a5 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -99,12 +99,14 @@ + android:configChanges="orientation|screenSize" + android:exported="true"/> + android:configChanges="orientation|screenSize" + android:exported="true"/> = Build.VERSION_CODES.P) { + penaltyListener(penaltyExecutor) { violation -> + val knownIssue = knownThreadViolations.any { it(violation) } + if (!knownIssue) Log.w("StrictMode", null, violation) + } + } else { + penaltyLog() + } + } + .penaltyDeathOnNetwork() + .build() + + private val knownThreadViolations: List Boolean> by lazy { + listOf( + // add known violations if any + ) + } + + private val vmPolicy: VmPolicy + get() = VmPolicy.Builder() + .apply { + detectActivityLeaks() + detectLeakedSqlLiteObjects() + detectLeakedClosableObjects() + detectLeakedRegistrationObjects() + detectFileUriExposure() + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) { + detectCleartextNetwork() + } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + detectContentUriWithoutPermission() + detectUntaggedSockets() // okhttp "issue" + } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + detectCredentialProtectedWhileLocked() + } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + detectIncorrectContextUse() // countly has known issue + detectUnsafeIntentLaunch() + } + } + .apply { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) { + penaltyListener(penaltyExecutor) { violation -> + val knownIssue = knownVmViolations.any { it(violation) } + if (!knownIssue) Log.w("StrictMode", null, violation) + } + } else { + penaltyLog() + } + } + .penaltyDeathOnFileUriExposure() + .build() + + private val knownVmViolations: List Boolean> by lazy { + listOfNotNull( + { + this is UntaggedSocketViolation && stackTrace.any { + it.className.contains("ImmediateRequestMaker") || it.className.contains("ConnectionProcessor") // countly + } + }, + ) + } + + @JvmStatic + fun configure() { + StrictMode.setThreadPolicy(threadPolicy) + StrictMode.setVmPolicy(vmPolicy) + } +} diff --git a/gradle.properties b/gradle.properties index 7f471cbdd..19eb87c0c 100644 --- a/gradle.properties +++ b/gradle.properties @@ -22,7 +22,7 @@ org.gradle.configureondemand=true android.useAndroidX=true android.enableJetifier=true # RELEASE FIELD SECTION -VERSION_NAME=26.1.2 +VERSION_NAME=26.1.3 GROUP=ly.count.android POM_URL=https://github.com/Countly/countly-sdk-android POM_SCM_URL=https://github.com/Countly/countly-sdk-android diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java index 438ad7d5c..720850dbe 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java @@ -539,9 +539,19 @@ public void createWindowParams_correctTypeAndFlags() { Assert.assertEquals("Type should be TYPE_APPLICATION", WindowManager.LayoutParams.TYPE_APPLICATION, params.type); + // Expected base flags match the production set in createWindowParams: + // FLAG_NOT_FOCUSABLE + FLAG_WATCH_OUTSIDE_TOUCH let the host + // activity keep IME focus while still receiving outside-touch + // events the overlay routes back via dispatchTouchEvent. + // FLAG_NOT_TOUCHABLE is added only while content is still loading + // (gates touches until the WebView is visible). The test + // constructs the overlay with about:blank and never waits for + // afterPageFinished, so isContentLoaded stays false here. int expectedFlags = WindowManager.LayoutParams.FLAG_LAYOUT_IN_SCREEN | WindowManager.LayoutParams.FLAG_LAYOUT_INSET_DECOR | WindowManager.LayoutParams.FLAG_NOT_TOUCH_MODAL + | WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE + | WindowManager.LayoutParams.FLAG_WATCH_OUTSIDE_TOUCH | WindowManager.LayoutParams.FLAG_NOT_TOUCHABLE; Assert.assertEquals("Flags should match", expectedFlags, params.flags); @@ -790,4 +800,63 @@ public void contentUrlAction_noQueryParams_returnsFalse() { Assert.assertFalse(overlay.contentUrlAction(url, overlay.webView)); }); } + + // ===================== Memory leak prevention (issue #556) ===================== + + /** + * Structural invariant: the overlay's View.mContext must not pin the + * constructing Activity. The overlay outlives activity transitions, and + * View.mContext can never be swapped after construction — if it's the + * Activity, that Activity stays GC-pinned for the overlay's full lifetime. + * + * The exact context type is API-dependent (see ContentOverlayView#resolveOverlayContext): + * - Pre-API 31: Application context. + * - API 31+: createConfigurationContext from the Activity — a ContextImpl + * wrapper that holds an IBinder token, not the Activity instance, so + * GC isn't blocked. Required for StrictMode#detectIncorrectContextUse. + * + * In both cases, getApplicationContext() resolves to the same Application. + * The test asserts both that the context is NOT the Activity and that it + * routes back to the right Application — which is the actual leak-avoidance + * contract independent of API level. + */ + @Test + public void constructor_usesApplicationContext_notActivity() { + withActivity(activity -> { + overlay = createOverlay(activity); + Assert.assertNotSame( + "ContentOverlayView.mContext must not be the constructing Activity — " + + "View.mContext can never be swapped, so binding it to an Activity leaks " + + "that Activity for the lifetime of the overlay.", + activity, overlay.getContext()); + Assert.assertSame( + "ContentOverlayView.mContext must resolve to the same Application as the " + + "constructing Activity (Application directly on { + overlay = createOverlay(activity); + Assert.assertNotNull("WebView should be created during construction", overlay.webView); + Assert.assertNotSame( + "ContentOverlayView's WebView.mContext must not be the constructing Activity.", + activity, overlay.webView.getContext()); + Assert.assertSame( + "ContentOverlayView's WebView.mContext must resolve to the same Application " + + "as the constructing Activity.", + activity.getApplicationContext(), + overlay.webView.getContext().getApplicationContext()); + }); + } } diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/CountlyStoreExplicitModeTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/CountlyStoreExplicitModeTests.java index 46fbf4184..7ef2793ec 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/CountlyStoreExplicitModeTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/CountlyStoreExplicitModeTests.java @@ -43,6 +43,15 @@ public class CountlyStoreExplicitModeTests { @Before public void setUp() { + // Reset the shared Countly singleton — without this, init() state from a prior + // test class in the suite leaks into our new Countly() instances and dirties the + // event/request caches before the "this should perform no write" assertions can + // measure them. The other suites (ContentOverlayViewTests, + // ModuleConfigurationTests, ...) do the same halt+clear in setUp; this test class + // was the odd one out and produced ordering-dependent flakes. + Countly.sharedInstance().halt(); + TestUtils.getCountlyStore().clear(); + Countly.sharedInstance().setLoggingEnabled(true); store = new CountlyStore(TestUtils.getContext(), mock(ModuleLog.class), false); sp = store; diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/DeviceIdTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/DeviceIdTests.java index 9ddc2a852..1b8f7e8c1 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/DeviceIdTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/DeviceIdTests.java @@ -308,8 +308,8 @@ public void sessionDurationScenario_1() throws InterruptedException, JSONExcepti countly.deviceId().changeWithoutMerge("ff"); // this will generate a request with "end_session", "session_duration" fields and reset duration + begin_session assertEquals(6, TestUtils.getCurrentRQ().length); // not 5 anymore, it will send orientation event as well - TestUtils.validateRequest("ff_merge", TestUtils.map("old_device_id", "1234"), 1); - ModuleEventsTests.validateEventInRQ("ff_merge", "[CLY]_orientation", null, 1, 0.0d, 0.0d, "_CLY_", "_CLY_", "_CLY_", "_CLY_", 2, -1, 0, 1); + ModuleEventsTests.validateEventInRQ(TestUtils.commonDeviceId, "[CLY]_orientation", null, 1, 0.0d, 0.0d, "_CLY_", "_CLY_", "_CLY_", "_CLY_", 1, -1, 0, 1); + TestUtils.validateRequest("ff_merge", TestUtils.map("old_device_id", "1234"), 2); ModuleUserProfileTests.validateUserProfileRequest("ff_merge", 3, 6, TestUtils.map(), TestUtils.map("prop2", 123, "prop1", "string", "prop3", false)); ModuleSessionsTests.validateSessionEndRequest(4, 3, "ff_merge"); diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleContentTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleContentTests.java index 521a48aee..599d9008b 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleContentTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleContentTests.java @@ -1,5 +1,6 @@ package ly.count.android.sdk; +import android.app.Activity; import androidx.test.ext.junit.runners.AndroidJUnit4; import java.util.ArrayList; import java.util.List; @@ -11,6 +12,8 @@ import org.junit.Test; import org.junit.runner.RunWith; +import static org.mockito.Mockito.mock; + @RunWith(AndroidJUnit4.class) public class ModuleContentTests { @@ -68,6 +71,12 @@ private void setIsCurrentlyInContentZone(ModuleContent module, boolean value) th field.set(module, value); } + private Activity getCurrentActivity(ModuleContent module) throws Exception { + java.lang.reflect.Field field = ModuleContent.class.getDeclaredField("currentActivity"); + field.setAccessible(true); + return (Activity) field.get(module); + } + // ======== previewContent public API tests ======== /** @@ -158,4 +167,92 @@ public void validateResponse() throws JSONException { valid.put("html", ""); Assert.assertTrue(mc.validateResponse(valid)); } + + // ======== Activity reference / leak prevention tests (issue #556) ======== + + /** + * onActivityDestroyed must null out currentActivity when the destroyed activity + * is the one currently tracked. This is the core leak fix. + */ + @Test + public void onActivityDestroyed_clearsCurrentActivity_whenIdentityMatches() throws Exception { + Countly countly = initWithConsent(true); + ModuleContent mc = countly.moduleContent; + + Activity act = mock(Activity.class); + mc.onActivityStarted(act, 1); + Assert.assertSame(act, getCurrentActivity(mc)); + + mc.onActivityDestroyed(act); + Assert.assertNull(getCurrentActivity(mc)); + } + + /** + * Destroying an activity other than the currently tracked one must NOT clear the field. + * This protects against losing the active activity reference when an old, already-replaced + * activity is finally destroyed. + */ + @Test + public void onActivityDestroyed_doesNotClear_whenDifferentActivity() throws Exception { + Countly countly = initWithConsent(true); + ModuleContent mc = countly.moduleContent; + + Activity tracked = mock(Activity.class); + Activity unrelated = mock(Activity.class); + mc.onActivityStarted(tracked, 1); + + mc.onActivityDestroyed(unrelated); + Assert.assertSame(tracked, getCurrentActivity(mc)); + } + + /** + * Rotation race regression: when onActivityStarted for the new activity fires before + * onActivityDestroyed for the old one, destroying the old activity must not wipe out + * the new tracked activity. + */ + @Test + public void onActivityDestroyed_doesNotClearNewerActivity_afterRotationRace() throws Exception { + Countly countly = initWithConsent(true); + ModuleContent mc = countly.moduleContent; + + Activity oldAct = mock(Activity.class); + Activity newAct = mock(Activity.class); + + mc.onActivityStarted(oldAct, 1); + mc.onActivityStarted(newAct, 2); + Assert.assertSame(newAct, getCurrentActivity(mc)); + + // Old activity is finally destroyed after the new one has already taken over. + mc.onActivityDestroyed(oldAct); + Assert.assertSame(newAct, getCurrentActivity(mc)); + } + + /** + * onActivityDestroyed must not throw when no activity has been tracked yet. + */ + @Test + public void onActivityDestroyed_isSafe_whenNoActivityTracked() throws Exception { + Countly countly = initWithConsent(true); + ModuleContent mc = countly.moduleContent; + + Activity stray = mock(Activity.class); + mc.onActivityDestroyed(stray); + Assert.assertNull(getCurrentActivity(mc)); + } + + /** + * The seeded activity path (onInitialActivitySeeded) must also be cleared on destroy. + */ + @Test + public void onActivityDestroyed_clearsSeededActivity() throws Exception { + Countly countly = initWithConsent(true); + ModuleContent mc = countly.moduleContent; + + Activity seeded = mock(Activity.class); + mc.onInitialActivitySeeded(seeded); + Assert.assertSame(seeded, getCurrentActivity(mc)); + + mc.onActivityDestroyed(seeded); + Assert.assertNull(getCurrentActivity(mc)); + } } diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleFeedbackTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleFeedbackTests.java index 012aee87a..dcba891cf 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/ModuleFeedbackTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/ModuleFeedbackTests.java @@ -1,5 +1,6 @@ package ly.count.android.sdk; +import android.app.Activity; import androidx.annotation.NonNull; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -694,4 +695,90 @@ private void fillFeedbackWidgetSegmentationParams(Map segmentati segmentation.put("app_version", "1.0"); segmentation.put("widget_id", widgetId); } + + private Activity getCurrentActivity(ModuleFeedback module) throws Exception { + java.lang.reflect.Field field = ModuleFeedback.class.getDeclaredField("currentActivity"); + field.setAccessible(true); + return (Activity) field.get(module); + } + + // ======== Activity reference / leak prevention tests (issue #556) ======== + + /** + * onActivityDestroyed must null out currentActivity when the destroyed activity + * is the one currently tracked. This is the core leak fix. + */ + @Test + public void onActivityDestroyed_clearsCurrentActivity_whenIdentityMatches() throws Exception { + ModuleFeedback mf = mCountly.moduleFeedback; + + Activity act = mock(Activity.class); + mf.onActivityStarted(act, 1); + Assert.assertSame(act, getCurrentActivity(mf)); + + mf.onActivityDestroyed(act); + Assert.assertNull(getCurrentActivity(mf)); + } + + /** + * Destroying an activity other than the currently tracked one must NOT clear the field. + */ + @Test + public void onActivityDestroyed_doesNotClear_whenDifferentActivity() throws Exception { + ModuleFeedback mf = mCountly.moduleFeedback; + + Activity tracked = mock(Activity.class); + Activity unrelated = mock(Activity.class); + mf.onActivityStarted(tracked, 1); + + mf.onActivityDestroyed(unrelated); + Assert.assertSame(tracked, getCurrentActivity(mf)); + } + + /** + * Rotation race regression: when onActivityStarted for the new activity fires before + * onActivityDestroyed for the old one, destroying the old activity must not wipe out + * the new tracked activity. + */ + @Test + public void onActivityDestroyed_doesNotClearNewerActivity_afterRotationRace() throws Exception { + ModuleFeedback mf = mCountly.moduleFeedback; + + Activity oldAct = mock(Activity.class); + Activity newAct = mock(Activity.class); + + mf.onActivityStarted(oldAct, 1); + mf.onActivityStarted(newAct, 2); + Assert.assertSame(newAct, getCurrentActivity(mf)); + + mf.onActivityDestroyed(oldAct); + Assert.assertSame(newAct, getCurrentActivity(mf)); + } + + /** + * onActivityDestroyed must not throw when no activity has been tracked yet. + */ + @Test + public void onActivityDestroyed_isSafe_whenNoActivityTracked() throws Exception { + ModuleFeedback mf = mCountly.moduleFeedback; + + Activity stray = mock(Activity.class); + mf.onActivityDestroyed(stray); + Assert.assertNull(getCurrentActivity(mf)); + } + + /** + * The seeded activity path (onInitialActivitySeeded) must also be cleared on destroy. + */ + @Test + public void onActivityDestroyed_clearsSeededActivity() throws Exception { + ModuleFeedback mf = mCountly.moduleFeedback; + + Activity seeded = mock(Activity.class); + mf.onInitialActivitySeeded(seeded); + Assert.assertSame(seeded, getCurrentActivity(mf)); + + mf.onActivityDestroyed(seeded); + Assert.assertNull(getCurrentActivity(mf)); + } } diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/TestUtils.java b/sdk/src/androidTest/java/ly/count/android/sdk/TestUtils.java index 82b516f1f..768baf222 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/TestUtils.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/TestUtils.java @@ -44,7 +44,7 @@ public class TestUtils { public final static String commonAppKey = "appkey"; public final static String commonDeviceId = "1234"; public final static String SDK_NAME = "java-native-android"; - public final static String SDK_VERSION = "26.1.2"; + public final static String SDK_VERSION = "26.1.3"; public static final int MAX_THREAD_COUNT_PER_STACK_TRACE = 50; public static class Activity2 extends Activity { diff --git a/sdk/src/androidTest/java/ly/count/android/sdk/scUP_UserProfileTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/scUP_UserProfileTests.java index e3e87068f..4b81799ce 100644 --- a/sdk/src/androidTest/java/ly/count/android/sdk/scUP_UserProfileTests.java +++ b/sdk/src/androidTest/java/ly/count/android/sdk/scUP_UserProfileTests.java @@ -53,7 +53,8 @@ public void eventSaveScenario_manualSessions() throws JSONException { countly.events().recordEvent("test_event3"); countly.userProfile().setProperty("theme", "light_mode"); - TestUtils.assertRQSize(3); // no request is generated on the way, not 2 anymore because of orientation event + // UPDATE: setProperty will trigger saving events + TestUtils.assertRQSize(4); // no request is generated on the way, not 2 anymore because of orientation event countly.sessions().endSession(); // begin_session + first user property request + 3 events + user property request with light_mode + end_session @@ -84,7 +85,7 @@ public void eventSaveScenario_onTimer() throws InterruptedException, JSONExcepti countly.events().recordEvent("test_event3"); countly.userProfile().setProperty("theme", "light_mode"); - TestUtils.assertRQSize(1); // no request is generated on the way + TestUtils.assertRQSize(2); // no request is generated on the way + UPDATE: user properties will trigger saving events Thread.sleep(3000); @@ -112,7 +113,8 @@ public void eventSaveScenario_changeDeviceIDWithoutMerge() throws JSONException countly.events().recordEvent("test_event3"); countly.userProfile().setProperty("theme", "light_mode"); - TestUtils.assertRQSize(1); // no request is generated on the way + // UPDATE: this will trigger saving events + TestUtils.assertRQSize(2); // no request is generated on the way countly.deviceId().changeWithoutMerge("new_device_id"); // this will begin a new session @@ -287,12 +289,12 @@ public void UP_203_CNR_A_events() throws JSONException { countly.events().recordEvent("A"); countly.events().recordEvent("B"); - sendSameData(countly); - countly.events().recordEvent("C"); - sendSameData(countly); - countly.events().recordEvent("D"); - sendSameData(countly); - countly.events().recordEvent("E"); + sendSameData(countly); // UPDATE: this will trigger A&B + countly.events().recordEvent("C"); // remaining UP + sendSameData(countly); // UPDATE: this will trigger C + countly.events().recordEvent("D"); // remaining UP + sendSameData(countly); // UPDATE: this will trigger D + countly.events().recordEvent("E"); // remaining UP TestUtils.assertRQSize(6); @@ -402,10 +404,10 @@ public void UP_207_CNR_M() throws JSONException { countly.sessions().beginSession(); countly.events().recordEvent("A"); countly.events().recordEvent("B"); - sendSameData(countly); + sendSameData(countly); // UPDATE: this will trigger A&B countly.sessions().endSession(); countly.events().recordEvent("C"); - sendUserData(countly); + sendUserData(countly); // UPDATE: this will trigger C countly.sessions().endSession(); countly.deviceId().changeWithMerge("merge_id"); sendSameData(countly); @@ -424,9 +426,9 @@ public void UP_207_CNR_M() throws JSONException { ModuleSessionsTests.validateSessionEndRequest(3, null, TestUtils.commonDeviceId); - TestUtils.validateRequest("merge_id", TestUtils.map("old_device_id", TestUtils.commonDeviceId), 4); + ModuleEventsTests.validateEventInRQ("C", 4, 0, 1); - ModuleEventsTests.validateEventInRQ("merge_id", "C", 5, 0, 1); + TestUtils.validateRequest("merge_id", TestUtils.map("old_device_id", TestUtils.commonDeviceId), 5); validateUserDataRequest(6, 8, "4", "merge_id"); @@ -477,9 +479,10 @@ public void UP_208_CR_CG_M() throws JSONException { ModuleUserProfileTests.validateUserProfileRequest(3, 9, TestUtils.map(), TestUtils.map("a12345", "4")); ModuleSessionsTests.validateSessionEndRequest(4, null, TestUtils.commonDeviceId); - TestUtils.validateRequest("merge_id", TestUtils.map("old_device_id", TestUtils.commonDeviceId), 5); - ModuleEventsTests.validateEventInRQ("merge_id", "C", 6, 0, 1); + ModuleEventsTests.validateEventInRQ("C", 5, 0, 1); + + TestUtils.validateRequest("merge_id", TestUtils.map("old_device_id", TestUtils.commonDeviceId), 6); validateUserDataRequest(7, 9, "4", "merge_id"); diff --git a/sdk/src/main/java/ly/count/android/sdk/ContentOverlayView.java b/sdk/src/main/java/ly/count/android/sdk/ContentOverlayView.java index 18507be95..c55f4edcd 100644 --- a/sdk/src/main/java/ly/count/android/sdk/ContentOverlayView.java +++ b/sdk/src/main/java/ly/count/android/sdk/ContentOverlayView.java @@ -15,6 +15,7 @@ import android.util.Log; import android.view.Gravity; import android.view.KeyEvent; +import android.view.MotionEvent; import android.view.View; import android.view.ViewGroup; import android.view.ViewTreeObserver; @@ -28,6 +29,7 @@ import android.widget.FrameLayout; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import java.lang.ref.WeakReference; import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -56,13 +58,38 @@ class ContentOverlayView extends FrameLayout { private ComponentCallbacks orientationCallback; private Application.ActivityLifecycleCallbacks activityLifecycleCallbacks; + // Returns a Context suitable for constructing the overlay's Views without retaining + // a strong Java reference to the constructing Activity: + // - Pre-API 31: Application context (current behavior; no StrictMode UI-context check exists). + // - API 31+: createConfigurationContext from the Activity. The returned ContextImpl is a + // lightweight wrapper that does not strongly retain the Activity instance — only an + // IBinder activity token, which does not pin the Activity for GC. + // Note: createConfigurationContext does not produce a UI context per Android's mIsUiContext + // contract; the StrictMode#detectIncorrectContextUse fix for getSystemService(WINDOW_SERVICE) + // lives in UtilsDevice.obtainWindowManager (which uses createWindowContext as a fallback). + @NonNull + private static Context resolveOverlayContext(@NonNull Activity activity) { + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + try { + return activity.createConfigurationContext(activity.getResources().getConfiguration()); + } catch (Throwable ignored) { + // Fall back to Application context if config-context creation fails. + } + } + return activity.getApplicationContext(); + } + @SuppressLint("SetJavaScriptEnabled") ContentOverlayView(@NonNull Activity activity, @NonNull TransparentActivityConfig portrait, @NonNull TransparentActivityConfig landscape, int orientation, @Nullable ContentCallback callback, @NonNull Runnable onClose) { - super(activity); + // View.mContext must not pin the constructing activity (overlay outlives activity + // transitions; window attachment uses currentHostActivity). On API 31+ we additionally + // need a UI context to satisfy StrictMode#detectIncorrectContextUse — see + // resolveOverlayContext above. + super(resolveOverlayContext(activity)); this.configPortrait = portrait; this.configLandscape = landscape; @@ -96,45 +123,112 @@ class ContentOverlayView extends FrameLayout { private void registerActivityLifecycleCallback(@NonNull Activity activity) { unregisterActivityLifecycleCallback(); - activityLifecycleCallbacks = new Application.ActivityLifecycleCallbacks() { - @Override public void onActivityCreated(@NonNull Activity a, @Nullable android.os.Bundle b) { - } - - @Override public void onActivityStarted(@NonNull Activity a) { - } + activityLifecycleCallbacks = new OverlayLifecycleCallbacks(this); + activity.getApplication().registerActivityLifecycleCallbacks(activityLifecycleCallbacks); + } - @Override public void onActivityResumed(@NonNull Activity a) { - } + /** + * Static class to avoid implicit this$0 reference to the outer ContentOverlayView. + * Anonymous-inner-class callbacks registered with Application#mActivityLifecycleCallbacks + * leak the outer instance for the entire process lifetime if any future code path drops + * the overlay without invoking destroy()/close(). The WeakReference + self-cleanup pattern + * lets the overlay be GC'd as soon as nothing else holds a strong ref to it; the dormant + * callback then removes itself from the Application's list on its next invocation. + * + * NOTE on two separate, non-fixable LeakCanary reports — both architectural, neither + * is true memory growth: + * + * 1. System-singleton reattachment retention: when the same overlay is shown across + * multiple activity transitions (View.mWindowAttachCount grows), various Android + * system singletons hold a reference to the currently-attached ViewRootImpl. The + * two retainers we've observed are InputMethodManager#mCurRootView (set when the + * user types into the WebView) and WindowManagerGlobal#mRoots (the process-wide + * list of attached ViewRootImpls, always populated for any attached window). + * LeakCanary may report the overlay as "leaking" while its own analyzer also + * reports the View is currently attached — that combination is the heuristic + * false-positive signal (one overlay reused, not N overlays leaked). Both + * references are released by the framework when the overlay's window is detached + * or rebound to another window. + * + * 2. ModuleContent.contentOverlay retention while backgrounded: when no activities + * are visible, ModuleContent.onActivityStopped(count=0) calls detachFromWindow on + * the overlay (to avoid WindowLeaked) but intentionally keeps the contentOverlay + * field non-null so the same instance can be re-attached when the user returns. + * LeakCanary sees a detached View still strongly referenced through the Countly + * singleton and reports a "leak". This is bounded (single field, one overlay + * instance) and released the next time ModuleContent replaces or clears the cached + * overlay reference. Not a growing-over-time leak — intentional caching for the + * user-returns-to-same-content UX. Process kill (SIGKILL) reclaims it along with + * everything else, so persistence-on-kill is a non-concern. + */ + private static final class OverlayLifecycleCallbacks implements Application.ActivityLifecycleCallbacks { + private final WeakReference overlayRef; - @Override public void onActivityPaused(@NonNull Activity a) { - } + OverlayLifecycleCallbacks(@NonNull ContentOverlayView overlay) { + this.overlayRef = new WeakReference<>(overlay); + } - @Override - public void onActivityStopped(@NonNull Activity a) { - if (a == currentHostActivity && isAddedToWindow && a.isFinishing()) { - Log.d(Countly.TAG, "[ContentOverlayView] onActivityStopped, host activity is finishing, removing from window"); - removeFromWindow(); + /** Returns the overlay if still alive; otherwise self-unregisters and returns null. */ + @Nullable + private ContentOverlayView resolveOrCleanup(@NonNull Activity a) { + ContentOverlayView overlay = overlayRef.get(); + if (overlay == null) { + try { + a.getApplication().unregisterActivityLifecycleCallbacks(this); + } catch (Exception ignored) { + // Application may already be tearing down; safe to ignore. } } + return overlay; + } + + @Override public void onActivityCreated(@NonNull Activity a, @Nullable android.os.Bundle b) { + } - @Override public void onActivitySaveInstanceState(@NonNull Activity a, @NonNull android.os.Bundle b) { + @Override public void onActivityStarted(@NonNull Activity a) { + // Probe in start callbacks too so dead callbacks don't linger across activity navigations. + resolveOrCleanup(a); + } + + @Override public void onActivityResumed(@NonNull Activity a) { + } + + @Override public void onActivityPaused(@NonNull Activity a) { + } + + @Override + public void onActivityStopped(@NonNull Activity a) { + ContentOverlayView overlay = resolveOrCleanup(a); + if (overlay == null) return; + if (a == overlay.currentHostActivity && overlay.isAddedToWindow && a.isFinishing()) { + Log.d(Countly.TAG, "[ContentOverlayView] onActivityStopped, host activity is finishing, removing from window"); + overlay.removeFromWindow(); } + } - @Override - public void onActivityDestroyed(@NonNull Activity a) { - if (a == currentHostActivity && isAddedToWindow) { + @Override public void onActivitySaveInstanceState(@NonNull Activity a, @NonNull android.os.Bundle b) { + } + + @Override + public void onActivityDestroyed(@NonNull Activity a) { + ContentOverlayView overlay = resolveOrCleanup(a); + if (overlay == null) return; + if (a == overlay.currentHostActivity) { + if (overlay.isAddedToWindow) { Log.d(Countly.TAG, "[ContentOverlayView] onActivityDestroyed, host activity destroyed, removing from window"); - removeFromWindow(); + overlay.removeFromWindow(); } + // Drop the strong reference to the destroyed activity so it can be GC'd. + // The overlay is reattached via ModuleContent.onActivityStarted, which calls attachToActivity() + // and re-sets currentHostActivity for the next host. + overlay.currentHostActivity = null; } - }; - activity.getApplication().registerActivityLifecycleCallbacks(activityLifecycleCallbacks); + } } private void unregisterActivityLifecycleCallback() { if (activityLifecycleCallbacks != null) { try { - getContext().getApplicationContext(); ((Application) getContext().getApplicationContext()) .unregisterActivityLifecycleCallbacks(activityLifecycleCallbacks); } catch (Exception e) { @@ -145,27 +239,54 @@ private void unregisterActivityLifecycleCallback() { } private void registerOrientationCallback(@NonNull Context context) { - orientationCallback = new ComponentCallbacks() { - @Override - public void onConfigurationChanged(@NonNull Configuration newConfig) { - if (isClosed || currentOrientation == newConfig.orientation) { - return; - } - Log.d(Countly.TAG, "[ContentOverlayView] onConfigurationChanged, orientation changed from [" + currentOrientation + "] to [" + newConfig.orientation + "]"); - currentOrientation = newConfig.orientation; + Context appContext = context.getApplicationContext(); + orientationCallback = new OverlayComponentCallbacks(this, appContext); + appContext.registerComponentCallbacks(orientationCallback); + } + + /** + * Static class to avoid implicit this$0 reference to the outer ContentOverlayView. + * Same leak-avoidance pattern as OverlayLifecycleCallbacks — see that class for details. + */ + private static final class OverlayComponentCallbacks implements ComponentCallbacks { + private final WeakReference overlayRef; + private final WeakReference appContextRef; + + OverlayComponentCallbacks(@NonNull ContentOverlayView overlay, @NonNull Context appContext) { + this.overlayRef = new WeakReference<>(overlay); + this.appContextRef = new WeakReference<>(appContext); + } - Activity activity = currentHostActivity; - if (activity != null && !activity.isFinishing()) { - handleOrientationChange(activity); + @Override + public void onConfigurationChanged(@NonNull Configuration newConfig) { + ContentOverlayView overlay = overlayRef.get(); + if (overlay == null) { + Context appContext = appContextRef.get(); + if (appContext != null) { + try { + appContext.unregisterComponentCallbacks(this); + } catch (Exception ignored) { + // App may be tearing down; safe to ignore. + } } + return; + } + if (overlay.isClosed || overlay.currentOrientation == newConfig.orientation) { + return; } + Log.d(Countly.TAG, "[ContentOverlayView] onConfigurationChanged, orientation changed from [" + overlay.currentOrientation + "] to [" + newConfig.orientation + "]"); + overlay.currentOrientation = newConfig.orientation; - @Override - public void onLowMemory() { - // no-op + Activity activity = overlay.currentHostActivity; + if (activity != null && !activity.isFinishing()) { + overlay.handleOrientationChange(activity); } - }; - context.getApplicationContext().registerComponentCallbacks(orientationCallback); + } + + @Override + public void onLowMemory() { + // no-op + } } private void unregisterOrientationCallback() { @@ -187,6 +308,50 @@ public boolean dispatchKeyEvent(KeyEvent event) { return super.dispatchKeyEvent(event); } + @Override + public boolean dispatchTouchEvent(MotionEvent ev) { + // Dynamically transfer IME focus between the overlay and the host activity: + // - ACTION_OUTSIDE (delivered because of FLAG_WATCH_OUTSIDE_TOUCH): user + // touched outside the overlay's bounds; the touch already passed through + // to the activity via FLAG_NOT_TOUCH_MODAL, but we additionally restore + // FLAG_NOT_FOCUSABLE so the activity's EditText can claim IME focus. + // - ACTION_DOWN (touch inside the overlay's bounds): clear FLAG_NOT_FOCUSABLE + // so the WebView's form fields can bring up the keyboard. + // Consumes nothing; existing touch handling (WebView, etc.) continues normally. + if (ev.getAction() == MotionEvent.ACTION_OUTSIDE) { + setWindowFocusable(false); + return true; + } + if (ev.getAction() == MotionEvent.ACTION_DOWN) { + setWindowFocusable(true); + } + return super.dispatchTouchEvent(ev); + } + + private void setWindowFocusable(boolean focusable) { + if (!isAddedToWindow || windowManager == null) { + return; + } + try { + WindowManager.LayoutParams lp = (WindowManager.LayoutParams) getLayoutParams(); + if (lp == null) { + return; + } + boolean alreadyFocusable = (lp.flags & WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE) == 0; + if (alreadyFocusable == focusable) { + return; + } + if (focusable) { + lp.flags &= ~WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE; + } else { + lp.flags |= WindowManager.LayoutParams.FLAG_NOT_FOCUSABLE; + } + windowManager.updateViewLayout(this, lp); + } catch (Exception e) { + Log.w(Countly.TAG, "[ContentOverlayView] setWindowFocusable, failed to update flags", e); + } + } + private TransparentActivityConfig getCurrentConfig() { if (currentOrientation == Configuration.ORIENTATION_LANDSCAPE) { return configLandscape; @@ -210,9 +375,19 @@ private WindowManager.LayoutParams createWindowParams(@NonNull Activity activity } } + // FLAG_NOT_FOCUSABLE: overlay does NOT grab IME focus by default, so the + // underlying Activity's EditText can receive keyboard input even while the + // overlay is shown. Toggled off in dispatchTouchEvent on inside touches so + // the WebView's /