Fix Nullsafe FIXMEs for DevSupportManagerBase.java (#50068)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/50068

Note: this involved tightening up some signatures in Kotlin files

Gone trough all the FIXMEs added in the previous diff by the nullsafe tool, marked the class as nullsafe and ensured no remaining violations.

Changelog: [Android][Fixed] Made DevSupportManagerBase.java nullsafe

Reviewed By: mdvacca

Differential Revision: D71126381

fbshipit-source-id: 01c08e2fc61eff885911fa9c295d504fa7ea334c

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevServerHelper.java

@@ -327,7 +327,7 @@ public class DevServerHelper {
       DevBundleDownloadListener callback,
       File outputFile,
       String bundleURL,
-      BundleDownloader.BundleInfo bundleInfo) {
+      @Nullable BundleDownloader.BundleInfo bundleInfo) {
     mBundleDownloader.downloadBundleFromURL(callback, outputFile, bundleURL, bundleInfo);
   }
 
@@ -335,7 +335,7 @@ public class DevServerHelper {
       DevBundleDownloadListener callback,
       File outputFile,
       String bundleURL,
-      BundleDownloader.BundleInfo bundleInfo,
+      @Nullable BundleDownloader.BundleInfo bundleInfo,
       Request.Builder requestBuilder) {
     mBundleDownloader.downloadBundleFromURL(
         callback, outputFile, bundleURL, bundleInfo, requestBuilder);

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManagerBase.java

@@ -34,6 +34,7 @@ import androidx.annotation.Nullable;
 import androidx.annotation.UiThread;
 import com.facebook.common.logging.FLog;
 import com.facebook.infer.annotation.Assertions;
+import com.facebook.infer.annotation.Nullsafe;
 import com.facebook.react.R;
 import com.facebook.react.bridge.DefaultJSExceptionHandler;
 import com.facebook.react.bridge.JSBundleLoader;
@@ -74,6 +75,7 @@ import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 
+@Nullsafe(Nullsafe.Mode.LOCAL)
 public abstract class DevSupportManagerBase implements DevSupportManager {
 
   public interface CallbackWithBundleLoader {
@@ -222,16 +224,13 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
 
     if (e instanceof JavascriptException) {
       FLog.e(ReactConstants.TAG, "Exception in native call from JS", e);
-      showNewError(
-          // NULLSAFE_FIXME[Nullable Dereference]
-          e.getMessage().toString(), new StackFrame[] {}, JSEXCEPTION_ERROR_COOKIE, ErrorType.JS);
+      showNewError(e.getMessage(), new StackFrame[] {}, JSEXCEPTION_ERROR_COOKIE, ErrorType.JS);
     } else {
       showNewJavaError(message.toString(), e);
     }
   }
 
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public void showNewJavaError(@Nullable String message, Throwable e) {
     FLog.e(ReactConstants.TAG, "Exception in native call", e);
     showNewError(
@@ -244,19 +243,17 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
    * called.
    */
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public void addCustomDevOption(String optionName, DevOptionHandler optionHandler) {
     mCustomDevOptions.put(optionName, optionHandler);
   }
 
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
-  public void showNewJSError(String message, ReadableArray details, int errorCookie) {
+  public void showNewJSError(
+      @Nullable String message, @Nullable ReadableArray details, int errorCookie) {
     showNewError(message, StackTraceHelper.convertJsStackTrace(details), errorCookie, ErrorType.JS);
   }
 
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public void registerErrorCustomizer(ErrorCustomizer errorCustomizer) {
     if (mErrorCustomizers == null) {
       mErrorCustomizers = new ArrayList<>();
@@ -265,7 +262,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
   }
 
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public Pair<String, StackFrame[]> processErrorCustomizers(Pair<String, StackFrame[]> errorInfo) {
     if (mErrorCustomizers != null) {
       for (ErrorCustomizer errorCustomizer : mErrorCustomizers) {
@@ -287,14 +283,14 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
     mRedBoxSurfaceDelegate.hide();
   }
 
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public @Nullable View createRootView(String appKey) {
     return mReactInstanceDevHelper.createRootView(appKey);
   }
 
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
-  public void destroyRootView(View rootView) {
-    mReactInstanceDevHelper.destroyRootView(rootView);
+  public void destroyRootView(@Nullable View rootView) {
+    if (rootView != null) {
+      mReactInstanceDevHelper.destroyRootView(rootView);
+    }
   }
 
   private void hideDevOptionsDialog() {
@@ -526,14 +522,12 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
     mDevOptionsDialog =
         new AlertDialog.Builder(context)
             .setCustomTitle(header)
-            // NULLSAFE_FIXME[Not Vetted Third-Party]
             .setAdapter(
                 adapter,
                 (dialog, which) -> {
                   optionHandlers[which].onOptionSelected();
                   mDevOptionsDialog = null;
                 })
-            // NULLSAFE_FIXME[Not Vetted Third-Party]
             .setOnCancelListener(dialog -> mDevOptionsDialog = null)
             .create();
     mDevOptionsDialog.show();
@@ -544,11 +538,10 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
     }
   }
 
-  private String getJSExecutorDescription() {
+  private @Nullable String getJSExecutorDescription() {
     try {
       return getReactInstanceDevHelper().getJavaScriptExecutorFactory().toString();
     } catch (IllegalStateException e) {
-      // NULLSAFE_FIXME[Return Not Nullable]
       return null;
     }
   }
@@ -634,19 +627,20 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
     if (mIsDevSupportEnabled && mJSBundleDownloadedFile.exists()) {
       try {
         String packageName = mApplicationContext.getPackageName();
-        PackageInfo thisPackage =
-            // NULLSAFE_FIXME[Nullable Dereference]
-            mApplicationContext.getPackageManager().getPackageInfo(packageName, 0);
-        if (mJSBundleDownloadedFile.lastModified() > thisPackage.lastUpdateTime) {
-          // Base APK has not been updated since we downloaded JS, but if app is using exopackage
-          // it may only be a single dex that has been updated. We check for exopackage dir update
-          // time in that case.
-          File exopackageDir =
-              new File(String.format(Locale.US, EXOPACKAGE_LOCATION_FORMAT, packageName));
-          if (exopackageDir.exists()) {
-            return mJSBundleDownloadedFile.lastModified() > exopackageDir.lastModified();
+        PackageManager packageManager = mApplicationContext.getPackageManager();
+        if (packageManager != null) {
+          PackageInfo thisPackage = packageManager.getPackageInfo(packageName, 0);
+          if (mJSBundleDownloadedFile.lastModified() > thisPackage.lastUpdateTime) {
+            // Base APK has not been updated since we downloaded JS, but if app is using exopackage
+            // it may only be a single dex that has been updated. We check for exopackage dir update
+            // time in that case.
+            File exopackageDir =
+                new File(String.format(Locale.US, EXOPACKAGE_LOCATION_FORMAT, packageName));
+            if (exopackageDir.exists()) {
+              return mJSBundleDownloadedFile.lastModified() > exopackageDir.lastModified();
+            }
+            return true;
           }
-          return true;
         }
       } catch (PackageManager.NameNotFoundException e) {
         // Ignore this error and just fallback to loading JS from assets
@@ -675,8 +669,10 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
     if (mCurrentReactContext != null) {
       try {
         URL sourceUrl = new URL(getSourceUrl());
-        // NULLSAFE_FIXME[Nullable Dereference]
-        String path = sourceUrl.getPath().substring(1); // strip initial slash in path
+        String path = sourceUrl.getPath();
+        if (path != null) {
+          path = path.substring(1); // strip initial slash in path
+        }
         String host = sourceUrl.getHost();
         String scheme = sourceUrl.getProtocol();
         int port = sourceUrl.getPort() != -1 ? sourceUrl.getPort() : sourceUrl.getDefaultPort();
@@ -798,7 +794,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
                 }
 
                 @Override
-                // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
                 public void onFailure(Exception cause) {
                   UiThreadUtil.runOnUiThread(
                       DevSupportManagerBase.this::hideSplitBundleDevLoadingView);
@@ -807,7 +802,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
               },
               bundleFile,
               bundleUrl,
-              // NULLSAFE_FIXME[Parameter Not Nullable]
               null);
         });
   }
@@ -836,7 +830,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
   }
 
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public @Nullable File downloadBundleResourceFromUrlSync(
       final String resourceURL, final File outputFile) {
     return mDevServerHelper.downloadBundleResourceFromUrlSync(resourceURL, outputFile);
@@ -904,7 +897,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
           }
 
           @Override
-          // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
           public void onFailure(final Exception cause) {
             hideDevLoadingView();
             if (mBundleDownloadListener != null) {
@@ -991,10 +983,16 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
 
       // start shake gesture detector
       if (!mIsShakeDetectorStarted) {
-        mShakeDetector.start(
-            // NULLSAFE_FIXME[Parameter Not Nullable]
-            (SensorManager) mApplicationContext.getSystemService(Context.SENSOR_SERVICE));
-        mIsShakeDetectorStarted = true;
+        SensorManager sensorManager =
+            (SensorManager) mApplicationContext.getSystemService(Context.SENSOR_SERVICE);
+        if (sensorManager != null) {
+          mShakeDetector.start(sensorManager);
+          mIsShakeDetectorStarted = true;
+        } else {
+          FLog.w(
+              ReactConstants.TAG,
+              "Couldn't register shake gesture detector, sensor service is null");
+        }
       }
 
       // register reload app broadcast receiver
@@ -1078,7 +1076,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
   }
 
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public void setPackagerLocationCustomizer(
       DevSupportManager.PackagerLocationCustomizer packagerLocationCustomizer) {
     mPackagerLocationCustomizer = packagerLocationCustomizer;
@@ -1090,7 +1087,6 @@ public abstract class DevSupportManagerBase implements DevSupportManager {
   }
 
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public @Nullable SurfaceDelegate createSurfaceDelegate(String moduleName) {
     if (mSurfaceDelegateFactory == null) {
       return null;

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/ReleaseDevSupportManager.kt

@@ -36,11 +36,11 @@ public open class ReleaseDevSupportManager : DevSupportManager {
 
   private val defaultJSExceptionHandler: DefaultJSExceptionHandler = DefaultJSExceptionHandler()
 
-  public override fun showNewJavaError(message: String?, e: Throwable?): Unit = Unit
+  public override fun showNewJavaError(message: String?, e: Throwable): Unit = Unit
 
   public override fun addCustomDevOption(
-      optionName: String?,
-      optionHandler: DevOptionHandler?
+      optionName: String,
+      optionHandler: DevOptionHandler
   ): Unit = Unit
 
   public override fun showNewJSError(
@@ -49,7 +49,7 @@ public open class ReleaseDevSupportManager : DevSupportManager {
       errorCookie: Int
   ): Unit = Unit
 
-  public override fun createRootView(appKey: String?): View? = null
+  public override fun createRootView(appKey: String): View? = null
 
   public override fun destroyRootView(rootView: View?): Unit = Unit
 
@@ -111,7 +111,7 @@ public open class ReleaseDevSupportManager : DevSupportManager {
 
   public override fun downloadBundleResourceFromUrlSync(
       resourceURL: String,
-      outputFile: File?
+      outputFile: File
   ): File? = null
 
   public override val lastErrorTitle: String?
@@ -125,14 +125,14 @@ public open class ReleaseDevSupportManager : DevSupportManager {
 
   public override val lastErrorCookie: Int = 0
 
-  public override fun registerErrorCustomizer(errorCustomizer: ErrorCustomizer?): Unit = Unit
+  public override fun registerErrorCustomizer(errorCustomizer: ErrorCustomizer): Unit = Unit
 
   public override fun processErrorCustomizers(
-      errorInfo: Pair<String, Array<StackFrame>>?
-  ): Pair<String, Array<StackFrame>>? = errorInfo
+      errorInfo: Pair<String, Array<StackFrame>>
+  ): Pair<String, Array<StackFrame>> = errorInfo
 
   public override fun setPackagerLocationCustomizer(
-      packagerLocationCustomizer: PackagerLocationCustomizer?
+      packagerLocationCustomizer: PackagerLocationCustomizer
   ): Unit = Unit
 
   public override fun handleException(e: Exception) {
@@ -145,7 +145,7 @@ public open class ReleaseDevSupportManager : DevSupportManager {
   public override val currentReactContext: ReactContext?
     get() = null
 
-  public override fun createSurfaceDelegate(moduleName: String?): SurfaceDelegate? = null
+  public override fun createSurfaceDelegate(moduleName: String): SurfaceDelegate? = null
 
   public override fun openDebugger(): Unit = Unit
 

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevBundleDownloadListener.kt

@@ -12,5 +12,5 @@ public interface DevBundleDownloadListener {
 
   public fun onProgress(status: String?, done: Int?, total: Int?)
 
-  public fun onFailure(cause: Exception?)
+  public fun onFailure(cause: Exception)
 }

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/devsupport/interfaces/DevSupportManager.kt

@@ -38,11 +38,11 @@ public interface DevSupportManager : JSExceptionHandler {
 
   public var devSupportEnabled: Boolean
 
-  public fun showNewJavaError(message: String?, e: Throwable?)
+  public fun showNewJavaError(message: String?, e: Throwable)
 
-  public fun addCustomDevOption(optionName: String?, optionHandler: DevOptionHandler?)
+  public fun addCustomDevOption(optionName: String, optionHandler: DevOptionHandler)
 
-  public fun createRootView(appKey: String?): View?
+  public fun createRootView(appKey: String): View?
 
   public fun destroyRootView(rootView: View?)
 
@@ -78,15 +78,15 @@ public interface DevSupportManager : JSExceptionHandler {
 
   public fun toggleElementInspector()
 
-  public fun downloadBundleResourceFromUrlSync(resourceURL: String, outputFile: File?): File?
+  public fun downloadBundleResourceFromUrlSync(resourceURL: String, outputFile: File): File?
 
-  public fun registerErrorCustomizer(errorCustomizer: ErrorCustomizer?)
+  public fun registerErrorCustomizer(errorCustomizer: ErrorCustomizer)
 
   public fun processErrorCustomizers(
-      errorInfo: Pair<String, Array<StackFrame>>?
-  ): Pair<String, Array<StackFrame>>?
+      errorInfo: Pair<String, Array<StackFrame>>
+  ): Pair<String, Array<StackFrame>>
 
-  public fun setPackagerLocationCustomizer(packagerLocationCustomizer: PackagerLocationCustomizer?)
+  public fun setPackagerLocationCustomizer(packagerLocationCustomizer: PackagerLocationCustomizer)
 
   /**
    * Create the surface delegate that the provided module should use to interact with
@@ -94,7 +94,7 @@ public interface DevSupportManager : JSExceptionHandler {
    * @param moduleName the module name that helps decide which surface it should interact with
    * @return a [SurfaceDelegate] instance
    */
-  public fun createSurfaceDelegate(moduleName: String?): SurfaceDelegate?
+  public fun createSurfaceDelegate(moduleName: String): SurfaceDelegate?
 
   /** Attempt to open the JS debugger on the host machine. */
   public fun openDebugger()