Enable deferred installation of Crashlytics NDK signal handler. (#2986)

Some app development platforms, including Unity, will overwrite our
NDK signal handler if it is installed too quickly upon app launch.
This change prevents our signal handler from being immediately installed,
instead waiting for FirebaseCrashlyticsNdk#installSignalHander().

Unity-specific logic was added to CrashlyticsNdkRegistrar.

Author: mrichards

firebase-crashlytics-ndk/src/androidTest/java/com/google/firebase/crashlytics/ndk/FirebaseCrashlyticsNdkTest.java

@@ -29,7 +29,7 @@ public class FirebaseCrashlyticsNdkTest extends TestCase {
   protected void setUp() throws Exception {
     super.setUp();
     mockController = mock(CrashpadController.class);
-    nativeComponent = new FirebaseCrashlyticsNdk(mockController);
+    nativeComponent = new FirebaseCrashlyticsNdk(mockController, true);
   }
 
   @Override

firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/CrashlyticsNdkRegistrar.java

@@ -20,6 +20,7 @@
 import com.google.firebase.components.ComponentRegistrar;
 import com.google.firebase.components.Dependency;
 import com.google.firebase.crashlytics.internal.CrashlyticsNativeComponent;
+import com.google.firebase.crashlytics.internal.unity.ResourceUnityVersionProvider;
 import com.google.firebase.platforminfo.LibraryVersionComponent;
 import java.util.Arrays;
 import java.util.List;
@@ -38,6 +39,10 @@ public List<Component<?>> getComponents() {
 
   private CrashlyticsNativeComponent buildCrashlyticsNdk(ComponentContainer container) {
     Context context = container.get(Context.class);
-    return FirebaseCrashlyticsNdk.create(context);
+    // The signal handler is installed immediately for non-Unity apps. For Unity apps, it will
+    // be installed when the Firebase Unity SDK explicitly calls installSignalHandler().
+    boolean installHandlerDuringPrepSession =
+        (ResourceUnityVersionProvider.resolveUnityEditorVersion(context) == null);
+    return FirebaseCrashlyticsNdk.create(context, installHandlerDuringPrepSession);
   }
 }

firebase-crashlytics-ndk/src/main/java/com/google/firebase/crashlytics/ndk/FirebaseCrashlyticsNdk.java

@@ -28,36 +28,62 @@ class FirebaseCrashlyticsNdk implements CrashlyticsNativeComponent {
   /** Relative sub-path to use for storing files. */
   private static final String FILES_PATH = ".com.google.firebase.crashlytics-ndk";
 
-  static FirebaseCrashlyticsNdk create(@NonNull Context context) {
+  private static FirebaseCrashlyticsNdk instance;
+
+  static FirebaseCrashlyticsNdk create(
+      @NonNull Context context, boolean installHandlerDuringPrepareSession) {
     final File rootDir = new File(context.getFilesDir(), FILES_PATH);
 
     final CrashpadController controller =
         new CrashpadController(
             context, new JniNativeApi(context), new NdkCrashFilesManager(rootDir));
-    return new FirebaseCrashlyticsNdk(controller);
+
+    instance = new FirebaseCrashlyticsNdk(controller, installHandlerDuringPrepareSession);
+    return instance;
   }
 
   private final CrashpadController controller;
 
-  FirebaseCrashlyticsNdk(@NonNull CrashpadController controller) {
+  private interface SignalHandlerInstaller {
+    void installHandler();
+  }
+
+  private boolean installHandlerDuringPrepareSession;
+  private SignalHandlerInstaller signalHandlerInstaller;
+
+  FirebaseCrashlyticsNdk(
+      @NonNull CrashpadController controller, boolean installHandlerDuringPrepareSession) {
     this.controller = controller;
+    this.installHandlerDuringPrepareSession = installHandlerDuringPrepareSession;
   }
 
   @Override
   public boolean hasCrashDataForSession(@NonNull String sessionId) {
     return controller.hasCrashDataForSession(sessionId);
   }
 
+  /**
+   * Prepares the session to be opened. If installSignalHandlerDuringPrepareSession was false at the
+   * constructor, the signal handler will not be fully installed until {@link
+   * FirebaseCrashlyticsNdk#installSignalHandler()} is called.
+   */
   @Override
-  public void openSession(
+  public synchronized void prepareNativeSession(
       @NonNull String sessionId,
       @NonNull String generator,
       long startedAtSeconds,
       @NonNull StaticSessionData sessionData) {
 
-    Logger.getLogger().d("Opening native session: " + sessionId);
-    if (!controller.initialize(sessionId, generator, startedAtSeconds, sessionData)) {
-      Logger.getLogger().w("Failed to initialize Crashlytics NDK for session " + sessionId);
+    signalHandlerInstaller =
+        () -> {
+          Logger.getLogger().d("Initializing native session: " + sessionId);
+          if (!controller.initialize(sessionId, generator, startedAtSeconds, sessionData)) {
+            Logger.getLogger().w("Failed to initialize Crashlytics NDK for session " + sessionId);
+          }
+        };
+
+    if (installHandlerDuringPrepareSession) {
+      signalHandlerInstaller.installHandler();
     }
   }
 
@@ -77,4 +103,43 @@ public NativeSessionFileProvider getSessionFileProvider(@NonNull String sessionI
     // equivalent objects.
     return new SessionFilesProvider(controller.getFilesForSession(sessionId));
   }
+
+  /**
+   * Installs the native signal handler, if the session has already been prepared. Otherwise,
+   * calling this method will result in the native signal handler being installed as soon as the
+   * session is prepared. Used by Firebase Crashlytics for Unity.
+   */
+  public synchronized void installSignalHandler() {
+    // If the handler is already initialized, execute it immediately.
+    // Otherwise, set installHandlerDuringPrepareSession=true so it will be installed as soon as it
+    // is available.
+    if (signalHandlerInstaller != null) {
+      signalHandlerInstaller.installHandler();
+      return;
+    }
+    if (installHandlerDuringPrepareSession) {
+      // If installHandlerDuringPrepareSession is already true, we can no-op. The signal handler
+      // was likely already installed (during prep). This method probably should not have been
+      // called, so log a warning.
+      Logger.getLogger().w("Native signal handler already installed; skipping re-install.");
+    } else {
+      Logger.getLogger()
+          .d(
+              "Deferring signal handler installation until the FirebaseCrashlyticsNdk session has been prepared");
+      installHandlerDuringPrepareSession = true;
+    }
+  }
+
+  /**
+   * Gets the singleton {@link FirebaseCrashlyticsNdk} instance. Used by Firebase Unity.
+   *
+   * @throws NullPointerException if create() has not already been called.
+   */
+  @NonNull
+  public static FirebaseCrashlyticsNdk getInstance() {
+    if (instance == null) {
+      throw new NullPointerException("FirebaseCrashlyticsNdk component is not present.");
+    }
+    return instance;
+  }
 }

firebase-crashlytics/src/androidTest/java/com/google/firebase/crashlytics/internal/CrashlyticsNativeComponentDeferredProxyTest.java

@@ -63,9 +63,10 @@ public void whenAvailable(
             0, "model", 1, 1000, 2000, false, 0, "manufacturer", "modelClass");
     StaticSessionData sessionData = StaticSessionData.create(appData, osData, deviceData);
 
-    proxy.openSession(TEST_SESSION_ID, TEST_GENERATOR, TEST_START_TIME, sessionData);
+    proxy.prepareNativeSession(TEST_SESSION_ID, TEST_GENERATOR, TEST_START_TIME, sessionData);
     Mockito.verify(component, Mockito.times(1))
-        .openSession(eq(TEST_SESSION_ID), eq(TEST_GENERATOR), eq(TEST_START_TIME), eq(sessionData));
+        .prepareNativeSession(
+            eq(TEST_SESSION_ID), eq(TEST_GENERATOR), eq(TEST_START_TIME), eq(sessionData));
 
     proxy.finalizeSession(TEST_SESSION_ID);
     Mockito.verify(component, Mockito.times(1)).finalizeSession(eq(TEST_SESSION_ID));

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/CrashlyticsNativeComponent.java

@@ -21,7 +21,11 @@ public interface CrashlyticsNativeComponent {
 
   boolean hasCrashDataForSession(@NonNull String sessionId);
 
-  void openSession(
+  /**
+   * Prepares the native session for opening. Whether or not Crashlytics is fully initialized to
+   * handle native symbols is implementation dependent.
+   */
+  void prepareNativeSession(
       @NonNull String sessionId,
       @NonNull String generator,
       long startedAtSeconds,

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/CrashlyticsNativeComponentDeferredProxy.java

@@ -47,7 +47,7 @@ public boolean hasCrashDataForSession(@NonNull String sessionId) {
   }
 
   @Override
-  public void openSession(
+  public void prepareNativeSession(
       @NonNull String sessionId,
       @NonNull String generator,
       long startedAtSeconds,
@@ -57,7 +57,9 @@ public void openSession(
 
     this.deferredNativeComponent.whenAvailable(
         nativeComponent -> {
-          nativeComponent.get().openSession(sessionId, generator, startedAtSeconds, sessionData);
+          nativeComponent
+              .get()
+              .prepareNativeSession(sessionId, generator, startedAtSeconds, sessionData);
         });
   }
 

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CommonUtils.java

@@ -67,9 +67,6 @@ public class CommonUtils {
       "com.google.firebase.crashlytics.mapping_file_id";
   static final String LEGACY_MAPPING_FILE_ID_RESOURCE_NAME = "com.crashlytics.android.build_id";
 
-  private static final String UNITY_EDITOR_VERSION =
-      "com.google.firebase.crashlytics.unity_version";
-
   private static final long UNCALCULATED_TOTAL_RAM = -1;
   static final int BYTES_IN_A_GIGABYTE = 1073741824;
   static final int BYTES_IN_A_MEGABYTE = 1048576;
@@ -601,20 +598,6 @@ public static String getMappingFileId(Context context) {
     return mappingFileId;
   }
 
-  /**
-   * @return the Unity Editor version resolved from String resources, or <code>null</code> if the
-   *     value was not present.
-   */
-  public static String resolveUnityEditorVersion(Context context) {
-    String version = null;
-    final int id = CommonUtils.getResourcesIdentifier(context, UNITY_EDITOR_VERSION, "string");
-    if (id != 0) {
-      version = context.getResources().getString(id);
-      Logger.getLogger().v("Unity Editor version is: " + version);
-    }
-    return version;
-  }
-
   public static void closeQuietly(Closeable closeable) {
     if (closeable != null) {
       try {

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/common/CrashlyticsController.java

@@ -577,7 +577,7 @@ private void doOpenSession() {
     StaticSessionData.OsData osData = createOsData(getContext());
     StaticSessionData.DeviceData deviceData = createDeviceData(getContext());
 
-    nativeComponent.openSession(
+    nativeComponent.prepareNativeSession(
         sessionIdentifier,
         generator,
         startedAtSeconds,

firebase-crashlytics/src/main/java/com/google/firebase/crashlytics/internal/unity/ResourceUnityVersionProvider.java

@@ -15,28 +15,43 @@
 package com.google.firebase.crashlytics.internal.unity;
 
 import android.content.Context;
+import com.google.firebase.crashlytics.internal.Logger;
 import com.google.firebase.crashlytics.internal.common.CommonUtils;
 
 public class ResourceUnityVersionProvider implements UnityVersionProvider {
 
+  private static final String UNITY_EDITOR_VERSION =
+      "com.google.firebase.crashlytics.unity_version";
+
+  private static boolean isUnityVersionSet = false;
+  private static String unityVersion = null;
+
   private final Context context;
 
-  private boolean hasRead = false;
-  private String unityVersion;
+  /**
+   * @return the Unity Editor version resolved from String resources, or <code>null</code> if the
+   *     value was not present. This method can be invoked directly; access via the instance method
+   *     from UnityVersionProvider is provided to support mocking while testing.
+   */
+  public static synchronized String resolveUnityEditorVersion(Context context) {
+    if (isUnityVersionSet) {
+      return unityVersion;
+    }
+    final int id = CommonUtils.getResourcesIdentifier(context, UNITY_EDITOR_VERSION, "string");
+    if (id != 0) {
+      unityVersion = context.getResources().getString(id);
+      isUnityVersionSet = true;
+      Logger.getLogger().v("Unity Editor version is: " + unityVersion);
+    }
+    return unityVersion;
+  }
 
   public ResourceUnityVersionProvider(Context context) {
     this.context = context;
   }
 
   @Override
   public String getUnityVersion() {
-    if (!hasRead) {
-      unityVersion = CommonUtils.resolveUnityEditorVersion(context);
-      hasRead = true;
-    }
-    if (unityVersion != null) {
-      return unityVersion;
-    }
-    return null;
+    return resolveUnityEditorVersion(this.context);
   }
 }