Skip to content

Commit

Permalink
Address comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@jeremyjiang-dev
jeremyjiang-dev committed Mar 24, 2021
1 parent fafa98f commit c0ddadb
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 59 deletions.
7 changes: 7 additions & 0 deletions firebase-perf/src/main/java/com/google/firebase/perf/FirebasePerfRegistrar.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ public List<Component<?>> getComponents() {
.add(Dependency.requiredProvider(TransportFactory.class))
.factory(FirebasePerfRegistrar::providesFirebasePerformance)
.build(),
/**
* Fireperf SDK is lazily by {@link FirebasePerformanceInitializer} during {@link
* com.google.firebase.perf.application.AppStateMonitor#onActivityResumed(Activity)}. we use
* "lazy" dependency for some components that are not required during initialization so as
* not to force initialize them at app startup (refer
* https://github.com/google/guice/wiki/InjectingProviders#providers-for-lazy-loading)*
*/
LibraryVersionComponent.create("fire-perf", BuildConfig.VERSION_NAME));
}

Expand Down
61 changes: 24 additions & 37 deletions firebase-perf/src/main/java/com/google/firebase/perf/application/AppStateMonitor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ public static AppStateMonitor getInstance() {

private ApplicationProcessState mCurrentState = ApplicationProcessState.BACKGROUND;

private Set<WeakReference<AppStateCallback>> appStateClients =
private Set<WeakReference<AppStateCallback>> appStateSubscribers =
new HashSet<WeakReference<AppStateCallback>>();
private Set<AppColdStartCallback> appColdStartClients = new HashSet<AppColdStartCallback>();
private Set<AppColdStartCallback> appColdStartSubscribers = new HashSet<AppColdStartCallback>();

private boolean hasFrameMetricsAggregator = false;
private FrameMetricsAggregator mFrameMetricsAggregator;
Expand Down Expand Up @@ -236,62 +236,49 @@ public ApplicationProcessState getAppState() {
}

/**
* Register a client to receive app state update.
* Register a subscriber to receive app state update.
*
* @param client an AppStateCallback instance.
* @hide
*/
/** @hide */
public void registerForAppState(WeakReference<AppStateCallback> client) {
synchronized (appStateClients) {
appStateClients.add(client);
public void registerForAppState(WeakReference<AppStateCallback> subscriber) {
synchronized (appStateSubscribers) {
appStateSubscribers.add(subscriber);
}
}

/**
* Unregister the client to stop receiving app state update.
* Unregister the subscriber to stop receiving app state update.
*
* @param client an AppStateCallback instance.
* @hide
*/
/** @hide */
public void unregisterForAppState(WeakReference<AppStateCallback> client) {
synchronized (appStateClients) {
appStateClients.remove(client);
}
}

/**
* Register a client to receive app cold start update.
*
* @param client an AppColdStartCallback instance.
* @param subscriber an AppStateCallback instance.
* @hide
*/
/** @hide */
public void registerForAppColdStart(AppColdStartCallback client) {
synchronized (appStateClients) {
appColdStartClients.add(client);
public void unregisterForAppState(WeakReference<AppStateCallback> subscriber) {
synchronized (appStateSubscribers) {
appStateSubscribers.remove(subscriber);
}
}

/**
* Unregister the client to stop receiving app cold start update.
* Register a subscriber to receive app cold start update.
*
* @param client an AppColdStartCallback instance.
* @param subscriber the {@link AppColdStartCallback} instance.
* @hide
*/
/** @hide */
public void unregisterForColdStart(AppColdStartCallback client) {
synchronized (appStateClients) {
appColdStartClients.remove(client);
public void registerForAppColdStart(AppColdStartCallback subscriber) {
synchronized (appStateSubscribers) {
appColdStartSubscribers.add(subscriber);
}
}

/** Send update state update to registered clients. */
/** Send update state update to registered subscribers. */
private void updateAppState(ApplicationProcessState newState) {
mCurrentState = newState;
synchronized (appStateClients) {
for (Iterator<WeakReference<AppStateCallback>> i = appStateClients.iterator();
synchronized (appStateSubscribers) {
for (Iterator<WeakReference<AppStateCallback>> i = appStateSubscribers.iterator();
i.hasNext(); ) {
AppStateCallback callback = i.next().get();
if (callback != null) {
Expand All @@ -305,10 +292,10 @@ private void updateAppState(ApplicationProcessState newState) {
}
}

/** Send cold start update to registered clients. */
/** Send cold start update to registered subscribers. */
private void sendAppColdStartUpdate() {
synchronized (appStateClients) {
for (Iterator<AppColdStartCallback> i = appColdStartClients.iterator(); i.hasNext(); ) {
synchronized (appStateSubscribers) {
for (Iterator<AppColdStartCallback> i = appColdStartSubscribers.iterator(); i.hasNext(); ) {
AppColdStartCallback callback = i.next();
if (callback != null) {
callback.onAppColdStart();
Expand Down Expand Up @@ -466,7 +453,7 @@ private boolean hasFrameMetricsAggregatorClass() {
}

/**
* An interface to be implemented by clients which needs to receive app state update.
* An interface to be implemented by subscribers which needs to receive app state update.
*
* @hide
*/
Expand All @@ -478,7 +465,7 @@ public static interface AppStateCallback {
}

/**
* An interface to be implemented by clients which needs to receive app cold start update.
* An interface to be implemented by subscribers which needs to receive app cold start update.
*
* @hide
*/
Expand Down
70 changes: 48 additions & 22 deletions firebase-perf/src/test/java/com/google/firebase/perf/application/AppStateMonitorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -676,55 +676,55 @@ public void backgroundTrace_perfMonDeactivated_traceCreated() {
}

@Test
public void activityStateChanges_singleClient_callbackIsCalled() {
public void activityStateChanges_singleSubscriber_callbackIsCalled() {
AppStateMonitor monitor = new AppStateMonitor(transportManager, mClock);
Map<Integer, ApplicationProcessState> clientState = new HashMap<>();
Map<Integer, ApplicationProcessState> subscriberState = new HashMap<>();

final int client1 = 1;
final int subscriber1 = 1;
monitor.registerForAppState(
new WeakReference<>(newState -> clientState.put(client1, newState)));
new WeakReference<>(newState -> subscriberState.put(subscriber1, newState)));

// Activity comes to Foreground
monitor.onActivityResumed(activity1);
assertThat(clientState.get(client1)).isEqualTo(ApplicationProcessState.FOREGROUND);
assertThat(subscriberState.get(subscriber1)).isEqualTo(ApplicationProcessState.FOREGROUND);

// Activity goes to Background
monitor.onActivityStopped(activity1);
assertThat(clientState.get(client1)).isEqualTo(ApplicationProcessState.BACKGROUND);
assertThat(subscriberState.get(subscriber1)).isEqualTo(ApplicationProcessState.BACKGROUND);
}

@Test
public void activityStateChanges_multipleClients_callbackCalledOnEachClient() {
public void activityStateChanges_multipleSubscribers_callbackCalledOnEachSubscriber() {
AppStateMonitor monitor = new AppStateMonitor(transportManager, mClock);
Map<Integer, ApplicationProcessState> clientState = new HashMap<>();
Map<Integer, ApplicationProcessState> subscriberState = new HashMap<>();

final int client1 = 1;
final int subscriber1 = 1;
monitor.registerForAppState(
new WeakReference<>(newState -> clientState.put(client1, newState)));
new WeakReference<>(newState -> subscriberState.put(subscriber1, newState)));

final int client2 = 2;
final int subscriber2 = 2;
monitor.registerForAppState(
new WeakReference<>(newState -> clientState.put(client2, newState)));
new WeakReference<>(newState -> subscriberState.put(subscriber2, newState)));

final int client3 = 3;
final int subscriber3 = 3;
monitor.registerForAppState(
new WeakReference<>(newState -> clientState.put(client3, newState)));
new WeakReference<>(newState -> subscriberState.put(subscriber3, newState)));

// Activity comes to Foreground
monitor.onActivityResumed(activity1);
assertThat(clientState.get(client1)).isEqualTo(ApplicationProcessState.FOREGROUND);
assertThat(clientState.get(client2)).isEqualTo(ApplicationProcessState.FOREGROUND);
assertThat(clientState.get(client3)).isEqualTo(ApplicationProcessState.FOREGROUND);
assertThat(subscriberState.get(subscriber1)).isEqualTo(ApplicationProcessState.FOREGROUND);
assertThat(subscriberState.get(subscriber2)).isEqualTo(ApplicationProcessState.FOREGROUND);
assertThat(subscriberState.get(subscriber3)).isEqualTo(ApplicationProcessState.FOREGROUND);

// Activity goes to Background
monitor.onActivityStopped(activity1);
assertThat(clientState.get(client1)).isEqualTo(ApplicationProcessState.BACKGROUND);
assertThat(clientState.get(client2)).isEqualTo(ApplicationProcessState.BACKGROUND);
assertThat(clientState.get(client3)).isEqualTo(ApplicationProcessState.BACKGROUND);
assertThat(subscriberState.get(subscriber1)).isEqualTo(ApplicationProcessState.BACKGROUND);
assertThat(subscriberState.get(subscriber2)).isEqualTo(ApplicationProcessState.BACKGROUND);
assertThat(subscriberState.get(subscriber3)).isEqualTo(ApplicationProcessState.BACKGROUND);
}

@Test
public void appColdStart_singleClient_callbackIsCalled() {
public void appColdStart_singleSubscriber_callbackIsCalled() {
AppStateMonitor monitor = new AppStateMonitor(transportManager, mClock);
FirebasePerformanceInitializer mockInitializer = mock(FirebasePerformanceInitializer.class);
monitor.registerForAppColdStart(mockInitializer);
Expand All @@ -735,7 +735,7 @@ public void appColdStart_singleClient_callbackIsCalled() {
}

@Test
public void appHotStart_singleClient_callbackIsNotCalled() {
public void appHotStart_singleSubscriber_callbackIsNotCalled() {
AppStateMonitor monitor = new AppStateMonitor(transportManager, mClock);
FirebasePerformanceInitializer mockInitializer = mock(FirebasePerformanceInitializer.class);
monitor.registerForAppColdStart(mockInitializer);
Expand All @@ -752,6 +752,32 @@ public void appHotStart_singleClient_callbackIsNotCalled() {
verify(mockInitializer, times(1)).onAppColdStart();
}

@Test
public void appColdStart_multipleSubscriber_callbackIsCalled() {
AppStateMonitor monitor = new AppStateMonitor(transportManager, mClock);
FirebasePerformanceInitializer mockInitializer1 = mock(FirebasePerformanceInitializer.class);
FirebasePerformanceInitializer mockInitializer2 = mock(FirebasePerformanceInitializer.class);
monitor.registerForAppColdStart(mockInitializer1);
monitor.registerForAppColdStart(mockInitializer2);

// Activity comes to Foreground
monitor.onActivityResumed(activity1);
verify(mockInitializer1, times(1)).onAppColdStart();
verify(mockInitializer2, times(1)).onAppColdStart();
}

@Test
public void appColdStart_singleSubscriberRegistersForMultipleTimes_oneCallbackIsCalled() {
AppStateMonitor monitor = new AppStateMonitor(transportManager, mClock);
FirebasePerformanceInitializer mockInitializer1 = mock(FirebasePerformanceInitializer.class);
monitor.registerForAppColdStart(mockInitializer1);
monitor.registerForAppColdStart(mockInitializer1);

// Activity comes to Foreground
monitor.onActivityResumed(activity1);
verify(mockInitializer1, times(1)).onAppColdStart();
}

private static Activity createFakeActivity(boolean isHardwareAccelerated) {
ActivityController<Activity> fakeActivityController = Robolectric.buildActivity(Activity.class);

Expand Down

0 comments on commit c0ddadb

Please sign in to comment.