Android: Fix crash when WindowInsets is null on ReactRootView #32989

enahum · 2022-01-28T13:53:24Z

Summary

Fixes a potential crash was introduced by #30919 that aimed to get the keyboard height on devices with a Notch. The problem is that it considers that any ReactRootView will have an insets available.

When using react-native-navigation and assigning a Navigation button to the TopBar as a component, the component gets registered as a RootView but won't have any insets attach to the view.

getRootWindowInsets() in fact return a WindowInset only available if the view is attached, so when executing checkForKeyboardEvents method from ReactRootView, is trying to access the DisplayCutout of a null object, leading to a crash.

Changelog

[Android] [Fixed] - Fix potential crash if ReactRootView does not have insets attached.

Test Plan

Without the code change: Notice how the second screen being push contains a React Component on the top right of the navigation bar, and when component is unmounted (going back) the app crashes.

rn67-crash.1.mp4

crash log

2022-01-28 10:27:52.902 15600-15600/com.mattermost.rnbeta E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.mattermost.rnbeta, PID: 15600
    java.lang.NullPointerException: Attempt to invoke virtual method 'android.view.DisplayCutout android.view.WindowInsets.getDisplayCutout()' on a null object reference
        at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.checkForKeyboardEvents(ReactRootView.java:778)
        at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.onGlobalLayout(ReactRootView.java:769)
        at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061)
        at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3214)
        at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2143)
        at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8665)
        at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1037)
        at android.view.Choreographer.doCallbacks(Choreographer.java:845)
        at android.view.Choreographer.doFrame(Choreographer.java:780)
        at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1022)
        at android.os.Handler.handleCallback(Handler.java:938)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loopOnce(Looper.java:201)
        at android.os.Looper.loop(Looper.java:288)
        at android.app.ActivityThread.main(ActivityThread.java:7839)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)

After applying the patch which is only a null check validation and does not change any previous behavior

rn67-fix.1.mp4

…oid ReactRootView checkForKeyboardEvents method

analysis-bot · 2022-01-28T14:28:32Z

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 384e1a0
Branch: main

cortinico

Change looks sane to me. Also from the documentation:
https://developer.android.com/reference/android/view/View#getRootWindowInsets()

WindowInsets from the top of the view hierarchy or null if View is detached

cortinico · 2022-01-28T14:28:07Z

ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java

        }
+        }


Indentation is odd here

indentation fixed ;)

cortinico · 2022-01-28T14:28:26Z

ReactAndroid/src/main/java/com/facebook/react/ReactRootView.java

