Merge remote-tracking branch 'origin/master' into ExpectedExceptionEqualsTrueToAssertThrows

Author: dconeybe

firebase-dynamic-links/src/main/java/com/google/firebase/dynamiclinks/PendingDynamicLinkData.java

@@ -71,12 +71,12 @@ protected PendingDynamicLinkData(
   }
 
   /**
-   * Return the {@link Bundle} so that 1P dynamic links extensions can access extension data. The
+   * Returns the {@link Bundle} so that 1P dynamic links extensions can access extension data. The
    * data is stored bundle with keys defined by the extension. The bundle is shared with all
    * extensions, so the keys should be unique using the package name of the extension to define the
    * namespace.
    *
-   * @return A bundle will all extension data.
+   * @return A bundle with all extensions data.
    * @hide
    */
   @KeepForSdk
@@ -89,7 +89,7 @@ public Bundle getExtensions() {
   }
 
   /**
-   * Return the link parameter of the dynamic link.
+   * Returns the link parameter of the Firebase Dynamic Link.
    *
    * <p>This link will be set as data in the launch Intent, see {@link Intent#setData(Uri)}, which
    * will match {@link android.content.IntentFilter} to deep link into the app.
@@ -109,8 +109,8 @@ public Uri getLink() {
   }
 
   /**
-   * Return the {@link Bundle} which contains utm parameters associated with the firebase dynamic
-   * link.
+   * Returns the {@link Bundle} which contains utm parameters associated with the Firebase Dynamic
+   * Link.
    *
    * @return Bundle of utm parameters associated with firebase dynamic link.
    */
@@ -124,9 +124,9 @@ public Bundle getUtmParameters() {
   }
 
   /**
-   * The minimum app version requested to process the dynamic link that can be compared directly
-   * with {@link android.content.pm.PackageInfo#versionCode}. If the minimum version code is higher
-   * than the installed app version code, the app can upgrade using {@link
+   * Gets the minimum app version requested to process the Firebase Dynamic Link that can be
+   * compared directly with {@link android.content.pm.PackageInfo#versionCode}. If the minimum
+   * version code is higher than the installed app version code, the app can upgrade using {@link
    * #getUpdateAppIntent(Context)}.
    *
    * @return minimum version code set on the dynamic link, or 0 if not specified.
@@ -139,8 +139,8 @@ public int getMinimumAppVersion() {
   }
 
   /**
-   * The time that the user clicked on the dynamic link. This can be used to determine the amount of
-   * time that has passed since the user selected the link until the app is launched.
+   * Gets the time that the user clicked on the Firebase Dynamic Link. This can be used to determine
+   * the amount of time that has passed since the user selected the link until the app is launched.
    *
    * @return The number of milliseconds that have elapsed since January 1, 1970.
    */
