Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update context tracking to use lifecycle callbacks #238

Merged
merged 6 commits into from
Feb 13, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion example/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

<uses-permission android:name="android.permission.INTERNET" />
<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
<uses-permission android:name="android.permission.GET_TASKS" />

<application
android:name=".ExampleApplication"
Expand Down
1 change: 0 additions & 1 deletion sdk/src/androidTest/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
android:versionName="1.0">

<uses-permission android:name="android.permission.ACCESS_NETWORK_STATE" />
<uses-permission android:name="android.permission.GET_TASKS" />

<application android:label="Bugsnag Android Tests"></application>

Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package com.bugsnag.android;

import android.support.test.InstrumentationRegistry;
import android.util.Pair;

import org.junit.Before;
import org.junit.Test;

import java.util.Date;

import static com.bugsnag.android.BugsnagTestUtils.*;
import static com.bugsnag.android.BugsnagTestUtils.generateClient;
import static com.bugsnag.android.BugsnagTestUtils.generateSessionStore;
import static com.bugsnag.android.BugsnagTestUtils.generateSessionTrackingApiClient;
import static junit.framework.Assert.assertEquals;
import static junit.framework.Assert.assertFalse;
import static junit.framework.Assert.assertNotNull;
Expand Down Expand Up @@ -95,6 +94,7 @@ public void testIncrementCounts() throws Exception {
public void testBasicInForeground() throws Exception {
assertFalse(sessionTracker.isInForeground());
assertNull(sessionTracker.getCurrentSession());
assertNull(sessionTracker.getContextActivity());

sessionTracker.updateForegroundTracker(ACTIVITY_NAME, true, System.currentTimeMillis());
assertTrue(sessionTracker.isInForeground());
Expand All @@ -104,13 +104,16 @@ public void testBasicInForeground() throws Exception {
sessionTracker.updateForegroundTracker("other", true, System.currentTimeMillis());
assertTrue(sessionTracker.isInForeground());
assertEquals(firstSession, sessionTracker.getCurrentSession());
assertEquals("other", sessionTracker.getContextActivity());

sessionTracker.updateForegroundTracker("other", false, System.currentTimeMillis());
assertTrue(sessionTracker.isInForeground());
assertEquals(ACTIVITY_NAME, sessionTracker.getContextActivity());

sessionTracker.updateForegroundTracker(ACTIVITY_NAME, false, System.currentTimeMillis());
assertFalse(sessionTracker.isInForeground());
assertEquals(firstSession, sessionTracker.getCurrentSession());
assertNull(sessionTracker.getContextActivity());
}

@Test
Expand All @@ -129,7 +132,10 @@ public void testInForegroundDuration() throws Exception {
assertEquals(100, sessionTracker.getDurationInForegroundMs(now + 100));

sessionTracker.updateForegroundTracker(ACTIVITY_NAME, false, now);
assertEquals(0, sessionTracker.getDurationInForegroundMs(now + 200));
assertEquals(200, sessionTracker.getDurationInForegroundMs(now + 200));

sessionTracker.updateForegroundTracker(ACTIVITY_NAME, false, now);
assertEquals(0, sessionTracker.getDurationInForegroundMs(now + 300));
}

@Test
Expand Down
22 changes: 6 additions & 16 deletions sdk/src/main/java/com/bugsnag/android/AppData.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import android.support.annotation.Nullable;

import java.io.IOException;
import java.util.List;

/**
* Information about the running Android app, including app name, version and release stage.
Expand Down Expand Up @@ -80,18 +79,7 @@ private static String getAppName(@NonNull Context appContext) {

@Nullable
String getActiveScreenClass() {
try {
ActivityManager activityManager =
(ActivityManager) appContext.getSystemService(Context.ACTIVITY_SERVICE);
List<ActivityManager.RunningTaskInfo> tasks =
activityManager.getRunningTasks(1);
ActivityManager.RunningTaskInfo runningTask = tasks.get(0);
return runningTask.topActivity.getClassName();
} catch (Exception exception) {
Logger.warn("Could not get active screen information,"
+ " we recommend granting the 'android.permission.GET_TASKS' permission");
}
return null;
return sessionTracker.getContextActivity();
}

/**
Expand All @@ -111,10 +99,12 @@ private static Boolean isLowMemory(@NonNull Context appContext) {
try {
ActivityManager activityManager =
(ActivityManager) appContext.getSystemService(Context.ACTIVITY_SERVICE);
ActivityManager.MemoryInfo memInfo = new ActivityManager.MemoryInfo();
activityManager.getMemoryInfo(memInfo);

return memInfo.lowMemory;
if (activityManager != null) {
ActivityManager.MemoryInfo memInfo = new ActivityManager.MemoryInfo();
activityManager.getMemoryInfo(memInfo);
return memInfo.lowMemory;
}
} catch (Exception exception) {
Logger.warn("Could not check lowMemory status");
}
Expand Down
32 changes: 24 additions & 8 deletions sdk/src/main/java/com/bugsnag/android/SessionTracker.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,13 @@
import java.util.Collection;
import java.util.Date;
import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentLinkedDeque;
import java.util.concurrent.ConcurrentLinkedQueue;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.Semaphore;
import java.util.concurrent.atomic.AtomicLong;
Expand All @@ -24,8 +27,8 @@ class SessionTracker implements Application.ActivityLifecycleCallbacks {
private static final String KEY_LIFECYCLE_CALLBACK = "ActivityLifecycle";
private static final int DEFAULT_TIMEOUT_MS = 30000;

private final ConcurrentHashMap<String, Boolean>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the change here? I'd be a bit concerned as this set is accessed using a few different code paths. Even if only the key is used, all this does is derive a concurrency-safe set.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kattrali I've changed it to a LinkedHashSet as that retains the ordering of values. The testBasicInForeground covers the case where multiple activities launch, then one stops, and we need to access the previous activity name.

Maybe a ConcurrentLinkedDeque would be a better choice for this, if you're happy with the general approach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah the approach is solid - ConcurrentLinkedDeque seems ideal for this case as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use a ConcurrentLinkedDeque.

foregroundActivities = new ConcurrentHashMap<>();
private final Collection<String>
foregroundActivities = new ConcurrentLinkedQueue<>();
private final Configuration configuration;
private final long timeoutMs;
private final Client client;
Expand Down Expand Up @@ -73,7 +76,7 @@ void startNewSession(@NonNull Date date, @Nullable User user, boolean autoCaptur
* Determines whether or not a session should be tracked. If this is true, the session will be
* stored and sent to the Bugsnag API, otherwise no action will occur in this method.
*
* @param session the session
* @param session the session
*/
private void trackSessionIfNeeded(final Session session) {
boolean notifyForRelease = configuration.shouldNotifyForReleaseStage(getReleaseStage());
Expand Down Expand Up @@ -250,7 +253,7 @@ void startFirstSession(Activity activity) {
long nowMs = System.currentTimeMillis();
activityFirstStartedAtMs.set(nowMs);
startNewSession(new Date(nowMs), client.user, true);
foregroundActivities.put(getActivityName(activity), true);
foregroundActivities.add(getActivityName(activity));
}
}

Expand All @@ -263,9 +266,9 @@ void startFirstSession(Activity activity) {
* If an activity comes to the foreground and is the only foreground activity, a new session
* should be started, unless the app is within a timeout period.
*
* @param activityName the activity name
* @param activityName the activity name
* @param activityStarting whether the activity is being started or not
* @param nowMs The current time in ms
* @param nowMs The current time in ms
*/
void updateForegroundTracker(String activityName, boolean activityStarting, long nowMs) {
if (activityStarting) {
Expand All @@ -279,7 +282,7 @@ void updateForegroundTracker(String activityName, boolean activityStarting, long
activityFirstStartedAtMs.set(nowMs);
startNewSession(new Date(nowMs), client.user, true);
}
foregroundActivities.put(activityName, true);
foregroundActivities.add(activityName);
} else {
foregroundActivities.remove(activityName);
activityLastStoppedAtMs.set(nowMs);
Expand All @@ -301,4 +304,17 @@ long getDurationInForegroundMs(long nowMs) {
return durationMs > 0 ? durationMs : 0;
}

@Nullable
String getContextActivity() {
if (foregroundActivities.isEmpty()) {
return null;
} else {
// linked hash set retains order of added activity and ensures uniqueness
// therefore obtain the most recently added
int size = foregroundActivities.size();
String[] activities = foregroundActivities.toArray(new String[size]);
return activities[size - 1];
}
}

}