@@ -826,10 +827,13 @@ private void checkForKeyboardEvents() {
      getRootView().getWindowVisibleDisplayFrame(mVisibleViewArea);
      int notchHeight = 0;
      if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
-        DisplayCutout displayCutout = getRootView().getRootWindowInsets().getDisplayCutout();
+        WindowInsets insets = getRootView().getRootWindowInsets();
+        if (insets != null) {


nit: That's why we need Kotlin :)

...and million other reasons too 😅

analysis-bot · 2022-01-28T14:30:28Z

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,297,729	-2,673
android	hermes	armeabi-v7a	7,634,842	-4,100
android	hermes	x86	8,771,019	-4,829
android	hermes	x86_64	8,708,089	-4,668
android	jsc	arm64-v8a	9,783,495	-1,340
android	jsc	armeabi-v7a	8,768,801	-1,920
android	jsc	x86	9,749,365	-2,095
android	jsc	x86_64	10,345,314	-2,313

Base commit: 384e1a0
Branch: main

facebook-github-bot · 2022-01-28T14:51:42Z

@ShikaSD has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

react-native-bot · 2022-01-31T16:00:04Z

This pull request was successfully merged by @enahum in 6239e2f.

^{When will my fix make it into a release? | Upcoming Releases}

Summary: Fixes a potential crash was introduced by #30919 that aimed to get the keyboard height on devices with a Notch. The problem is that it considers that any ReactRootView will have an insets available. When using [react-native-navigation](https://github.com/wix/react-native-navigation) and assigning a Navigation button to the TopBar as a component, the component gets registered as a RootView but won't have any insets attach to the view. [getRootWindowInsets()](https://developer.android.com/reference/android/view/View#getRootWindowInsets()) in fact return a `WindowInset` only available if the view is attached, so when executing `checkForKeyboardEvents` method from ReactRootView, is trying to access the `DisplayCutout` of a null object, leading to a crash. ## Changelog [Android] [Fixed] - Fix potential crash if ReactRootView does not have insets attached. Pull Request resolved: #32989 Test Plan: Without the code change: Notice how the second screen being push contains a React Component on the top right of the navigation bar, and when component is unmounted (going back) the app crashes. https://user-images.githubusercontent.com/6757047/151558235-39b9a8b5-be73-4c31-8053-02ce188637b8.mp4 crash log ``` 2022-01-28 10:27:52.902 15600-15600/com.mattermost.rnbeta E/AndroidRuntime: FATAL EXCEPTION: main Process: com.mattermost.rnbeta, PID: 15600 java.lang.NullPointerException: Attempt to invoke virtual method 'android.view.DisplayCutout android.view.WindowInsets.getDisplayCutout()' on a null object reference at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.checkForKeyboardEvents(ReactRootView.java:778) at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.onGlobalLayout(ReactRootView.java:769) at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061) at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3214) at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2143) at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8665) at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1037) at android.view.Choreographer.doCallbacks(Choreographer.java:845) at android.view.Choreographer.doFrame(Choreographer.java:780) at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1022) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:201) at android.os.Looper.loop(Looper.java:288) at android.app.ActivityThread.main(ActivityThread.java:7839) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003) ``` After applying the patch which is only a null check validation and does not change any previous behavior https://user-images.githubusercontent.com/6757047/151558429-9ff1a608-abb6-4168-8db9-df0c3c71d79e.mp4 Reviewed By: cortinico Differential Revision: D33844955 Pulled By: ShikaSD fbshipit-source-id: ed5579ad3afeed009c61cc1851eee45c70087cf5

…ok#32989) Summary: Fixes a potential crash was introduced by facebook#30919 that aimed to get the keyboard height on devices with a Notch. The problem is that it considers that any ReactRootView will have an insets available. When using [react-native-navigation](https://github.com/wix/react-native-navigation) and assigning a Navigation button to the TopBar as a component, the component gets registered as a RootView but won't have any insets attach to the view. [getRootWindowInsets()](https://developer.android.com/reference/android/view/View#getRootWindowInsets()) in fact return a `WindowInset` only available if the view is attached, so when executing `checkForKeyboardEvents` method from ReactRootView, is trying to access the `DisplayCutout` of a null object, leading to a crash. ## Changelog [Android] [Fixed] - Fix potential crash if ReactRootView does not have insets attached. Pull Request resolved: facebook#32989 Test Plan: Without the code change: Notice how the second screen being push contains a React Component on the top right of the navigation bar, and when component is unmounted (going back) the app crashes. https://user-images.githubusercontent.com/6757047/151558235-39b9a8b5-be73-4c31-8053-02ce188637b8.mp4 crash log ``` 2022-01-28 10:27:52.902 15600-15600/com.mattermost.rnbeta E/AndroidRuntime: FATAL EXCEPTION: main Process: com.mattermost.rnbeta, PID: 15600 java.lang.NullPointerException: Attempt to invoke virtual method 'android.view.DisplayCutout android.view.WindowInsets.getDisplayCutout()' on a null object reference at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.checkForKeyboardEvents(ReactRootView.java:778) at com.facebook.react.ReactRootView$CustomGlobalLayoutListener.onGlobalLayout(ReactRootView.java:769) at android.view.ViewTreeObserver.dispatchOnGlobalLayout(ViewTreeObserver.java:1061) at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:3214) at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:2143) at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:8665) at android.view.Choreographer$CallbackRecord.run(Choreographer.java:1037) at android.view.Choreographer.doCallbacks(Choreographer.java:845) at android.view.Choreographer.doFrame(Choreographer.java:780) at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:1022) at android.os.Handler.handleCallback(Handler.java:938) at android.os.Handler.dispatchMessage(Handler.java:99) at android.os.Looper.loopOnce(Looper.java:201) at android.os.Looper.loop(Looper.java:288) at android.app.ActivityThread.main(ActivityThread.java:7839) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003) ``` After applying the patch which is only a null check validation and does not change any previous behavior https://user-images.githubusercontent.com/6757047/151558429-9ff1a608-abb6-4168-8db9-df0c3c71d79e.mp4 Reviewed By: cortinico Differential Revision: D33844955 Pulled By: ShikaSD fbshipit-source-id: ed5579ad3afeed009c61cc1851eee45c70087cf5

check if WindowInsets is null before calling getDisplayCutout on Andr…

832dd57

…oid ReactRootView checkForKeyboardEvents method

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jan 28, 2022

react-native-bot added Bug Platform: Android Android applications. labels Jan 28, 2022

kelset requested review from ShikaSD and cortinico January 28, 2022 14:23

cortinico approved these changes Jan 28, 2022

View reviewed changes

ShikaSD approved these changes Jan 28, 2022

View reviewed changes

Fix indentation

52a31e7

zzorba mentioned this pull request Jan 31, 2022

Crash when using TopBar ButtonComponents + BottomTabs + Pop wix/react-native-navigation#7435

Closed

facebook-github-bot closed this in 6239e2f Jan 31, 2022

react-native-bot added the Merged This PR has been merged. label Jan 31, 2022

snyk-bot mentioned this pull request Feb 22, 2022

[Snyk] Upgrade react-native from 0.63.3 to 0.67.2 NOUIY/react-native-macos#12

Open

This was referenced Nov 14, 2022

i18n fallback testing discord/react-native#30

Closed

Alter font loading code to support fallback fonts in fontFamily discord/react-native#31

Merged

stevenpetryk mentioned this pull request Nov 21, 2022

Remove RN Android's bizarre font weight extension pattern discord/react-native#32

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android: Fix crash when WindowInsets is null on ReactRootView #32989

Android: Fix crash when WindowInsets is null on ReactRootView #32989

enahum commented Jan 28, 2022

analysis-bot commented Jan 28, 2022 •

edited

Loading

cortinico left a comment

cortinico Jan 28, 2022

enahum Jan 28, 2022

cortinico Jan 28, 2022

ShikaSD Jan 28, 2022

analysis-bot commented Jan 28, 2022 •

edited

Loading

facebook-github-bot commented Jan 28, 2022

react-native-bot commented Jan 31, 2022

Android: Fix crash when WindowInsets is null on ReactRootView #32989

Android: Fix crash when WindowInsets is null on ReactRootView #32989

Conversation

enahum commented Jan 28, 2022

Summary

Changelog

Test Plan

analysis-bot commented Jan 28, 2022 • edited Loading

cortinico left a comment

Choose a reason for hiding this comment

cortinico Jan 28, 2022

Choose a reason for hiding this comment

enahum Jan 28, 2022

Choose a reason for hiding this comment

cortinico Jan 28, 2022

Choose a reason for hiding this comment

ShikaSD Jan 28, 2022

Choose a reason for hiding this comment

analysis-bot commented Jan 28, 2022 • edited Loading

facebook-github-bot commented Jan 28, 2022

react-native-bot commented Jan 31, 2022

analysis-bot commented Jan 28, 2022 •

edited

Loading

analysis-bot commented Jan 28, 2022 •

edited

Loading