@@ -152,7 +152,7 @@ public long getClickTimestamp() {
   }
 
   /**
-   * Provides the redirect url, which is used as the alternative to opening the app. This url may
+   * Gets the redirect url, which is used as the alternative to opening the app. This url may
    * install the app or go to an app specific website.
    *
    * @return Url that can be used to create an intent to launch an activity.
@@ -168,7 +168,7 @@ public Uri getRedirectUrl() {
   }
 
   /**
-   * Provide an intent to update the app to the version in the Play Store.
+   * Gets the intent to update the app to the version in the Play Store.
    *
    * <p>An intent is returned to be used as a parameter to {@link
    * android.app.Activity#startActivity(Intent)} to launch the Play Store update flow for the app.

firebase-messaging/src/main/java/com/google/firebase/messaging/FcmLifecycleCallbacks.java

@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.firebase.messaging;
 
+import static com.google.firebase.messaging.Constants.TAG;
+
 import android.app.Activity;
 import android.app.Application;
 import android.content.Intent;
@@ -21,6 +23,7 @@
 import android.os.Bundle;
 import android.os.Handler;
 import android.os.Looper;
+import android.util.Log;
 import java.util.Collections;
 import java.util.Set;
 import java.util.WeakHashMap;
@@ -74,12 +77,19 @@ public void onActivityResumed(Activity activity) {}
   public void onActivitySaveInstanceState(Activity activity, Bundle bundle) {}
 
   private void logNotificationOpen(Intent startingIntent) {
-    Bundle extras = startingIntent.getExtras();
-    if (extras != null) {
-      Bundle analyticsData = extras.getBundle(Constants.MessageNotificationKeys.ANALYTICS_DATA);
-      if (MessagingAnalytics.shouldUploadScionMetrics(analyticsData)) {
-        MessagingAnalytics.logNotificationOpen(analyticsData);
+    Bundle analyticsData = null;
+    try {
+      Bundle extras = startingIntent.getExtras();
+      if (extras != null) {
+        analyticsData = extras.getBundle(Constants.MessageNotificationKeys.ANALYTICS_DATA);
       }
+    } catch (RuntimeException e) {
+      // Don't crash if there was a problem trying to get the analytics data Bundle since the
+      // Intent could be coming from anywhere and could be incorrectly formatted.
+      Log.w(TAG, "Failed trying to get analytics data from Intent extras.", e);
+    }
+    if (MessagingAnalytics.shouldUploadScionMetrics(analyticsData)) {
+      MessagingAnalytics.logNotificationOpen(analyticsData);
     }
   }
 }

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

@@ -146,20 +146,7 @@ public void onActivityCreated(Activity activity, Bundle savedInstanceState) {}
   public void onActivityDestroyed(Activity activity) {}
 
   @Override
-  public synchronized void onActivityStarted(Activity activity) {
-    if (isScreenTraceSupported(activity) && configResolver.isPerformanceMonitoringEnabled()) {
-      // Starts recording frame metrics for this activity.
-      /**
-       * TODO: Only add activities that are hardware acceleration enabled so that calling {@link
-       * FrameMetricsAggregator#remove(Activity)} will not throw exceptions.
-       */
-      frameMetricsAggregator.add(activity);
-      // Start the Trace
-      Trace screenTrace = new Trace(getScreenTraceName(activity), transportManager, clock, this);
-      screenTrace.start();
-      activityToScreenTraceMap.put(activity, screenTrace);
-    }
-  }
+  public synchronized void onActivityStarted(Activity activity) {}
 
   @Override
   public synchronized void onActivityStopped(Activity activity) {
@@ -181,7 +168,8 @@ public synchronized void onActivityResumed(Activity activity) {
     // cases:
     // 1. At app startup, first activity comes to foreground.
     // 2. app switch from background to foreground.
-    // 3. app already in foreground, current activity is replaced by another activity.
+    // 3. app already in foreground, current activity is replaced by another activity, or the
+    // current activity was paused then resumed without onStop, for example by an AlertDialog
     if (activityToResumedMap.isEmpty()) {
       // The first resumed activity means app comes to foreground.
       resumeTime = clock.getTime();
@@ -198,9 +186,24 @@ public synchronized void onActivityResumed(Activity activity) {
         updateAppState(ApplicationProcessState.FOREGROUND);
       }
     } else {
-      // case 3: app already in foreground, current activity is replaced by another activity.
+      // case 3: app already in foreground, current activity is replaced by another activity, or the
+      // current activity was paused then resumed without onStop, for example by an AlertDialog
       activityToResumedMap.put(activity, true);
     }
+
+    // Screen trace is after session update so the sessionId is not added twice to the Trace
+    if (isScreenTraceSupported(activity) && configResolver.isPerformanceMonitoringEnabled()) {
+      // Starts recording frame metrics for this activity.
+      /**
+       * TODO: Only add activities that are hardware acceleration enabled so that calling {@link
+       * FrameMetricsAggregator#remove(Activity)} will not throw exceptions.
+       */
+      frameMetricsAggregator.add(activity);
+      // Start the Trace
+      Trace screenTrace = new Trace(getScreenTraceName(activity), transportManager, clock, this);
+      screenTrace.start();
+      activityToScreenTraceMap.put(activity, screenTrace);
+    }
   }
 
   /** Returns if this is the cold start of the app. */

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

@@ -366,7 +366,7 @@ public void screenTrace_twoActivities_traceStartedAndStoppedWithActivityLifecycl
       int startTime = i * 100;
       int endTime = startTime + 10;
       currentTime = startTime;
-      monitor.onActivityStarted(activity);
+      monitor.onActivityResumed(activity);
       assertThat(monitor.getActivity2ScreenTrace()).hasSize(1);
       currentTime = endTime;
       monitor.onActivityPostPaused(activity);
@@ -418,7 +418,7 @@ public void screenTrace_perfMonEnabledSwitchAtRuntime_traceCreationDependsOnRunt
 
     // Assert that screen trace has been created.
     currentTime = 100;
-    monitor.onActivityStarted(activityWithNonHardwareAcceleratedView);
+    monitor.onActivityResumed(activityWithNonHardwareAcceleratedView);
     assertThat(monitor.getActivity2ScreenTrace()).hasSize(1);
     currentTime = 200;
     monitor.onActivityPostPaused(activityWithNonHardwareAcceleratedView);

firebase-storage/src/test/java/com/google/firebase/storage/StorageReferenceTest.java

@@ -148,9 +148,11 @@ public void initWithDefaultAndCustomUriTest() throws Exception {
     Assert.assertEquals("gs://foo-bar.appspot.com/", customRef.toString());
   }
 
-  @Test(expected = IllegalArgumentException.class)
+  @Test
   public void badInitTest() throws Exception {
-    FirebaseStorage.getInstance().getReference("gs://fooey.appspot.com/child");
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> FirebaseStorage.getInstance().getReference("gs://fooey.appspot.com/child"));
   }
 
   @Test
