Attempt to fix crash on ProductListToolbarHelper

[hichamboushaba]

Closes: #11089 Closes: #11480

Description

This is just an attempt to fix the crash occurring on ProductListToolbarHelper, my theory of the crash is this: "if we check the events in Sentry, we can see that it always occur during a configuration change, and since we post the action to refresh the menu to the Toolbar's handler, there seems to be a race condition where the posting happens, and then a configuration change happens before the callback is executed, which means that when it does, the app's NavHostFragment could be still not attached"

To solve this, I did two changes to the ProductListToolbarHelper:

1. I updated it to observe the View's Lifecycle instead of the Fragment's one, this is needed to ensure onDestroy is called during the View's detachment, and not after.
2. I added logic to remove the menu refresh Runnable from the Toolbar's Handler when the view is destroyed.

Testing instructions

I couldn't reproduce the crash, so it's just code review, and ensuring nothing breaks.

Images/gif

• I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

[hichamboushaba]

@kidinov @jd-alexander I requested your review since you seem to have some past background about this, I'm keeping the PR as draft for now, my goal is just to start a discussion, and see if the theory above makes sense, then discuss if the fix is the appropriate one.

[hichamboushaba]

This change also ensures that we don't leak the fragment's view, we were setting binding to null in the fragment's onDestroy due to tying it to its Lifecycle, but now it'll be executed correctly on a matching callback of onDestroyView.

[wpmobilebot]

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Flavor	Jalapeno
	Build Type	Debug
	Commit	b8867f0
	Direct Download	woocommerce-prototype-build-pr11484-b8867f0.apk

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 40.88%. Comparing base (e734909) to head (53d96e7).

❗ Current head 53d96e7 differs from pull request most recent head b8867f0. Consider uploading reports for the commit b8867f0 to get more accurate results

Files	Patch %	Lines
...droid/ui/products/list/ProductListToolbarHelper.kt	0.00%	6 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #11484      +/-   ##
============================================
+ Coverage     40.84%   40.88%   +0.04%     
  Complexity     5180     5180              
============================================
  Files          1068     1067       -1     
  Lines         62283    62208      -75     
  Branches       8495     8484      -11     
============================================
- Hits          25437    25436       -1     
+ Misses        34559    34486      -73     
+ Partials       2287     2286       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kidinov]

@hichamboushaba Thank you! The potential fix makes sense to me

Have you considered adding a line in the release notes?

[hichamboushaba]

Thanks @kidinov for the review.

Have you considered adding a line in the release notes?

Done.

[sentry-io]

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

• ‼️ IllegalStateException: Fragment NavHostFragment{158d7f8} (e5f27f72-9492-48d8-bd99-77f34de9d016) has not been attached yet. Dashboard View Issue

_{Did you find this useful? React with a 👍 or 👎}