1244722 - Tab Queue issues with "Draw over other apps" disabled

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Teodora Vermesan (:TeoVermesan)]

Steps to reproduce:
1. Go to Settings -> Apps -> Draw over other apps -> Aurora/Nightly and disable the option
2. Open Firefox and go to Settings -> General and tap "Open multiple links" option
3. From the "Open multiple links" dialog tap on Settings (to enable 'Draw over other apps'
4. Enable the option
5. Go back

Expected results:
- "Open multiple links" option is enabled in Settings

Actual results: 
- "Open multiple links" dialog is still displayed
- The option is not enabled

Note;
- I am not able to reproduce this every time 
- Please take a look at the following video: https://www.youtube.com/watch?v=CYJaY_Ldn7Q&feature=youtu.be

Also:
- sometimes with "Draw over other apps" disabled, with a clean Aurora profile, go to Settings -> General and turn on "Open multiple links" option => the option is enabled. No dialog to turn on "Draw over other apps" is shown.
- please take a look at the following video: https://youtu.be/eOsRBy8WLrM

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Teodora Vermesan (:TeoVermesan) from comment #0)
> - sometimes with "Draw over other apps" disabled, with a clean Aurora
> profile, go to Settings -> General and turn on "Open multiple links" option
> => the option is enabled. No dialog to turn on "Draw over other apps" is
> shown.
> - please take a look at the following video: https://youtu.be/eOsRBy8WLrM

I have seen this a couple of times today too. This is really strange. ContextCompat.canDrawOverlays() seems to return true while the toggle is actually off. This almost looks like a bug in the Android system. Actually trying to draw over other apps fails in this situation.
Currently I can't get this in an unbroken state again on my device - even re-install, clear data, reboot does not fix this.

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

This looks a lot like this:
https://code.google.com/p/android/issues/detail?id=198671

If we are hitting this then the return value of canDrawOverlays() is influenced by other Firefox installations on the device (e.g. if Nightly can draw over other apps then we might not show the prompt in Aurora). This needs some more investigation.

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Sebastian Kaspari (:sebastian) from comment #2)
> If we are hitting this then the return value of canDrawOverlays() is
> influenced by other Firefox installations on the device (e.g. if Nightly can
> draw over other apps then we might not show the prompt in Aurora). This
> needs some more investigation.

Yeah, in my case I could "fix" the issue by enabling or disabling overdraw for all installed Firefox versions.

@Teodora: Do you have multiple Firefox versions installed on the device? Do you still see the issue if all Firefox versions are using the same setting?

Comment 4

[Teodora Vermesan (:TeoVermesan)]

I have both Nightly and Aurora installed on this device. 
Scenarios:

1. First both versions have "Draw over other apps" set to "No":
-On Aurora I try to enable "Open multiple links" feature: but the dialog is still displayed and the option is not enabled. "Draw over other apps" is set to "Yes".
-On Nightly works ok. "Draw over other apps" is set to "Yes" also.

Now both versions have "Draw over other apps" set to "Yes". I clear data, set the option to "No" and try the same steps. 
-First tried with Nightly and works ok
-Then tried with Aurora: "Open multiple links" option is enabled and no dialog to turn on "Draw over other apps" is shown. "Draw over other apps" remains set to "No" for Aurora and "Yes" to Nightly.

2. Restart the device and repeat the above steps with the other version first:

Both versions have "Draw over other apps" set to "No":
-On Nightly I try to enable "Open multiple links" feature: but the dialog is still displayed and the option is not enabled. "Draw over other apps" is set to "Yes"
-On Aurora works ok. "Draw over other apps" is set to "Yes", also

Now both versions have "Draw over other apps" set to "Yes". I clear data, set the option to "No" and try the same steps. 
-First tried with Aurora and works ok.
-Then tried with Nightly: "Open multiple links" option is enabled and no dialog to turn on "Draw over other apps" is shown. "Draw over other apps" remains set to "No" for Nightly and "Yes" to Aurora

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

Unfortunately it seems like we won't be able to fix that - at least until the Android team fixes this bug:
https://code.google.com/p/android/issues/detail?id=198671

This is caused by us using "sharedUserId" and then Android mixing up the settings. Even though it seems like we do not need "sharedUserId" anymore, we can't get rid of it because this would revoke access to our own files:
https://code.google.com/p/android/issues/detail?id=1227
https://code.google.com/p/android/issues/detail?id=14074

Android never implemented a "downgrade path" for removing or changing a sharedUserId of an existing application.

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Sebastian Kaspari (:sebastian) from comment #5)
> Unfortunately it seems like we won't be able to fix that - at least until
> the Android team fixes this bug:
> https://code.google.com/p/android/issues/detail?id=198671

There seems to be no movement in this bug at all.

As a last resort we could try to not use Settings.canDrawOverlays() and instead try to add an invisible overlay: If successful -> We can add overlays. Remove it again. If exception: Seems like we do not have permission.

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

I looked at the code again and added a comment to the bug report:
https://code.google.com/p/android/issues/detail?id=198671#c7

This method is obviously broken for apps that use sharedUserId. I implemented the workaround mentioned in comment 5. This seems to work just fine. I'll test some more and then push it to review.

Another option would be to use reflection to access Settings.isCallingPackageAllowedToDrawOverlays() and provide it with the correct package name. But this is prone to break if this non-public API changes.

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r?ahunt

Review commit: https://reviewboard.mozilla.org/r/43193/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/43193/

Comment 9

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8736275 [details]
MozReview Request: Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r?ahunt

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/43193/diff/1-2/

Comment 10

[Andrzej Hunt :ahunt]

Comment on attachment 8736275 [details]
MozReview Request: Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r?ahunt

https://reviewboard.mozilla.org/r/43193/#review40105

Comment 11

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/a70b50e2830f3316d5a1307bb9f8231ca88efc52
Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r=ahunt

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/a70b50e2830f

Comment 13

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8736275 [details]
MozReview Request: Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r?ahunt

We should uplift this. I somehow forgot to request this.

Approval Request Comment

[Feature/regressing bug #]: Runtime permissions - Introduced with Firefox 46.0 - Bug 1212830. Maybe it's too late for 46, but requesting uplift anyways.

[User impact if declined]: If multiple versions of Firefox are installed locally (say Release and Beta) then the prompt for "Draw over other apps" might not be shown when it should or shown even if we already have the permission.

[Describe test coverage new/current, TreeHerder]: Local testing with multiple installations.

[Risks and why]: Low - This has been in Nightly for two weeks now.

[String/UUID change made/needed]: -

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

I think this can wait till 47. We are very close to the 46 release, this is a very particular case (having more than one version installed) and doesn't reproduce every time.

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hi Teodora, could you please verify this bug is fixed as expected on a latest Nightly build? Thanks!

Comment 16

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8736275 [details]
MozReview Request: Bug 1244722 - TabQueueHelper.canDrawOverlays(): Implement workaround for Android bug. r?ahunt

Recent regression, was manually verified by dev, Aurora47+

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5b3bc1699878

Comment 18

[Carsten Book [:Tomcat]]

backed out for bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=2430277&repo=mozilla-aurora

Comment 19

[Teodora Vermesan (:TeoVermesan)]

Based on steps from comment 4:

1. First both versions have "Draw over other apps" set to "No":
-On Aurora: affected
-On Nightly works ok. "Draw over other apps" is set to "Yes". "Tab queue" is enabled. The prompt is dismissed

Now both versions have "Draw over other apps" set to "Yes". I clear data, set the option to "No" and try the same steps. 
-First tried with Nightly and works ok. "Draw over other apps" is set to "Yes". "Tab queue" is enabled. The prompt is dismissed
-Then tried with Aurora: "Open multiple links" option is enabled and no dialog to turn on "Draw over other apps" is shown. "Draw over other apps" remains set to "No" for Aurora and "Yes" to Nightly.

2. Restart the device and repeat the above steps with the other version first:

Both versions have "Draw over other apps" set to "No":
-On Nightly works ok. "Draw over other apps" is set to "Yes". "Tab queue" is enabled. The prompt is dismissed
-On Aurora works ok. "Draw over other apps" is set to "Yes", also

Now both versions have "Draw over other apps" set to "Yes". I clear data, set the option to "No" and try the same steps. 
-First tried with Aurora and works ok. 
-Then tried with Nightly: and works ok. 

3. "Draw over other apps" set to "Yes" for Aurora and "No" for Nightly
-On Nightly works ok.


Verified as fixed using:
Device: Nexus 6 (Android 6.0)
Build: Firefox for Android 48.0a1 (2016-04-17)

Comment 20

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 1244722-TQ-Aurora.patch

Modified patch for Aurora (Fixing imports)

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/892d0f1308f8

Comment 22

[Carsten Book [:Tomcat]]

(In reply to Sebastian Kaspari (:sebastian) from comment #20)
> Created attachment 8744348 [details] [diff] [review]
> 1244722-TQ-Aurora.patch
> 
> Modified patch for Aurora (Fixing imports)

landed by ryan

Comment 23

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Verified based on comment 19