Skip to content

Commit 7fbc55d

Browse files
Merge branch 'staging' into fix452/strict_mode_content
2 parents bb26fd5 + 00d34fb commit 7fbc55d

11 files changed

Lines changed: 434 additions & 55 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,11 @@
11
## XX.XX.XX
2+
* Added gradle configuration cache support to upload symbols plugin.
23
* Improved user properties auto-save conditions to flush event queue with every user property call.
34

45
* Mitigated StrictMode `IncorrectContextUseViolation` warnings logged when the SDK retrieved device display metrics and constructed the content overlay view from a non-UI context.
6+
* Mitigated an issue where content overlays and feedback widgets prevented keyboard input on the underlying activity's text fields while displayed.
7+
* 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.
8+
* Mitigated a memory leak where the content overlay retained the activity it was first opened in across subsequent activity transitions.
59

610
## 26.1.2
711
* 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.

sdk/src/androidTest/java/ly/count/android/sdk/ContentOverlayViewTests.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -790,4 +790,48 @@ public void contentUrlAction_noQueryParams_returnsFalse() {
790790
Assert.assertFalse(overlay.contentUrlAction(url, overlay.webView));
791791
});
792792
}
793+
794+
// ===================== Memory leak prevention (issue #556) =====================
795+
796+
/**
797+
* Structural invariant: the overlay's View.mContext must be the Application, not the
798+
* constructing activity. This is what allows the overlay to outlive activity transitions
799+
* without leaking the activity it was first opened in.
800+
*
801+
* Regression guard: if anyone changes the constructor's super(...) call back to the
802+
* activity, this test will fail and surface the leak before users do.
803+
*/
804+
@Test
805+
public void constructor_usesApplicationContext_notActivity() {
806+
withActivity(activity -> {
807+
overlay = createOverlay(activity);
808+
Assert.assertNotSame(
809+
"ContentOverlayView.mContext must not be the constructing Activity — "
810+
+ "View.mContext can never be swapped, so binding it to an Activity leaks "
811+
+ "that Activity for the lifetime of the overlay.",
812+
activity, overlay.getContext());
813+
Assert.assertSame(
814+
"ContentOverlayView.mContext must be the Application context.",
815+
activity.getApplicationContext(), overlay.getContext());
816+
});
817+
}
818+
819+
/**
820+
* Same invariant for the embedded WebView. Even with the wrapper View using App context,
821+
* a WebView constructed with Activity context would still pin the constructing activity
822+
* via its own mContext.
823+
*/
824+
@Test
825+
public void webView_usesApplicationContext_notActivity() {
826+
withActivity(activity -> {
827+
overlay = createOverlay(activity);
828+
Assert.assertNotNull("WebView should be created during construction", overlay.webView);
829+
Assert.assertNotSame(
830+
"ContentOverlayView's WebView.mContext must not be the constructing Activity.",
831+
activity, overlay.webView.getContext());
832+
Assert.assertSame(
833+
"ContentOverlayView's WebView.mContext must be the Application context.",
834+
activity.getApplicationContext(), overlay.webView.getContext());
835+
});
836+
}
793837
}

sdk/src/androidTest/java/ly/count/android/sdk/ModuleContentTests.java

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package ly.count.android.sdk;
22

3+
import android.app.Activity;
34
import androidx.test.ext.junit.runners.AndroidJUnit4;
45
import java.util.ArrayList;
56
import java.util.List;
@@ -11,6 +12,8 @@
1112
import org.junit.Test;
1213
import org.junit.runner.RunWith;
1314

15+
import static org.mockito.Mockito.mock;
16+
1417
@RunWith(AndroidJUnit4.class)
1518
public class ModuleContentTests {
1619

@@ -68,6 +71,12 @@ private void setIsCurrentlyInContentZone(ModuleContent module, boolean value) th
6871
field.set(module, value);
6972
}
7073

74+
private Activity getCurrentActivity(ModuleContent module) throws Exception {
75+
java.lang.reflect.Field field = ModuleContent.class.getDeclaredField("currentActivity");
76+
field.setAccessible(true);
77+
return (Activity) field.get(module);
78+
}
79+
7180
// ======== previewContent public API tests ========
7281

