Fenix: [Bug]Crash after changing autoplay setting

[AndiAJ]

Steps to reproduce

1. Set the Autoplay setting to "Block audio and video on cellular data only"
2. Open cbssports.com
3. Open settings and set the Autoplay setting to "Block audio and video"
4. Tap 3x the back button and exit to the page view

Expected behavior

The website is properly displayed

Actual behavior

Crash

Device information

• Android device:
  • Samsung Galaxy Tab S6 (Android 9)
  • Google Pixel 3XL (Android 9)
  • OnePlus A3 (Android 6.0.1)

• Fenix version:
  • Beta 81.1.0-beta.2
  • Nightly 200903

Notes

❗ Reproducible on the following websites:
https://videojs.github.io/autoplay-tests/plain/attr/autoplay.html
https://www.walkinto.in/tour/WyKhAFf0iXZJgF2CKzAj7

ff280b8f-c892-4f31-b967-5beacbe58386
java.util.ConcurrentModificationException

• Sentry: https://sentry.prod.mozaws.net/operations/firefox-beta/?query=923dc0b0c28945318f91edfd68b40aaf

* Socorro: https://crash-stats.mozilla.org/report/index/bp-2e021900-961e-4a2b-9589-31c3f0200903

java.util.ConcurrentModificationException
at java.util.LinkedHashMap$LinkedHashIterator.nextEntry(LinkedHashMap.java:346)
at java.util.LinkedHashMap$KeyIterator.next(LinkedHashMap.java:366)
at mozilla.components.support.base.observer.ObserverRegistry.notifyObservers(ObserverRegistry.kt:2)
at mozilla.components.support.base.observer.ObserverRegistry.notifyAtLeastOneObserver(ObserverRegistry.kt:3)
at org.mozilla.fenix.wifi.WifiConnectionMonitor.notifyAtLeastOneObserver(WifiConnectionMonitor.kt)
at org.mozilla.fenix.wifi.WifiConnectionMonitor$frameworkListener$1.onAvailable(WifiConnectionMonitor.kt:1)
at android.net.ConnectivityManager$CallbackHandler.handleMessage(ConnectivityManager.java:2345)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:152)
at android.os.HandlerThread.run(HandlerThread.java:61)

► Video

[pocmo]

java.util.ConcurrentModificationException

I see that WifiConnectionMonitor is using AC's ObserverRegistry under the hood. ObserverRegistry has protection against concurrent modifications of the list of observers from multiple threads. The only way this can still happen is if an observer, when getting notified, is also calling into the ObserverRegistry to add/remove observers (maybe remove itself).

Ideally we avoid that. But if that is not possible, a workaround we used in AC is to not call into ObserverRegistry from the observer directly, but instead launch a coroutine that will do it. This will automatically prevent the concurrent modification.

Looks like 2d066d77ad721eb64db2e82eb00286ed8827e99c (Jul 16) refactored WifiConnectionMonitor to use ObserverRegistry and previously it used its own callback list. To be honest, since we are probably eventually trying to nuke ObserverRegistry from AC (switching to Store implementations only), it may be the best to just reverse this change.

[sv-ohorvath]

Still reproducing on Nightly 9/10 with a HTC Desire 820 (Android 6.0.1)

[ekager]

@sv-ohorvath is it a different stack trace on that device now?

[Diana-Rus]

Hi, reproduced the issue with Sony Xperia Z5 (Android 7) on Firefox Beta 81.1.1-beta.3

Crash link: https://crash-stats.mozilla.org/report/index/6f53f42a-5763-4ccf-80a6-409560200911

Stack trace is slightly a bit different

java.util.ConcurrentModificationException
at java.util.LinkedHashMap$LinkedHashIterator.nextEntry(LinkedHashMap.java:411)
at java.util.LinkedHashMap$KeyIterator.next(LinkedHashMap.java:422)
at org.mozilla.fenix.wifi.WifiConnectionMonitor$frameworkListener$1.onAvailable(WifiConnectionMonitor.kt:2)
at android.net.ConnectivityManager$CallbackHandler.handleMessage(ConnectivityManager.java:2716)
at android.os.Handler.dispatchMessage(Handler.java:102)
at android.os.Looper.loop(Looper.java:241)
at android.os.HandlerThread.run(HandlerThread.java:61)

[abodea]

Still reproducible on Beta 82.0.0-beta1 with Google Pixel 4 XL (Android 11).

[eliserichards]

I am not able to reproduce this crash on my Pixel 3, but it looks like the "blocking" settings aren't actually working -- I still see a best buy ad at the top of the site that is playing.

@abodea could you try it again with your Pixel 4 XL and see if you're getting the same result?

[AndiAJ]

Hi @eliserichards had a quick chat with @abodea and told me that he could reproduce the crash. ☺️
On the other hand, I've managed to reproduce the crash on the latest Nightly 201015 build using a Google Pixel 3a (Android 11) on https://www.walkinto.in/tour/WyKhAFf0iXZJgF2CKzAj7

It doesn't happen quite often, in my case, crashed 2x in 20 attempts

ee44f5ee-710f-4441-8f04-501b94fa7958
java.util.ConcurrentModificationException

• Sentry: https://sentry.prod.mozaws.net/operations/fenix/?query=eb5f0da2d5fd4ea988700851f53ba83b

* Socorro: https://crash-stats.mozilla.org/report/index/bp-e07222aa-2388-4da4-bd99-bc7470201015

java.util.ConcurrentModificationException
at java.util.LinkedHashMap$LinkedHashIterator.nextNode(LinkedHashMap.java:757)
at java.util.LinkedHashMap$LinkedKeyIterator.next(LinkedHashMap.java:780)
at org.mozilla.fenix.wifi.WifiConnectionMonitor$frameworkListener$1.onAvailable(WifiConnectionMonitor.kt:2)
at android.net.ConnectivityManager$NetworkCallback.onAvailable(ConnectivityManager.java:3322)
at android.net.ConnectivityManager$CallbackHandler.handleMessage(ConnectivityManager.java:3607)
at android.os.Handler.dispatchMessage(Handler.java:106)
at android.os.Looper.loop(Looper.java:223)
at android.os.HandlerThread.run(HandlerThread.java:67)

[eliserichards]

Thank you so much @AndiAJ! It looks there's a low number of users that this is impacting, so I'm going to file a followup issue (#15934) and we will triage & prioritize from there :)