Update Android targetSdkVersion to 29 (Android 10), by 2020-11 deadline

[gnprice]

The next round after #3563 (and #3075). We should update our targetSdkVersion to 29, aka Android 10.

Deadline is 2020-11-02. But we could do this any time (indeed could have as soon as Android 10 was out last year), and in any case filing an issue to give us a place to track tidbits we learn.

• General docs on this upgrade are here.
• The list of behavior changes triggered by actually bumping the targetSdkVersion is here.

The important steps for this upgrade are:

1. Read about the potentially-breaking changes, and identify those that might affect us.
2. Make a WIP change to targetSdkVersion.
3. Test the WIP change thoroughly, especially in the areas highlighted in step 1. Fix things as needed, and repeat.

Specific tidbits we've learned:

• While validating the previous upgrade, to API 28 aka Android 9 Pie, @ray-kraesig noticed we are said to use some "weakly-deprecated APIs": Update Android targetSdkVersion to 28, by 2019-11 deadline #3563 (comment)
  • Of these, one is... IIUC Android 10 puts it in the "greylist-max-p" list, which means that a targetSdkVersion of 29 or greater prevents using it.

[gnprice]

The Play Console has started showing me a warning when I upload a new release; first saw it today with the 27.154 release (now in beta). Following some links, the deadline is 2020-11-02.