7382
/**
@@ -158,4 +167,92 @@ public void validateResponse() throws JSONException {
158167
valid.put("html", "<html></html>");
159168
Assert.assertTrue(mc.validateResponse(valid));
160169
}
170+
171+
// ======== Activity reference / leak prevention tests (issue #556) ========
172+
173+
/**
174+
* onActivityDestroyed must null out currentActivity when the destroyed activity
175+
* is the one currently tracked. This is the core leak fix.
176+
*/
177+
@Test
178+
public void onActivityDestroyed_clearsCurrentActivity_whenIdentityMatches() throws Exception {
179+
Countly countly = initWithConsent(true);
180+
ModuleContent mc = countly.moduleContent;
181+
182+
Activity act = mock(Activity.class);
183+
mc.onActivityStarted(act, 1);
184+
Assert.assertSame(act, getCurrentActivity(mc));
185+
186+
mc.onActivityDestroyed(act);
187+
Assert.assertNull(getCurrentActivity(mc));
188+
}
189+
190+
/**
191+
* Destroying an activity other than the currently tracked one must NOT clear the field.
192+
* This protects against losing the active activity reference when an old, already-replaced
193+
* activity is finally destroyed.
194+
*/
195+
@Test
196+
public void onActivityDestroyed_doesNotClear_whenDifferentActivity() throws Exception {
197+
Countly countly = initWithConsent(true);
198+
ModuleContent mc = countly.moduleContent;
199+
200+
Activity tracked = mock(Activity.class);
201+
Activity unrelated = mock(Activity.class);
202+
mc.onActivityStarted(tracked, 1);
203+
204+
mc.onActivityDestroyed(unrelated);
205+
Assert.assertSame(tracked, getCurrentActivity(mc));
206+
}
207+
208+
/**
209+
* Rotation race regression: when onActivityStarted for the new activity fires before
210+
* onActivityDestroyed for the old one, destroying the old activity must not wipe out
211+
* the new tracked activity.
212+
*/
213+
@Test
214+
public void onActivityDestroyed_doesNotClearNewerActivity_afterRotationRace() throws Exception {
215+
Countly countly = initWithConsent(true);
216+
ModuleContent mc = countly.moduleContent;
217+
218+
Activity oldAct = mock(Activity.class);
219+
Activity newAct = mock(Activity.class);
220+
221+
mc.onActivityStarted(oldAct, 1);
222+
mc.onActivityStarted(newAct, 2);
223+
Assert.assertSame(newAct, getCurrentActivity(mc));
224+
225+
// Old activity is finally destroyed after the new one has already taken over.
226+
mc.onActivityDestroyed(oldAct);
227+
Assert.assertSame(newAct, getCurrentActivity(mc));
228+
}
229+
230+
/**
231+
* onActivityDestroyed must not throw when no activity has been tracked yet.
232+
*/
233+
@Test
234+
public void onActivityDestroyed_isSafe_whenNoActivityTracked() throws Exception {
235+
Countly countly = initWithConsent(true);
236+
ModuleContent mc = countly.moduleContent;
237+
238+
Activity stray = mock(Activity.class);
239+
mc.onActivityDestroyed(stray);
240+
Assert.assertNull(getCurrentActivity(mc));
241+
}
242+
243+
/**
244+
* The seeded activity path (onInitialActivitySeeded) must also be cleared on destroy.
245+
*/
246+
@Test
247+
public void onActivityDestroyed_clearsSeededActivity() throws Exception {
248+
Countly countly = initWithConsent(true);
249+
ModuleContent mc = countly.moduleContent;
250+
251+
Activity seeded = mock(Activity.class);
252+
mc.onInitialActivitySeeded(seeded);
253+
Assert.assertSame(seeded, getCurrentActivity(mc));
254+
255+
mc.onActivityDestroyed(seeded);
256+
Assert.assertNull(getCurrentActivity(mc));
257+
}
161258
}

sdk/src/androidTest/java/ly/count/android/sdk/ModuleFeedbackTests.java

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package ly.count.android.sdk;
22

