Fix ProductImageMap ConcurrentModificationException by Safely Managing WeakReferences

[jd-alexander]

Closes: #10901

Description

In addressing the ConcurrentModificationException within the ProductImageMap class, we opted to refine our approach by removing the previously implemented CopyOnWriteArrayList and instead directly tackling the issue with WeakReference objects in the observer list. Thanks to @malinajirka for this exploration.

The revised solution is an improvement to the PR #10902 It involves a two-step modification process: initially, the code iterates over the observer list, identifying any WeakReference objects that have been cleared due to garbage collection and adding them to a temporary list. This method prevents the direct modification of the observer list during iteration, which was the primary cause of the exception.
Following this, the code iterates over the temporary list to safely remove the identified references from the original observer list. By reverting the CopyOnWriteArrayList implementation and focusing on the specific management of WeakReference objects, this solution effectively addresses the root cause of the exception, ensuring the integrity of the observer list without incurring the overhead associated with introducing CopyOnWriteArrayList.

Testing instructions

1. Switch from one store to the next.
2. Open the Orders tab.
3. Notice no crash takes place.

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.

[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	49a6057
	Direct Download	woocommerce-prototype-build-pr10910-49a6057.apk

[malinajirka]

I've reviewed and re-tested the code and LGTM. I've left one minor suggestions.

I think it might be better to merge this after the code-freeze as it's Friday evening, having said that, it's up to you.

P.S. We could potentially also use an iterator which allows us to remove elements during iteration

val observersIterator = observers.iterator()
while (observersIterator.hasNext()) {
    // notify the observer or collect it for removal if it's been garbage collected
    observersIterator.next().get()?.onProductFetched(remoteProductId) 
        ?: observersIterator.remove()
}

Having said that, I'm not convinced it's more readable with the iterator.

[jd-alexander]

I think it might be better to merge this after the code-freeze as it's Friday evening, having said that, it's up to you.

Totally fine with merging after the code freeze.

[codecov-commenter]

Codecov Report

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

Project coverage is 41.27%. Comparing base (672695b) to head (49a6057).
Report is 56 commits behind head on trunk.

Files	Patch %	Lines
...n/com/woocommerce/android/tools/ProductImageMap.kt	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##              trunk   #10910   +/-   ##
=========================================
  Coverage     41.27%   41.27%           
  Complexity     5084     5084           
=========================================
  Files          1034     1034           
  Lines         59556    59556           
  Branches       7989     7989           
=========================================
+ Hits          24582    24583    +1     
+ Misses        32788    32787    -1     
  Partials       2186     2186           

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