Fenix: [Bug] Crash when denying app's access to camera, but allowing access to the site

[softvision-miralobontiu]

Steps to reproduce

1. From the device's settings, disable all permissions from Fenix.
2. In Fenix, go to permissions.site and tap on Camera.
3. When Fenix asks permission to allow the camera, deny it.
4. When the permission.site asks for permission, allow it.

Expected behavior

The permission is denied, the camera does not have permission.

Actual behavior

Fenix crashes.
Tried the same scenario on microphone, location, notifications, no crashes occur.

Device information

• Android devices: Samsung Galaxy S8+ (Android 8), Huawei P9 Lite (Android 7), and Samsung Galaxy Tab S6 (Android 9)
• Fenix version: Beta 4.1.0-beta.2, and on Nightly from 3/13
• Not reproducible on Beta 4.0.0-beta.5 build

--------- beginning of crash
03-13 16:04:52.530 5167-5409/? E/AndroidRuntime: FATAL EXCEPTION: VideoCaptureAndroidSurfaceTextureHelper
    Process: org.mozilla.fenix.beta, PID: 5167
    java.lang.SecurityException
        at android.os.BinderProxy.transactNative(Native Method)
        at android.os.BinderProxy.transact(Binder.java:527)
        at android.hardware.ICameraService$Stub$Proxy.connectDevice(ICameraService.java:364)
        at java.lang.reflect.Method.invoke(Native Method)
        at android.hardware.camera2.utils.Decorator.invoke(Decorator.java:80)
        at java.lang.reflect.Proxy.invoke(Proxy.java:393)
        at $Proxy12.connectDevice(Unknown Source)
        at android.hardware.camera2.CameraManager.openCameraDeviceUserAsync(CameraManager.java:357)
        at android.hardware.camera2.CameraManager.openCamera(CameraManager.java:503)
        at org.webrtc.Camera2Session.<init>(Camera2Session.java:46)
        at org.webrtc.Camera2Capturer.createCameraSession(Camera2Capturer.java:2)
        at org.webrtc.CameraCapturer$5.run(CameraCapturer.java:10)
        at android.os.Handler.handleCallback(Handler.java:739)
        at android.os.Handler.dispatchMessage(Handler.java:95)
        at android.os.Looper.loop(Looper.java:152)
        at android.os.HandlerThread.run(HandlerThread.java:61)
03-13 16:04:52.530 5167-5409/? D/AppTracker: App Event: crash

[ebalazs-sv]

This is also reproducible on Beta 4.1.0-beta.2 with Huawei P9 Lite (Android 7.0) and OnePlus A3 (Android 6.0.1) following the steps:

1. Go to permission.site, allow some site permissions.
2. In system settings, go to Apps>Firefox Preview>Permissions and turn off all permissions.
3. Remove Fenix from Recent apps.
4. Launch Fenix by tapping the icon.

Fenix crashes with the same signature:
--------- beginning of crash 03-13 16:04:52.530 5167-5409/? E/AndroidRuntime: FATAL EXCEPTION: VideoCaptureAndroidSurfaceTextureHelper

[pocmo]

Adding "needs:gv" label. Since that stacktrace points to webrtc code without Fenix or AC in the callstack, I assume we cannot effectively prevent that on our side.

[Amejia481]

@pocmo it looks like gv changed the order in which we receive the callbacks for showing permissions prompts.

We have two callbacks onMediaPermissionRequest and onAndroidPermissionsRequest:

• onMediaPermissionRequest : Indicates that a site permissions like location, notification, camera etc is requested. We show the permission dialog and the user could grant or deny.

• onAndroidPermissionsRequest: Indicates that an Android permission is needed for site permissions and it needs to be granted before the site permission can be used. We show the system permission dialog.

It used to call these callbacks in the below order:

First onMediaPermissionRequest is called we show the site permissions dialog

if (allowed) {
    //  `onAndroidPermissionsRequest` was getting called
} else {
    //   `onAndroidPermissionsRequest`wasn't getting called and the site permission wasn't grantted
}

Second onAndroidPermissionsRequest is called we show the system permission dialog.

```
if (allowed) {
// The site permission was granted and the system permission was used
} else {
// The site permission was not granted NOR the system permission was used
}

### Now it is getting called backwards:

**First** `onAndroidPermissionsRequest` is called we show the system permission dialog.

 ```
if (allowed) {
    //  The system permission is granted and onMediaPermissionRequest is called  
} else {
    //  onMediaPermissionRequest is called
}

Second onMediaPermissionRequest is called we show the site permissions dialog

if (allowed) {
    //   The site permission is granted for the site and the system permissions is used
} else {
    //  The site permission is denied for the site and the system permissions is used
}

[Amejia481]

I noticed the change in order is only for mic/ camera. I'm not sure if we should rely on this order and do something similar to what GeckoView example app is doing

[liuche]

Okay - @Amejia481 is this an AC fix then, not a Fenix one? I think we can release 4.1 without this change, and pick it up in 4.2.

[Amejia481]

@liuche I'm still working on a fix, it will be on the AC side.

[Amejia481]

This issue will be addressed on https://github.com/mozilla-mobile/android-components/pull/6301

[Amejia481]

@softvision-miralobontiu would you mind re-testing? :)

[sv-sdeiac]

Hi, I can't reproduce the issue using the latest version of Firefox Preview Nightly 3/26 #20860605) but I can reproduce it using the latest version of Firefox Preview Beta4.2.0-beta.3.

Device:

• Samsung Galaxy Tab S6 (Android 9).
  @Amejia481, this crash should be fixed in the latest beta or will be merged in the next version? Till I have updates, I'll remove the qa:needed flag, thanks.

[Amejia481]

@sv-sdeiac Beta will get this fix, when it gets the latest AC update :)

[softvision-miralobontiu]

This is no longer reproducible on the Beta 4.3.0-beta.1 build, and 4/15 Nightly build with Google Pixel (Android 10), OnePlus 5T (Android 9), HTC 10 (Android 8), Samsung Galaxy Tab A6 (Android 5.1.1).
After denying the camera access, the site no longer asks for permission.