Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[android][v10] Add guard to prevent NRE on map.getStyle().styleLayers #3595

Conversation

FrederickEngelhardt
Copy link
Contributor

@FrederickEngelhardt FrederickEngelhardt commented Aug 25, 2024

Description

Fixes a crash (I believe it's unreported) that was blocking a google review with sdk 10.1.28 due to the devices running the aab getting this NRE.

Crash Info

Exception java.lang.NullPointerException:
  at com.rnmapbox.rnmbx.components.mapview.RNMBXMapView.setSourceVisibility (RNMBXMapView.kt:1133)
  at com.rnmapbox.rnmbx.components.mapview.NativeMapViewModule$setSourceVisibility$1.invoke (NativeMapViewModule.kt:66)
  at com.rnmapbox.rnmbx.components.mapview.NativeMapViewModule$setSourceVisibility$1.invoke (NativeMapViewModule.kt:65)
  at com.rnmapbox.rnmbx.utils.ViewTagResolver.withViewResolved$lambda$4 (ViewTagResolver.kt:64)
  at android.os.Handler.handleCallback (Handler.java:938)
  at android.os.Handler.dispatchMessage (Handler.java:99)
  at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage (MessageQueueThreadHandler.java:27)
  at android.os.Looper.loop (Looper.java:223)
  at android.app.ActivityThread.main (ActivityThread.java:7664)
  at java.lang.reflect.Method.invoke
  at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:592)
  at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:947)

Steps to reproduce.

Be on react-native 0.74.3. Build a AAR. Have the mapbox-gl map launch immediately after location permissions are given. The map should not be static, but be reliant on some tiles that could take a bit of time to render.

About the project

  1. This is a map app with two maps overlayed over each-other.
  2. First map is is absolutely positioned underneath the other and both are synced.
  3. The map loads almost immediately after the onboarding screens, and the data all should exist, but there might be a brief moment or race condition which causes the maps to get that null reference.

Cannot replicate on my devices

  • Note 20 ultra
  • Z Fold 3, Z Fold 6
  • Tablets: Samsung Tab S9+.
  • Notably these are ALL running on operating systems flavors of samsung, so it might be an issue with Pixels, and Motorola devices which I believe use stock android. Also all of these devices are high performant, even the note 20 ultra is likely 30-40% faster than these reported devices.

Reported devices

  • Motorola G20 and a Pixel5 both on SDK 30.

Locally with an emulator I cannot replicate the issue. Maybe network, ram, and cpu throttling would do the trick.

Screenshot 2024-08-24 at 10 07 47 PM

Checklist

  • I've read CONTRIBUTING.md
  • I updated the doc/other generated code with running yarn generate in the root folder
  • I have tested the new feature on /example app.
    • In V11 mode/ios
    • In New Architecture mode/ios
    • In V11 mode/android
    • In New Architecture mode/android
  • I added/updated a sample - if a new feature was implemented (/example)

Screenshot OR Video

Component to reproduce the issue you're fixing

@FrederickEngelhardt FrederickEngelhardt changed the title [android] Add guard to prevent NRE on map.getStyle().styleLayers [android][v10] Add guard to prevent NRE on map.getStyle().styleLayers Aug 25, 2024
Copy link
Contributor

@mfazekas mfazekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks much looks good

…MBXMapView.kt


Use early return instead of optional chaining an iterator.

Co-authored-by: Miklós Fazekas <mfazekas@szemafor.com>
@mfazekas mfazekas merged commit b96cdb9 into rnmapbox:main Aug 27, 2024
11 checks passed
@FrederickEngelhardt FrederickEngelhardt deleted the fix/add-null-guards-for-getstyle-stylelayers branch August 30, 2024 05:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants