Stop declaring photo/video permissions for stream media playback (#5507)

docs/developer-guide/Advanced-Topics-Under-The-Hood.asciidoc

@@ -167,6 +167,12 @@ If none of the services are defined to true then plus, auth, base, analytics, gc
 |android.blockExternalStoragePermission
 |Boolean true/false defaults to false. Disables the external storage (SD card) permission
 
+|android.blockReadMediaPermissions
+|Boolean true/false, defaults to the value of `android.blockExternalStoragePermission`. Suppresses the `READ_MEDIA_VIDEO` and `READ_MEDIA_AUDIO` permissions that playing a URI adds on API 33 and above
+
+|android.requestReadMediaPermissions
+|Boolean true/false defaults to false. Declares `READ_MEDIA_IMAGES`, `READ_MEDIA_VIDEO` and `READ_MEDIA_AUDIO` on API 33 and above even when the build detected no media playback. `READ_MEDIA_IMAGES` is only ever added by this hint
+
 |android.min_sdk_version
 |The least SDK required to run this app, the default value changes based on functionality but can be as low as 7. This corresponds to the XML attribute `android:minSdkVersion`.
 
@@ -776,6 +782,10 @@ a specific permission came up. This maps Android permissions to the methods/clas
 
 `android.permission.READ_CONTACTS` - requested when accessing the device address book through `Display.getAllContacts()` and related APIs.
 
+`android.permission.READ_MEDIA_VIDEO` & `android.permission.READ_MEDIA_AUDIO` - added on API 33 and above when the app plays a URI through `MediaManager.createMedia(String, boolean)` or `createMediaAsync(String, boolean, Runnable)`, since that URI can point at the shared media store. The `InputStream` overloads don't add them: the Android port copies the stream into app private storage and reads nothing shared. Block them with `android.blockReadMediaPermissions=true`.
+
+`android.permission.READ_MEDIA_IMAGES` - never added by media playback, because no Codename One API requests it at runtime. Declaring it alongside `READ_MEDIA_VIDEO` places the app under Google Play's Photo and Video Permissions policy, so ask for it only when you need it, with `android.requestReadMediaPermissions=true`.
+
 ==== Permissions under Marshmallow (Android 6+)
 
 Starting with Marshmallow (Android 6+ API level 23) Android shifted to a permissions system that prompts users for permission the first time an API is used for example: when accessing contacts the user will receive a prompt whether to allow contacts access.

maven/codenameone-maven-plugin/src/main/java/com/codename1/builders/AndroidGradleBuilder.java

@@ -369,6 +369,87 @@ static String pendingPushReplayCode(int detectedPushVersion) {
                 + "        }, this);\n";
     }
 
+    /**
+     * Whether a createMedia call reads media the app does not own, and so
+     * needs the READ_MEDIA_* permissions on API 33 and up.
+     *
+     * <p>Only the URI overloads can: {@code createMedia(String,boolean)}
+     * and {@code createMediaAsync(String,boolean,Runnable)} hand the
+     * string to the platform, which may resolve it against the
+     * MediaStore. The InputStream overloads cannot -- the Android
+     * implementation either plays an already-open FileInputStream's
+     * descriptor or copies the stream into a temp file in app-private
+     * storage, and asks for no permission at any point.</p>
+     *
+     * <p>They were told apart by name alone, so they were not told apart
+     * at all, and an app playing a bundled resource through a stream was
+     * built asking to read the user's photos and videos. That is not
+     * merely a spurious permission: READ_MEDIA_IMAGES and
+     * READ_MEDIA_VIDEO put the app under Play's Photo and Video
+     * Permissions policy, so the author gets a declaration form and a
+     * compliance deadline for something the app never does (issue
+     * #5507).</p>
+     *
+     * <p>A null descriptor is treated as the URI overload. Over-declaring
+     * costs a permission; under-declaring costs a SecurityException on a
+     * user's device.</p>
+     */
+    static boolean readsSharedMediaForPlayback(String cls, String method,
+            String descriptor) {
+        if (cls == null || method == null) {
+            return false;
+        }
+        if (cls.indexOf("com/codename1/media/MediaManager") != 0
+                && cls.indexOf("com/codename1/ui/Display") != 0) {
+            return false;
+        }
+        if (method.indexOf("createMedia") < 0
+                || method.indexOf("createMediaRecorder") > -1) {
+            return false;
+        }
+        return descriptor == null
+                || descriptor.startsWith("(Ljava/lang/String;");
+    }
+
+    /**
+     * The READ_MEDIA_* permissions the manifest should declare, in
+     * manifest order.
+     *
+     * <p>Images are declared only when the app asked for them outright
+     * with {@code android.requestReadMediaPermissions}, never because
+     * media playback was detected. Playback cannot read an image: the one
+     * runtime request site in the Android port passes
+     * {@code PERMISSION_READ_VIDEO} or {@code PERMISSION_READ_AUDIO}, and
+     * nothing anywhere in the port passes
+     * {@code PERMISSION_READ_IMAGES}, so an inferred READ_MEDIA_IMAGES
+     * was a permission the app had no way to use.</p>
+     *
+     * <p>It was not free either. READ_MEDIA_IMAGES together with
+     * READ_MEDIA_VIDEO is what puts an app under Play's Photo and Video
+     * Permissions policy, so an app that only plays audio was handed a
+     * declaration form and a compliance deadline for a capability it does
+     * not have (issue #5507).</p>
+     *
+     * <p>Video and audio stay paired because the runtime picks between
+     * them on the {@code isVideo} flag of the call, which is a value this
+     * scan does not read.</p>
+     */
+    static List<String> readMediaPermissionNames(boolean blocked,
+            int targetSdkVersion, boolean mediaPlayback,
+            boolean requestedOutright) {
+        List<String> out = new ArrayList<String>();
+        if (blocked || targetSdkVersion < 33
+                || (!mediaPlayback && !requestedOutright)) {
+            return out;
+        }
+        if (requestedOutright) {
+            out.add("android.permission.READ_MEDIA_IMAGES");
+        }
+        out.add("android.permission.READ_MEDIA_VIDEO");
+        out.add("android.permission.READ_MEDIA_AUDIO");
+        return out;
+    }
+
     private boolean wakeLock;
     private boolean recordAudio;
     private boolean mediaPlaybackPermission;
@@ -1704,6 +1785,15 @@ public void usesClassMethodWithBooleanArgument(String cls,
                     }
                 }
 
+                @Override
+                public void usesClassMethodWithDescriptor(String cls,
+                        String method, String descriptor) {
+                    if (readsSharedMediaForPlayback(cls, method,
+                            descriptor)) {
+                        mediaPlaybackPermission = true;
+                    }
+                }
+
                 @Override
                 public void usesClassMethod(String cls, String method) {
                     // The catalog first: it decides frameworks, gradle
@@ -1905,12 +1995,10 @@ public void usesClassMethod(String cls, String method) {
                     if (cls.indexOf("com/codename1/ui/Display") == 0 && method.indexOf("createMediaRecorder") > -1) {
                         recordAudio = true;
                     }
-                    if (cls.indexOf("com/codename1/media/MediaManager") == 0 && method.indexOf("createMedia") > -1 && method.indexOf("createMediaRecorder") < 0) {
-                        mediaPlaybackPermission = true;
-                    }
-                    if (cls.indexOf("com/codename1/ui/Display") == 0 && method.indexOf("createMedia") > -1 && method.indexOf("createMediaRecorder") < 0) {
-                        mediaPlaybackPermission = true;
-                    }
+                    // createMedia is handled in
+                    // usesClassMethodWithDescriptor: which overload was
+                    // called decides whether any shared media is read,
+                    // and the name alone cannot say.
                     if (cls.indexOf("com/codename1/ui/Display") == 0 && method.indexOf("createContact") > -1) {
                         contactsWritePermission = true;
                     }
@@ -3986,10 +4074,12 @@ && compareVersions(declaredPlugin, kotlinFloor) < 0) {
         boolean blockReadMediaPermissions = request.getArg("android.blockReadMediaPermissions", blockExternalStoragePermission ? "true" : "false").equals("true");
         boolean requestReadMediaPermissions = request.getArg("android.requestReadMediaPermissions", "false").equals("true");
         String readMediaPermissions = "";
-        if (!blockReadMediaPermissions && targetSDKVersionInt >= 33 && (mediaPlaybackPermission || requestReadMediaPermissions)) {
-            readMediaPermissions += permissionAdd(request, "\"android.permission.READ_MEDIA_IMAGES\"", "    <uses-permission android:name=\"android.permission.READ_MEDIA_IMAGES\" android:required=\"false\" />\n");
-            readMediaPermissions += permissionAdd(request, "\"android.permission.READ_MEDIA_VIDEO\"", "    <uses-permission android:name=\"android.permission.READ_MEDIA_VIDEO\" android:required=\"false\" />\n");
-            readMediaPermissions += permissionAdd(request, "\"android.permission.READ_MEDIA_AUDIO\"", "    <uses-permission android:name=\"android.permission.READ_MEDIA_AUDIO\" android:required=\"false\" />\n");
+        for (String p : readMediaPermissionNames(blockReadMediaPermissions,
+                targetSDKVersionInt, mediaPlaybackPermission,
+                requestReadMediaPermissions)) {
+            readMediaPermissions += permissionAdd(request, "\"" + p + "\"",
+                    "    <uses-permission android:name=\"" + p
+                            + "\" android:required=\"false\" />\n");
         }
         String xmlizedDisplayName = xmlize(request.getDisplayName());
 

maven/codenameone-maven-plugin/src/main/java/com/codename1/builders/Executor.java

@@ -500,6 +500,34 @@ public default void declaresConcreteType(String cls) {
         public default void usesClassMethodWithBooleanArgument(String cls,
                 String method, Boolean value) {
         }
+
+        /**
+         * Reports a call together with the descriptor of the method it
+         * resolves to.
+         *
+         * <p>{@link #usesClassMethod(String, String)} reports the name
+         * alone, which cannot separate overloads. That is not a detail
+         * where the overloads do different things:
+         * {@code MediaManager.createMedia(String,boolean)} plays a URI
+         * that may point at the MediaStore, while
+         * {@code createMedia(InputStream,String)} copies the stream into
+         * app-private storage and reads no shared media at all. Keyed on
+         * the name, an app doing the second was built asking for
+         * {@code READ_MEDIA_VIDEO}, which costs its author a Play Console
+         * Photo and Video Permissions declaration for a permission the
+         * app never uses.</p>
+         *
+         * <p>{@code descriptor} is the call site's descriptor, so it is
+         * what the compiler resolved rather than what runs -- close
+         * enough for overload selection, which is all it is for. It is
+         * null when the scan could not recover one; a caller must treat
+         * null as "may be the overload that needs the permission",
+         * because under-declaring costs a SecurityException on a user's
+         * device.</p>
+         */
+        public default void usesClassMethodWithDescriptor(String cls,
+                String method, String descriptor) {
+        }
     }
 
     public static interface InternalClassRemapper {
@@ -755,14 +783,16 @@ public void visitFieldInsn(int i, String string, String string1, String string2)
                                 }
 
                                 @Override
-                                public void visitMethodInsn(int i, String owner, String name, String string2) {
+                                public void visitMethodInsn(int i, String owner, String name, String descriptor) {
                                     Boolean arg = pushedBoolean;
                                     pushedBoolean = null;
                                     scanner.usesClass(owner);
                                     if (name != null && !name.equals("<init>")) {
                                         scanner.usesClassMethod(owner, name);
                                         scanner.usesClassMethodWithBooleanArgument(
                                                 owner, name, arg);
+                                        scanner.usesClassMethodWithDescriptor(
+                                                owner, name, descriptor);
                                     }
                                 }
 
@@ -775,6 +805,8 @@ public void visitMethodInsn(int opcode, String owner, String name, String descri
                                         scanner.usesClassMethod(owner, name);
                                         scanner.usesClassMethodWithBooleanArgument(
                                                 owner, name, arg);
+                                        scanner.usesClassMethodWithDescriptor(
+                                                owner, name, descriptor);
                                     }
                                 }
 
@@ -823,6 +855,16 @@ public void visitInvokeDynamicInsn(String name,
                                             scanner.usesClassMethodWithBooleanArgument(
                                                     h.getOwner(), h.getName(),
                                                     null);
+                                            // The Handle does carry the
+                                            // descriptor of the exact
+                                            // overload the reference was
+                                            // resolved against, so a
+                                            // MediaManager::createMedia
+                                            // reference is as selectable
+                                            // as a direct call.
+                                            scanner.usesClassMethodWithDescriptor(
+                                                    h.getOwner(), h.getName(),
+                                                    h.getDesc());
                                         }
                                     }
                                 }