@@ -178,15 +180,18 @@ public void initWithApp() throws Exception {
     Assert.assertEquals("child", ref.getName());
   }
 
-  @Test(expected = IllegalArgumentException.class)
+  @Test
   public void badInitWithApp1() throws Exception {
-    FirebaseStorage.getInstance().getReference("gs://bucket/child");
+    assertThrows(
+        IllegalArgumentException.class,
+        () -> FirebaseStorage.getInstance().getReference("gs://bucket/child"));
   }
 
   @SuppressWarnings("ConstantConditions")
-  @Test(expected = IllegalArgumentException.class)
+  @Test
   public void badInitWithApp2() throws Exception {
-    FirebaseStorage.getInstance().getReference(null);
+    assertThrows(
+        IllegalArgumentException.class, () -> FirebaseStorage.getInstance().getReference(null));
   }
 
   @Test

smoke-tests/build.gradle

@@ -71,7 +71,6 @@ dependencies {
   // Firebase
   implementation "com.google.firebase:firebase-analytics"
   implementation "com.google.firebase:firebase-annotations"
-  implementation "com.google.firebase:firebase-appdistribution"
   implementation "com.google.firebase:firebase-appindexing"
   implementation "com.google.firebase:firebase-auth"
   implementation "com.google.firebase:firebase-common"

smoke-tests/src/main/java/com/google/firebase/testing/BuildOnlyTest.java

@@ -21,7 +21,6 @@
 import com.google.firebase.messaging.FirebaseMessaging;
 import com.google.firebase.ml.modeldownloader.FirebaseModelDownloader;
 import com.google.firebase.perf.FirebasePerformance;
-import com.google.firebase.appdistribution.FirebaseAppDistribution;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.junit.runners.JUnit4;
@@ -56,9 +55,4 @@ public void modelDownloader_IsNotNull() {
   public void performance_IsNotNull() {
     assertThat(FirebasePerformance.getInstance()).isNotNull();
   }
-
-  @Test
-  public void appDistribution_IsNotNull(){
-    assertThat(FirebaseAppDistribution.getInstance()).isNotNull();
-  }
 }