3+
import android.app.Activity;
34
import androidx.annotation.NonNull;
45
import androidx.test.core.app.ApplicationProvider;
56
import androidx.test.ext.junit.runners.AndroidJUnit4;
@@ -694,4 +695,90 @@ private void fillFeedbackWidgetSegmentationParams(Map<String, Object> segmentati
694695
segmentation.put("app_version", "1.0");
695696
segmentation.put("widget_id", widgetId);
696697
}
698+
699+
private Activity getCurrentActivity(ModuleFeedback module) throws Exception {
700+
java.lang.reflect.Field field = ModuleFeedback.class.getDeclaredField("currentActivity");
701+
field.setAccessible(true);
702+
return (Activity) field.get(module);
703+
}
704+
705+
// ======== Activity reference / leak prevention tests (issue #556) ========
706+
707+
/**
708+
* onActivityDestroyed must null out currentActivity when the destroyed activity
709+
* is the one currently tracked. This is the core leak fix.
710+
*/
711+
@Test
712+
public void onActivityDestroyed_clearsCurrentActivity_whenIdentityMatches() throws Exception {
713+
ModuleFeedback mf = mCountly.moduleFeedback;
714+
715+
Activity act = mock(Activity.class);
716+
mf.onActivityStarted(act, 1);
717+
Assert.assertSame(act, getCurrentActivity(mf));
718+
719+
mf.onActivityDestroyed(act);
720+
Assert.assertNull(getCurrentActivity(mf));
721+
}
722+
723+
/**
724+
* Destroying an activity other than the currently tracked one must NOT clear the field.
725+
*/
726+
@Test
727+
public void onActivityDestroyed_doesNotClear_whenDifferentActivity() throws Exception {
728+
ModuleFeedback mf = mCountly.moduleFeedback;
729+
730+
Activity tracked = mock(Activity.class);
731+
Activity unrelated = mock(Activity.class);
732+
mf.onActivityStarted(tracked, 1);
733+
734+
mf.onActivityDestroyed(unrelated);
735+
Assert.assertSame(tracked, getCurrentActivity(mf));
736+
}
737+
738+
/**
739+
* Rotation race regression: when onActivityStarted for the new activity fires before
740+
* onActivityDestroyed for the old one, destroying the old activity must not wipe out
741+
* the new tracked activity.
742+
*/
743+
@Test
744+
public void onActivityDestroyed_doesNotClearNewerActivity_afterRotationRace() throws Exception {
745+
ModuleFeedback mf = mCountly.moduleFeedback;
746+
747+
Activity oldAct = mock(Activity.class);
748+
Activity newAct = mock(Activity.class);
749+
750+
mf.onActivityStarted(oldAct, 1);
751+
mf.onActivityStarted(newAct, 2);
752+
Assert.assertSame(newAct, getCurrentActivity(mf));
753+
754+
mf.onActivityDestroyed(oldAct);
755+
Assert.assertSame(newAct, getCurrentActivity(mf));
756+
}
757+
758+
/**
759+
* onActivityDestroyed must not throw when no activity has been tracked yet.
760+
*/
761+
@Test
762+
public void onActivityDestroyed_isSafe_whenNoActivityTracked() throws Exception {
763+
ModuleFeedback mf = mCountly.moduleFeedback;
764+
765+
Activity stray = mock(Activity.class);
766+
mf.onActivityDestroyed(stray);
767+
Assert.assertNull(getCurrentActivity(mf));
768+
}
769+
770+
/**
771+
* The seeded activity path (onInitialActivitySeeded) must also be cleared on destroy.
772+
*/
773+
@Test
774+
public void onActivityDestroyed_clearsSeededActivity() throws Exception {
775+
ModuleFeedback mf = mCountly.moduleFeedback;
776+
777+
Activity seeded = mock(Activity.class);
778+
mf.onInitialActivitySeeded(seeded);
779+
Assert.assertSame(seeded, getCurrentActivity(mf));
780+
781+
mf.onActivityDestroyed(seeded);
782+
Assert.assertNull(getCurrentActivity(mf));
783+
}
697784
}

0 commit comments

Comments
 (0)