mapbox / mapbox-navigation-android

Mapbox Navigation SDK for Android
https://docs.mapbox.com/android/navigation/overview/
Other
622 stars 319 forks source link

Crash when use navigationview with Hilt #4582

Open peytonwupeixin opened 3 years ago

peytonwupeixin commented 3 years ago

Android API: 28 Mapbox Navigation SDK version: 1.6.1

Steps to trigger behavior

1.We use hilt to inject ViewModel in our navigation fragment 2 .Call NavigationView.onDestroy() in Fragment.onDestroyView() to avoid memory leaks

@AndroidEntryPoint
NaviagationFragment: BaseNaviagationFragment(){

@HiltViewModel
private val viewModel by viewModels<MyViewModel>()

...
 override fun onDestroyView() {
        navigationView.onDestroy()
        super.onDestroyView()
   }
...

}
abstract BaseNaviagationFragment:Fragment(){...}
  1. When we click back key, app crash.
  2. Log as below

image

Expected behavior

Not crash

Actual behavior

App crash.

Reason

1: NaviagationView.onDestory will call isChangingConfigurations

  private boolean isChangingConfigurations() {
    try {
      return ((FragmentActivity) getContext()).isChangingConfigurations();
    } catch (ClassCastException exception) {
      throw new ClassCastException("Please ensure that the provided Context is a valid FragmentActivity");
    }
  }
  1. When we use Hilt to inject something. Hilt will change our fragment's parent class to its generated intermediate class. It will change the context of the LayoutInflater and so the context of its layout and child view It will change our code like below 2.1 NaviagationFragment extends Hilt_NavigationFragment...{} 2.2 Hilt_NavigationFragment extends Fragment...{} 2.3 image

image

cafesilencio commented 3 years ago

@peytonwupeixin Thanks for reporting this. Ultimately the code will need a FragmentActivity reference in order to call the isChangingConfigurations() method. However as you point out the call to getContext() is assuming the returned object will be a FragmentActivity.

In the same class we have a method to better return the appropriate context reference here.

  private FragmentActivity getFragmentActivity() {
    Context context = getContext();
    // unwrap the context if needed, see https://github.com/mapbox/mapbox-navigation-android/issues/2777
    while (!(context instanceof FragmentActivity) && context instanceof ContextWrapper) {
      context = ((ContextWrapper) context).getBaseContext();
    }

    if (context instanceof FragmentActivity) {
      return (FragmentActivity) context;
    } else {
      throw new ClassCastException("Please ensure that the provided Context is a valid FragmentActivity");
    }
  }

This call isn't being used in the code you pointed out unfortunately. As I mentioned even if the code snippet were using the NavigationView::getFragmentActivity() the dependency injection framework you're using would need to be providing an instance of a FragementActivity so the NavigationView can call isChangingConfigurations() so that might be something to confirm in the meantime.

Perhaps as a temporary workaround you could override the Hilt getContext() to use the getFragmentActivity() I pasted above. I'm not sure if that will work for you, it's just an idea.

abhishek1508 commented 2 years ago

Thanks for using the Mapbox Navigation SDK for Android and being a valued customer.

Mapbox will be soon deprecating any support for v0 and v1 versions of the SDK. To facilitate this transition we’re launching a new drop-in UI component into v2, equivalent to the existing NavigationView v1 in its design goals, however with a more modern and customizable API.

We plan to launch this new drop-in UI component as a Developer Preview feature in April, as part of the v2.5 series. Since you are using NavigationView with v1, we’d love to hear your feedback so that we can incorporate it ahead of a GA release.

If you’re interested in having early access to the upcoming drop-in UI for v2 and its documentation, drop a comment on this ticket or send an email to abhishek.kejriwal@mapbox.com

/cc @zugaldia @AhmerKhan1