Closed Bug 1369107 Opened 3 years ago Closed 3 years ago

Remove GeckoAppShell.ContextGetter

Categories

(GeckoView :: General, enhancement)

50 Branch
Unspecified
Android
enhancement
Not set

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jchen, Assigned: jchen)

References

(Depends on 1 open bug)

Details

Attachments

(4 files)

ContextGetter doesn't play well with GeckoView.
Depends on: 1369108
Replace usages of GeckoAppShell.getContext() with
getApplicationContext() if possible, or the current Activity if the
usage expects an Activity context.
Attachment #8873483 - Flags: review?(droeh)
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
Attachment #8873484 - Flags: review?(esawin)
Use getApplicationContext or GeckoApp.getPluginContext instead of
getContext so we can get rid of ContextGetter.
Attachment #8873485 - Flags: review?(snorp)
Remove ContextGetter and all implementations now that it's no longer
being used.
Attachment #8873486 - Flags: review?(snorp)
Attachment #8873485 - Flags: review?(snorp) → review+
Attachment #8873486 - Flags: review?(snorp) → review+
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.
Attachment #8873484 - Flags: review?(esawin) → review+
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.
Attachment #8873483 - Flags: review?(droeh) → review+
Pushed by nchen@mozilla.com:
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
Depends on: 1311813
Product: Firefox for Android → GeckoView
Target Milestone: Firefox 55 → mozilla55
You need to log in before you can comment on or make changes to this bug.