1277117 - Move Gecko env var mapping code to IntentUtils.getEnvVarMap

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, defect, P5)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

There is some duplicated code to read environment variables in GeckoLoader & GeckoApp:
  https://dxr.mozilla.org/mozilla-central/search?q=path%3Amobile%2Fandroid+env0&redirect=true

I wrote a utility function in bug 1270191 to get the environment variable methods and pass it into Experiments & TelemetryUploadService. We should move the existing Gecko code to use these methods.

Note there is also some duplication in my resulting code for Experiments & TelemetryUploadService (they set the value, throw if the value was already set, and throw if the value is accessed before they are retrieved, etc.) so it could be worth building a better (but simple! ;) abstraction around this than the map.

There is also some ambiguity about how the environment variables are used (e.g. do we want to check if the key exists in the map? What if the key is the empty string? Should we passing more specific values, e.g. `true` & `false`?) so it could be worth ironing that out too.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

From bug 1277214 comment 8:
> SafeIntentUtils breaks the env var algorithm:
> 
>         while (envValue != null) {
> ...
>             envValue = intent.getStringExtra("env" + i);
> 
> SafeIntentUtils returns null when there is an error – a runtime exception or
> OOM – so we'd early return and miss some of our env vars. We should instead
> be checking containsKey or similar.

bug 1277214 will fix this for IntentUtils so if we use IntentUtils, we should get the improvements. A good quick fix here might be to call out to env var utils for these methods, with the proper fix to only call getEnvVarMap once.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Michael Comella (:mcomella) from comment #1)
> A good quick fix here might be to call out to
> env var utils for these methods, with the proper fix to only call
> getEnvVarMap once.

Slight detour – util/ is not accessible from mozglue/ so we can't call out to IntentUtils in GeckoLoader. I filed bug 1277631 because I think this behavior is incorrect – soft dep. A quick fix is to move IntentUtils to mozglue/ because mozglue seems accessible from everywhere.

Comment 3

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.