Incorporating review comments (grouping variables)

Author: ramanpreetSinghKhinda

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

@@ -55,9 +55,34 @@
 public class AppStateMonitor implements ActivityLifecycleCallbacks {
 
   private static final AndroidLogger logger = AndroidLogger.getInstance();
+  private static final String FRAME_METRICS_AGGREGATOR_CLASSNAME =
+      "androidx.core.app.FrameMetricsAggregator";
 
   private static volatile AppStateMonitor instance;
 
+  private final WeakHashMap<Activity, Boolean> activityToResumedMap = 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<>();
+
+  /* Count for TRACE_STARTED_NOT_STOPPED */
+  private final AtomicInteger tsnsCount = new AtomicInteger(0);
+
+  private final TransportManager transportManager;
+  private final ConfigResolver configResolver;
+  private final Clock clock;
+
+  private FrameMetricsAggregator frameMetricsAggregator;
+
+  private Timer resumeTime; // The time app comes to foreground
+  private Timer stopTime; // The time app goes to background
+
+  private ApplicationProcessState currentAppState = ApplicationProcessState.BACKGROUND;
+
+  private boolean isRegisteredForLifecycleCallbacks = false;
+  private boolean isColdStart = true;
+  private boolean hasFrameMetricsAggregator = false;
+
   public static AppStateMonitor getInstance() {
     if (instance == null) {
       synchronized (AppStateMonitor.class) {
@@ -69,29 +94,6 @@ public static AppStateMonitor getInstance() {
     return instance;
   }
 
-  private static final String FRAME_METRICS_AGGREGATOR_CLASSNAME =
-      "androidx.core.app.FrameMetricsAggregator";
-  private boolean isRegisteredForLifecycleCallbacks = false;
-  private final TransportManager transportManager;
-  private ConfigResolver configResolver;
-  private final Clock clock;
-  private boolean isColdStart = true;
-  private final WeakHashMap<Activity, Boolean> activityToResumedMap = new WeakHashMap<>();
-  private Timer stopTime; // The time app goes to background
-  private Timer resumeTime; // The time app comes to foreground
-  private final Map<String, Long> metricToCountMap = new HashMap<>();
-  /* Count for TRACE_STARTED_NOT_STOPPED */
-  private AtomicInteger tsnsCount = new AtomicInteger(0);
-
-  private ApplicationProcessState currentAppState = ApplicationProcessState.BACKGROUND;
-
-  private Set<WeakReference<AppStateCallback>> appStateSubscribers =
-      new HashSet<WeakReference<AppStateCallback>>();
-
-  private boolean hasFrameMetricsAggregator = false;
-  private FrameMetricsAggregator frameMetricsAggregator;
-  private final WeakHashMap<Activity, Trace> activityToScreenTraceMap = new WeakHashMap<>();
-
   AppStateMonitor(TransportManager transportManager, Clock clock) {
     this.transportManager = transportManager;
     this.clock = clock;

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

@@ -28,15 +28,16 @@
 /** @hide */
 public abstract class AppStateUpdateHandler implements AppStateCallback {
 
-  private AppStateMonitor appStateMonitor;
-  private ApplicationProcessState currentAppState =
-      ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN;
-  private boolean isRegisteredForAppState = false;
+  private final AppStateMonitor appStateMonitor;
   // The weak reference to register/unregister with AppStateMonitor.
   // It must be a weak reference because unregisterForAppState() is called typically from
   // Trace.stop() and user may forget to call Trace.stop(), if it was a strong reference,
   // the registration in AppStateMonitor will prevent Trace to be deallocated.
-  private WeakReference<AppStateCallback> appStateCallback;
+  private final WeakReference<AppStateCallback> appStateCallback;
+
+  private boolean isRegisteredForAppState = false;
+  private ApplicationProcessState currentAppState =
+      ApplicationProcessState.APPLICATION_PROCESS_STATE_UNKNOWN;
 
   /** @hide */
   /** @hide */

firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java

@@ -55,11 +55,12 @@
 public class ConfigResolver {
 
   private static final AndroidLogger logger = AndroidLogger.getInstance();
+
   private static volatile ConfigResolver instance;
 
   // Configuration Storage objects.
+  private final RemoteConfigManager remoteConfigManager;
   private ImmutableBundle metadataBundle;
-  private RemoteConfigManager remoteConfigManager;
   private DeviceCacheManager deviceCacheManager;
 
   /**

firebase-perf/src/main/java/com/google/firebase/perf/config/DeviceCacheManager.java

@@ -36,16 +36,16 @@
 public class DeviceCacheManager {
 
   private static final AndroidLogger logger = AndroidLogger.getInstance();
-  private static DeviceCacheManager instance;
-
   private static final String PREFS_NAME = Constants.PREFS_NAME;
+
+  private static DeviceCacheManager instance;
   // The key-value pairs are written to XML files that persist across user sessions, even if the
   // app is killed.
   // https://developer.android.com/guide/topics/data/data-storage.html#pref
   private volatile SharedPreferences sharedPref;
 
   // Used for retrieving the shared preferences on a separate thread.
-  private ExecutorService serialExecutor;
+  private final ExecutorService serialExecutor;
 
   @VisibleForTesting
   public DeviceCacheManager(ExecutorService serialExecutor) {

firebase-perf/src/main/java/com/google/firebase/perf/config/RemoteConfigManager.java

@@ -46,23 +46,20 @@
 public class RemoteConfigManager {
 
   private static final AndroidLogger logger = AndroidLogger.getInstance();
-
-  private static final String FIREPERF_FRC_NAMESPACE_NAME = "fireperf";
-
   private static final RemoteConfigManager instance = new RemoteConfigManager();
+  private static final String FIREPERF_FRC_NAMESPACE_NAME = "fireperf";
   private static final long TIME_AFTER_WHICH_A_FETCH_IS_CONSIDERED_STALE_MS =
       TimeUnit.HOURS.toMillis(12);
-
   private static final long FETCH_NEVER_HAPPENED_TIMESTAMP_MS = 0;
+
+  private final ConcurrentHashMap<String, FirebaseRemoteConfigValue> allRcConfigMap;
+  private final Executor executor;
+
   private long firebaseRemoteConfigLastFetchTimestampMs = FETCH_NEVER_HAPPENED_TIMESTAMP_MS;
 
   @Nullable private Provider<RemoteConfigComponent> firebaseRemoteConfigProvider;
   @Nullable private FirebaseRemoteConfig firebaseRemoteConfig;
 
-  private final Executor executor;
-
-  private final ConcurrentHashMap<String, FirebaseRemoteConfigValue> allRcConfigMap;
-
   private RemoteConfigManager() {
     this(
         new ThreadPoolExecutor(

firebase-perf/src/main/java/com/google/firebase/perf/metrics/AppStartTrace.java

@@ -53,7 +53,9 @@
  */
 /** @hide */
 public class AppStartTrace implements ActivityLifecycleCallbacks {
+
   private static final long MAX_LATENCY_BEFORE_UI_INIT = TimeUnit.MINUTES.toMicros(1);
+
   private static volatile AppStartTrace instance;
 
   /**

firebase-perf/src/main/java/com/google/firebase/perf/metrics/Counter.java

@@ -26,8 +26,9 @@
  */
 /** @hide */
 public class Counter implements Parcelable {
+
   private final String name;
-  private AtomicLong count;
+  private final AtomicLong count;
 
   /**
    * Creates a Counter with given name.

firebase-perf/src/main/java/com/google/firebase/perf/metrics/HttpMetric.java

@@ -39,9 +39,10 @@ public class HttpMetric implements FirebasePerformanceAttributable {
 
   private static final AndroidLogger logger = AndroidLogger.getInstance();
 
-  private NetworkRequestMetricBuilder networkMetricBuilder;
-  private Timer timer;
+  private final NetworkRequestMetricBuilder networkMetricBuilder;
+  private final Timer timer;
   private final Map<String, String> customAttributesMap;
+
   private boolean isStopped = false;
   private boolean isDisabled = false;
 

firebase-perf/src/main/java/com/google/firebase/perf/metrics/NetworkRequestMetricBuilder.java

@@ -52,7 +52,6 @@ public final class NetworkRequestMetricBuilder extends AppStateUpdateHandler
     implements SessionAwareObject {
 
   private static final AndroidLogger logger = AndroidLogger.getInstance();
-
   private static final char HIGHEST_CONTROL_CHAR = '\u001f';
   private static final char HIGHEST_ASCII_CHAR = '\u007f';
 
@@ -61,16 +60,14 @@ public final class NetworkRequestMetricBuilder extends AppStateUpdateHandler
   private final GaugeManager gaugeManager;
   private final TransportManager transportManager;
 
-  private final NetworkRequestMetric.Builder networkMetricBuilder =
-      NetworkRequestMetric.newBuilder();
+  private final NetworkRequestMetric.Builder builder = NetworkRequestMetric.newBuilder();
+  private final WeakReference<SessionAwareObject> weakReference = new WeakReference<>(this);
 
   @Nullable private String userAgent;
 
   private boolean isReportSent;
   private boolean isManualNetworkRequestMetric;
 
-  private final WeakReference<SessionAwareObject> weakReference = new WeakReference<>(this);
-
   /** @hide */
   @Override
   /** @hide */
@@ -134,15 +131,15 @@ public NetworkRequestMetricBuilder setUrl(@Nullable String url) {
       url = Utils.stripSensitiveInfo(url);
 
       // Limits the URL length to 2000 chars.
-      networkMetricBuilder.setUrl(Utils.truncateURL(url, Constants.MAX_URL_LENGTH));
+      builder.setUrl(Utils.truncateURL(url, Constants.MAX_URL_LENGTH));
     }
 
     return this;
   }
 
   /** Gets the url for the current {@link NetworkRequestMetric}. */
   public String getUrl() {
-    return networkMetricBuilder.getUrl();
+    return builder.getUrl();
   }
 
   /** Sets the user-agent for the current network request. */
@@ -196,31 +193,31 @@ public NetworkRequestMetricBuilder setHttpMethod(@Nullable String method) {
           httpMethod = HttpMethod.HTTP_METHOD_UNKNOWN;
           break;
       }
-      networkMetricBuilder.setHttpMethod(httpMethod);
+      builder.setHttpMethod(httpMethod);
     }
     return this;
   }
 
   /** Sets the httpResponseCode for the current {@link NetworkRequestMetric}. */
   public NetworkRequestMetricBuilder setHttpResponseCode(int code) {
-    networkMetricBuilder.setHttpResponseCode(code);
+    builder.setHttpResponseCode(code);
     return this;
   }
 
   /** Answers whether the builder has an HttpResponseCode. */
   public boolean hasHttpResponseCode() {
-    return networkMetricBuilder.hasHttpResponseCode();
+    return builder.hasHttpResponseCode();
   }
 
   /** Sets the requestPayloadBytes for the current {@link NetworkRequestMetric}. */
   public NetworkRequestMetricBuilder setRequestPayloadBytes(long bytes) {
-    networkMetricBuilder.setRequestPayloadBytes(bytes);
+    builder.setRequestPayloadBytes(bytes);
     return this;
   }
 
   /** Sets the customAttributes for the current {@link NetworkRequestMetric}. */
   public NetworkRequestMetricBuilder setCustomAttributes(Map<String, String> attributes) {
-    networkMetricBuilder.clearCustomAttributes().putAllCustomAttributes(attributes);
+    builder.clearCustomAttributes().putAllCustomAttributes(attributes);
     return this;
   }
 
@@ -238,7 +235,7 @@ public NetworkRequestMetricBuilder setRequestStartTimeMicros(long time) {
     PerfSession perfSession = sessionManager.perfSession();
     SessionManager.getInstance().registerForSessionUpdates(weakReference);
 
-    networkMetricBuilder.setClientStartTimeUs(time);
+    builder.setClientStartTimeUs(time);
     updateSession(perfSession);
 
     if (perfSession.isGaugeAndEventCollectionEnabled()) {
@@ -250,19 +247,19 @@ public NetworkRequestMetricBuilder setRequestStartTimeMicros(long time) {
 
   /** Sets the timeToRequestCompletedUs for the current {@link NetworkRequestMetric}. */
   public NetworkRequestMetricBuilder setTimeToRequestCompletedMicros(long time) {
-    networkMetricBuilder.setTimeToRequestCompletedUs(time);
+    builder.setTimeToRequestCompletedUs(time);
     return this;
   }
 
   /** Sets the timeToResponseInitiatedUs for the current {@link NetworkRequestMetric}. */
   public NetworkRequestMetricBuilder setTimeToResponseInitiatedMicros(long time) {
-    networkMetricBuilder.setTimeToResponseInitiatedUs(time);
+    builder.setTimeToResponseInitiatedUs(time);
     return this;
   }
 
   /** Gets the timeToResponseInitiatedUs for the current {@link NetworkRequestMetric}. */
   public long getTimeToResponseInitiatedMicros() {
-    return networkMetricBuilder.getTimeToResponseInitiatedUs();
+    return builder.getTimeToResponseInitiatedUs();
   }
 
   /**
@@ -275,7 +272,7 @@ public long getTimeToResponseInitiatedMicros() {
    * @see PerfSession#isGaugeAndEventCollectionEnabled()
    */
   public NetworkRequestMetricBuilder setTimeToResponseCompletedMicros(long time) {
-    networkMetricBuilder.setTimeToResponseCompletedUs(time);
+    builder.setTimeToResponseCompletedUs(time);
 
     if (SessionManager.getInstance().perfSession().isGaugeAndEventCollectionEnabled()) {
       gaugeManager.collectGaugeMetricOnce(SessionManager.getInstance().perfSession().getTimer());
@@ -286,19 +283,19 @@ public NetworkRequestMetricBuilder setTimeToResponseCompletedMicros(long time) {
 
   /** Sets the responsePayloadBytes for the current {@link NetworkRequestMetric}. */
   public NetworkRequestMetricBuilder setResponsePayloadBytes(long bytes) {
-    networkMetricBuilder.setResponsePayloadBytes(bytes);
+    builder.setResponsePayloadBytes(bytes);
     return this;
   }
 
   /** Sets the responseContentType for the current {@link NetworkRequestMetric}. */
   public NetworkRequestMetricBuilder setResponseContentType(@Nullable String contentType) {
     if (contentType == null) {
-      networkMetricBuilder.clearResponseContentType();
+      builder.clearResponseContentType();
       return this;
     }
 
     if (isValidContentType(contentType)) {
-      networkMetricBuilder.setResponseContentType(contentType);
+      builder.setResponseContentType(contentType);
     } else {
       logger.info("The content type of the response is not a valid content-type:" + contentType);
     }
@@ -310,7 +307,7 @@ public NetworkRequestMetricBuilder setResponseContentType(@Nullable String conte
    * the current {@link NetworkRequestMetric}.
    */
   public NetworkRequestMetricBuilder setNetworkClientErrorReason() {
-    networkMetricBuilder.setNetworkClientErrorReason(NetworkClientErrorReason.GENERIC_CLIENT_ERROR);
+    builder.setNetworkClientErrorReason(NetworkClientErrorReason.GENERIC_CLIENT_ERROR);
     return this;
   }
 
@@ -322,10 +319,10 @@ public NetworkRequestMetric build() {
     com.google.firebase.perf.v1.PerfSession[] perfSessions =
         PerfSession.buildAndSort(getSessions());
     if (perfSessions != null) {
-      networkMetricBuilder.addAllPerfSessions(Arrays.asList(perfSessions));
+      builder.addAllPerfSessions(Arrays.asList(perfSessions));
     }
 
-    NetworkRequestMetric metric = networkMetricBuilder.build();
+    NetworkRequestMetric metric = builder.build();
 
     if (!isAllowedUserAgent(userAgent)) {
       logger.debug("Dropping network request from a 'User-Agent' that is not allowed");
@@ -349,12 +346,12 @@ public NetworkRequestMetric build() {
 
   /** Checks if the network request has stopped. */
   private boolean isStopped() {
-    return networkMetricBuilder.hasTimeToResponseCompletedUs();
+    return builder.hasTimeToResponseCompletedUs();
   }
 
   /** Checks if the network request has started. */
   private boolean hasStarted() {
-    return networkMetricBuilder.hasClientStartTimeUs();
+    return builder.hasClientStartTimeUs();
   }
 
   @VisibleForTesting
@@ -384,7 +381,7 @@ void setReportSent() {
 
   @VisibleForTesting
   void clearBuilderFields() {
-    networkMetricBuilder.clear();
+    builder.clear();
   }
 
   /**