Skip to content

Remove expanded layout for MapboxNavigationNotification#1441

Closed
danesfeder wants to merge 2 commits into
masterfrom
dan-notification
Closed

Remove expanded layout for MapboxNavigationNotification#1441
danesfeder wants to merge 2 commits into
masterfrom
dan-notification

Conversation

@danesfeder
Copy link
Copy Markdown
Contributor

Closes #1188

This exception seems to result from updating both expanded / normal content views in the Notification - we has seen this a while back in #355 and was supposedly fixed in #564.

Once #1059 was merged, we began seeing #1188 again, so my hunch is it's somehow related. Noting that this is reproducible after running navigation for a long time. Something within the notification framework is not being cleared properly, causing a build up of some sort. On our side, from what I can see we are doing everything we can to ensure as little updates as possible (checking that we actually need to update, rather than blindly updating the notification).

@danesfeder danesfeder added bug Defect to be fixed. ✓ ready for review labels Oct 19, 2018
@danesfeder danesfeder added this to the 0.22.0 milestone Oct 19, 2018
@danesfeder danesfeder self-assigned this Oct 19, 2018
@Guardiola31337
Copy link
Copy Markdown
Contributor

#1188 could happen if:

  • we're passing too much information through the Intents
  • we're storing a bunch of data (especially images) in the custom views (RemoteViews) used in the Notification

There's a max of what can be added into a Bundle so we need to make sure that we recreate (clean) the builder up and only notify with the new changes.

Something within the notification framework is not being cleared properly, causing a build up of some sort.

Yeah that's the issue. In our case, updating two views (collapsed / expanded) seems to be two fold i.e. too much causing the exception (that's why removing the expanded layout works). Although it seems it's an Android issue we could workaround it 👀

https://stackoverflow.com/questions/45087779/android-notification-manager-transactiontoolargeexception

I solved it for 99% for my users by 2 steps: 1. Made sure when I call notify I change only what needed. I removed all the unnecessary remoteViews.setTextViewText(R.id.notification_text, data) that I had. 2. I called the setupNotification() function each time it made scene. looks like it reseted transaction size and allowed me to call more notify() afterwards

A similar use case as ours (they have collapsed and expanded too) TeamNewPipe/NewPipe#799 (comment)

This is solved by simply recreating the notification builder every time it is being updated, see BackgroundPlayer.

We could give it a try and see if implementing ☝️ also reproduces TransactionTooLargeException. If that's the case, we could remove the expanded layout.

@danesfeder
Copy link
Copy Markdown
Contributor Author

@Guardiola31337 re-opening this as we are still seeing the crash with the code in #1455

@danesfeder danesfeder reopened this Oct 29, 2018
@Guardiola31337
Copy link
Copy Markdown
Contributor

@danesfeder unfortunately I was also able to reproduce as well with the code in here 😥

Fatal Exception: java.lang.RuntimeException: android.os.TransactionTooLargeException: data parcel size 1039948 bytes
       at android.app.NotificationManager.notifyAsUser(NotificationManager.java:408)
       at android.app.NotificationManager.notify(NotificationManager.java:370)
       at android.app.NotificationManager.notify(NotificationManager.java:346)
       at com.mapbox.services.android.navigation.v5.navigation.MapboxNavigationNotification.updateNotificationViews(MapboxNavigationNotification.java:152)
       at com.mapbox.services.android.navigation.v5.navigation.MapboxNavigationNotification.updateNotification(MapboxNavigationNotification.java:76)
       at com.mapbox.services.android.navigation.v5.navigation.NavigationNotificationProvider.updateNavigationNotification(NavigationNotificationProvider.java:23)
       at com.mapbox.services.android.navigation.v5.navigation.RouteProcessorThreadListener.onNewRouteProgress(RouteProcessorThreadListener.java:33)
       at com.mapbox.services.android.navigation.v5.navigation.RouteProcessorRunnable$1.run(RouteProcessorRunnable.java:119)
       at android.os.Handler.handleCallback(Handler.java:873)
       at android.os.Handler.dispatchMessage(Handler.java:99)
       at android.os.Looper.loop(Looper.java:193)
       at android.app.ActivityThread.main(ActivityThread.java:6669)
       at java.lang.reflect.Method.invoke(Method.java)
       at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
       at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)

We should keep 🕵️ if it's less frequent though because that'd mean that the issue could be related to the resources we're updating in the notification (especially the images i.e. maneuverResource).

@danesfeder
Copy link
Copy Markdown
Contributor Author

Closing in favor of #1474

@danesfeder danesfeder closed this Oct 29, 2018
@Guardiola31337 Guardiola31337 mentioned this pull request Mar 29, 2020
10 tasks
@etl etl deleted the dan-notification branch April 22, 2021 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Defect to be fixed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MapboxNavigationNotification#updateNotificationViews TransactionTooLargeException

3 participants