undereference when activity is destroyed

Author: leotianlizhan

firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java

@@ -20,7 +20,6 @@
 import android.content.Context;
 import android.os.Bundle;
 import androidx.annotation.NonNull;
-import androidx.core.app.FrameMetricsAggregator;
 import androidx.fragment.app.FragmentActivity;
 import com.google.android.gms.common.util.VisibleForTesting;
 import com.google.firebase.perf.config.ConfigResolver;
@@ -57,7 +56,10 @@ public class AppStateMonitor implements ActivityLifecycleCallbacks {
   private final boolean hasFrameMetricsAggregator;
 
   private final WeakHashMap<Activity, Boolean> activityToResumedMap = new WeakHashMap<>();
-  private final WeakHashMap<Activity, FrameMetricsRecorder> activityToRecorderMap = new WeakHashMap<>();
+  private final WeakHashMap<Activity, FrameMetricsRecorder> activityToRecorderMap =
+      new WeakHashMap<>();
+  private final WeakHashMap<Activity, FragmentStateMonitor> activityToFragmentStateMonitorMap =
+      new WeakHashMap<>();
   private final WeakHashMap<Activity, Trace> activityToScreenTraceMap = new WeakHashMap<>();
   private final Map<String, Long> metricToCountMap = new HashMap<>();
   private final Set<WeakReference<AppStateCallback>> appStateSubscribers = new HashSet<>();
@@ -90,11 +92,7 @@ public static AppStateMonitor getInstance() {
   }
 
   AppStateMonitor(TransportManager transportManager, Clock clock) {
-    this(
-        transportManager,
-        clock,
-        ConfigResolver.getInstance(),
-        hasFrameMetricsAggregatorClass());
+    this(transportManager, clock, ConfigResolver.getInstance(), hasFrameMetricsAggregatorClass());
   }
 
   AppStateMonitor(
@@ -153,19 +151,29 @@ public void incrementTsnsCount(int value) {
   public void onActivityCreated(Activity activity, Bundle savedInstanceState) {
     if (isScreenTraceSupported() && configResolver.isPerformanceMonitoringEnabled()) {
       FrameMetricsRecorder recorder = new FrameMetricsRecorder(activity);
+      activityToRecorderMap.put(activity, recorder);
       if (activity instanceof FragmentActivity) {
+        FragmentStateMonitor fragmentStateMonitor =
+            new FragmentStateMonitor(clock, transportManager, this, recorder);
+        activityToFragmentStateMonitorMap.put(activity, fragmentStateMonitor);
         FragmentActivity fragmentActivity = (FragmentActivity) activity;
         fragmentActivity
             .getSupportFragmentManager()
-            .registerFragmentLifecycleCallbacks(
-                new FragmentStateMonitor(clock, transportManager, this, recorder),
-                true);
+            .registerFragmentLifecycleCallbacks(fragmentStateMonitor, true);
       }
     }
   }
 
   @Override
-  public void onActivityDestroyed(Activity activity) {}
+  public void onActivityDestroyed(Activity activity) {
+    activityToRecorderMap.remove(activity);
+    if (activityToFragmentStateMonitorMap.containsKey(activity)) {
+      FragmentActivity fragmentActivity = (FragmentActivity) activity;
+      fragmentActivity
+          .getSupportFragmentManager()
+          .unregisterFragmentLifecycleCallbacks(activityToFragmentStateMonitorMap.remove(activity));
+    }
+  }
 
   @Override
   public synchronized void onActivityStarted(Activity activity) {
@@ -326,18 +334,16 @@ public void onActivityPaused(Activity activity) {}
    * @param activity activity object.
    */
   private void sendScreenTrace(Activity activity) {
-    if (!activityToScreenTraceMap.containsKey(activity)) {
-      return;
-    }
     Trace screenTrace = activityToScreenTraceMap.get(activity);
     if (screenTrace == null) {
       return;
     }
     activityToScreenTraceMap.remove(activity);
 
-    Optional<FrameMetricsCalculator.PerfFrameMetrics> perfFrameMetrics = activityToRecorderMap.get(activity).stop();
+    Optional<FrameMetricsCalculator.PerfFrameMetrics> perfFrameMetrics =
+        activityToRecorderMap.get(activity).stop();
     if (!perfFrameMetrics.isAvailable()) {
-      logger.warn("Failed to record frame data for %s", activity.getClass().getSimpleName());
+      logger.warn("Failed to record frame data for %s.", activity.getClass().getSimpleName());
       return;
     }
     ScreenTraceUtil.addFrameCounters(screenTrace, perfFrameMetrics.get());

firebase-perf/src/main/java/com/google/firebase/perf/application/FragmentStateMonitor.java

@@ -15,7 +15,6 @@
 package com.google.firebase.perf.application;
 
 import androidx.annotation.NonNull;
-import androidx.core.app.FrameMetricsAggregator;
 import androidx.fragment.app.Fragment;
 import androidx.fragment.app.FragmentManager;
 import com.google.android.gms.common.util.VisibleForTesting;
@@ -98,8 +97,7 @@ public void onFragmentPaused(@NonNull FragmentManager fm, @NonNull Fragment f) {
       logger.warn("onFragmentPaused: recorder failed to trace %s", f.getClass().getSimpleName());
       return;
     }
-    ScreenTraceUtil.addFrameCounters(
-        fragmentTrace, data.get());
+    ScreenTraceUtil.addFrameCounters(fragmentTrace, data.get());
     fragmentTrace.stop();
   }
 

firebase-perf/src/main/java/com/google/firebase/perf/application/FrameMetricsRecorder.java

@@ -96,8 +96,8 @@ public Optional<PerfFrameMetrics> stop() {
     }
     Optional<PerfFrameMetrics> data = this.snapshot();
     /**
-     * {@link FrameMetricsAggregator#remove(Activity)} will throw exceptions for hardware acceleration
-     * disabled activities.
+     * {@link FrameMetricsAggregator#remove(Activity)} will throw exceptions for hardware
+     * acceleration disabled activities.
      */
     try {
       frameMetricsAggregator.remove(activity);

firebase-perf/src/test/java/com/google/firebase/perf/application/FragmentStateMonitorTest.java

@@ -23,10 +23,8 @@
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
 import static org.mockito.MockitoAnnotations.initMocks;
 
-import android.app.Activity;
 import android.os.Bundle;
 import android.util.SparseIntArray;
 import androidx.appcompat.app.AppCompatActivity;
@@ -45,7 +43,6 @@
 import com.google.firebase.perf.util.Timer;
 import com.google.firebase.perf.v1.ApplicationProcessState;
 import com.google.firebase.perf.v1.TraceMetric;
-import com.google.testing.timing.FakeDirectExecutorService;
 import java.util.WeakHashMap;
 import org.junit.Assert;
 import org.junit.Before;
@@ -80,7 +77,6 @@ public class FragmentStateMonitorTest extends FirebasePerformanceTestBase {
   private static final String longFragmentName =
       "_st_NeverGonnaGiveYouUpNeverGonnaLetYouDownNeverGonnaRunAroundAndDesertYouNeverGonnaMakeYouCryNeverGonnaSayGoodbyeNeverGonnaTellALieAndHurtYou";
 
-
   /**
    * Array of SparseIntArray to mock the return value from {@link
    * FrameMetricsAggregator#getMetrics()}
@@ -97,11 +93,12 @@ public void setUp() {
 
     DeviceCacheManager.clearInstance();
 
-//    ConfigResolver configResolver = ConfigResolver.getInstance();
-//    configResolver.setDeviceCacheManager(new DeviceCacheManager(new FakeDirectExecutorService()));
-//    ConfigResolver spyConfigResolver = spy(configResolver);
+    //    ConfigResolver configResolver = ConfigResolver.getInstance();
+    //    configResolver.setDeviceCacheManager(new DeviceCacheManager(new
+    // FakeDirectExecutorService()));
+    //    ConfigResolver spyConfigResolver = spy(configResolver);
     doReturn(true).when(configResolver).isPerformanceMonitoringEnabled();
-//    this.configResolver = spyConfigResolver;
+    //    this.configResolver = spyConfigResolver;
 
     // fmaMetrics1 should have 1+3+1=5 total frames, 3+1=4 slow frames, and 1 frozen frames.
     SparseIntArray sparseIntArray = new SparseIntArray();
@@ -198,7 +195,8 @@ public void lifecycleCallbacks_onPausedCalledBeforeOnResume_doesNotLogFragmentSc
   public void lifecycleCallbacks_cleansUpMap_duringActivityTransitions() {
     // Simulate call order of activity + fragment lifecycle events
     Bundle savedInstanceState = mock(Bundle.class);
-    AppStateMonitor appStateMonitor = new AppStateMonitor(mockTransportManager, clock, configResolver, true);
+    AppStateMonitor appStateMonitor =
+        new AppStateMonitor(mockTransportManager, clock, configResolver, true);
     FragmentStateMonitor fragmentMonitor =
         new FragmentStateMonitor(clock, mockTransportManager, appStateMonitor, recorder);
     doReturn(true).when(configResolver).isPerformanceMonitoringEnabled();