Skip to content

Commit 9ab737a

Browse files
Fix inaccurate activity screen metrics. (#2736)
* Fix inaccurate activity screen metrics. * Add javadoc for hc acceleratin disabled. * Add comments and TODOs.
1 parent 011b65b commit 9ab737a

File tree

2 files changed

+17
-16
lines changed

2 files changed

+17
-16
lines changed

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

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import android.content.Context;
2121
import android.os.Bundle;
2222
import android.util.SparseIntArray;
23-
import android.view.WindowManager;
2423
import androidx.annotation.NonNull;
2524
import androidx.core.app.FrameMetricsAggregator;
2625
import com.google.android.gms.common.util.VisibleForTesting;
@@ -150,6 +149,10 @@ public void onActivityDestroyed(Activity activity) {}
150149
public synchronized void onActivityStarted(Activity activity) {
151150
if (isScreenTraceSupported(activity) && configResolver.isPerformanceMonitoringEnabled()) {
152151
// Starts recording frame metrics for this activity.
152+
/**
153+
* TODO: Only add activities that are hardware acceleration enabled so that calling {@link
154+
* FrameMetricsAggregator#remove(Activity)} will not throw exceptions.
155+
*/
153156
frameMetricsAggregator.add(activity);
154157
// Start the Trace
155158
Trace screenTrace = new Trace(getScreenTraceName(activity), transportManager, clock, this);
@@ -290,7 +293,8 @@ public void onActivitySaveInstanceState(Activity activity, Bundle outState) {}
290293
public void onActivityPaused(Activity activity) {}
291294

292295
/**
293-
* Send screen trace.
296+
* Send screen trace. If hardware acceleration is not enabled, all frame metrics will be zero and
297+
* the trace will not be sent.
294298
*
295299
* @param activity activity object.
296300
*/
@@ -307,8 +311,14 @@ private void sendScreenTrace(Activity activity) {
307311
int totalFrames = 0;
308312
int slowFrames = 0;
309313
int frozenFrames = 0;
310-
// Stops recording metrics for this Activity and returns the currently-collected metrics
311-
SparseIntArray[] arr = frameMetricsAggregator.remove(activity);
314+
/**
315+
* Resets the metrics data and returns the currently-collected metrics. Note that {@link
316+
* FrameMetricsAggregator#reset()} will not stop recording for the activity. The reason of using
317+
* {@link FrameMetricsAggregator#reset()} is that {@link
318+
* FrameMetricsAggregator#remove(Activity)} will throw exceptions for hardware acceleration
319+
* disabled activities.
320+
*/
321+
SparseIntArray[] arr = frameMetricsAggregator.reset();
312322
if (arr != null) {
313323
SparseIntArray frameTimes = arr[FrameMetricsAggregator.TOTAL_INDEX];
314324
if (frameTimes != null) {
@@ -391,21 +401,13 @@ private void sendSessionLog(String name, Timer startTime, Timer endTime) {
391401
}
392402

393403
/**
394-
* Only send screen trace if FrameMetricsAggregator exists and the activity is hardware
395-
* accelerated.
404+
* Only send screen trace if FrameMetricsAggregator exists.
396405
*
397406
* @param activity The Activity for which we're monitoring the screen rendering performance.
398407
* @return true if supported, false if not.
399408
*/
400409
private boolean isScreenTraceSupported(Activity activity) {
401-
return hasFrameMetricsAggregator
402-
// This check is needed because we can't observe frame rates for a non hardware accelerated
403-
// view.
404-
// See b/133827763.
405-
&& activity.getWindow() != null
406-
&& ((activity.getWindow().getAttributes().flags
407-
& WindowManager.LayoutParams.FLAG_HARDWARE_ACCELERATED)
408-
!= 0);
410+
return hasFrameMetricsAggregator;
409411
}
410412

411413
/**

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,13 +373,12 @@ public void screenTrace_twoActivities_traceStartedAndStoppedWithActivityLifecycl
373373
}
374374

375375
@Test
376-
public void screenTrace_noHardwareAccelerated_traceNotCreated() {
376+
public void screenTrace_noHardwareAccelerated_noExceptionThrown() {
377377
AppStateMonitor monitor = new AppStateMonitor(transportManager, clock);
378378
Activity activityWithNonHardwareAcceleratedView =
379379
createFakeActivity(/* isHardwareAccelerated =*/ false);
380380

381381
monitor.onActivityStarted(activityWithNonHardwareAcceleratedView);
382-
assertThat(monitor.getActivity2ScreenTrace()).isEmpty();
383382

384383
// Confirm that this doesn't throw an exception.
385384
monitor.onActivityStopped(activityWithNonHardwareAcceleratedView);

0 commit comments

Comments
 (0)