1369107 - Remove GeckoAppShell.ContextGetter

Status: RESOLVED FIXED

(GeckoView :: General, enhancement)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

ContextGetter doesn't play well with GeckoView.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: 1. Remove usages of GeckoAppShell.getContext() (v1)

Replace usages of GeckoAppShell.getContext() with
getApplicationContext() if possible, or the current Activity if the
usage expects an Activity context.

Comment 2

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: 2. Ask for app location permission from JS (v1)

For location permission, we first show a prompt for the website, then on
higher Android versions, we show a prompt for the app. For the app
prompt, right now we show that from GeckoView code in GeckoAppShell.
However, if we move the app prompt into ContentPermissionPrompt.js
alongside the website prompt, we can then remove the code in
GeckoAppShell and eliminate this app permission dependency from
GeckoView code.

Right now we ask for the app location permission from GeckoView code.
However, we can move the permission prompt to JS, where we

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: 3. Don't use GeckoAppShell.getContext for plugin code (v1)

Use getApplicationContext or GeckoApp.getPluginContext instead of
getContext so we can get rid of ContextGetter.

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: 4. Remove GeckoAppShell.ContextGetter (v1)

Remove ContextGetter and all implementations now that it's no longer
being used.

Comment 5

[Eugen Sawin [:esawin]]

Comment on attachment 8873484 [details] [diff] [review]
2. Ask for app location permission from JS (v1)

Review of attachment 8873484 [details] [diff] [review]:
-----------------------------------------------------------------

The patch description is cut off, you may want to shorten it.

::: mobile/android/components/ContentPermissionPrompt.js
@@ +91,5 @@
> +      if (perm.type === "geolocation") {
> +        RuntimePermissions.waitForPermissions(
> +          RuntimePermissions.ACCESS_FINE_LOCATION).then((granted) => {
> +            (granted ? request.allow : request.cancel)();
> +          });

return; for consistency instead of if-else.

Comment 6

[Dylan Roeh (:droeh) (he/him)]

Comment on attachment 8873483 [details] [diff] [review]
1. Remove usages of GeckoAppShell.getContext() (v1)

Review of attachment 8873483 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b6337d9fa55e
1. Remove usages of GeckoAppShell.getContext(); r=droeh
https://hg.mozilla.org/integration/mozilla-inbound/rev/685f48a31f21
2. Ask for app location permission from JS; r=esawin
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c6b25ec2b19
3. Don't use GeckoAppShell.getContext for plugin code; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/49051e18e276
4. Remove GeckoAppShell.ContextGetter; r=snorp

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/b6337d9fa55e
https://hg.mozilla.org/mozilla-central/rev/685f48a31f21
https://hg.mozilla.org/mozilla-central/rev/5c6b25ec2b19
https://hg.mozilla.org/mozilla-central/rev/49051e18e276