Skip to content

Navigation: fix crash when restoring state that has custom Parcelable arrays#5307

Merged
0nko merged 4 commits intodevelopfrom
fix-navigation-crash
Nov 22, 2021
Merged

Navigation: fix crash when restoring state that has custom Parcelable arrays#5307
0nko merged 4 commits intodevelopfrom
fix-navigation-crash

Conversation

@hichamboushaba
Copy link
Copy Markdown
Member

@hichamboushaba hichamboushaba commented Nov 22, 2021

Description

The current version of navigation library generates a function fromSavedStateHandle for the generated NavArgs, but it doesn't handle restoration after a process-death properly if one of the arguments is a custom Parcelable[].
To fix the crash, this PR reverts to using fromBundle temporarily, and I filed the following issue to have Google fix this https://issuetracker.google.com/issues/207315994

Testing instructions

  1. Enable "Don't keep activities" from the developer options.
  2. Open Product Images screen of on the products.
  3. Navigate to another app, then re-open the app.
  4. Confirm that the app doesn't crash.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@hichamboushaba hichamboushaba added the type: crash The worst kind of bug. label Nov 22, 2021
@hichamboushaba hichamboushaba added this to the 8.1 milestone Nov 22, 2021
@0nko 0nko self-assigned this Nov 22, 2021
@peril-woocommerce
Copy link
Copy Markdown

peril-woocommerce bot commented Nov 22, 2021 •
edited
Loading

You can test the changes on this Pull Request by downloading the APK here.

@hichamboushaba
Copy link
Copy Markdown
Member Author

hichamboushaba commented Nov 22, 2021

@0nko the tests are failing, I'll take a look, sorry for the ping before everything is ready.

0nko
0nko approved these changes Nov 22, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@0nko 0nko left a comment

Choose a reason for hiding this comment

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

The fix works! We can merge once the tests are fixed.

@0nko 0nko enabled auto-merge November 22, 2021 16:31
@hichamboushaba
Copy link
Copy Markdown
Member Author

hichamboushaba commented Nov 22, 2021

Thanks @0nko, I pushed the commit aee3c33 to keep using fromSavedStateHandle when running unit tests, and fallback to fromBundle when running on the device.

@0nko 0nko merged commit ced9625 into develop Nov 22, 2021
@0nko 0nko deleted the fix-navigation-crash branch November 22, 2021 16:41
@0nko 0nko mentioned this pull request Nov 22, 2021
Closed
@hichamboushaba hichamboushaba mentioned this pull request Dec 24, 2021
Closed
1 task
@hichamboushaba hichamboushaba mentioned this pull request Mar 25, 2022
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@anitaa1990 anitaa1990 Awaiting requested review from anitaa1990

1 more reviewer

@0nko 0nko 0nko approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@0nko 0nko

Labels

type: crash The worst kind of bug.

Projects

None yet

Milestone

8.1

Development

Successfully merging this pull request may close these issues.

2 participants

@hichamboushaba @0nko