Merge pull request #394 from bugsnag/in-foreground-fixes

Fix calculation of durationInForeground when autoCaptureSessions is false

Author: fractalwrench

features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/InForegroundScenario.kt

@@ -0,0 +1,32 @@
+package com.bugsnag.android.mazerunner.scenarios
+
+import android.content.Context
+import com.bugsnag.android.Bugsnag
+import com.bugsnag.android.Configuration
+import java.lang.Thread
+import android.os.Handler
+import android.os.HandlerThread
+
+/**
+ * Sends a handled exception to Bugsnag, which has a short delay to allow the app to remain
+ * in the foreground for ~1 second
+ */
+internal class InForegroundScenario(config: Configuration,
+                                    context: Context) : Scenario(config, context) {
+    init {
+        config.setAutoCaptureSessions(false)
+    }
+
+    override fun run() {
+        super.run()
+
+        val thread = HandlerThread("HandlerThread")
+        thread.start()
+        Handler(thread.looper).post {
+            Thread.sleep(5000)
+            Bugsnag.notify(generateException())
+        }
+
+    }
+
+}

features/in_foreground.feature

@@ -0,0 +1,18 @@
+Feature: In foreground field populates correctly
+
+Scenario: Test handled exception after delay
+    When I run "InForegroundScenario"
+    Then I should receive a request
+    And the request is a valid for the error reporting API
+    And the event "app.inForeground" is true
+
+#    Duration in foreground should be a non-zero integer
+    And the payload field "events.0.app.durationInForeground" is greater than 0
+
+Scenario: Test handled exception in background
+    When I run "InForegroundScenario" and press the home button
+    And I press the home button
+    Then I should receive a request
+    And the request is a valid for the error reporting API
+    And the event "app.inForeground" is false
+    And the payload field "events.0.app.durationInForeground" equals 0

features/steps/build_steps.rb

@@ -175,3 +175,8 @@
     assert_not_nil(frame['lineNumber'], "The lineNumber of frame #{index} is nil")
   end
 end
+
+Then(/^the payload field "(.+)" is greater than (\d+)(?: for request (\d+))?$/) do |field_path, int_value, request_index|
+  observed_value = read_key_path(find_request(request_index)[:body], field_path)
+  assert(observed_value > int_value)
+end

sdk/src/main/java/com/bugsnag/android/SessionTracker.java

@@ -36,10 +36,10 @@ class SessionTracker extends Observable implements Application.ActivityLifecycle
     private final SessionStore sessionStore;
 
     // This most recent time an Activity was stopped.
-    private AtomicLong activityLastStoppedAtMs = new AtomicLong(0);
+    private AtomicLong lastExitedForegroundMs = new AtomicLong(0);
 
     // The first Activity in this 'session' was started at this time.
-    private AtomicLong activityFirstStartedAtMs = new AtomicLong(0);
+    private AtomicLong lastEnteredForegroundMs = new AtomicLong(0);
     private AtomicReference<Session> currentSession = new AtomicReference<>();
     private Semaphore flushingRequest = new Semaphore(1);
 
@@ -261,7 +261,7 @@ void startFirstSession(Activity activity) {
         Session session = currentSession.get();
         if (session == null) {
             long nowMs = System.currentTimeMillis();
-            activityFirstStartedAtMs.set(nowMs);
+            lastEnteredForegroundMs.set(nowMs);
             startNewSession(new Date(nowMs), client.getUser(), true);
             foregroundActivities.add(getActivityName(activity));
         }
@@ -282,20 +282,24 @@ void startFirstSession(Activity activity) {
      */
     void updateForegroundTracker(String activityName, boolean activityStarting, long nowMs) {
         if (activityStarting) {
-            long noActivityRunningForMs = nowMs - activityLastStoppedAtMs.get();
+            long noActivityRunningForMs = nowMs - lastExitedForegroundMs.get();
 
             //FUTURE:SM Race condition between isEmpty and put
-            if (foregroundActivities.isEmpty()
-                && noActivityRunningForMs >= timeoutMs
-                && configuration.shouldAutoCaptureSessions()) {
+            if (foregroundActivities.isEmpty()) {
+                lastEnteredForegroundMs.set(nowMs);
 
-                activityFirstStartedAtMs.set(nowMs);
-                startNewSession(new Date(nowMs), client.getUser(), true);
+                if (noActivityRunningForMs >= timeoutMs
+                    && configuration.shouldAutoCaptureSessions()) {
+                    startNewSession(new Date(nowMs), client.getUser(), true);
+                }
             }
             foregroundActivities.add(activityName);
         } else {
             foregroundActivities.remove(activityName);
-            activityLastStoppedAtMs.set(nowMs);
+
+            if (foregroundActivities.isEmpty()) {
+                lastExitedForegroundMs.set(nowMs);
+            }
         }
         setChanged();
         notifyObservers(new NativeInterface.Message(
@@ -310,7 +314,7 @@ boolean isInForeground() {
     //FUTURE:SM This shouldnt be here
     long getDurationInForegroundMs(long nowMs) {
         long durationMs = 0;
-        long sessionStartTimeMs = activityFirstStartedAtMs.get();
+        long sessionStartTimeMs = lastEnteredForegroundMs.get();
 
         if (isInForeground() && sessionStartTimeMs != 0) {
             durationMs = nowMs - sessionStartTimeMs;