Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TIMOB-26260] Android: Updated "Ti.Android.Service" to handle Android P "FOREGROUND_SERVICE" permission #10314

Merged
merged 5 commits into from Oct 4, 2018

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Sep 8, 2018

JIRA:
https://jira.appcelerator.org/browse/TIMOB-26260

Summary:
Note that if you target API Level 28 (Android 9.0) or higher, then Google now requires that you add "android.permission.FOREGROUND_SERVICE" to the "AndroidManifest.xml" file. If you don't have this permission, then a permission exception will be thrown when attempting to set up a foreground service.

This PR does not prevent an exception. Instead, it'll throw the below exception message instead to help clarify what is needed. Also, this PR will throw this exception on older Android OS versions too if the permission is missing, which Google does not do, but it is intended to help developers who don't have an Android P device to help with.

[Developer Error] Ti.Android.Service.foregroundNotify() requires manifest permission: android.permission.FOREGROUND_SERVICE

Also note that the above only applies when targeting Android 9.0 (aka: API Level 28) or higher.

Test:

  1. Set up a test project as shown in PR [TIMOB-16066] Android: Added "foreground" service support #10076.
  2. Set up the "tiapp.xml" to target Android API Level 27.
<uses-sdk android:minSdkVersion="16" android:targetSdkVersion="27"/>
  1. Build and run the app on Android 9.
  2. Verify that a foreground service notification appears on the status bar.
  3. Set up the "tiapp.xml" to target Android API Level 28.
<uses-sdk android:minSdkVersion="16" android:targetSdkVersion="28"/>
  1. Build and the app on Android 9.
  2. Verify that a "[Developer Error]" exception appears.
  3. Run the app on an Android OS older that 9.0.
  4. Verify that a "[Developer Error]" exception appears.
  5. Add the following Android manifest permission to the "tiapp.xml" file.
<uses-permission android:name="android.permission.FOREGROUND_SERVICE"/>
  1. Build and run the app on an Android 9 device.
  2. Verify that a foreground service notification appears on the status bar.
  3. Run the app on an Android OS older that 9.0.
  4. Verify that a foreground service notification appears on the status bar.

@build
Copy link
Contributor

build commented Sep 14, 2018

Messages
📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

@lokeshchdhry
Copy link
Contributor

lokeshchdhry commented Oct 2, 2018

@jquick-axway , I am using the FOREGROUND_SERVICE test from #10076.
I am not seeing any permission error even if I do not have android.permission.FOREGROUND_SERVICE in my tiapp.xml on Android 9.0. Also, no runtime error.
I do get the location updates every 20 sec when the app is backgrounded even with no permission.

@jquick-axway
Copy link
Contributor Author

jquick-axway commented Oct 2, 2018

Oh shoot. I forgot to write up the test for this PR.

You'll only get a permission error if you target API Level 28, the android.permission.FOREGROUND_SERVICE is missing, and you attempt to run a foreground service.

The key thing here is the permission check won't happen unless you "target" API Level 28. So, if you target an older API Level (we currently target 27 by default) then an Android P device will not have a permission error because it'll run the app in backward compatibility mode.

@lokeshchdhry
Copy link
Contributor

FR Passed.

Without the permission & targeting API 28 an exception is thrown with an error to add android.permission.FOREGROUND_SERVICE.
With permission no exception is thrown & it works as expected.

Studio Ver: 5.1.1.201809051655
SDK Ver: 7.4.1.v20180928073117
OS Ver: 10.13.5
Xcode Ver: Xcode 9.4.1
Appc NPM: 4.2.13
Appc CLI: 7.0.6
Daemon Ver: 1.1.3
Ti CLI Ver: 5.1.1
Alloy Ver: 1.13.2
Node Ver: 8.9.1
NPM Ver: 5.5.1
Java Ver: 10.0.2
Devices: ⇨ google Nexus 5 (Android 6.0.1)
⇨ google Pixel (Android 9)
Emulator: ⇨ Android 4.1.2

@lokeshchdhry lokeshchdhry merged commit 75294d0 into tidev:master Oct 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants