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 1dd6be92a..372c3e524 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,10 +2,11 @@ * 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 StrictMode `IncorrectContextUseViolation` warnings logged when the SDK retrieved device display metrics and constructed the content overlay view from a non-UI context. -* Mitigated an issue where content overlays and feedback widgets prevented keyboard input on the underlying activity's text fields while displayed. -* Mitigated a memory retention issue where content overlays and feedback widgets could be briefly held in memory after closing, surfacing under repeated open/close cycles. -* Mitigated a memory leak where the content overlay retained the activity it was first opened in across subsequent activity transitions. +* 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. 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/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java b/sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java index 5d425cee5..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); @@ -794,12 +804,21 @@ public void contentUrlAction_noQueryParams_returnsFalse() { // ===================== Memory leak prevention (issue #556) ===================== /** - * Structural invariant: the overlay's View.mContext must be the Application, not the - * constructing activity. This is what allows the overlay to outlive activity transitions - * without leaking the activity it was first opened in. + * 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. * - * Regression guard: if anyone changes the constructor's super(...) call back to the - * activity, this test will fail and surface the leak before users do. + * 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() { @@ -811,15 +830,19 @@ public void constructor_usesApplicationContext_notActivity() { + "that Activity for the lifetime of the overlay.", activity, overlay.getContext()); Assert.assertSame( - "ContentOverlayView.mContext must be the Application context.", - activity.getApplicationContext(), overlay.getContext()); + "ContentOverlayView.mContext must resolve to the same Application as the " + + "constructing Activity (Application directly on = Build.VERSION_CODES.S) { @@ -120,51 +123,112 @@ private static Context resolveOverlayContext(@NonNull Activity activity) { 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) { - if (isAddedToWindow) { - Log.d(Countly.TAG, "[ContentOverlayView] onActivityDestroyed, host activity destroyed, removing from window"); - 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. - currentHostActivity = null; + @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"); + 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) { @@ -175,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); + } - Activity activity = currentHostActivity; - if (activity != null && !activity.isFinishing()) { - handleOrientationChange(activity); + /** + * 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); + } + + @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() { @@ -445,20 +536,21 @@ private void removeFromWindow() { // Expedite any pending scrollbar-fade Runnables: scrolling inside the WebView // schedules a ScrollabilityCache fade Runnable on the main MessageQueue. If // the View is detached before that Runnable fires, the pending Message keeps - // ViewRootImpl alive (and through it, this overlay) for ~550ms — visible as - // a transient leak under repeated show/close cycles. Calling awakenScrollBars(0) - // re-schedules the existing fade with zero delay, so the Message drains on the - // next message-loop iteration (~16ms) and the View becomes GC-eligible promptly. - // No-op if mScrollCache wasn't created (no scrolling occurred). - try { - // CountlyWebView#expediteScrollbarFade exposes protected View#awakenScrollBars(int) - // (only callable through inheritance, hence the wrapper on the subclass). - if (webView instanceof CountlyWebView) { + // ViewRootImpl alive (and through it, this overlay) for the platform's + // scrollbar-fade window — visible as a transient leak under repeated show/close + // cycles. Calling awakenScrollBars(0) re-schedules the existing fade with zero + // delay, so the Message drains on the next message-loop iteration and the View + // becomes GC-eligible promptly. No-op if mScrollCache wasn't created (no scrolling + // occurred). Only the WebView can have a scroll cache here — the FrameLayout + // itself is non-scrollable. + if (webView instanceof CountlyWebView) { + try { ((CountlyWebView) webView).expediteScrollbarFade(); + } catch (Exception e) { + // Best-effort drain; if this ever throws, leave a breadcrumb so a future + // regression doesn't silently re-introduce the transient ViewRoot retention. + Log.w(Countly.TAG, "[ContentOverlayView] removeFromWindow, scrollbar fade expedite failed", e); } - awakenScrollBars(0); - } catch (Exception ignored) { - // Public API, but defensive against any edge-case throws during teardown. } try { diff --git a/sdk/src/main/java/ly/count/android/sdk/CountlyWebView.java b/sdk/src/main/java/ly/count/android/sdk/CountlyWebView.java index f7805e44e..905119747 100644 --- a/sdk/src/main/java/ly/count/android/sdk/CountlyWebView.java +++ b/sdk/src/main/java/ly/count/android/sdk/CountlyWebView.java @@ -19,8 +19,9 @@ public boolean onCheckIsTextEditor() { /** * Expedites any pending scrollbar-fade Runnable by re-scheduling it with zero delay. * Used by ContentOverlayView before detach to drain MessageQueue entries that would - * otherwise hold the ViewRootImpl alive for ~550ms (transient leak under stress). - * No-op if no scroll cache exists. Calls protected View#awakenScrollBars(int). + * otherwise hold the ViewRootImpl alive for up to the platform's scrollbar-fade window + * (sub-second; transient leak under repeated open/close stress). No-op if no scroll + * cache exists. Calls protected View#awakenScrollBars(int). */ void expediteScrollbarFade() { awakenScrollBars(0);