(I'll edit the description with more info.)

[gnprice]

Bumped the priority, as this deadline is getting sooner.

[gnprice]

1. Read about the potentially-breaking changes, and identify those that might affect us.

Here's notes from reading through that list of behavior changes triggered by actually bumping the targetSdkVersion. I intend to update this as I learn more.

• Privacy changes, which have their own docs page.
  • External storage access scoped to app files and media. This sounds like it's describing a new feature, not a breaking change. This is a very welcome feature for some types of apps, but I think not helpful for ours: our use cases for accessing files (and photos etc.) are almost entirely about reading data created by other apps, and about saving data (e.g. downloading an image) to make it available to other apps.
    • Well, yes and no, as it turned out. The description on this docs page was misleading: it says apps with the new "scoped storage" model get access to certain things without needing to request any permissions, but doesn't mention that they don't get access to certain other files, even with the same permissions granted by the user as on previous versions. As a result, things broke: Update Android targetSdkVersion to 29 (Android 10), by 2020-11 deadline #3665 (comment)
    • On the other hand, happily, this is like a new feature -- temporarily -- in that it's optional: Update Android targetSdkVersion to 29 (Android 10), by 2020-11 deadline #3665 (comment) . So to unblock this targetSdkVersion change, we'll opt out for now.
  • Access to device location in the background requires permission. We don't use device location at all, let alone while in the background, so this doesn't affect us.
  • Restrictions on starting activities from the background. This sounds like it's intended not to affect usages like ours, but isn't 100% reassuring on that; we should read the details.
    • OK, read them -- and probably the most important detail here is that these restrictions aren't affected by our targetSdkVersion. Rather they apply to apps running on Android 10 and higher, regardless of target. I and lots of our users have been on Android 10 (or 11) for a year at this point, so if there was a problem here we'd likely already have a bug report. Even if there is an edge case lurking, it won't be a regression associated with the targetSdkVersion bump, so doesn't need to block this issue.
  • Removal of contacts affinity. We don't use contacts in the first place, so this doesn't affect us.
  • MAC address randomization. We don't look at or care about the device's MAC address, so this doesn't affect us.
  • Restriction on access to /proc/net filesystem. Same story as for shared memory, below.
  • Restriction on non-resettable device identifiers. We don't use any of these in our own code. It's conceivable that perhaps Sentry could be using these for us (though that'd be a rather disappointing choice on Sentry's part if so.) ... Aha, but if so (or if anything in our app did), it would already be crashing (at least on Android 10) because we don't have the READ_PHONE_STATE permission in our manifest. (Since 2cc6a52 we explicitly deny it in our source manifest, presumably because some dependency was injecting it; at this point it looks like none of our dependencies try to inject it in the first place, either.) So we must indeed not be using these, and this change doesn't affect us.
  • Limited access to clipboard data. We don't ever try to read from the clipboard, let alone without focus; we write to it only on certain explicit user actions in our UI, so only when we have focus. So this doesn't affect us.
  • Protection of USB device serial number. We don't ever try to read this, so this doesn't affect us.
  • Restriction on access to camera details and metadata. I'm not sure if we use this getCameraCharacteristics method -- it seems like something that might well be used in the course of the "take a photo" feature offered in the compose menu, for which we rely on a third-party library (react-native-image-picker). The way this could conceivably be a problem is if for some reason that library tries to use that method before it secures the CAMERA permission.
    • Test the WIP change by attempting to take a photo, in a fresh install of the app so it doesn't already have that permission.
  • Restriction on enabling and disabling Wi-Fi. We don't do this.
  • Restrictions on direct access to configured Wi-Fi networks. We don't try to access this.
  • Some telephony, Bluetooth, Wi-Fi APIs require FINE location permission. We don't use telephony, don't use Bluetooth, and don't use Wi-Fi (beyond simply making network requests over the device's network connection, which these changes don't affect.)
  • Restricted access to screen contents. We don't request any of the three permissions affected by this change.
  • User-facing permission check on legacy apps. This affects only apps targeting an ancient version of Android (which haven't been publishable on the Play Store for years.)
  • Physical activity recognition. We don't attempt to use this feature.
  • Permission groups removed from UI. We don't use this SDK feature.
• Updates to non-SDK interface restrictions -- i.e. they've added to the lists of undocumented system interfaces that are forbidden to use.
  • Go through the list (here) and try to identify anything likely to affect us. … OK, I filtered out the "False Positive" lines, where they determined that nobody's actually using them in the first place; that leaves 306 lines, and I read through those. None look like things we're likely to be using in our own code. It's possible RN or a third-party library is using something; for that we'll rely on other analysis and testing.
  • Also look at what the Google Play Console flags as things we seem to be using.
  • Test the WIP change to try to spot breakage.
• Shared memory. We certainly don't have any code of our own that touches this -- we'd only be affected if something in RN was affected, and if so it'd probably be something quite central that virtually all RN apps relied on. We're using RN v0.62, which came out well after Android 10, so it seems unlikely there's a problem of that form.
• Removed execute permission for app home directory. Same story as shared memory.
• Android runtime only accepts system-generated OAT files. Same story.
• Enforcing AOT correctness in ART. Same story.
• Permissions changes for fullscreen intents. We don't do this.
• Support for foldables.
  • Changes to the meanings of onResume and onPause, with the new concept of "topmost resumed". I see two ways this could matter:
    • It means several activities can be in the "resumed" state at once. We only ever have one activity at a time, and I don't see a reason we would care whether some other activity is also resumed. So I believe this doesn't affect us.
    • It means an activity can be "resumed" but not "topmost resumed". But... the only difference this seems to make is that some other activity is then "topmost resumed", and potentially has focus. I believe that makes it a subset of the previous point.
  • Change to the behavior of android:resizeableActivity="false" in the manifest. The default is true, and we don't set it, so this doesn't affect us.
  • New manifest attribute android:minAspectRatio. This seems like not actually a behavior change -- it's a new opt-in feature. Copied this to a separate list of those, below.

---

New features to possibly use, learned incidentally in this process:

• New manifest attribute android:minAspectRatio.

[gnprice]

OK, I've now completed the steps of this that are about reading through the documentation. The results are expanded out into the checklist in the preceding comment. Almost all those items are checked off, but there are a couple of them remaining that correspond to deeper investigation and/or testing.

Here's a fresh task list, covering those remaining items above and zooming back out to the overall issue as set out in the description:

• Non-SDK interfaces: Look at what the Google Play Console flags as things we seem to be using.
  • The pre-launch report for v27.156 is here for those with access.
  • There are 10 API issues flagged; all "unsupported APIs", no "restricted APIs". Inspecting the definitions of those terms with the hover-help text, this means that none of them are blocked in Android 10, or for that matter Android 11. So they'd be good to avoid using, but will not interact with bumping our targetSdkVersion; they don't affect this issue.
  • Additional info while I'm looking at this: Of the 10 issues, 9 of them have stack traces going through either RN or OkHttp, which is the library RN pulls in for making HTTP requests. So those seem like issues for RN to deal with, if anyone. The remaining one is a bit mysterious, as the stack begins with android.os.Handler; what code queued that task is unknown. For more details: it looks like these 10 are the same 10 as @rk-for-zulip investigated a year ago at Update Android targetSdkVersion to 28, by 2019-11 deadline #3563 (comment) .
• Make a WIP change that bumps the targetSdkVersion.
• Test that change:
  • Attempt to take a photo, in a fresh install of the app so it doesn't already have the CAMERA permission.
  • Test thoroughly in general, looking for breakage.
  • Fix things as needed, and repeat.
• Commit and push the change.
• Optional followups:
  • Consider using the new manifest attribute android:minAspectRatio.

[gnprice]

Attempt to take a photo, in a fresh install of the app so it doesn't already have the CAMERA permission.

Hmm, well -- I get this error modal:

Specifically, my steps were:

• Start a debug build, in a fresh install, with targetSdkVersion at 29, and with Flipper disabled to avoid this issue which I discovered when trying to test this.
• Attempt to take a photo.
• Get the two permissions prompts as usual; grant both.
• This error modal appears.

Then:

• Try hitting the "camera" button again.
• The same error modal appears.

So it seems there is some problem. It may not be the getCameraCharacteristics problem I anticipated above; that doesn't seem to fit with having gotten successfully through granting camera permissions.

Hmm, thinking some more, ISTR that the library we use for getting images from the camera (react-native-image-picker) does something a bit odd where it first has the system take a photo and store it, then it goes and reads the resulting file by name. That sounds like the kind of thing that would be blocked by the change "External storage access scoped to app files and media" -- what it's actually doing is perfectly innocent (reading the photo the user just explicitly chose to take for use with Zulip), but it's unnecessarily using APIs that could provide access to arbitrary files and are now blocked. And these symptoms seem like a perfect fit for that hypothesis.

So that's a potential lead for investigation; if that's what's happening, we'll want either a newer version of this library, or some other library if it's effectively unmaintained and has no such version, or some other solution.

[gnprice]

Well, here's a sad but adequate workaround for the camera issues:

--- a/android/app/src/main/AndroidManifest.xml
+++ b/android/app/src/main/AndroidManifest.xml
@@ -25,2 +25,3 @@
-        android:theme="@style/AppTheme">
+        android:theme="@style/AppTheme"
+        android:requestLegacyExternalStorage="true">
         <activity

That fixes the trouble mentioned in my previous comment, and also a related issue that appears when trying to pick an existing image.

It's a temporary solution -- effectively it opts out of this aspect of the effects of targeting Android 10, and when in the future we target Android 11 the opt-out will be ignored. But it's enough to unblock this for now.

I'll file a separate issue for the follow-up, with what I learned from investigating this today.

[gnprice]

After the issues with react-native-image-picker described in the previous two comments (affecting both the take-photo and upload-existing-photo features, behind the "camera" and "photo" icons in the compose menu) where it hasn't adapted to the new "scoped storage", I tested out the other feature that seemed likely to be affected by scoped storage: the upload-file feature, behind the "document" icon.

That one, happily, seems to work just fine when targeting Android 10 aka API 29, without needing the requestLegacyExternalStorage opt-out / workaround. That feature has a similar flavor but is provided by a different library -- react-native-document-picker -- so I guess the latter library has successfully adapted to the new APIs.