Fenix: [Bug] Crash when starting Fenix whose all search engines deleted

[hokonch]

Steps to reproduce

1. Add a custom search engine (for example, example.com/?q=%s)
2. Delete search engines except for the custom engine.
3. Switch system language (for example, en -> ja)
4. Restart fenix and you can see local search engines and the custom engine enabled
5. Delete the custom engine
6. Switch back system language (ja -> en)
7. Crash and can't start fenix

Crash report

7647615f-5c59-4224-884a-06b2ae3c75a1
java.lang.IndexOutOfBoundsException: Index: 0, Size: 0

• Sentry: https://sentry.prod.mozaws.net/operations/firefox-nightly/?query=798bbc0fa2984dba9d6944046d58f444

* Socorro: https://crash-stats.mozilla.org/report/index/bp-5dbf983c-f3e9-40f5-a68f-7908d0200908

java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
at java.util.ArrayList.get(ArrayList.java:437)
at mozilla.components.browser.search.SearchEngineManager.getProvidedDefaultSearchEngineAsync(SearchEngineManager.kt:7)
at mozilla.components.browser.search.SearchEngineManager.getDefaultSearchEngineAsync(SearchEngineManager.kt:7)
at mozilla.components.browser.search.SearchEngineManager$getDefaultSearchEngineAsync$1.invokeSuspend(Unknown Source:12)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:3)
at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:21)
at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:1)
at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:11)
`

[Diana-Rus]

Reproducing the crash with Samsung Galaxy S9 (Android 8) on Nightly 9/8 this issue is similar to the crash mentioned here #8406

[Mugurell]

Well.. this is interesting..
I don't think the user should ever get into the situation in which she can delete all search engines.
Or maybe if that's the case we should just re-add a default one because otherwise how would search work anymore?
But what if there is not a default one for the locale?

[hokonch]

For your information, I note the background of my encounter with this crash. Its numbers correspond to the reproducing procedure.

1. Because Fenix 80.1.2 in the ja locale does not provide DuckDuckGo as presets
2. Because DuckDuckGo itself provides !Bang features similar to Fenix's search engine shortcut
3. During app translation I need switch language frequently
4. Skip
5. There is also a preset version of DuckDuckGo in the en locale, so I deleted the custom engine as a duplicate.
6. Same as 3

[TrianguloY]

When you open firefox for the first time you have a set of default search engines for your locale. If the user somehow is able to delete all search engines (which leaves the browser in a inconsistent state) a failsafe should trigger to 'install' the default set of locales again.
Restoring all locales will avoid the issue to choose a default one (the browser should have a non empty set for any locale, so no need to change or choose anything) and although the user manually deleted them, they managed to delete all of them, so it should not come as a surprise.
But if that's a concern perhaps a small toast or a dialog could popup like "all search engine were deleted, defaults restored". This is a failsafe for a very particular sequence of steps anyway.

[apbitner]

Pinging @brampitoyo for search feedback

[brampitoyo]

@Mugurell @TrianguloY @hokonch I think the right approach is to disallow removing search engine, if there’s only one engine remaining on the list.

This means that the last search engine – whatever it may be – cannot ever be removed. And people won’t ever be stuck with “zero” search engine installed.

What do you think?

[cadeyrn]

FWIW Firefox for desktop has the same behaviour as suggested by @brampitoyo - the last remaining search engine cannot be removed.

[TrianguloY]

That's already a feature, you can't delete the last search engine when there is only one. If you check the steps you will see that the bug happens because when changing locales firefox changes the default search engines too.
So a real fix should be something like don't change the search engines when changing locales.

But then maybe in a few weeks another user discovers another unintended behaviour that also allows you to remove all search engines, you fix that, and then a third steps is found, etc. Or what if somehow a user data is corrupted?

The list of search engines is a list, so firefox needs to be prepared to manage it when there is no items there, even if that's 'normally' impossible.

That's why I mentioned the failsafe, with the (in my opinion) better approach to simply restore the default engines as if 'something bad happened, let me restore you the default list'

[hokonch]

another unintended behaviour that also allows you to remove all search engines

Upgrade Firefox 80 whose all engines deleted except for Twitter

[Mugurell]

Thank you all for chiming in!
Based on the feedback here looks like we should have a fallback in adding back
the default engine / first available for that locale
if otherwise the list of engines would be empty.

[baldurmen]

FWIW, I've just hit this in Fennec 81.1.3 (it wasn't a problem in 81.1.1) and although I did remove all the pre-configured search engines, I still have a custom one.

foobar:/data/data/org.mozilla.fennec_fdroid/shared_prefs # cat fenix-search-engine-provider.xml                                                                                                                                          
<?xml version='1.0' encoding='utf-8' standalone='yes' ?>
<map>
    <set name="fenix-installed-search-engines-en-CA-CA-fallback">
        <string>DuckDuckGo Lite</string>
    </set>
</map>

foobar:/data/data/org.mozilla.fennec_fdroid/shared_prefs # cat custom-search-engines.xml                                                                                                                                                 
<?xml version='1.0' encoding='utf-8' standalone='yes' ?>
<map>
    <set name="pref_custom_search_engines">
        <string>DuckDuckGo Lite</string>
    </set>
    <string name="DuckDuckGo Lite">&lt;?xml version=&quot;1.0&quot; encoding=&quot;UTF-8&quot;?&gt;&lt;OpenSearchDescription xmlns=&quot;http://a9.com/-/spec/opensearch/1.1/&quot;&gt;&lt;ShortName&gt;DuckDuckGo Lite&lt;/ShortName&gt;&lt;Image width=&quot;16&quot; height=&quot;16&quot;&gt;data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGAAAABgCAYAAADimHc4AAAAAXNSR0IArs4c6QAAAARzQklUCAgI&#10;CHwIZIgAAAGaSURBVHic7dw/TgJBGEDxb/wTjcSahsTEyoorGEoqOuMBtvAYVl7Bxk5L7Cg5gIkJ&#10;xcTSgppwBy2ElWCz0V1eNr5ftUO2+DKP2Wohxbb+w8ePz1SfXKTN5ffCjd+tVYivAG4+Ixdpj57h&#10;v0t++1meAJgBYAaAGQBmAJgBYAaAGQBmAJgBYAaAGQBmAJgBYAaAGQBmAJgBYAaAGQBmAJgBYAaA&#10;GQBmAJgBYAaAGQBmANgBPUAVpyeH8Xh3Wa6fJu8xns65gWrUigDHR/sxGpyV69e3JThNvXwEwQwA&#10;MwDMADADwAwAMwDMADADwAwAMwDMADADwAwAMwDMADADwAwAMwDMADADwFrxVsS2m6uLuB6eV77/&#10;eTqP2/tZgxP9XisD9Lqd6HU7le9/yYsGp/kbH0Ew/zcU5gmAGQBmAJgBYAaAGQBmAJgBYAaAGQBm&#10;AJgBYAaAGQBmAJgBYAaAGQBmAJgBYAaAGQBmAJgBYAaAGQBmAJgBYAaAGQBmAFiKiPA3ApBcJE8A&#10;LJVXnoLdykWK2AywZohmrTZ+7RNY+Sh2/K4PjAAAAABJRU5ErkJggg==&#10;&lt;/Image&gt;&lt;Description&gt;DuckDuckGo Lite&lt;/Description&gt;&lt;Url type=&quot;text/html&quot; template=&quot;https://lite.duckduckgo.com/lite?q={searchTerms}&quot;/&gt;&lt;/OpenSearchDescription&gt;</string>
</map>



md5-7b9a5b1d05ca63ca645ef018eafe2116



10-13 18:19:03.204 E/AndroidRuntime(28255): java.lang.IndexOutOfBoundsException: Index: 0, Size: 0
10-13 18:19:03.204 E/AndroidRuntime(28255):     at java.util.ArrayList.get(ArrayList.java:437)
10-13 18:19:03.204 E/AndroidRuntime(28255):     at mozilla.components.browser.search.SearchEngineManager.getProvidedDefaultSearchEngineAsync(SearchEngineManager.kt:7)
10-13 18:19:03.204 E/AndroidRuntime(28255):     at mozilla.components.browser.search.SearchEngineManager.getDefaultSearchEngineAsync(SearchEngineManager.kt:7)
10-13 18:19:03.204 E/AndroidRuntime(28255):     at mozilla.components.browser.search.SearchEngineManager$getDefaultSearchEngineAsync$1.invokeSuspend(Unknown Source:12)
10-13 18:19:03.204 E/AndroidRuntime(28255):     at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:3)
10-13 18:19:03.204 E/AndroidRuntime(28255):     at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:21)
10-13 18:19:03.204 E/AndroidRuntime(28255):     at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:1)
10-13 18:19:03.204 E/AndroidRuntime(28255):     at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:11)
10-13 18:19:03.208 W/ActivityTaskManager(  669):   Force finishing activity org.mozilla.fennec_fdroid/org.mozilla.fenix.HomeActivitycom.android.systemui.media.NotificationPlayer$CreationAndCompletionThread.run(NotificationPlayer.java:163)

[EchedeyLR]

@baldurmen same situation that you

[Mugurell]

This seems to have been fixed in the latest Nightly.

[softvision-miralobontiu]

Cannot reproduce the crash due to the fact that the user cannot delete all the search engines.
Environment:

• 10/29 Nightly build, RC 82.1.1, and beta 83.0.0-beta.2
• devices: Sony Xperia Z5 Premium (Android 7.1.1), Samsung Galaxy Tab S6 (Android 10).
  I'll remove the qa:needed label and close the issue.

[EchedeyLR]

You just needed to add custom engines and delete all the defaults. In last version is solved the issue though.

[EchedeyLR]

The issue is present until 81.1.5 and solved in the next version.