Fix Nullsafe FIXMEs for DevSupportManagerBase.java (#50068)

Summary:

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

Author: GijsWeterings

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

@@ -327,15 +327,15 @@ public void downloadBundleFromURL(
       DevBundleDownloadListener callback,
       File outputFile,
       String bundleURL,
-      BundleDownloader.BundleInfo bundleInfo) {
+      @Nullable BundleDownloader.BundleInfo bundleInfo) {
     mBundleDownloader.downloadBundleFromURL(callback, outputFile, bundleURL, bundleInfo);
   }
 
   public void downloadBundleFromURL(
       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.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.Map;
 import java.util.Set;
 
+@Nullsafe(Nullsafe.Mode.LOCAL)
 public abstract class DevSupportManagerBase implements DevSupportManager {
 
   public interface CallbackWithBundleLoader {
@@ -222,16 +224,13 @@ private void logJSException(Exception e) {
 
     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 void showNewJavaError(@Nullable String message, Throwable e) {
    * 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 void registerErrorCustomizer(ErrorCustomizer errorCustomizer) {
   }
 
   @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 void hideRedboxDialog() {
     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 View getView(int position, @Nullable View convertView, ViewGroup parent)
     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 View getView(int position, @Nullable View convertView, ViewGroup parent)
     }
   }
 
-  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 boolean hasUpToDateJSBundleInCache() {
     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 @@ private void resetCurrentContext(@Nullable ReactContext reactContext) {
     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 void onProgress(
                 }
 
                 @Override
-                // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
                 public void onFailure(Exception cause) {
                   UiThreadUtil.runOnUiThread(
                       DevSupportManagerBase.this::hideSplitBundleDevLoadingView);
@@ -807,7 +802,6 @@ public void onFailure(Exception cause) {
               },
               bundleFile,
               bundleUrl,
-              // NULLSAFE_FIXME[Parameter Not Nullable]
               null);
         });
   }
@@ -836,7 +830,6 @@ public void isPackagerRunning(final PackagerStatusCallback callback) {
   }
 
   @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 void onProgress(
           }
 
           @Override
-          // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
           public void onFailure(final Exception cause) {
             hideDevLoadingView();
             if (mBundleDownloadListener != null) {
@@ -991,10 +983,16 @@ private void reload() {
 
       // 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 @@ private static String getReloadAppAction(Context context) {
   }
 
   @Override
-  // NULLSAFE_FIXME[Inconsistent Subclass Parameter Annotation]
   public void setPackagerLocationCustomizer(
       DevSupportManager.PackagerLocationCustomizer packagerLocationCustomizer) {
     mPackagerLocationCustomizer = packagerLocationCustomizer;
@@ -1090,7 +1087,6 @@ public void setPackagerLocationCustomizer(
   }
 
   @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,23 +78,23 @@ 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
    *
    * @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()