diff options
81 files changed, 3429 insertions, 791 deletions
diff --git a/Android.bp b/Android.bp index 93d8daeb4..4dd572385 100644 --- a/Android.bp +++ b/Android.bp @@ -61,6 +61,7 @@ android_app { resource_dirs: [ "res", + "photopicker/res", ], srcs: [ ":mediaprovider-sources", diff --git a/AndroidManifest.xml b/AndroidManifest.xml index e4249ce1c..a19e718b5 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -54,6 +54,10 @@ <uses-permission android:name="com.android.providers.media.permission.ACCESS_OEM_METADATA" /> + <!-- Permission required to update OEM metadata. Declared by us --> + <uses-permission + android:name="com.android.providers.media.permission.UPDATE_OEM_METADATA" /> + <!-- Permission required to bind to OemMetadataService --> <uses-permission android:name="com.android.providers.media.permission.BIND_OEM_METADATA_SERVICE" /> @@ -74,6 +78,9 @@ <permission android:name="com.android.providers.media.permission.ACCESS_OEM_METADATA" android:protectionLevel="signature|privileged" /> + <permission android:name="com.android.providers.media.permission.UPDATE_OEM_METADATA" + android:protectionLevel="signature|privileged" /> + <permission android:name="com.android.providers.media.permission.BIND_OEM_METADATA_SERVICE" android:protectionLevel="signature"/> diff --git a/TEST_MAPPING b/TEST_MAPPING index 1f37f6d72..9a38fb418 100644 --- a/TEST_MAPPING +++ b/TEST_MAPPING @@ -180,11 +180,6 @@ ] }, { - // This is a typo and is tracked in b/155715039 but flaky on CF. - // Will fix this once the root cause of flake is fixed. - "name": "AdoptableHostTest" - }, - { "name": "CtsScopedStorageCoreHostTest", "options": [ { diff --git a/apex/framework/api/system-current.txt b/apex/framework/api/system-current.txt index 1d11267f4..f002605a1 100644 --- a/apex/framework/api/system-current.txt +++ b/apex/framework/api/system-current.txt @@ -64,6 +64,7 @@ package android.provider { } public final class MediaStore { + method @FlaggedApi("com.android.providers.media.flags.enable_oem_metadata_update") public static void bulkUpdateOemMetadataInNextScan(@NonNull android.content.Context); method @NonNull public static android.net.Uri rewriteToLegacy(@NonNull android.net.Uri); method @NonNull @WorkerThread public static android.net.Uri scanFile(@NonNull android.content.ContentResolver, @NonNull java.io.File); method @WorkerThread public static void scanVolume(@NonNull android.content.ContentResolver, @NonNull String); @@ -72,6 +73,7 @@ package android.provider { field public static final String AUTHORITY_LEGACY = "media_legacy"; field @NonNull public static final android.net.Uri AUTHORITY_LEGACY_URI; field public static final String QUERY_ARG_DEFER_SCAN = "android:query-arg-defer-scan"; + field @FlaggedApi("com.android.providers.media.flags.enable_oem_metadata_update") public static final String UPDATE_OEM_METADATA_PERMISSION = "com.android.providers.media.permission.UPDATE_OEM_METADATA"; } @FlaggedApi("com.android.providers.media.flags.enable_oem_metadata") public abstract class OemMetadataService extends android.app.Service { diff --git a/apex/framework/java/android/provider/MediaStore.java b/apex/framework/java/android/provider/MediaStore.java index c9ee81196..b8ff19503 100644 --- a/apex/framework/java/android/provider/MediaStore.java +++ b/apex/framework/java/android/provider/MediaStore.java @@ -32,6 +32,7 @@ import android.annotation.WorkerThread; import android.app.Activity; import android.app.AppOpsManager; import android.app.PendingIntent; +import android.app.compat.CompatChanges; import android.compat.annotation.UnsupportedAppUsage; import android.content.ClipData; import android.content.ContentProvider; @@ -304,6 +305,9 @@ public final class MediaStore { "revoke_all_media_grants_for_package"; /** @hide */ + public static final String BULK_UPDATE_OEM_METADATA_CALL = "bulk_update_oem_metadata"; + + /** @hide */ public static final String OPEN_FILE_CALL = "open_file_call"; @@ -663,6 +667,11 @@ public final class MediaStore { public static final String INTENT_ACTION_VIDEO_CAMERA = "android.media.action.VIDEO_CAMERA"; /** + * This is a copy of the flag that exists in MediaProvider. + */ + private static final long EXCLUDE_UNRELIABLE_STORAGE_VOLUMES = 391360514L; + + /** * Standard Intent action that can be sent to have the camera application * capture an image and return it. * <p> @@ -1342,6 +1351,16 @@ public final class MediaStore { public static final String ACCESS_OEM_METADATA_PERMISSION = "com.android.providers.media.permission.ACCESS_OEM_METADATA"; + /** + * Permission that grants ability to trigger update of {@link MediaColumns#OEM_METADATA}. + * + * @hide + */ + @FlaggedApi(Flags.FLAG_ENABLE_OEM_METADATA_UPDATE) + @SystemApi + public static final String UPDATE_OEM_METADATA_PERMISSION = + "com.android.providers.media.permission.UPDATE_OEM_METADATA"; + /** @hide */ @IntDef(flag = true, prefix = { "MATCH_" }, value = { MATCH_DEFAULT, @@ -4734,7 +4753,15 @@ public final class MediaStore { case Environment.MEDIA_MOUNTED_READ_ONLY: { final String volumeName = sv.getMediaStoreVolumeName(); if (volumeName != null) { - res.add(volumeName); + File directory = sv.getDirectory(); + if (shouldExcludeUnReliableStorageVolumes() + && directory != null + && directory.getAbsolutePath() != null + && directory.getAbsolutePath().startsWith("/mnt/")) { + Log.d(TAG, "skipping unreliable volume : " + volumeName); + } else { + res.add(volumeName); + } } break; } @@ -4744,6 +4771,14 @@ public final class MediaStore { } /** + * Checks if the EXCLUDE_UNRELIABLE_STORAGE_VOLUMES appcompat flag is enabled. + */ + private static boolean shouldExcludeUnReliableStorageVolumes() { + return CompatChanges.isChangeEnabled(EXCLUDE_UNRELIABLE_STORAGE_VOLUMES) + && Flags.excludeUnreliableVolumes(); + } + + /** * Works exactly the same as * {@link ContentResolver#openFileDescriptor(Uri, String, CancellationSignal)}, but only works * for {@link Uri} whose scheme is {@link ContentResolver#SCHEME_CONTENT} and its authority is @@ -5734,4 +5769,26 @@ public final class MediaStore { throw e.rethrowAsRuntimeException(); } } + + /** + * Allows bulk update of {@link MediaColumns#OEM_METADATA} column in next scan. + * Requires calling package to hold {@link UPDATE_OEM_METADATA_PERMISSION} permission. Updates + * {@link MediaColumns#OEM_METADATA} to NULL for OEM supported media files and re-fetch + * the latest values in the next scan. + * Caller can enforce file/volume scan after this to update MediaStore with the latest OEM + * metadata. If not done, next scan by MediaStore will fetch and update the latest data. + * + * @hide + */ + @FlaggedApi(Flags.FLAG_ENABLE_OEM_METADATA_UPDATE) + @SystemApi + public static void bulkUpdateOemMetadataInNextScan(@NonNull Context context) { + final ContentResolver resolver = context.getContentResolver(); + try (ContentProviderClient client = resolver.acquireContentProviderClient(AUTHORITY)) { + final Bundle extras = new Bundle(); + client.call(BULK_UPDATE_OEM_METADATA_CALL, /* arg= */ null, /* extras= */ extras); + } catch (RemoteException e) { + throw e.rethrowAsRuntimeException(); + } + } } diff --git a/jni/FuseDaemon.cpp b/jni/FuseDaemon.cpp index 816fbcff7..d22758bba 100644 --- a/jni/FuseDaemon.cpp +++ b/jni/FuseDaemon.cpp @@ -2651,6 +2651,7 @@ std::unique_ptr<FdAccessResult> FuseDaemon::CheckFdAccess(int fd, uid_t uid) con return std::make_unique<FdAccessResult>(string(), false); } + std::lock_guard<std::recursive_mutex> guard(fuse->lock); const node* node = node::LookupInode(fuse->root, ino); if (!node) { PLOG(DEBUG) << "CheckFdAccess no node found with given ino"; diff --git a/mediaprovider_flags.aconfig b/mediaprovider_flags.aconfig index 7a6c28299..de617c438 100644 --- a/mediaprovider_flags.aconfig +++ b/mediaprovider_flags.aconfig @@ -158,6 +158,15 @@ flag { } flag { + name: "enable_photopicker_datescrubber" + is_exported: true + namespace: "mediaprovider" + description: "This flag controls whether to enable datescrubber feature in photopicker" + bug: "312640456" + is_fixed_read_only: true +} + +flag { name: "cloud_media_provider_search" is_exported: true namespace: "mediaprovider" @@ -282,3 +291,21 @@ flag { is_fixed_read_only: true bug: "376910932" } + +flag { + name: "enable_oem_metadata_update" + is_exported: true + namespace: "mediaprovider" + description: "Controls ability to update oem_metadata column" + bug: "352528913" + is_fixed_read_only: true +} + +flag { + name: "enable_local_media_provider_capabilities" + namespace: "mediaprovider" + description: "This flag controls the Capabilities APIs in the local media provider, i.e. PhotoPickerProvider." + bug: "402379523" + is_fixed_read_only: true + is_exported: true +} diff --git a/pdf/framework-v/java/android/graphics/pdf/PdfRenderer.java b/pdf/framework-v/java/android/graphics/pdf/PdfRenderer.java index 6c8a59066..0296fd01d 100644 --- a/pdf/framework-v/java/android/graphics/pdf/PdfRenderer.java +++ b/pdf/framework-v/java/android/graphics/pdf/PdfRenderer.java @@ -935,6 +935,11 @@ public final class PdfRenderer implements AutoCloseable { /** * Update the given {@link PdfPageObject} to the page. * <p> + * Note: This method only updates the parameters of the PageObject whose setters + * are available. Attempting to update fields with no corresponding setters will + * have no effect. + * + * <p> * {@link PdfRenderer#write} needs to be called to get the updated PDF stream after calling * this method. {@link PdfRenderer.Page} instance can be closed before calling * {@link PdfRenderer#write}. diff --git a/pdf/framework/Android.bp b/pdf/framework/Android.bp index d67506690..d2d9459e9 100644 --- a/pdf/framework/Android.bp +++ b/pdf/framework/Android.bp @@ -136,6 +136,7 @@ java_library { libs: [ // To add StatsLog as a dependency of the generated file. "framework-statsd.stubs.module_lib", + "androidx.annotation_annotation", ], apex_available: [ "com.android.mediaprovider", diff --git a/pdf/framework/api/current.txt b/pdf/framework/api/current.txt index 3715cbdef..feef20bcf 100644 --- a/pdf/framework/api/current.txt +++ b/pdf/framework/api/current.txt @@ -80,23 +80,25 @@ package android.graphics.pdf.component { @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_text_annotations") public final class FreeTextAnnotation extends android.graphics.pdf.component.PdfAnnotation { ctor public FreeTextAnnotation(@NonNull android.graphics.RectF, @NonNull String); method @ColorInt public int getBackgroundColor(); + method @NonNull public android.graphics.RectF getBounds(); method @ColorInt public int getTextColor(); method @NonNull public String getTextContent(); method public void setBackgroundColor(@ColorInt int); + method public void setBounds(@NonNull android.graphics.RectF); method public void setTextColor(@ColorInt int); method public void setTextContent(@NonNull String); } @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_annotations") public final class HighlightAnnotation extends android.graphics.pdf.component.PdfAnnotation { - ctor public HighlightAnnotation(@NonNull android.graphics.RectF); + ctor public HighlightAnnotation(@NonNull java.util.List<android.graphics.RectF>); + method @NonNull public java.util.List<android.graphics.RectF> getBounds(); method @ColorInt public int getColor(); + method public void setBounds(@NonNull java.util.List<android.graphics.RectF>); method public void setColor(@ColorInt int); } @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_annotations") public abstract class PdfAnnotation { - method @NonNull public android.graphics.RectF getBounds(); method public int getPdfAnnotationType(); - method public void setBounds(@NonNull android.graphics.RectF); } @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_annotations") public final class PdfAnnotationType { @@ -119,6 +121,13 @@ package android.graphics.pdf.component { method public void transform(float, float, float, float, float, float); } + @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_page_objects") public final class PdfPageObjectRenderMode { + field public static final int FILL = 0; // 0x0 + field public static final int FILL_STROKE = 2; // 0x2 + field public static final int STROKE = 1; // 0x1 + field public static final int UNKNOWN = -1; // 0xffffffff + } + @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_page_objects") public final class PdfPageObjectType { method public static boolean isValidType(int); field public static final int IMAGE = 3; // 0x3 @@ -129,36 +138,58 @@ package android.graphics.pdf.component { @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_page_objects") public final class PdfPagePathObject extends android.graphics.pdf.component.PdfPageObject { ctor public PdfPagePathObject(@NonNull android.graphics.Path); - method @Nullable public android.graphics.Color getFillColor(); - method @Nullable public android.graphics.Color getStrokeColor(); + method @ColorInt public int getFillColor(); + method public int getRenderMode(); + method @ColorInt public int getStrokeColor(); method public float getStrokeWidth(); - method public void setFillColor(@Nullable android.graphics.Color); - method public void setStrokeColor(@Nullable android.graphics.Color); + method public void setFillColor(@ColorInt int); + method public void setRenderMode(int); + method public void setStrokeColor(@ColorInt int); method public void setStrokeWidth(float); method @NonNull public android.graphics.Path toPath(); } @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_text_objects") public final class PdfPageTextObject extends android.graphics.pdf.component.PdfPageObject { - ctor public PdfPageTextObject(@NonNull String, @NonNull android.graphics.Typeface, float); - method @Nullable public android.graphics.Color getFillColor(); + ctor public PdfPageTextObject(@NonNull String, @NonNull android.graphics.pdf.component.PdfPageTextObjectFont, float); + method @ColorInt public int getFillColor(); + method @NonNull public android.graphics.pdf.component.PdfPageTextObjectFont getFont(); method public float getFontSize(); - method @NonNull public android.graphics.Color getStrokeColor(); + method public int getRenderMode(); + method @ColorInt public int getStrokeColor(); method public float getStrokeWidth(); method @NonNull public String getText(); - method @NonNull public android.graphics.Typeface getTypeface(); - method public void setFillColor(@Nullable android.graphics.Color); - method public void setFontSize(float); - method public void setStrokeColor(@NonNull android.graphics.Color); + method public void setFillColor(@ColorInt int); + method public void setRenderMode(int); + method public void setStrokeColor(@ColorInt int); method public void setStrokeWidth(float); method public void setText(@NonNull String); - method public void setTypeface(@NonNull android.graphics.Typeface); + } + + @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_text_objects") public class PdfPageTextObjectFont { + ctor public PdfPageTextObjectFont(int, boolean, boolean); + ctor public PdfPageTextObjectFont(@NonNull android.graphics.pdf.component.PdfPageTextObjectFont); + method public int getFontFamily(); + method public boolean isBold(); + method public boolean isItalic(); + method public void setBold(boolean); + method public void setFontFamily(int); + method public void setItalic(boolean); + } + + @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_text_objects") public class PdfPageTextObjectFontFamily { + field public static final int COURIER = 0; // 0x0 + field public static final int HELVETICA = 1; // 0x1 + field public static final int SYMBOL = 2; // 0x2 + field public static final int TIMES_NEW_ROMAN = 3; // 0x3 } @FlaggedApi("android.graphics.pdf.flags.enable_edit_pdf_stamp_annotations") public final class StampAnnotation extends android.graphics.pdf.component.PdfAnnotation { ctor public StampAnnotation(@NonNull android.graphics.RectF); method public void addObject(@NonNull android.graphics.pdf.component.PdfPageObject); + method @NonNull public android.graphics.RectF getBounds(); method @NonNull public java.util.List<android.graphics.pdf.component.PdfPageObject> getObjects(); method public void removeObject(@IntRange(from=0) int); + method public void setBounds(@NonNull android.graphics.RectF); } } diff --git a/pdf/framework/java/android/graphics/pdf/PdfRendererPreV.java b/pdf/framework/java/android/graphics/pdf/PdfRendererPreV.java index d5081ae98..2ab35f1f3 100644 --- a/pdf/framework/java/android/graphics/pdf/PdfRendererPreV.java +++ b/pdf/framework/java/android/graphics/pdf/PdfRendererPreV.java @@ -768,6 +768,11 @@ public final class PdfRendererPreV implements AutoCloseable { /** * Update the given {@link PdfPageObject} to the page. * <p> + * Note: This method only updates the parameters of the PageObject whose setters + * are available. Attempting to update fields with no corresponding setters will + * have no effect. + * + * <p> * {@link PdfRenderer#write} needs to be called to get the updated PDF stream after calling * this method. {@link PdfRenderer.Page} instance can be closed before calling * {@link PdfRenderer#write}. diff --git a/pdf/framework/java/android/graphics/pdf/component/FreeTextAnnotation.java b/pdf/framework/java/android/graphics/pdf/component/FreeTextAnnotation.java index 32d0762f6..303dd3ff0 100644 --- a/pdf/framework/java/android/graphics/pdf/component/FreeTextAnnotation.java +++ b/pdf/framework/java/android/graphics/pdf/component/FreeTextAnnotation.java @@ -22,6 +22,7 @@ import android.annotation.NonNull; import android.graphics.Color; import android.graphics.RectF; import android.graphics.pdf.flags.Flags; +import android.graphics.pdf.utils.Preconditions; /** * Represents a free text annotation in a PDF document. @@ -38,6 +39,7 @@ import android.graphics.pdf.flags.Flags; */ @FlaggedApi(Flags.FLAG_ENABLE_EDIT_PDF_TEXT_ANNOTATIONS) public final class FreeTextAnnotation extends PdfAnnotation { + @NonNull private RectF mBounds; @NonNull private String mTextContent; private @ColorInt int mTextColor; private @ColorInt int mBackgroundColor; @@ -51,13 +53,34 @@ public final class FreeTextAnnotation extends PdfAnnotation { * @param textContent The text content of the annotation */ public FreeTextAnnotation(@NonNull RectF bounds, @NonNull String textContent) { - super(PdfAnnotationType.FREETEXT, bounds); + super(PdfAnnotationType.FREETEXT); + this.mBounds = bounds; this.mTextContent = textContent; this.mTextColor = Color.BLACK; this.mBackgroundColor = Color.WHITE; } /** + * Sets the bounding rectangle of the freetext annotation. + * + * @param bounds The new bounding rectangle. + * @throws NullPointerException if given bounds is null + */ + public void setBounds(@NonNull RectF bounds) { + Preconditions.checkNotNull(bounds, "Bounds should not be null"); + this.mBounds = bounds; + } + + /** + * Returns the bounding rectangle of the freetext annotation. + * + * @return The bounding rectangle. + */ + @NonNull public RectF getBounds() { + return mBounds; + } + + /** * Sets the text content of the annotation. * * @param text The new text content. diff --git a/pdf/framework/java/android/graphics/pdf/component/HighlightAnnotation.java b/pdf/framework/java/android/graphics/pdf/component/HighlightAnnotation.java index d3f5d27af..0f2661735 100644 --- a/pdf/framework/java/android/graphics/pdf/component/HighlightAnnotation.java +++ b/pdf/framework/java/android/graphics/pdf/component/HighlightAnnotation.java @@ -22,6 +22,9 @@ import android.annotation.NonNull; import android.graphics.Color; import android.graphics.RectF; import android.graphics.pdf.flags.Flags; +import android.graphics.pdf.utils.Preconditions; + +import java.util.List; /** * Represents a highlight annotation in a PDF document. @@ -31,6 +34,7 @@ import android.graphics.pdf.flags.Flags; */ @FlaggedApi(Flags.FLAG_ENABLE_EDIT_PDF_ANNOTATIONS) public final class HighlightAnnotation extends PdfAnnotation { + @NonNull private List<RectF> mBounds; private @ColorInt int mColor; /** @@ -40,12 +44,36 @@ public final class HighlightAnnotation extends PdfAnnotation { * * @param bounds The bounding rectangle of the annotation. */ - public HighlightAnnotation(@NonNull RectF bounds) { - super(PdfAnnotationType.HIGHLIGHT, bounds); + public HighlightAnnotation(@NonNull List<RectF> bounds) { + super(PdfAnnotationType.HIGHLIGHT); + this.mBounds = bounds; this.mColor = Color.YELLOW; } /** + * Sets the bounding rectangles of the highlight annotation. Each rect in the list mBounds + * represent an absolute position of highlight inside the page of the document + * + * @param bounds The new bounding rectangles. + * @throws NullPointerException if given bounds is null + * @throws IllegalArgumentException if the given bounds list is empty + */ + public void setBounds(@NonNull List<RectF> bounds) { + Preconditions.checkNotNull(bounds, "Bounds should not be null"); + Preconditions.checkArgument(!bounds.isEmpty(), "Bounds should not be empty"); + this.mBounds = bounds; + } + + /** + * Returns the bounding rectangles of the highlight annotation. + * + * @return The bounding rectangles. + */ + @NonNull public List<RectF> getBounds() { + return mBounds; + } + + /** * Returns the highlight color of the annotation. * * @return The highlight color. diff --git a/pdf/framework/java/android/graphics/pdf/component/PdfAnnotation.java b/pdf/framework/java/android/graphics/pdf/component/PdfAnnotation.java index 0d510f277..fb4772218 100644 --- a/pdf/framework/java/android/graphics/pdf/component/PdfAnnotation.java +++ b/pdf/framework/java/android/graphics/pdf/component/PdfAnnotation.java @@ -17,8 +17,6 @@ package android.graphics.pdf.component; import android.annotation.FlaggedApi; -import android.annotation.NonNull; -import android.graphics.RectF; import android.graphics.pdf.flags.Flags; import android.graphics.pdf.utils.Preconditions; @@ -30,23 +28,19 @@ import android.graphics.pdf.utils.Preconditions; */ @FlaggedApi(Flags.FLAG_ENABLE_EDIT_PDF_ANNOTATIONS) public abstract class PdfAnnotation { - private int mType; - @NonNull private RectF mBounds; + private final int mType; /** * Creates a new PDF annotation with the specified type and bounds. * - * @param type The type of annotation. See {@link PdfAnnotationType} for possible values. - * @param bounds The bounding rectangle of the annotation. + * @param type The type of annotation. See {@link PdfAnnotationType} for possible values. */ - PdfAnnotation(@PdfAnnotationType.Type int type, @NonNull RectF bounds) { - Preconditions.checkNotNull(bounds, "Bounds cannot be null"); + PdfAnnotation(@PdfAnnotationType.Type int type) { Preconditions.checkArgument(type == PdfAnnotationType.UNKNOWN || type == PdfAnnotationType.FREETEXT || type == PdfAnnotationType.HIGHLIGHT || type == PdfAnnotationType.STAMP, "Invalid Annotation Type"); this.mType = type; - this.mBounds = bounds; } /** @@ -57,23 +51,4 @@ public abstract class PdfAnnotation { public @PdfAnnotationType.Type int getPdfAnnotationType() { return mType; } - - /** - * Sets the bounding rectangle of the annotation. - * - * @param bounds The new bounding rectangle. - */ - public void setBounds(@NonNull RectF bounds) { - this.mBounds = bounds; - } - - /** - * Returns the bounding rectangle of the annotation. - * - * @return The bounding rectangle. - */ - @NonNull public RectF getBounds() { - return mBounds; - } - } diff --git a/pdf/framework/java/android/graphics/pdf/component/PdfPageObjectRenderMode.java b/pdf/framework/java/android/graphics/pdf/component/PdfPageObjectRenderMode.java new file mode 100644 index 000000000..c8603bb85 --- /dev/null +++ b/pdf/framework/java/android/graphics/pdf/component/PdfPageObjectRenderMode.java @@ -0,0 +1,72 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.graphics.pdf.component; + +import android.annotation.FlaggedApi; +import android.annotation.IntDef; +import android.graphics.pdf.flags.Flags; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Defines rendering modes for PDF page objects (fill, stroke, etc.). + * + * <p> + * This final class provides constants for specifying how graphical elements + * are rendered on a PDF page. It cannot be instantiated. + * + * <p> + * Rendering modes: + * <ul> + * <li>{@link #UNKNOWN}: Unknown mode. + * <li>{@link #FILL}: Fill object. + * <li>{@link #STROKE}: Stroke object. + * <li>{@link #FILL_STROKE}: Fill and stroke object. </ul> + */ +@FlaggedApi(Flags.FLAG_ENABLE_EDIT_PDF_PAGE_OBJECTS) +public final class PdfPageObjectRenderMode { + // Private constructor + private PdfPageObjectRenderMode() { + } + + /** + * Unknown Mode + */ + public static final int UNKNOWN = -1; + + /** + * Fill Mode + */ + public static final int FILL = 0; + + /** + * Stroke Mode + */ + public static final int STROKE = 1; + + /** + * FillStroke Mode + */ + public static final int FILL_STROKE = 2; + + /** @hide */ + @Retention(RetentionPolicy.SOURCE) + @IntDef({UNKNOWN, FILL, STROKE, FILL_STROKE}) + public @interface Type { + } +} diff --git a/pdf/framework/java/android/graphics/pdf/component/PdfPagePathObject.java b/pdf/framework/java/android/graphics/pdf/component/PdfPagePathObject.java index 08ed9ee3a..d19c37ae9 100644 --- a/pdf/framework/java/android/graphics/pdf/component/PdfPagePathObject.java +++ b/pdf/framework/java/android/graphics/pdf/component/PdfPagePathObject.java @@ -16,10 +16,9 @@ package android.graphics.pdf.component; +import android.annotation.ColorInt; import android.annotation.FlaggedApi; import android.annotation.NonNull; -import android.annotation.Nullable; -import android.graphics.Color; import android.graphics.Path; import android.graphics.pdf.flags.Flags; @@ -31,9 +30,10 @@ import android.graphics.pdf.flags.Flags; @FlaggedApi(Flags.FLAG_ENABLE_EDIT_PDF_PAGE_OBJECTS) public final class PdfPagePathObject extends PdfPageObject { private final Path mPath; - private Color mStrokeColor; + private @ColorInt int mStrokeColor; private float mStrokeWidth; - private Color mFillColor; + private @ColorInt int mFillColor; + private @PdfPageObjectRenderMode.Type int mRenderMode; /** * Constructor for the PdfPagePathObject. Sets the object type @@ -42,6 +42,7 @@ public final class PdfPagePathObject extends PdfPageObject { public PdfPagePathObject(@NonNull Path path) { super(PdfPageObjectType.PATH); this.mPath = path; + this.mRenderMode = PdfPageObjectRenderMode.FILL; } /** @@ -64,8 +65,7 @@ public final class PdfPagePathObject extends PdfPageObject { * * @return The stroke color of the object. */ - @Nullable - public Color getStrokeColor() { + public @ColorInt int getStrokeColor() { return mStrokeColor; } @@ -74,7 +74,7 @@ public final class PdfPagePathObject extends PdfPageObject { * * @param strokeColor The stroke color of the object. */ - public void setStrokeColor(@Nullable Color strokeColor) { + public void setStrokeColor(@ColorInt int strokeColor) { this.mStrokeColor = strokeColor; } @@ -101,8 +101,7 @@ public final class PdfPagePathObject extends PdfPageObject { * * @return The fill color of the object. */ - @Nullable - public Color getFillColor() { + public @ColorInt int getFillColor() { return mFillColor; } @@ -111,8 +110,27 @@ public final class PdfPagePathObject extends PdfPageObject { * * @param fillColor The fill color of the object. */ - public void setFillColor(@Nullable Color fillColor) { + public void setFillColor(@ColorInt int fillColor) { this.mFillColor = fillColor; } + /** + * Returns the {@link PdfPageObjectRenderMode} of the object. + * Returns {@link PdfPageObjectRenderMode#FILL} by default + * if {@link PdfPagePathObject#mRenderMode} is not set. + * + * @return The {@link PdfPageObjectRenderMode} of the object. + */ + public @PdfPageObjectRenderMode.Type int getRenderMode() { + return mRenderMode; + } + + /** + * Sets the {@link PdfPageObjectRenderMode} of the object. + * + * @param renderMode The {@link PdfPageObjectRenderMode} to be set. + */ + public void setRenderMode(@PdfPageObjectRenderMode.Type int renderMode) { + mRenderMode = renderMode; + } } diff --git a/pdf/framework/java/android/graphics/pdf/component/PdfPageTextObject.java b/pdf/framework/java/android/graphics/pdf/component/PdfPageTextObject.java index aa311fd5d..17662308e 100644 --- a/pdf/framework/java/android/graphics/pdf/component/PdfPageTextObject.java +++ b/pdf/framework/java/android/graphics/pdf/component/PdfPageTextObject.java @@ -16,11 +16,9 @@ package android.graphics.pdf.component; +import android.annotation.ColorInt; import android.annotation.FlaggedApi; import android.annotation.NonNull; -import android.annotation.Nullable; -import android.graphics.Color; -import android.graphics.Typeface; import android.graphics.pdf.flags.Flags; /** @@ -30,24 +28,29 @@ import android.graphics.pdf.flags.Flags; @FlaggedApi(Flags.FLAG_ENABLE_EDIT_PDF_TEXT_OBJECTS) public final class PdfPageTextObject extends PdfPageObject { private String mText; - private Typeface mTypeface; - private float mFontSize; - private Color mStrokeColor = new Color(); // Default is opaque black in the sRGB color space. + private final PdfPageTextObjectFont mFont; + private final float mFontSize; + private @ColorInt int mStrokeColor; private float mStrokeWidth = 1.0f; - private Color mFillColor; + private @ColorInt int mFillColor; + private @PdfPageObjectRenderMode.Type int mRenderMode; /** * Constructor for the PdfPageTextObject. * Sets the object type to TEXT and initializes the text color to black. * - * @param typeface The font of the text. + * @param font The font of the text. * @param fontSize The font size of the text. */ - public PdfPageTextObject(@NonNull String text, @NonNull Typeface typeface, float fontSize) { + public PdfPageTextObject(@NonNull String text, @NonNull PdfPageTextObjectFont font, + float fontSize) { super(PdfPageObjectType.TEXT); this.mText = text; - this.mTypeface = typeface; + this.mFont = font; this.mFontSize = fontSize; + if (Flags.enableEditPdfPageObjects()) { + this.mRenderMode = PdfPageObjectRenderMode.FILL; + } } /** @@ -79,31 +82,31 @@ public final class PdfPageTextObject extends PdfPageObject { } /** - * Sets the font size of the object. + * Returns the font of the text. * - * @param fontSize The font size to set. + * @return A copy of the font object. */ - public void setFontSize(float fontSize) { - mFontSize = fontSize; + @NonNull + public PdfPageTextObjectFont getFont() { + return new PdfPageTextObjectFont(mFont); } /** - * Returns the stroke color of the object. + * Returns the fill color of the object. * - * @return The stroke color of the object. + * @return The fill color of the object. */ - @NonNull - public Color getStrokeColor() { - return mStrokeColor; + public @ColorInt int getFillColor() { + return mFillColor; } /** - * Sets the stroke color of the object. + * Sets the fill color of the object. * - * @param strokeColor The stroke color of the object. + * @param fillColor The fill color of the object. */ - public void setStrokeColor(@NonNull Color strokeColor) { - this.mStrokeColor = strokeColor; + public void setFillColor(@ColorInt int fillColor) { + this.mFillColor = fillColor; } /** @@ -121,44 +124,42 @@ public final class PdfPageTextObject extends PdfPageObject { * @param strokeWidth The stroke width of the object. */ public void setStrokeWidth(float strokeWidth) { - this.mStrokeWidth = strokeWidth; + mStrokeWidth = strokeWidth; } /** - * Returns the font of the text. + * Returns the stroke color of the object. * - * @return The font. + * @return The stroke color of the object. */ - @NonNull - public Typeface getTypeface() { - return mTypeface; + public @ColorInt int getStrokeColor() { + return mStrokeColor; } /** - * Sets the font of the text. + * Sets the stroke color of the object. * - * @param typeface The font to set. + * @param strokeColor The stroke color of the object. */ - public void setTypeface(@NonNull Typeface typeface) { - this.mTypeface = typeface; + public void setStrokeColor(@ColorInt int strokeColor) { + this.mStrokeColor = strokeColor; } /** - * Returns the fill color of the object. + * Returns the render mode of the object. * - * @return The fill color of the object. + * @return The render mode of the object. */ - @Nullable - public Color getFillColor() { - return mFillColor; + public @PdfPageObjectRenderMode.Type int getRenderMode() { + return mRenderMode; } /** - * Sets the fill color of the object. + * Sets the render mode of the object. * - * @param fillColor The fill color of the object. + * @param renderMode The render mode to be set. */ - public void setFillColor(@Nullable Color fillColor) { - this.mFillColor = fillColor; + public void setRenderMode(@PdfPageObjectRenderMode.Type int renderMode) { + mRenderMode = renderMode; } } diff --git a/pdf/framework/java/android/graphics/pdf/component/PdfPageTextObjectFont.java b/pdf/framework/java/android/graphics/pdf/component/PdfPageTextObjectFont.java new file mode 100644 index 000000000..c38baa156 --- /dev/null +++ b/pdf/framework/java/android/graphics/pdf/component/PdfPageTextObjectFont.java @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.graphics.pdf.component; + +import android.annotation.FlaggedApi; +import android.annotation.NonNull; +import android.graphics.pdf.flags.Flags; + +@FlaggedApi(Flags.FLAG_ENABLE_EDIT_PDF_TEXT_OBJECTS) +public class PdfPageTextObjectFont { + private @PdfPageTextObjectFontFamily.Type int mFontFamily; + private boolean mIsBold; + private boolean mIsItalic; + + public PdfPageTextObjectFont(@PdfPageTextObjectFontFamily.Type int fontFamily, + boolean isBold, boolean isItalic) { + mFontFamily = fontFamily; + mIsBold = isBold; + mIsItalic = isItalic; + } + + public PdfPageTextObjectFont(@NonNull PdfPageTextObjectFont font) { + this.mFontFamily = font.getFontFamily(); + this.mIsBold = font.isBold(); + this.mIsItalic = font.isItalic(); + } + + /** + * Returns the font-family which is of type {@link PdfPageTextObjectFontFamily} + * + * @return The font-family. + */ + public @PdfPageTextObjectFontFamily.Type int getFontFamily() { + return mFontFamily; + } + + /** + * Set the font family of the object. + * + * @param fontFamily The font family to be set. + */ + public void setFontFamily(@PdfPageTextObjectFontFamily.Type int fontFamily) { + mFontFamily = fontFamily; + } + + /** + * Determines if the text is bold. + * + * @return true if the text is bold, false otherwise. + */ + public boolean isBold() { + return mIsBold; + } + + /** + * Sets whether the text should be bold or not. + * + * @param bold true if the text should be bold, false otherwise. + */ + public void setBold(boolean bold) { + mIsBold = bold; + } + + /** + * Determines if the text is italic. + * + * @return true if the text is italic, false otherwise. + */ + public boolean isItalic() { + return mIsItalic; + } + + /** + * Set whether the text should be italic or not. + * + * @param italic true if the text should be italic, false otherwise. + */ + public void setItalic(boolean italic) { + mIsItalic = italic; + } +} diff --git a/pdf/framework/java/android/graphics/pdf/component/PdfPageTextObjectFontFamily.java b/pdf/framework/java/android/graphics/pdf/component/PdfPageTextObjectFontFamily.java new file mode 100644 index 000000000..4a40f3df5 --- /dev/null +++ b/pdf/framework/java/android/graphics/pdf/component/PdfPageTextObjectFontFamily.java @@ -0,0 +1,60 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package android.graphics.pdf.component; + +import android.annotation.FlaggedApi; +import android.annotation.IntDef; +import android.graphics.pdf.flags.Flags; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * The class holds the set of font families supported by {@link PdfPageTextObject}. + * The specified font families are standard font families defined + * in the PDF Spec 1.7 - Page 146. + */ +@FlaggedApi(Flags.FLAG_ENABLE_EDIT_PDF_TEXT_OBJECTS) +public class PdfPageTextObjectFontFamily { + private PdfPageTextObjectFontFamily() {} + + /** + * Courier Font + */ + public static final int COURIER = 0; + + /** + * Helvetica Font + */ + public static final int HELVETICA = 1; + + /** + * Symbol Font (Note: Renders only symbols) + */ + public static final int SYMBOL = 2; + + /** + * TimesNewRoman Font + */ + public static final int TIMES_NEW_ROMAN = 3; + + /** @hide */ + @Retention(RetentionPolicy.SOURCE) + @IntDef({COURIER, HELVETICA, SYMBOL, TIMES_NEW_ROMAN}) + public @interface Type { + } +} diff --git a/pdf/framework/java/android/graphics/pdf/component/StampAnnotation.java b/pdf/framework/java/android/graphics/pdf/component/StampAnnotation.java index 2663cd869..3f3279415 100644 --- a/pdf/framework/java/android/graphics/pdf/component/StampAnnotation.java +++ b/pdf/framework/java/android/graphics/pdf/component/StampAnnotation.java @@ -35,19 +35,43 @@ import java.util.List; */ @FlaggedApi(Flags.FLAG_ENABLE_EDIT_PDF_STAMP_ANNOTATIONS) public final class StampAnnotation extends PdfAnnotation { + @NonNull private RectF mBounds; @NonNull private List<PdfPageObject> mObjects; /** - * Creates a new stamp annotation with the specified bounds + * Creates a new stamp annotation with the specified bounds. + * <p> + * The list of page objects inside the stamp annotation will be empty by default * * @param bounds The bounding rectangle of the annotation. */ public StampAnnotation(@NonNull RectF bounds) { - super(PdfAnnotationType.STAMP, bounds); + super(PdfAnnotationType.STAMP); + mBounds = bounds; mObjects = new ArrayList<>(); } /** + * Sets the bounding rectangle of the stamp annotation. + * + * @param bounds The new bounding rectangle. + * @throws NullPointerException if given bounds is null + */ + public void setBounds(@NonNull RectF bounds) { + Preconditions.checkNotNull(bounds, "Bounds should not be null"); + this.mBounds = bounds; + } + + /** + * Returns the bounding rectangle of the stamp annotation. + * + * @return The bounding rectangle. + */ + @NonNull public RectF getBounds() { + return mBounds; + } + + /** * Adds a PDF page object to the stamp annotation. * <p> * The page object should be a path, text or an image. diff --git a/pdf/framework/libs/pdfClient/Android.bp b/pdf/framework/libs/pdfClient/Android.bp index dad336fff..95272f3c5 100644 --- a/pdf/framework/libs/pdfClient/Android.bp +++ b/pdf/framework/libs/pdfClient/Android.bp @@ -108,7 +108,7 @@ cc_test { static_libs: [ "libbase_ndk", - "libpdfium_static", + "libpdfium_static_android_r_compatible", ], shared_libs: [ @@ -116,7 +116,6 @@ cc_test { "libjnigraphics", "libdl", "libft2", - "libicu", "libjpeg", "libz", ], diff --git a/pdf/framework/libs/pdfClient/annotation.cc b/pdf/framework/libs/pdfClient/annotation.cc index b1f12c3f0..653f62ac6 100644 --- a/pdf/framework/libs/pdfClient/annotation.cc +++ b/pdf/framework/libs/pdfClient/annotation.cc @@ -35,6 +35,20 @@ std::vector<PageObject*> StampAnnotation::GetObjects() const { return page_objects; } +bool updateExistingBounds(FPDF_ANNOTATION fpdf_annot, size_t num_bounds, + std::vector<Rectangle_f> bounds) { + for (auto bound_index = 0; bound_index < num_bounds; bound_index++) { + Rectangle_f rect = bounds[bound_index]; + FS_QUADPOINTSF quad_points = {rect.left, rect.top, rect.right, rect.top, + rect.left, rect.bottom, rect.right, rect.bottom}; + if (!FPDFAnnot_SetAttachmentPoints(fpdf_annot, bound_index, &quad_points)) { + LOGD("Failed to update the bounds of highlight annotation"); + return false; + } + } + return true; +} + bool StampAnnotation::PopulateFromPdfiumInstance(FPDF_ANNOTATION fpdf_annot, FPDF_PAGE page) { int num_of_objects = FPDFAnnot_GetObjectCount(fpdf_annot); @@ -207,16 +221,32 @@ bool HighlightAnnotation::UpdatePdfiumInstance(FPDF_ANNOTATION fpdf_annot, FPDF_ return false; } - Rectangle_f annotation_bounds = this->GetBounds(); - FS_RECTF rect; - rect.left = annotation_bounds.left; - rect.bottom = annotation_bounds.bottom; - rect.right = annotation_bounds.right; - rect.top = annotation_bounds.top; - - if (!FPDFAnnot_SetRect(fpdf_annot, &rect)) { - LOGE("Highlight Annotation bounds couldn't be updated"); - return false; + auto old_num_bounds = FPDFAnnot_CountAttachmentPoints(fpdf_annot); + std::vector<Rectangle_f> bounds = GetBounds(); + auto new_num_bounds = bounds.size(); + + if (old_num_bounds == new_num_bounds) { + if (!updateExistingBounds(fpdf_annot, old_num_bounds, bounds)) return false; + } else if (old_num_bounds < new_num_bounds) { + if (!updateExistingBounds(fpdf_annot, old_num_bounds, bounds)) return false; + for (auto bound_index = old_num_bounds; bound_index < new_num_bounds; bound_index++) { + Rectangle_f rect = bounds[bound_index]; + FS_QUADPOINTSF quad_points = {rect.left, rect.top, rect.right, rect.top, + rect.left, rect.bottom, rect.right, rect.bottom}; + if (!FPDFAnnot_AppendAttachmentPoints(fpdf_annot, &quad_points)) { + LOGD("Failed to update bounds of the highlight annotation"); + return false; + } + } + } else { + if (!updateExistingBounds(fpdf_annot, new_num_bounds, bounds)) return false; + for (auto bound_index = new_num_bounds; bound_index < old_num_bounds; bound_index++) { + FS_QUADPOINTSF quad_points = {0, 0, 0, 0, 0, 0, 0, 0}; + if (!FPDFAnnot_SetAttachmentPoints(fpdf_annot, bound_index, &quad_points)) { + LOGD("Failed to update bounds of the highlight annotation"); + return false; + } + } } Color new_color = this->GetColor(); @@ -318,4 +348,5 @@ bool FreeTextAnnotation::UpdatePdfiumInstance(FPDF_ANNOTATION fpdf_annot, FPDF_D return true; } + } // namespace pdfClient
\ No newline at end of file diff --git a/pdf/framework/libs/pdfClient/annotation.h b/pdf/framework/libs/pdfClient/annotation.h index 4c8cbc42b..518c45c64 100644 --- a/pdf/framework/libs/pdfClient/annotation.h +++ b/pdf/framework/libs/pdfClient/annotation.h @@ -34,14 +34,11 @@ class Annotation { public: enum class Type { UNKNOWN = 0, FreeText = 1, Highlight = 2, Stamp = 3 }; - Annotation(Type type, const Rectangle_f& bounds) : type_(type), bounds_(bounds) {} + Annotation(Type type) : type_(type) {} virtual ~Annotation() = default; Type GetType() const { return type_; } - Rectangle_f GetBounds() const { return bounds_; } - void SetBounds(Rectangle_f bounds) { bounds_ = bounds; } - virtual bool PopulateFromPdfiumInstance(FPDF_ANNOTATION fpdf_annot, FPDF_PAGE page) = 0; virtual ScopedFPDFAnnotation CreatePdfiumInstance(FPDF_DOCUMENT document, FPDF_PAGE page) = 0; virtual bool UpdatePdfiumInstance(FPDF_ANNOTATION fpdf_annot, FPDF_DOCUMENT document, @@ -49,7 +46,6 @@ class Annotation { private: Type type_; - Rectangle_f bounds_; }; // This class represents a stamp annotation on a page of the pdf document. It doesn't take the @@ -57,7 +53,10 @@ class Annotation { // underlying pdfium page objects class StampAnnotation : public Annotation { public: - StampAnnotation(const Rectangle_f& bounds) : Annotation(Type::Stamp, bounds) {} + StampAnnotation(const Rectangle_f& bounds) : Annotation(Type::Stamp) { bounds_ = bounds; } + + Rectangle_f GetBounds() const { return bounds_; } + void SetBounds(Rectangle_f bounds) { bounds_ = bounds; } // Return a const reference to the list // Stamp annotation will have the ownership of the page objects inside it @@ -80,12 +79,18 @@ class StampAnnotation : public Annotation { FPDF_PAGE page) override; private: + Rectangle_f bounds_; std::vector<std::unique_ptr<PageObject>> pageObjects_; }; class HighlightAnnotation : public Annotation { public: - HighlightAnnotation(const Rectangle_f& bounds) : Annotation(Type::Highlight, bounds) {} + HighlightAnnotation(const std::vector<Rectangle_f>& bounds) : Annotation(Type::Highlight) { + bounds_ = bounds; + } + + std::vector<Rectangle_f> GetBounds() const { return bounds_; } + void SetBounds(std::vector<Rectangle_f> bounds) { bounds_ = bounds; } Color GetColor() const { return color_; } void SetColor(Color color) { color_ = color; } @@ -96,13 +101,17 @@ class HighlightAnnotation : public Annotation { FPDF_PAGE page) override; private: + std::vector<Rectangle_f> bounds_; Color color_; }; class FreeTextAnnotation : public Annotation { public: static constexpr const char* kContents = "Contents"; - FreeTextAnnotation(const Rectangle_f& bounds) : Annotation(Type::FreeText, bounds) {} + FreeTextAnnotation(const Rectangle_f& bounds) : Annotation(Type::FreeText) { bounds_ = bounds; } + + Rectangle_f GetBounds() const { return bounds_; } + void SetBounds(Rectangle_f bounds) { bounds_ = bounds; } std::wstring GetTextContent() const { return text_content_; } void SetTextContent(std::wstring textContent) { text_content_ = textContent; } @@ -119,6 +128,7 @@ class FreeTextAnnotation : public Annotation { FPDF_PAGE page) override; private: + Rectangle_f bounds_; std::wstring text_content_; Color text_color_; Color background_color_; diff --git a/pdf/framework/libs/pdfClient/image_object.cc b/pdf/framework/libs/pdfClient/image_object.cc index 80de7a55e..69dedb0bc 100644 --- a/pdf/framework/libs/pdfClient/image_object.cc +++ b/pdf/framework/libs/pdfClient/image_object.cc @@ -26,6 +26,23 @@ namespace pdfClient { +BitmapFormat GetBitmapFormat(int bitmap_format) { + switch (bitmap_format) { + case FPDFBitmap_BGR: { + return BitmapFormat::BGR; + } + case FPDFBitmap_BGRA: { + return BitmapFormat::BGRA; + } + case FPDFBitmap_BGRx: { + return BitmapFormat::BGRx; + } + default: { + return BitmapFormat::Unknown; + } + } +} + ImageObject::ImageObject() : PageObject(Type::Image) {} ScopedFPDFPageObject ImageObject::CreateFPDFInstance(FPDF_DOCUMENT document, FPDF_PAGE page) { @@ -57,35 +74,46 @@ bool ImageObject::UpdateFPDFInstance(FPDF_PAGEOBJECT image_object, FPDF_PAGE pag } // Set the updated matrix. - if (!FPDFPageObj_SetMatrix(image_object, reinterpret_cast<FS_MATRIX*>(&matrix_))) { + if (!SetDeviceToPageMatrix(image_object, page)) { return false; } + // Set the updated dimensions. width_ = FPDFBitmap_GetWidth(bitmap_.get()); height_ = FPDFBitmap_GetHeight(bitmap_.get()); + // Set the updated bitmap format. + bitmap_format_ = GetBitmapFormat(FPDFBitmap_GetFormat(bitmap_.get())); + return true; } bool ImageObject::PopulateFromFPDFInstance(FPDF_PAGEOBJECT image_object, FPDF_PAGE page) { - // Get Bitmap + // Get bitmap. bitmap_ = ScopedFPDFBitmap(FPDFImageObj_GetBitmap(image_object)); if (bitmap_.get() == nullptr) { return false; } - // Get Matrix - if (!FPDFPageObj_GetMatrix(image_object, reinterpret_cast<FS_MATRIX*>(&matrix_))) { + // Get matrix. + if (!GetPageToDeviceMatrix(image_object, page)) { return false; } + // Get dimensions. width_ = FPDFBitmap_GetWidth(bitmap_.get()); height_ = FPDFBitmap_GetHeight(bitmap_.get()); + // Get bitmap format. + bitmap_format_ = GetBitmapFormat(FPDFBitmap_GetFormat(bitmap_.get())); + if (bitmap_format_ == BitmapFormat::Unknown) { + LOGE("Bitmap format unknown"); + return false; + } return true; } -void* ImageObject::GetBitmapReadableBuffer() const { +void* ImageObject::GetBitmapBuffer() const { return FPDFBitmap_GetBuffer(bitmap_.get()); } diff --git a/pdf/framework/libs/pdfClient/image_object.h b/pdf/framework/libs/pdfClient/image_object.h index 2e3aa9541..e3563ba8e 100644 --- a/pdf/framework/libs/pdfClient/image_object.h +++ b/pdf/framework/libs/pdfClient/image_object.h @@ -27,6 +27,13 @@ typedef unsigned int uint; namespace pdfClient { +enum class BitmapFormat { + Unknown = -1, + BGR, + BGRA, + BGRx, +}; + class ImageObject : public PageObject { public: ImageObject(); @@ -35,12 +42,13 @@ class ImageObject : public PageObject { bool UpdateFPDFInstance(FPDF_PAGEOBJECT image_object, FPDF_PAGE page) override; bool PopulateFromFPDFInstance(FPDF_PAGEOBJECT image_object, FPDF_PAGE page) override; - void* GetBitmapReadableBuffer() const; + void* GetBitmapBuffer() const; ~ImageObject(); - int width_ = 0; - int height_ = 0; + size_t width_ = 0; + size_t height_ = 0; + BitmapFormat bitmap_format_ = BitmapFormat::Unknown; ScopedFPDFBitmap bitmap_; }; diff --git a/pdf/framework/libs/pdfClient/jni_conversion.cc b/pdf/framework/libs/pdfClient/jni_conversion.cc index 202410273..46d5b2d9e 100644 --- a/pdf/framework/libs/pdfClient/jni_conversion.cc +++ b/pdf/framework/libs/pdfClient/jni_conversion.cc @@ -22,10 +22,14 @@ #include "image_object.h" #include "logging.h" #include "rect.h" +#include "text_object.h" using pdfClient::Annotation; +using pdfClient::BitmapFormat; using pdfClient::Color; using pdfClient::Document; +using pdfClient::Font; +using pdfClient::font_names; using pdfClient::FreeTextAnnotation; using pdfClient::HighlightAnnotation; using pdfClient::ICoordinateConverter; @@ -39,6 +43,7 @@ using pdfClient::Rectangle_f; using pdfClient::Rectangle_i; using pdfClient::SelectionBoundary; using pdfClient::StampAnnotation; +using pdfClient::TextObject; using std::string; using std::vector; @@ -60,6 +65,8 @@ static const char* kGotoLinkDestination = "android/graphics/pdf/content/PdfPageGotoLinkContent$Destination"; static const char* kGotoLink = "android/graphics/pdf/content/PdfPageGotoLinkContent"; static const char* kPageObject = "android/graphics/pdf/component/PdfPageObject"; +static const char* kTextFont = "android/graphics/pdf/component/PdfPageTextObjectFont"; +static const char* kTextObject = "android/graphics/pdf/component/PdfPageTextObject"; static const char* kPathObject = "android/graphics/pdf/component/PdfPagePathObject"; static const char* kImageObject = "android/graphics/pdf/component/PdfPageImageObject"; static const char* kStampAnnotation = "android/graphics/pdf/component/StampAnnotation"; @@ -129,8 +136,31 @@ jobject ToJavaString(JNIEnv* env, const std::string& s) { return env->NewStringUTF(s.c_str()); } -jobject ToJavaString(JNIEnv* env, const std::wstring& s) { - return env->NewString((jchar*)s.c_str(), s.length()); +jobject ToJavaString(JNIEnv* env, const std::wstring& ws) { + jsize len = ws.length(); + jchar* jchars = new jchar[len + 1]; // Null Termination + + for (size_t i = 0; i < len; ++i) { + jchars[i] = static_cast<jchar>(ws[i]); + } + jchars[len] = 0; + + jstring result = env->NewString(jchars, len); + + delete[] jchars; + return result; +} + +std::wstring ToNativeWideString(JNIEnv* env, jstring java_string) { + std::wstring value; + + const jchar* raw = env->GetStringChars(java_string, 0); + jsize len = env->GetStringLength(java_string); + + value.assign(raw, raw + len); + + env->ReleaseStringChars(java_string, raw); + return value; } // Copy a C++ vector to a java ArrayList, using the given function to convert. @@ -151,6 +181,22 @@ jobject ToJavaList(JNIEnv* env, const vector<T>& input, return java_list; } +template <class T> +jobject ToJavaList(JNIEnv* env, const vector<T>& input, ICoordinateConverter* converter, + jobject (*ToJavaObject)(JNIEnv* env, const T&, ICoordinateConverter* converter)) { + static jclass arraylist_class = GetPermClassRef(env, kArrayList); + static jmethodID init = env->GetMethodID(arraylist_class, "<init>", "(I)V"); + static jmethodID add = env->GetMethodID(arraylist_class, "add", funcsig("Z", kObject).c_str()); + + jobject java_list = env->NewObject(arraylist_class, init, input.size()); + for (size_t i = 0; i < input.size(); i++) { + jobject java_object = ToJavaObject(env, input[i], converter); + env->CallBooleanMethod(java_list, add, java_object); + env->DeleteLocalRef(java_object); + } + return java_list; +} + // Copy a C++ vector to a java ArrayList, using the given function to convert. template <class T> jobject ToJavaList(JNIEnv* env, const vector<T*>& input, ICoordinateConverter* converter, @@ -405,7 +451,41 @@ jobject ToJavaGotoLinks(JNIEnv* env, const vector<GotoLink>& links) { return ToJavaList(env, links, &ToJavaGotoLink); } -jobject ToJavaBitmap(JNIEnv* env, void* buffer, int width, int height) { +void ConvertBgrToRgba(uint32_t* rgba_pixel_array, uint8_t* bgr_pixel_array, size_t rgba_stride, + size_t bgr_stride, size_t width, size_t height) { + for (size_t y = 0; y < height; y++) { + uint32_t* rgba_row_ptr = rgba_pixel_array + y * (rgba_stride / 4); + uint8_t* bgr_row_ptr = bgr_pixel_array + y * (bgr_stride); + for (size_t x = 0; x < width; x++) { + // Extract BGR components stored. + uint8_t blue = bgr_row_ptr[x * 3]; + uint8_t green = bgr_row_ptr[x * 3 + 1]; + uint8_t red = bgr_row_ptr[x * 3 + 2]; + // Storing java bitmap components RGBA in little-endian. + rgba_row_ptr[x] = (0xFF << 24) | (blue << 16) | (green << 8) | red; + } + } +} + +void ConvertBgraToRgba(uint32_t* rgba_pixel_array, uint8_t* bgra_pixel_array, size_t rgba_stride, + size_t bgra_stride, size_t width, size_t height, bool ignore_alpha) { + for (size_t y = 0; y < height; y++) { + uint32_t* rgba_row_ptr = rgba_pixel_array + y * (rgba_stride / 4); + uint8_t* bgra_row_ptr = bgra_pixel_array + y * (bgra_stride); + for (size_t x = 0; x < width; x++) { + // Extract BGR components and determine alpha based on ignore_alpha flag. + uint8_t blue = bgra_row_ptr[x * 4]; + uint8_t green = bgra_row_ptr[x * 4 + 1]; + uint8_t red = bgra_row_ptr[x * 4 + 2]; + uint8_t alpha = ignore_alpha ? 0xFF : bgra_row_ptr[x * 4 + 3]; + // Storing java bitmap components RGBA in little-endian. + rgba_row_ptr[x] = (alpha << 24) | (blue << 16) | (green << 8) | red; + } + } +} + +jobject ToJavaBitmap(JNIEnv* env, void* buffer, BitmapFormat bitmap_format, size_t width, + size_t height, size_t native_stride) { // Find Java Bitmap class static jclass bitmap_class = GetPermClassRef(env, kBitmap); @@ -424,50 +504,46 @@ jobject ToJavaBitmap(JNIEnv* env, void* buffer, int width, int height) { jobject java_bitmap = env->CallStaticObjectMethod(bitmap_class, create_bitmap, width, height, argb8888); - // Lock the Bitmap pixels for copying + // Copy the buffer data into java bitmap. + AndroidBitmapInfo bitmap_info; + AndroidBitmap_getInfo(env, java_bitmap, &bitmap_info); + size_t java_stride = bitmap_info.stride; + void* bitmap_pixels; if (AndroidBitmap_lockPixels(env, java_bitmap, &bitmap_pixels) < 0) { return NULL; } - // Copy the buffer data into java Bitmap. - std::memcpy(bitmap_pixels, buffer, width * height); // 4 bytes per pixel (ARGB_8888) + uint32_t* java_pixel_array = static_cast<uint32_t*>(bitmap_pixels); + uint8_t* native_pixel_array = static_cast<uint8_t*>(buffer); + switch (bitmap_format) { + case BitmapFormat::BGR: { + ConvertBgrToRgba(java_pixel_array, native_pixel_array, java_stride, native_stride, + width, height); + break; + } + case BitmapFormat::BGRA: { + ConvertBgraToRgba(java_pixel_array, native_pixel_array, java_stride, native_stride, + width, height, false); + break; + } + case BitmapFormat::BGRx: { + ConvertBgraToRgba(java_pixel_array, native_pixel_array, java_stride, native_stride, + width, height, true); + break; + } + default: { + LOGE("Bitmap format unknown!"); + AndroidBitmap_unlockPixels(env, java_bitmap); + return NULL; + } + } - // Unlock the Bitmap pixels AndroidBitmap_unlockPixels(env, java_bitmap); return java_bitmap; } -jstring wstringToJstringUTF16(JNIEnv* env, const std::wstring& wstr) { - jsize len = wstr.length(); - jchar* jchars = new jchar[len + 1]; // +1 for null terminator - - for (size_t i = 0; i < len; ++i) { - jchars[i] = static_cast<jchar>(wstr[i]); - } - jchars[len] = 0; - - jstring result = env->NewString(jchars, len); - - delete[] jchars; - - return result; -} - -std::wstring jStringToWstring(JNIEnv* env, jstring java_string) { - std::wstring value; - - const jchar* raw = env->GetStringChars(java_string, 0); - jsize len = env->GetStringLength(java_string); - - value.assign(raw, raw + len); - - env->ReleaseStringChars(java_string, raw); - - return value; -} - int ToJavaColorInt(Color color) { // Get ARGB values from Native Color uint A = color.a; @@ -549,14 +625,12 @@ jobject ToJavaPath(JNIEnv* env, const std::vector<PathObject::Segment>& segments case PathObject::Segment::Command::Move: { static jmethodID move_to = env->GetMethodID(path_class, "moveTo", funcsig("V", "F", "F").c_str()); - env->CallVoidMethod(java_path, move_to, output.x, output.y); break; } case PathObject::Segment::Command::Line: { static jmethodID line_to = env->GetMethodID(path_class, "lineTo", funcsig("V", "F", "F").c_str()); - env->CallVoidMethod(java_path, line_to, output.x, output.y); break; } @@ -566,7 +640,6 @@ jobject ToJavaPath(JNIEnv* env, const std::vector<PathObject::Segment>& segments // Check if segment isClosed. if (segment.is_closed) { static jmethodID close = env->GetMethodID(path_class, "close", funcsig("V").c_str()); - env->CallVoidMethod(java_path, close); } } @@ -574,11 +647,56 @@ jobject ToJavaPath(JNIEnv* env, const std::vector<PathObject::Segment>& segments return java_path; } -jobject ToJavaPdfPathObject(JNIEnv* env, const PageObject* page_object, - ICoordinateConverter* converter) { - // Cast to PathObject - const PathObject* path_object = static_cast<const PathObject*>(page_object); +jobject ToJavaPdfTextObject(JNIEnv* env, const TextObject* text_object) { + // Find Java PdfTextObject Class. + static jclass text_object_class = GetPermClassRef(env, kTextObject); + + // Create Java Text String from TextObject Data String. + jobject java_string = ToJavaString(env, text_object->text_); + + // Get Native Font Object Data. + int font_family = static_cast<int>(text_object->font_.GetFamily()); + bool bold = text_object->font_.IsBold(); + bool italic = text_object->font_.IsItalic(); + + // Create Java TextObjectFont Instance. + static jclass text_font_class = GetPermClassRef(env, kTextFont); + static jmethodID init_text_font = + env->GetMethodID(text_font_class, "<init>", funcsig("V", "I", "Z", "Z").c_str()); + jobject java_font = env->NewObject(text_font_class, init_text_font, font_family, bold, italic); + + // Create Java PdfTextObject Instance. + static jmethodID init_text_object = env->GetMethodID( + text_object_class, "<init>", funcsig("V", kString, kTextFont, "F").c_str()); + float font_size = text_object->font_size_; + jobject java_text_object = + env->NewObject(text_object_class, init_text_object, java_string, java_font, font_size); + + // Set Java PdfTextObject Render Mode. + int render_mode = static_cast<int>(text_object->render_mode_); + static jmethodID set_render_mode = env->GetMethodID(text_object_class, "setRenderMode", "(I)V"); + env->CallVoidMethod(java_text_object, set_render_mode, render_mode); + + // Set Java PdfTextObject Fill Color. + static jmethodID set_fill_color = env->GetMethodID(text_object_class, "setFillColor", "(I)V"); + env->CallVoidMethod(java_text_object, set_fill_color, ToJavaColorInt(text_object->fill_color_)); + + // Set Java PdfTextObject Stroke Color. + static jmethodID set_stroke_color = + env->GetMethodID(text_object_class, "setStrokeColor", "(I)V"); + env->CallVoidMethod(java_text_object, set_stroke_color, + ToJavaColorInt(text_object->stroke_color_)); + + // Set Java PdfTextObject Stroke Width. + static jmethodID set_stroke_width = + env->GetMethodID(text_object_class, "setStrokeWidth", "(F)V"); + env->CallVoidMethod(java_text_object, set_stroke_width, text_object->stroke_width_); + return java_text_object; +} + +jobject ToJavaPdfPathObject(JNIEnv* env, const PathObject* path_object, + ICoordinateConverter* converter) { // Find Java PdfPathObject Class. static jclass path_object_class = GetPermClassRef(env, kPathObject); // Get Constructor Id. @@ -594,19 +712,19 @@ jobject ToJavaPdfPathObject(JNIEnv* env, const PageObject* page_object, // Set Java PdfPathObject FillColor. if (path_object->is_fill_) { static jmethodID set_fill_color = - env->GetMethodID(path_object_class, "setFillColor", funcsig("V", kColor).c_str()); + env->GetMethodID(path_object_class, "setFillColor", funcsig("V", "I").c_str()); env->CallVoidMethod(java_path_object, set_fill_color, - ToJavaColor(env, path_object->fill_color_)); + ToJavaColorInt(path_object->fill_color_)); } // Set Java PdfPathObject StrokeColor. if (path_object->is_stroke_) { static jmethodID set_stroke_color = - env->GetMethodID(path_object_class, "setStrokeColor", funcsig("V", kColor).c_str()); + env->GetMethodID(path_object_class, "setStrokeColor", funcsig("V", "I").c_str()); env->CallVoidMethod(java_path_object, set_stroke_color, - ToJavaColor(env, path_object->stroke_color_)); + ToJavaColorInt(path_object->stroke_color_)); } // Set Java Stroke Width. @@ -617,21 +735,24 @@ jobject ToJavaPdfPathObject(JNIEnv* env, const PageObject* page_object, return java_path_object; } -jobject ToJavaPdfImageObject(JNIEnv* env, const PageObject* page_object) { - // Cast to ImageObject - const ImageObject* image_object = static_cast<const ImageObject*>(page_object); - +jobject ToJavaPdfImageObject(JNIEnv* env, const ImageObject* image_object) { // Find Java ImageObject Class. static jclass image_object_class = GetPermClassRef(env, kImageObject); // Get Constructor Id. static jmethodID init_image = env->GetMethodID(image_object_class, "<init>", funcsig("V", kBitmap).c_str()); - // Get Bitmap readable buffer from ImageObject Data. - void* buffer = image_object->GetBitmapReadableBuffer(); - // Create Java Bitmap from Native Bitmap Buffer. - jobject java_bitmap = ToJavaBitmap(env, buffer, image_object->width_, image_object->height_); + void* buffer = image_object->GetBitmapBuffer(); + BitmapFormat bitmap_format = image_object->bitmap_format_; + size_t width = image_object->width_; + size_t height = image_object->height_; + int stride = FPDFBitmap_GetStride(image_object->bitmap_.get()); + jobject java_bitmap = ToJavaBitmap(env, buffer, bitmap_format, width, height, stride); + if (java_bitmap == NULL) { + LOGE("To java bitmap conversion failed!"); + return NULL; + } // Create Java PdfImageObject Instance. jobject java_image_object = env->NewObject(image_object_class, init_image, java_bitmap); @@ -650,11 +771,13 @@ jobject ToJavaPdfPageObject(JNIEnv* env, const PageObject* page_object, switch (page_object->GetType()) { case PageObject::Type::Path: { - java_page_object = ToJavaPdfPathObject(env, page_object, converter); + const PathObject* path_object = static_cast<const PathObject*>(page_object); + java_page_object = ToJavaPdfPathObject(env, path_object, converter); break; } case PageObject::Type::Image: { - java_page_object = ToJavaPdfImageObject(env, page_object); + const ImageObject* image_object = static_cast<const ImageObject*>(page_object); + java_page_object = ToJavaPdfImageObject(env, image_object); break; } default: @@ -672,7 +795,8 @@ jobject ToJavaPdfPageObject(JNIEnv* env, const PageObject* page_object, // Set Java PdfPageObject Matrix. static jmethodID set_matrix = env->GetMethodID(page_object_class, "setMatrix", funcsig("V", kMatrix).c_str()); - env->CallVoidMethod(java_page_object, set_matrix, ToJavaMatrix(env, page_object->matrix_)); + env->CallVoidMethod(java_page_object, set_matrix, + ToJavaMatrix(env, page_object->device_matrix_)); return java_page_object; } @@ -703,6 +827,105 @@ Color ToNativeColor(JNIEnv* env, jobject java_color) { return ToNativeColor(java_color_int); } +std::unique_ptr<TextObject> ToNativeTextObject(JNIEnv* env, jobject java_text_object) { + // Create TextObject Data Instance. + auto text_object = std::make_unique<TextObject>(); + + // Get Ref to Java PdfTextObject Class. + static jclass text_object_class = GetPermClassRef(env, kTextObject); + + // Get Java PdfTextObject Font. + static jmethodID get_text_font = + env->GetMethodID(text_object_class, "getFont", funcsig(kTextFont).c_str()); + jobject java_text_font = env->CallObjectMethod(java_text_object, get_text_font); + + // Find Java PdfTextObjectFont Class. + static jclass text_font_class = GetPermClassRef(env, kTextFont); + + // Get the Font Family for the PdfTextObjectFont. + static jmethodID get_font_family = env->GetMethodID(text_font_class, "getFontFamily", "()I"); + jint font_family = env->CallIntMethod(java_text_font, get_font_family); + + // Is PdfTextObjectFont Bold. + static jmethodID is_bold = env->GetMethodID(text_font_class, "isBold", "()Z"); + jboolean bold = env->CallBooleanMethod(java_text_font, is_bold); + + // Is PdfTextObjectFont Italic. + static jmethodID is_italic = env->GetMethodID(text_font_class, "isItalic", "()Z"); + jboolean italic = env->CallBooleanMethod(java_text_font, is_italic); + + // Set TextObject Data Font. + if (font_family < 0 || font_family >= font_names.size()) { + return nullptr; + } + text_object->font_ = + Font(font_names[font_family], static_cast<Font::Family>(font_family), bold, italic); + + // Get Java PdfTextObject font size. + static jmethodID get_font_size = env->GetMethodID(text_object_class, "getFontSize", "()F"); + jfloat font_size = env->CallFloatMethod(java_text_object, get_font_size); + + // Set TextObject Data font size. + text_object->font_size_ = font_size; + + // Get Java PdfTextObject Text. + static jmethodID get_text = + env->GetMethodID(text_object_class, "getText", funcsig(kString).c_str()); + jstring java_text = static_cast<jstring>(env->CallObjectMethod(java_text_object, get_text)); + + // Set TextObject Data Text. + text_object->text_ = ToNativeWideString(env, java_text); + + // Get Java PdfTextObject RenderMode. + static jmethodID get_render_mode = env->GetMethodID(text_object_class, "getRenderMode", "()I"); + jint render_mode = env->CallIntMethod(java_text_object, get_render_mode); + + // Set TextObject Data RenderMode. + switch (static_cast<TextObject::RenderMode>(render_mode)) { + case TextObject::RenderMode::Fill: { + text_object->render_mode_ = TextObject::RenderMode::Fill; + break; + } + case TextObject::RenderMode::Stroke: { + text_object->render_mode_ = TextObject::RenderMode::Stroke; + break; + } + case TextObject::RenderMode::FillStroke: { + text_object->render_mode_ = TextObject::RenderMode::FillStroke; + break; + } + default: { + text_object->render_mode_ = TextObject::RenderMode::Unknown; + break; + } + } + + // Get Java PdfTextObject Fill Color. + static jmethodID get_fill_color = env->GetMethodID(text_object_class, "getFillColor", "()I"); + jint java_fill_color = env->CallIntMethod(java_text_object, get_fill_color); + + // Set TextObject Data Fill Color + text_object->fill_color_ = ToNativeColor(java_fill_color); + + // Get Java PdfTextObject Stroke Color. + static jmethodID get_stroke_color = + env->GetMethodID(text_object_class, "getStrokeColor", "()I"); + jint java_stroke_color = env->CallIntMethod(java_text_object, get_stroke_color); + + // Set TextObject Data Stroke Color. + text_object->stroke_color_ = ToNativeColor(java_stroke_color); + + // Get Java PdfTextObject Stroke Width. + static jmethodID get_stroke_width = + env->GetMethodID(text_object_class, "getStrokeWidth", "()F"); + jfloat stroke_width = env->CallFloatMethod(java_text_object, get_stroke_width); + + // Set TextObject Data Stroke Width. + text_object->stroke_width_ = stroke_width; + + return text_object; +} + std::unique_ptr<PathObject> ToNativePathObject(JNIEnv* env, jobject java_path_object, ICoordinateConverter* converter) { // Create PathObject Data Instance. @@ -746,24 +969,24 @@ std::unique_ptr<PathObject> ToNativePathObject(JNIEnv* env, jobject java_path_ob // Get Java PathObject Fill Color. static jmethodID get_fill_color = - env->GetMethodID(path_object_class, "getFillColor", funcsig(kColor).c_str()); - jobject java_fill_color = env->CallObjectMethod(java_path_object, get_fill_color); + env->GetMethodID(path_object_class, "getFillColor", funcsig("I").c_str()); + jint java_fill_color = env->CallIntMethod(java_path_object, get_fill_color); // Set PathObject Data Fill Mode and Fill Color - path_object->is_fill_ = (java_fill_color != NULL); + path_object->is_fill_ = (java_fill_color != 0); if (path_object->is_fill_) { - path_object->fill_color_ = ToNativeColor(env, java_fill_color); + path_object->fill_color_ = ToNativeColor(java_fill_color); } // Get Java PathObject Stroke Color. static jmethodID get_stroke_color = - env->GetMethodID(path_object_class, "getStrokeColor", funcsig(kColor).c_str()); - jobject java_stroke_color = env->CallObjectMethod(java_path_object, get_stroke_color); + env->GetMethodID(path_object_class, "getStrokeColor", funcsig("I").c_str()); + jint java_stroke_color = env->CallIntMethod(java_path_object, get_stroke_color); // Set PathObject Data Stroke Mode and Stroke Color. - path_object->is_stroke_ = (java_stroke_color != NULL); + path_object->is_stroke_ = (java_stroke_color != 0); if (path_object->is_stroke_) { - path_object->stroke_color_ = ToNativeColor(env, java_stroke_color); + path_object->stroke_color_ = ToNativeColor(java_stroke_color); } // Get Java PathObject Stroke Width. @@ -777,6 +1000,23 @@ std::unique_ptr<PathObject> ToNativePathObject(JNIEnv* env, jobject java_path_ob return path_object; } +void CopyRgbaToBgra(uint8_t* rgba_pixel_array, size_t rgba_stride, uint32_t* bgra_pixel_array, + size_t bgra_stride, size_t width, size_t height) { + for (size_t y = 0; y < height; y++) { + uint8_t* rgba_row_ptr = rgba_pixel_array + y * rgba_stride; + uint32_t* bgra_row_ptr = bgra_pixel_array + y * (bgra_stride / 4); + for (size_t x = 0; x < width; x++) { + // Extract RGBA components stored. + uint8_t red = rgba_row_ptr[x * 4]; + uint8_t green = rgba_row_ptr[x * 4 + 1]; + uint8_t blue = rgba_row_ptr[x * 4 + 2]; + uint8_t alpha = rgba_row_ptr[x * 4 + 3]; + // Storing native bitmap components BGRA in little-endian. + bgra_row_ptr[x] = (alpha << 24) | (red << 16) | (green << 8) | blue; + } + } +} + std::unique_ptr<ImageObject> ToNativeImageObject(JNIEnv* env, jobject java_image_object) { // Create ImageObject Data Instance. auto image_object = std::make_unique<ImageObject>(); @@ -789,21 +1029,34 @@ std::unique_ptr<ImageObject> ToNativeImageObject(JNIEnv* env, jobject java_image env->GetMethodID(image_object_class, "getBitmap", funcsig(kBitmap).c_str()); jobject java_bitmap = env->CallObjectMethod(java_image_object, get_bitmap); - // Create an FPDF_BITMAP from the Android Bitmap. + // Get android bitmap info. + AndroidBitmapInfo bitmap_info; + AndroidBitmap_getInfo(env, java_bitmap, &bitmap_info); + if (bitmap_info.format != ANDROID_BITMAP_FORMAT_RGBA_8888) { + LOGE("Android bitmap is not in RGBA_8888 format"); + return nullptr; + } + size_t bitmap_width = bitmap_info.width; + size_t bitmap_height = bitmap_info.height; + size_t java_stride = bitmap_info.stride; + + // Create ImageObject data bitmap. + image_object->bitmap_ = ScopedFPDFBitmap(FPDFBitmap_Create(bitmap_width, bitmap_height, 1)); + size_t native_stride = FPDFBitmap_GetStride(image_object->bitmap_.get()); + + // Copy pixels from android bitmap. void* bitmap_pixels; if (AndroidBitmap_lockPixels(env, java_bitmap, &bitmap_pixels) < 0) { + LOGE("Android bitmap lock pixels failed!"); return nullptr; } - AndroidBitmapInfo bitmap_info; - AndroidBitmap_getInfo(env, java_bitmap, &bitmap_info); - const int stride = bitmap_info.width * 4; + uint8_t* java_pixel_array = static_cast<uint8_t*>(bitmap_pixels); + uint32_t* native_pixel_array = static_cast<uint32_t*>(image_object->GetBitmapBuffer()); - // Set ImageObject Data Bitmap - image_object->bitmap_ = ScopedFPDFBitmap(FPDFBitmap_CreateEx( - bitmap_info.width, bitmap_info.height, FPDFBitmap_BGRA, bitmap_pixels, stride)); + CopyRgbaToBgra(java_pixel_array, java_stride, native_pixel_array, native_stride, bitmap_width, + bitmap_height); - // Unlock the Android Bitmap AndroidBitmap_unlockPixels(env, java_bitmap); return image_object; @@ -846,9 +1099,9 @@ std::unique_ptr<PageObject> ToNativePageObject(JNIEnv* env, jobject java_page_ob env->GetFloatArrayRegion(java_matrix_array, 0, 9, transform); // Set PageObject Data Matrix. - page_object->matrix_ = {transform[0 /*kMScaleX*/], transform[3 /*kMSkewY*/], - transform[1 /*kMSkewX*/], transform[4 /*kMScaleY*/], - transform[2 /*kMTransX*/], transform[5 /*kMTransY*/]}; + page_object->device_matrix_ = {transform[0 /*kMScaleX*/], transform[3 /*kMSkewY*/], + transform[1 /*kMSkewX*/], transform[4 /*kMScaleY*/], + transform[2 /*kMTransX*/], transform[5 /*kMTransY*/]}; return page_object; } @@ -860,9 +1113,9 @@ jobject ToJavaPageAnnotations(JNIEnv* env, const vector<Annotation*>& annotation jobject ToJavaStampAnnotation(JNIEnv* env, const Annotation* annotation, ICoordinateConverter* converter) { - jobject java_bounds = ToJavaRectF(env, annotation->GetBounds(), converter); // Cast to StampAnnotation const StampAnnotation* stamp_annotation = static_cast<const StampAnnotation*>(annotation); + jobject java_bounds = ToJavaRectF(env, stamp_annotation->GetBounds(), converter); // Find Java StampAnnotation Class. static jclass stamp_annotation_class = GetPermClassRef(env, kStampAnnotation); @@ -890,16 +1143,17 @@ jobject ToJavaStampAnnotation(JNIEnv* env, const Annotation* annotation, jobject ToJavaHighlightAnnotation(JNIEnv* env, const Annotation* annotation, ICoordinateConverter* converter) { - jobject java_bounds = ToJavaRectF(env, annotation->GetBounds(), converter); // Cast to HighlightAnnotation const HighlightAnnotation* highlight_annotation = static_cast<const HighlightAnnotation*>(annotation); + jobject java_bounds = + ToJavaList(env, highlight_annotation->GetBounds(), converter, &ToJavaRectF); // Find Java HighlightAnnotation Class. static jclass highlight_annotation_class = GetPermClassRef(env, kHighlightAnnotation); // Get Constructor Id. static jmethodID init = - env->GetMethodID(highlight_annotation_class, "<init>", funcsig("V", kRectF).c_str()); + env->GetMethodID(highlight_annotation_class, "<init>", funcsig("V", kList).c_str()); // Create Java HighlightAnnotation Instance. jobject java_annotation = env->NewObject(highlight_annotation_class, init, java_bounds); @@ -917,11 +1171,11 @@ jobject ToJavaHighlightAnnotation(JNIEnv* env, const Annotation* annotation, jobject ToJavaFreeTextAnnotation(JNIEnv* env, const Annotation* annotation, ICoordinateConverter* converter) { - jobject java_bounds = ToJavaRectF(env, annotation->GetBounds(), converter); - // Cast to FreeText Annotation const FreeTextAnnotation* freetext_annotation = static_cast<const FreeTextAnnotation*>(annotation); + + jobject java_bounds = ToJavaRectF(env, freetext_annotation->GetBounds(), converter); // Find Java FreeTextAnnotation class. static jclass freetext_annotation_class = GetPermClassRef(env, kFreeTextAnnotation); // Get Constructor Id. @@ -929,7 +1183,7 @@ jobject ToJavaFreeTextAnnotation(JNIEnv* env, const Annotation* annotation, funcsig("V", kRectF, kString).c_str()); // Get Java String for text content. - jobject java_string = wstringToJstringUTF16(env, freetext_annotation->GetTextContent()); + jobject java_string = ToJavaString(env, freetext_annotation->GetTextContent()); // Create Java FreeTextAnnotation Object. jobject java_freetext_annotation = env->NewObject(freetext_annotation_class, init, java_bounds, java_string); @@ -980,14 +1234,18 @@ jobject ToJavaPageAnnotation(JNIEnv* env, const Annotation* annotation, } std::unique_ptr<Annotation> ToNativeStampAnnotation(JNIEnv* env, jobject java_annotation, - Rectangle_f native_bounds, ICoordinateConverter* converter) { - // Create StampAnnotation Instance. - auto stamp_annotation = std::make_unique<StampAnnotation>(native_bounds); - // Get Ref to Java StampAnnotation Class. static jclass stamp_annotation_class = GetPermClassRef(env, kStampAnnotation); + jmethodID get_bounds = + env->GetMethodID(stamp_annotation_class, "getBounds", funcsig(kRectF).c_str()); + jobject java_bounds = env->CallObjectMethod(java_annotation, get_bounds); + Rectangle_f native_bounds = ToNativeRectF(env, java_bounds, converter); + + // Create StampAnnotation Instance. + auto stamp_annotation = std::make_unique<StampAnnotation>(native_bounds); + // Get PdfPageObjects from stamp annotation static jmethodID get_objects = env->GetMethodID(stamp_annotation_class, "getObjects", funcsig(kList).c_str()); @@ -1008,13 +1266,30 @@ std::unique_ptr<Annotation> ToNativeStampAnnotation(JNIEnv* env, jobject java_an } std::unique_ptr<Annotation> ToNativeHighlightAnnotation(JNIEnv* env, jobject java_annotation, - Rectangle_f native_bounds) { - // Create HighlightAnnotation Instance. - auto highlight_annotation = std::make_unique<HighlightAnnotation>(native_bounds); - + ICoordinateConverter* converter) { // Get Ref to Java HighlightAnnotation Class. static jclass highlight_annotation_class = GetPermClassRef(env, kHighlightAnnotation); + jmethodID get_bounds = + env->GetMethodID(highlight_annotation_class, "getBounds", funcsig(kList).c_str()); + jobject java_bounds = env->CallObjectMethod(java_annotation, get_bounds); + + vector<Rectangle_f> native_bounds; + + jclass list_class = env->FindClass(kList); + jmethodID size_method = env->GetMethodID(list_class, "size", funcsig("I").c_str()); + jmethodID get_method = env->GetMethodID(list_class, "get", funcsig(kObject, "I").c_str()); + + jint listSize = env->CallIntMethod(java_bounds, size_method); + for (int i = 0; i < listSize; i++) { + jobject java_bound = env->CallObjectMethod(java_bounds, get_method, i); + Rectangle_f native_bound = ToNativeRectF(env, java_bound, converter); + native_bounds.push_back(native_bound); + } + + // Create HighlightAnnotation Instance. + auto highlight_annotation = std::make_unique<HighlightAnnotation>(native_bounds); + // Get and set highlight color // Get methodId for getColor @@ -1028,13 +1303,18 @@ std::unique_ptr<Annotation> ToNativeHighlightAnnotation(JNIEnv* env, jobject jav } std::unique_ptr<Annotation> ToNativeFreeTextAnnotation(JNIEnv* env, jobject java_annotation, - Rectangle_f native_bounds) { - // Create FreeTextAnnotation Instance. - auto freetext_annotation = std::make_unique<FreeTextAnnotation>(native_bounds); - + ICoordinateConverter* converter) { // Get Ref to Java FreeTextAnnotation Class. static jclass freetext_annotation_class = GetPermClassRef(env, kFreeTextAnnotation); + jmethodID get_bounds = + env->GetMethodID(freetext_annotation_class, "getBounds", funcsig(kRectF).c_str()); + jobject java_bounds = env->CallObjectMethod(java_annotation, get_bounds); + Rectangle_f native_bounds = ToNativeRectF(env, java_bounds, converter); + + // Create FreeTextAnnotation Instance. + auto freetext_annotation = std::make_unique<FreeTextAnnotation>(native_bounds); + // Get the TextContent from Java layer. static jmethodID get_text_content = env->GetMethodID(freetext_annotation_class, "getTextContent", funcsig(kString).c_str()); @@ -1042,7 +1322,7 @@ std::unique_ptr<Annotation> ToNativeFreeTextAnnotation(JNIEnv* env, jobject java static_cast<jstring>(env->CallObjectMethod(java_annotation, get_text_content)); // Set the TextContent - std::wstring native_text_content = jStringToWstring(env, java_text_content); + std::wstring native_text_content = ToNativeWideString(env, java_text_content); freetext_annotation->SetTextContent(native_text_content); // Get the text color @@ -1070,24 +1350,19 @@ std::unique_ptr<Annotation> ToNativePageAnnotation(JNIEnv* env, jobject java_ann env->GetMethodID(annotation_class, "getPdfAnnotationType", funcsig("I").c_str()); jint annotation_type = env->CallIntMethod(java_annotation, get_type); - // 2. Get bounds - jmethodID get_bounds = env->GetMethodID(annotation_class, "getBounds", funcsig(kRectF).c_str()); - jobject java_bounds = env->CallObjectMethod(java_annotation, get_bounds); - Rectangle_f native_bounds = ToNativeRectF(env, java_bounds, converter); - std::unique_ptr<Annotation> annotation = nullptr; switch (static_cast<Annotation::Type>(annotation_type)) { case Annotation::Type::Stamp: { - annotation = ToNativeStampAnnotation(env, java_annotation, native_bounds, converter); + annotation = ToNativeStampAnnotation(env, java_annotation, converter); break; } case Annotation::Type::Highlight: { - annotation = ToNativeHighlightAnnotation(env, java_annotation, native_bounds); + annotation = ToNativeHighlightAnnotation(env, java_annotation, converter); break; } case Annotation::Type::FreeText: { - annotation = ToNativeFreeTextAnnotation(env, java_annotation, native_bounds); + annotation = ToNativeFreeTextAnnotation(env, java_annotation, converter); break; } default: diff --git a/pdf/framework/libs/pdfClient/jni_conversion.h b/pdf/framework/libs/pdfClient/jni_conversion.h index 2b679405b..30b784250 100644 --- a/pdf/framework/libs/pdfClient/jni_conversion.h +++ b/pdf/framework/libs/pdfClient/jni_conversion.h @@ -117,8 +117,6 @@ jobject ToJavaGotoLink(JNIEnv* env, const GotoLink link); jobject ToJavaGotoLinks(JNIEnv* env, const vector<GotoLink>& links); -jobject ToJavaBitmap(JNIEnv* env, void* buffer, int width, int height); - jobject ToJavaColor(JNIEnv* env, Color color); jfloatArray ToJavaFloatArray(JNIEnv* env, const float arr[], size_t length); diff --git a/pdf/framework/libs/pdfClient/page.cc b/pdf/framework/libs/pdfClient/page.cc index a409a39c1..faa84b34b 100644 --- a/pdf/framework/libs/pdfClient/page.cc +++ b/pdf/framework/libs/pdfClient/page.cc @@ -858,25 +858,48 @@ void Page::PopulateAnnotations() { ScopedFPDFAnnotation scoped_annot(FPDFPage_GetAnnot(page_.get(), annotation_index)); int annotationType = FPDFAnnot_GetSubtype(scoped_annot.get()); - FS_RECTF rect; - if (!FPDFAnnot_GetRect(scoped_annot.get(), &rect)) { - LOGE("Failed to get the bounds of the annotation"); - break; - } - Rectangle_f bounds = Rectangle_f{rect.left, rect.top, rect.right, rect.bottom}; - std::unique_ptr<Annotation> annotation = nullptr; switch (annotationType) { case FPDF_ANNOT_STAMP: { + FS_RECTF rect; + if (!FPDFAnnot_GetRect(scoped_annot.get(), &rect)) { + LOGE("Failed to get the bounds of the annotation"); + break; + } + auto bounds = Rectangle_f{rect.left, rect.top, rect.right, rect.bottom}; annotation = std::make_unique<StampAnnotation>(bounds); break; } case FPDF_ANNOT_HIGHLIGHT: { + vector<Rectangle_f> bounds; + auto num_bounds = FPDFAnnot_CountAttachmentPoints(scoped_annot.get()); + if (num_bounds > 0) { + bounds.resize(num_bounds); + for (auto bound_index = 0; bound_index < num_bounds; bound_index++) { + FS_QUADPOINTSF quad_points; + if (!FPDFAnnot_GetAttachmentPoints(scoped_annot.get(), bound_index, + &quad_points)) { + LOGD("Failed to get quad points from pdfium"); + break; + } + + bounds[bound_index] = Rectangle_f(quad_points.x1, quad_points.y1, + quad_points.x2, quad_points.y4); + } + } else { + LOGD("Failed to find bounds for highlight annotation"); + } annotation = std::make_unique<HighlightAnnotation>(bounds); break; } case FPDF_ANNOT_FREETEXT: { + FS_RECTF rect; + if (!FPDFAnnot_GetRect(scoped_annot.get(), &rect)) { + LOGE("Failed to get the bounds of the annotation"); + break; + } + auto bounds = Rectangle_f{rect.left, rect.top, rect.right, rect.bottom}; annotation = std::make_unique<FreeTextAnnotation>(bounds); break; } diff --git a/pdf/framework/libs/pdfClient/page_object.cc b/pdf/framework/libs/pdfClient/page_object.cc index e78d0fd0b..c8cfd8313 100644 --- a/pdf/framework/libs/pdfClient/page_object.cc +++ b/pdf/framework/libs/pdfClient/page_object.cc @@ -23,6 +23,7 @@ #include "fpdf_edit.h" #include "fpdfview.h" #include "logging.h" +#include "rect.h" #define LOG_TAG "page_object" @@ -34,6 +35,58 @@ PageObject::Type PageObject::GetType() const { return type_; } +bool PageObject::GetPageToDeviceMatrix(FPDF_PAGEOBJECT page_object, FPDF_PAGE page) { + Matrix page_matrix; + if (!FPDFPageObj_GetMatrix(page_object, reinterpret_cast<FS_MATRIX*>(&page_matrix))) { + LOGE("GetPageMatrix failed!"); + return false; + } + + // Set identity transformation for GetBounds. + Matrix identity = {1, 0, 0, 1, 0, 0}; + FPDFPageObj_SetMatrix(page_object, reinterpret_cast<FS_MATRIX*>(&identity)); + + // Get Bounds. + Rectangle_f bounds; + FPDFPageObj_GetBounds(page_object, &bounds.left, &bounds.bottom, &bounds.right, &bounds.top); + + // Reset the original page matrix. + FPDFPageObj_SetMatrix(page_object, reinterpret_cast<FS_MATRIX*>(&page_matrix)); + + float page_height = FPDF_GetPageHeightF(page); + + // Page to device matrix. + device_matrix_.a = page_matrix.a; + device_matrix_.b = (page_matrix.b != 0) ? -page_matrix.b : 0; + device_matrix_.c = (page_matrix.c != 0) ? -page_matrix.c : 0; + device_matrix_.d = page_matrix.d; + device_matrix_.e = page_matrix.e + ((bounds.top + bounds.bottom) * page_matrix.c); + device_matrix_.f = page_height - page_matrix.f - ((bounds.top + bounds.bottom) * page_matrix.d); + + return true; +} + +bool PageObject::SetDeviceToPageMatrix(FPDF_PAGEOBJECT page_object, FPDF_PAGE page) { + // Reset Previous Transformation. + Matrix identity = {1, 0, 0, 1, 0, 0}; + if (!FPDFPageObj_SetMatrix(page_object, reinterpret_cast<FS_MATRIX*>(&identity))) { + LOGE("SetMatrix failed!"); + return false; + } + + Rectangle_f bounds; + FPDFPageObj_GetBounds(page_object, &bounds.left, &bounds.bottom, &bounds.right, &bounds.top); + + float page_height = FPDF_GetPageHeightF(page); + + FPDFPageObj_Transform(page_object, 1, 0, 0, 1, 0, -(bounds.top + bounds.bottom)); + FPDFPageObj_Transform(page_object, device_matrix_.a, -device_matrix_.b, -device_matrix_.c, + device_matrix_.d, device_matrix_.e, -device_matrix_.f); + FPDFPageObj_Transform(page_object, 1, 0, 0, 1, 0, page_height); + + return true; +} + PageObject::~PageObject() = default; } // namespace pdfClient
\ No newline at end of file diff --git a/pdf/framework/libs/pdfClient/page_object.h b/pdf/framework/libs/pdfClient/page_object.h index 561b57911..50df89616 100644 --- a/pdf/framework/libs/pdfClient/page_object.h +++ b/pdf/framework/libs/pdfClient/page_object.h @@ -19,6 +19,8 @@ #include <stdint.h> +#include <algorithm> + #include "cpp/fpdf_scopers.h" #include "fpdfview.h" @@ -56,6 +58,11 @@ struct Matrix { return a == other.a && b == other.b && c == other.c && d == other.d && e == other.e && f == other.f; } + + float operator-(const Matrix& other) const { + return std::max({abs(a - other.a), abs(b - other.b), abs(c - other.c), abs(d - other.d), + abs(e - other.e), abs(f - other.f)}); + } }; class PageObject { @@ -77,7 +84,7 @@ class PageObject { virtual ~PageObject(); - Matrix matrix_; // Matrix used to scale, rotate, shear and translate the page object. + Matrix device_matrix_; // Matrix used to scale, rotate, shear and translate the page object. Color fill_color_; Color stroke_color_; float stroke_width_ = 1.0f; @@ -85,10 +92,13 @@ class PageObject { protected: PageObject(Type type = Type::Unknown); + virtual bool GetPageToDeviceMatrix(FPDF_PAGEOBJECT page_object, FPDF_PAGE page); + virtual bool SetDeviceToPageMatrix(FPDF_PAGEOBJECT page_object, FPDF_PAGE page); + private: Type type_; }; } // namespace pdfClient -#endif // MEDIAPROVIDER_PDF_JNI_PDFCLIENT_PAGE_OBJECT_H_
\ No newline at end of file +#endif // MEDIAPROVIDER_PDF_JNI_PDFCLIENT_PAGE_OBJECT_H_ diff --git a/pdf/framework/libs/pdfClient/page_test.cc b/pdf/framework/libs/pdfClient/page_test.cc index 74c79eae2..79dfeea84 100644 --- a/pdf/framework/libs/pdfClient/page_test.cc +++ b/pdf/framework/libs/pdfClient/page_test.cc @@ -250,7 +250,7 @@ TEST(Test, AddImagePageObjectTest) { FPDFBitmap_FillRect(imageObject->bitmap_.get(), 0, 0, 100, 100, 0xFF000000); // Set Matrix. - imageObject->matrix_ = {1.0f, 0, 0, 1.0f, 0, 0}; + imageObject->device_matrix_ = {1.0f, 0, 0, 1.0f, 0, 0}; // Add the page object. ASSERT_EQ(page->AddPageObject(std::move(imageObject)), initialPageObjects.size()); @@ -289,7 +289,7 @@ TEST(Test, AddPathPageObject) { pathObject->is_stroke_ = true; // Set PathObject Matrix. - pathObject->matrix_ = {1.0f, 0, 0, 1.0f, 0, 0}; + pathObject->device_matrix_ = {1.0f, 0, 0, 1.0f, 0, 0}; // Add the page object. ASSERT_EQ(page->AddPageObject(std::move(pathObject)), initialPageObjects.size()); @@ -328,7 +328,7 @@ TEST(Test, AddTextPageObject) { auto textObject = std::make_unique<TextObject>(); // Set Font. - textObject->font_ = Font(font_names[index], true, true); + textObject->font_ = Font(font_names[index], static_cast<Font::Family>(index), true, true); // Set Font Size. textObject->font_size_ = 10.0f; @@ -343,7 +343,7 @@ TEST(Test, AddTextPageObject) { textObject->fill_color_ = Color(0, 0, 255, 255); // Set TextObject Matrix. - textObject->matrix_ = {1.0f, 0, 0, 1.0f, 0, 0}; + textObject->device_matrix_ = {1.0f, 0, 0, 1.0f, 0, 0}; // Add the page object. if (index != 2) { @@ -401,8 +401,8 @@ TEST(Test, UpdateImagePageObjectTest) { FPDFBitmap_FillRect(imageObject->bitmap_.get(), 0, 0, 100, 110, 0xFF0000FF); // Set Matrix. - Matrix update_matrix = {2.0f, 0, 0, 2.0f, 0, 0}; - imageObject->matrix_ = update_matrix; + Matrix update_matrix = {2.0f, 3.0f, 4.0f, 2.0f, 1.0f, -2.0f}; + imageObject->device_matrix_ = update_matrix; // Update the page object. EXPECT_TRUE(page->UpdatePageObject(0, std::move(imageObject))); @@ -420,7 +420,7 @@ TEST(Test, UpdateImagePageObjectTest) { 110); // Check for updated matrix. - ASSERT_EQ(updatedPageObjects[0]->matrix_, update_matrix); + ASSERT_EQ(updatedPageObjects[0]->device_matrix_, update_matrix); } TEST(Test, UpdatePathPageObjectTest) { @@ -442,8 +442,8 @@ TEST(Test, UpdatePathPageObjectTest) { pathObject->is_stroke_ = false; // Set Matrix. - Matrix update_matrix = {2.0f, 0, 0, 2.0f, 0, 0}; - pathObject->matrix_ = update_matrix; + Matrix update_matrix = {2.0f, -3.0f, 2.0f, 2.0f, -1.0f, 2.0f}; + pathObject->device_matrix_ = update_matrix; // Update the page object. EXPECT_TRUE(page->UpdatePageObject(1, std::move(pathObject))); @@ -459,7 +459,7 @@ TEST(Test, UpdatePathPageObjectTest) { ASSERT_EQ(static_cast<PathObject*>(updatedPageObjects[1])->is_stroke_, false); // Check for updated matrix. - ASSERT_EQ(updatedPageObjects[1]->matrix_, update_matrix); + ASSERT_EQ(updatedPageObjects[1]->device_matrix_, update_matrix); } TEST(Test, UpdateTextPageObjectTest) { @@ -482,7 +482,8 @@ TEST(Test, UpdateTextPageObjectTest) { ASSERT_EQ(initialTextObject->render_mode_, TextObject::RenderMode::Fill); // Check for initial matrix. - ASSERT_EQ(initialTextObject->matrix_, Matrix(1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 30.0f)); + Matrix initial_matrix(1.0f, 0.0f, 0.0f, 1.0f, 0.0f, 759.424f); + ASSERT_LT(initialTextObject->device_matrix_ - initial_matrix, 0.01f); // Check for initial fill color. ASSERT_EQ(initialTextObject->fill_color_, Color(0, 255, 0, 255)); @@ -502,8 +503,8 @@ TEST(Test, UpdateTextPageObjectTest) { textObject->fill_color_ = update_fill_color; // Set Matrix. - Matrix update_matrix = {2.0f, 5.0f, 0, 3.0f, 4.0f, 10.0f}; - textObject->matrix_ = update_matrix; + Matrix update_matrix = {2.0f, 5.0f, -2.0f, 3.0f, -4.0f, 10.0f}; + textObject->device_matrix_ = update_matrix; // Update the page object. EXPECT_TRUE(page->UpdatePageObject(2, std::move(textObject))); @@ -521,8 +522,13 @@ TEST(Test, UpdateTextPageObjectTest) { // Check for updated fill Color. ASSERT_EQ(updatedPageObjects[2]->fill_color_, update_fill_color); - // Check for updated matrix. - ASSERT_EQ(updatedPageObjects[2]->matrix_, update_matrix); + /* + * TextObject Transformation is dependent upon its bounds values. + * Pdfium calculation for bounds shows a little fluctuation which results + * in return matrix values to be not exactly the same. We should tolerate + * the difference which does not make any visible difference. + */ + ASSERT_LT(updatedPageObjects[2]->device_matrix_ - update_matrix, 0.01f); } TEST(Test, GetPageAnnotationsTest) { @@ -568,7 +574,7 @@ TEST(Test, AddStampAnnotationTest) { FPDFBitmap_FillRect(imageObject->bitmap_.get(), 0, 0, 100, 100, 0xFF000000); // Set Matrix. - imageObject->matrix_ = {1.0f, 0, 0, 1.0f, 0, 0}; + imageObject->device_matrix_ = {1.0f, 0, 0, 1.0f, 0, 0}; // Add the page object. stampAnnotation->AddObject(std::move(imageObject)); @@ -586,7 +592,7 @@ TEST(Test, AddStampAnnotationTest) { pathObject->is_stroke_ = true; // Set PathObject Matrix. - pathObject->matrix_ = {1.0f, 0, 0, 1.0f, 0, 0}; + pathObject->device_matrix_ = {1.0f, 2.0f, 3.0f, 1.0f, 3.0f, 2.0f}; // Add the page object. stampAnnotation->AddObject(std::move(pathObject)); diff --git a/pdf/framework/libs/pdfClient/path_object.cc b/pdf/framework/libs/pdfClient/path_object.cc index ff3cf8f9a..a0d68a95e 100644 --- a/pdf/framework/libs/pdfClient/path_object.cc +++ b/pdf/framework/libs/pdfClient/path_object.cc @@ -21,6 +21,7 @@ #include "fpdf_edit.h" #include "logging.h" +#include "rect.h" #define LOG_TAG "path_object" @@ -90,7 +91,7 @@ bool PathObject::UpdateFPDFInstance(FPDF_PAGEOBJECT path_object, FPDF_PAGE page) } // Set the updated matrix. - if (!FPDFPageObj_SetMatrix(path_object, reinterpret_cast<FS_MATRIX*>(&matrix_))) { + if (!SetDeviceToPageMatrix(path_object, page)) { return false; } @@ -150,7 +151,7 @@ bool PathObject::PopulateFromFPDFInstance(FPDF_PAGEOBJECT path_object, FPDF_PAGE is_stroke_ = stroke; // Get Matrix - if (!FPDFPageObj_GetMatrix(path_object, reinterpret_cast<FS_MATRIX*>(&matrix_))) { + if (!GetPageToDeviceMatrix(path_object, page)) { return false; } @@ -166,6 +167,44 @@ bool PathObject::PopulateFromFPDFInstance(FPDF_PAGEOBJECT path_object, FPDF_PAGE return true; } +bool PathObject::GetPageToDeviceMatrix(FPDF_PAGEOBJECT path_object, FPDF_PAGE page) { + Matrix page_matrix; + if (!FPDFPageObj_GetMatrix(path_object, reinterpret_cast<FS_MATRIX*>(&page_matrix))) { + LOGE("GetPageMatrix failed!"); + return false; + } + + float page_height = FPDF_GetPageHeightF(page); + + // Page to device matrix. + device_matrix_.a = page_matrix.a; + device_matrix_.b = (page_matrix.b != 0) ? -page_matrix.b : 0; + device_matrix_.c = (page_matrix.c != 0) ? -page_matrix.c : 0; + device_matrix_.d = page_matrix.d; + device_matrix_.e = page_matrix.e + (page_height * page_matrix.c); + device_matrix_.f = page_height - page_matrix.f - (page_height * page_matrix.d); + + return true; +} + +bool PathObject::SetDeviceToPageMatrix(FPDF_PAGEOBJECT path_object, FPDF_PAGE page) { + // Reset Previous Transformation. + Matrix identity = {1, 0, 0, 1, 0, 0}; + if (!FPDFPageObj_SetMatrix(path_object, reinterpret_cast<FS_MATRIX*>(&identity))) { + LOGE("SetMatrix failed!"); + return false; + } + + float page_height = FPDF_GetPageHeightF(page); + + FPDFPageObj_Transform(path_object, 1, 0, 0, 1, 0, -page_height); + FPDFPageObj_Transform(path_object, device_matrix_.a, -device_matrix_.b, -device_matrix_.c, + device_matrix_.d, device_matrix_.e, -device_matrix_.f); + FPDFPageObj_Transform(path_object, 1, 0, 0, 1, 0, page_height); + + return true; +} + PathObject::~PathObject() = default; } // namespace pdfClient
\ No newline at end of file diff --git a/pdf/framework/libs/pdfClient/path_object.h b/pdf/framework/libs/pdfClient/path_object.h index 6b5ebf4e4..fee76f70e 100644 --- a/pdf/framework/libs/pdfClient/path_object.h +++ b/pdf/framework/libs/pdfClient/path_object.h @@ -60,6 +60,10 @@ class PathObject : public PageObject { bool is_stroke_ = false; std::vector<Segment> segments_; + + protected: + bool GetPageToDeviceMatrix(FPDF_PAGEOBJECT path_object, FPDF_PAGE page) override; + bool SetDeviceToPageMatrix(FPDF_PAGEOBJECT path_object, FPDF_PAGE page) override; }; } // namespace pdfClient diff --git a/pdf/framework/libs/pdfClient/text_object.cc b/pdf/framework/libs/pdfClient/text_object.cc index 9d239f0a6..ba1e6a599 100644 --- a/pdf/framework/libs/pdfClient/text_object.cc +++ b/pdf/framework/libs/pdfClient/text_object.cc @@ -29,6 +29,8 @@ #define LOG_TAG "text_object" +using FontFamily = pdfClient::Font::Family; + namespace pdfClient { std::optional<Font> GetFont(FPDF_PAGEOBJECT text_object) { @@ -109,8 +111,8 @@ std::optional<std::wstring> GetText(FPDF_PAGEOBJECT text_object, FPDF_PAGE page) return pdfClient_utils::ToWideString(p_text_buffer.get(), text_len); } -Font::Font(std::string font_name, bool bold, bool italic) - : font_name_(font_name), bold_(bold), italic_(italic) {} +Font::Font(const std::string& font_name, Family family, bool bold, bool italic) + : font_name_(font_name), family_(family), bold_(bold), italic_(italic) {} std::string Font::GetName() { std::string name = font_name_; @@ -178,8 +180,8 @@ bool TextObject::UpdateFPDFInstance(FPDF_PAGEOBJECT text_object, FPDF_PAGE page) return false; } - // Set the updated matrix. - if (!FPDFPageObj_SetMatrix(text_object, reinterpret_cast<FS_MATRIX*>(&matrix_))) { + // Set the updated device matrix. + if (!SetDeviceToPageMatrix(text_object, page)) { LOGE("SetMatrix failed"); return false; } @@ -237,8 +239,8 @@ bool TextObject::PopulateFromFPDFInstance(FPDF_PAGEOBJECT text_object, FPDF_PAGE return false; } - // Get matrix. - if (!FPDFPageObj_GetMatrix(text_object, reinterpret_cast<FS_MATRIX*>(&matrix_))) { + // Get device matrix. + if (!GetPageToDeviceMatrix(text_object, page)) { LOGE("GetMatrix failed"); return false; } @@ -270,22 +272,22 @@ TextObject::~TextObject() = default; // Font Mapper std::unordered_map<std::string, Font> font_mapper = { - {Courier, Font(Courier)}, - {Courier + Bold, Font(Courier, true)}, - {Courier + Oblique, Font(Courier, false, true)}, - {Courier + BoldOblique, Font(Courier, true, true)}, - - {Helvetica, Font(Helvetica)}, - {Helvetica + Bold, Font(Helvetica, true)}, - {Helvetica + Oblique, Font(Helvetica, false, true)}, - {Helvetica + BoldOblique, Font(Helvetica, true, true)}, - - {TimesRoman, Font(TimesRoman)}, - {Times + Bold, Font(Times, true)}, - {Times + Italic, Font(Times, false, true)}, - {Times + BoldItalic, Font(Times, true, true)}, - - {Symbol, Font(Symbol)}}; + {Courier, Font(Courier, FontFamily::Courier)}, + {Courier + Bold, Font(Courier, FontFamily::Courier, true)}, + {Courier + Oblique, Font(Courier, FontFamily::Courier, false, true)}, + {Courier + BoldOblique, Font(Courier, FontFamily::Courier, true, true)}, + + {Helvetica, Font(Helvetica, FontFamily::Helvetica)}, + {Helvetica + Bold, Font(Helvetica, FontFamily::Helvetica, true)}, + {Helvetica + Oblique, Font(Helvetica, FontFamily::Helvetica, false, true)}, + {Helvetica + BoldOblique, Font(Helvetica, FontFamily::Helvetica, true, true)}, + + {TimesRoman, Font(TimesRoman, FontFamily::TimesRoman)}, + {Times + Bold, Font(Times, FontFamily::TimesRoman, true)}, + {Times + Italic, Font(Times, FontFamily::TimesRoman, false, true)}, + {Times + BoldItalic, Font(Times, FontFamily::TimesRoman, true, true)}, + + {Symbol, Font(Symbol, FontFamily::Symbol)}}; // Standard Font Names. std::vector<std::string> font_names = {CourierNew, Helvetica, Symbol, TimesNewRoman}; diff --git a/pdf/framework/libs/pdfClient/text_object.h b/pdf/framework/libs/pdfClient/text_object.h index 05b084ca1..84a150b6b 100644 --- a/pdf/framework/libs/pdfClient/text_object.h +++ b/pdf/framework/libs/pdfClient/text_object.h @@ -31,15 +31,26 @@ namespace pdfClient { class Font { public: - Font() {}; - Font(std::string font_name, bool bold = false, bool italic = false); + enum class Family { + Unknown = -1, + Courier, + Helvetica, + Symbol, + TimesRoman, + }; + + Font() : family_(Family::Unknown), bold_(false), italic_(false) {}; + Font(const std::string& font_name, Family family = Family::Unknown, bool bold = false, + bool italic = false); std::string GetName(); - bool IsBold() { return bold_; } - bool IsItalic() { return italic_; } + Family GetFamily() const { return family_; } + bool IsBold() const { return bold_; } + bool IsItalic() const { return italic_; } private: std::string font_name_; + Family family_; bool bold_; bool italic_; }; diff --git a/photopicker/res/values/feature_search_strings.xml b/photopicker/res/values/feature_search_strings.xml index 78af2a470..dacf372f7 100644 --- a/photopicker/res/values/feature_search_strings.xml +++ b/photopicker/res/values/feature_search_strings.xml @@ -24,6 +24,9 @@ <!-- Search view placeholder text when videos MIME type filter is applied--> <string name="photopicker_search_videos_placeholder_text" translation_description="Place holder text shown in Search Bar for videos MIME type filter">Search your videos</string> + <!-- Search Bar trailing icon description text--> + <string name="photopicker_search_clear_text" translation_description="Description for trailing icon to clear search text in Search Bar">Clear search text</string> + <!-- Empty state title when the search has no results --> <string name="photopicker_search_result_empty_state_title" translation_description="Title of the message shown to the user when there are no search results to show">No results found</string> diff --git a/photopicker/src/com/android/photopicker/MainActivity.kt b/photopicker/src/com/android/photopicker/MainActivity.kt index 22bf6bd16..8b148df60 100644 --- a/photopicker/src/com/android/photopicker/MainActivity.kt +++ b/photopicker/src/com/android/photopicker/MainActivity.kt @@ -56,6 +56,7 @@ import com.android.photopicker.core.events.Telemetry import com.android.photopicker.core.events.dispatchReportPhotopickerApiInfoEvent import com.android.photopicker.core.events.dispatchReportPhotopickerMediaItemStatusEvent import com.android.photopicker.core.events.dispatchReportPhotopickerSessionInfoEvent +import com.android.photopicker.core.events.dispatchReportPickerAppMediaCapabilities import com.android.photopicker.core.features.FeatureManager import com.android.photopicker.core.features.LocalFeatureManager import com.android.photopicker.core.selection.GrantsAwareSelectionImpl @@ -289,6 +290,12 @@ class MainActivity : Hilt_MainActivity() { pickerIntentAction = intentAction, lazyFeatureManager = featureManager, ) + + dispatchReportPickerAppMediaCapabilities( + coroutineScope = lifecycleScope, + lazyEvents = events, + photopickerConfiguration = configurationManager.configuration.value, + ) } /** diff --git a/photopicker/src/com/android/photopicker/core/configuration/ConfigurationManager.kt b/photopicker/src/com/android/photopicker/core/configuration/ConfigurationManager.kt index 5054f17c7..d333f4b20 100644 --- a/photopicker/src/com/android/photopicker/core/configuration/ConfigurationManager.kt +++ b/photopicker/src/com/android/photopicker/core/configuration/ConfigurationManager.kt @@ -330,6 +330,7 @@ class ConfigurationManager( /* defaultValue= */ FEATURE_PICKER_CHOICE_MANAGED_SELECTION.second, ), PICKER_SEARCH_ENABLED = Flags.enablePhotopickerSearch(), + PICKER_DATESCRUBBER_ENABLED = Flags.enablePhotopickerDatescrubber(), PICKER_TRANSCODING_ENABLED = Flags.enablePhotopickerTranscoding(), ) } diff --git a/photopicker/src/com/android/photopicker/core/configuration/PhotopickerConfiguration.kt b/photopicker/src/com/android/photopicker/core/configuration/PhotopickerConfiguration.kt index 74bde6db9..4cd6037b8 100644 --- a/photopicker/src/com/android/photopicker/core/configuration/PhotopickerConfiguration.kt +++ b/photopicker/src/com/android/photopicker/core/configuration/PhotopickerConfiguration.kt @@ -95,13 +95,14 @@ data class PhotopickerConfiguration( * the intent to check for CrossProfileIntentForwarder's. Rather than exposing intent as a * public field, this method can be called to do the check, if an Intent exists. * - * @param packageManager the PM of the process owner - * @param handle the [UserHandle] of the target user + * @param packageManager the PM of the "from" user + * @param targetUserHandle the [UserHandle] of the target user * @return Whether the current Intent Photopicker may be running under has a matching * CrossProfileIntentForwarderActivity */ fun doesCrossProfileIntentForwarderExists( packageManager: PackageManager, + fromUserHandle: UserHandle, targetUserHandle: UserHandle, ): Boolean { @@ -124,7 +125,11 @@ data class PhotopickerConfiguration( it.setPackage(null) for (info: ResolveInfo? in - packageManager.queryIntentActivities(it, PackageManager.MATCH_DEFAULT_ONLY)) { + packageManager.queryIntentActivitiesAsUser( + it, + PackageManager.MATCH_DEFAULT_ONLY, + fromUserHandle, + )) { info?.let { if (it.isCrossProfileIntentForwarderActivity()) { @@ -147,7 +152,7 @@ data class PhotopickerConfiguration( it::class.java.getDeclaredField("targetUserId").apply { isAccessible = true } - property?.get(it) as? Int + property.get(it) as? Int } catch (e: Exception) { when (e) { is NoSuchFieldException, @@ -192,6 +197,7 @@ data class PhotopickerConfiguration( "No intent available for checking cross-profile access.", ) + // Nothing left to check return false } diff --git a/photopicker/src/com/android/photopicker/core/configuration/PhotopickerFlags.kt b/photopicker/src/com/android/photopicker/core/configuration/PhotopickerFlags.kt index 271892df9..dc5a9aff7 100644 --- a/photopicker/src/com/android/photopicker/core/configuration/PhotopickerFlags.kt +++ b/photopicker/src/com/android/photopicker/core/configuration/PhotopickerFlags.kt @@ -45,6 +45,7 @@ data class PhotopickerFlags( val PRIVATE_SPACE_ENABLED: Boolean = FEATURE_PRIVATE_SPACE_ENABLED.second, val MANAGED_SELECTION_ENABLED: Boolean = FEATURE_PICKER_CHOICE_MANAGED_SELECTION.second, val PICKER_SEARCH_ENABLED: Boolean = Flags.enablePhotopickerSearch(), + val PICKER_DATESCRUBBER_ENABLED: Boolean = Flags.enablePhotopickerDatescrubber(), val PICKER_TRANSCODING_ENABLED: Boolean = Flags.enablePhotopickerTranscoding(), val OWNED_PHOTOS_ENABLED: Boolean = Flags.revokeAccessOwnedPhotos(), val EXPRESSIVE_THEME_ENABLED: Boolean = Flags.enablePhotopickerExpressiveTheme(), @@ -62,6 +63,7 @@ data class PhotopickerFlags( if (PRIVATE_SPACE_ENABLED != other.PRIVATE_SPACE_ENABLED) return false if (MANAGED_SELECTION_ENABLED != other.MANAGED_SELECTION_ENABLED) return false if (PICKER_SEARCH_ENABLED != other.PICKER_SEARCH_ENABLED) return false + if (PICKER_DATESCRUBBER_ENABLED != other.PICKER_DATESCRUBBER_ENABLED) return false if (PICKER_TRANSCODING_ENABLED != other.PICKER_TRANSCODING_ENABLED) return false return true @@ -79,6 +81,7 @@ data class PhotopickerFlags( PRIVATE_SPACE_ENABLED, MANAGED_SELECTION_ENABLED, PICKER_SEARCH_ENABLED, + PICKER_DATESCRUBBER_ENABLED, PICKER_TRANSCODING_ENABLED, ) } diff --git a/photopicker/src/com/android/photopicker/core/events/Dispatchers.kt b/photopicker/src/com/android/photopicker/core/events/Dispatchers.kt index 3ec63d3f3..e27cb75fd 100644 --- a/photopicker/src/com/android/photopicker/core/events/Dispatchers.kt +++ b/photopicker/src/com/android/photopicker/core/events/Dispatchers.kt @@ -16,6 +16,8 @@ package com.android.photopicker.core.events +import android.media.ApplicationMediaCapabilities +import android.media.MediaFeature import android.provider.MediaStore import com.android.photopicker.core.configuration.PhotopickerConfiguration import com.android.photopicker.core.configuration.PhotopickerRuntimeEnv @@ -49,18 +51,18 @@ fun dispatchReportPhotopickerApiInfoEvent( val sessionId = photopickerConfiguration.sessionId // We always launch the picker in collapsed state. We track the state change as UI event. val pickerSize = Telemetry.PickerSize.COLLAPSED - val mediaFilters = - photopickerConfiguration.mimeTypes - .map { mimeType -> - when { - mimeType.contains("image") && mimeType.contains("video") -> - Telemetry.MediaType.PHOTO_VIDEO - mimeType.startsWith("image/") -> Telemetry.MediaType.PHOTO - mimeType.startsWith("video/") -> Telemetry.MediaType.VIDEO - else -> Telemetry.MediaType.UNSET_MEDIA_TYPE - } - } - .ifEmpty { listOf(Telemetry.MediaType.UNSET_MEDIA_TYPE) } + val mimeTypes = photopickerConfiguration.mimeTypes + val mediaFilter = + when { + mimeTypes.size > 1 && + mimeTypes.any { it.startsWith("image/") } && + mimeTypes.any { it.startsWith("video/") } -> Telemetry.MediaType.PHOTO_VIDEO + mimeTypes.size == 1 && mimeTypes.first().startsWith("image/") -> + Telemetry.MediaType.PHOTO + mimeTypes.size == 1 && mimeTypes.first().startsWith("video/") -> + Telemetry.MediaType.VIDEO + else -> Telemetry.MediaType.UNSET_MEDIA_TYPE + } val maxPickedItemsCount = photopickerConfiguration.selectionLimit val selectedTab = when (photopickerConfiguration.startDestination) { @@ -80,29 +82,85 @@ fun dispatchReportPhotopickerApiInfoEvent( val isCloudSearchEnabled = lazyFeatureManager.get().isFeatureEnabled(SearchFeature::class.java) // TODO(b/376822503): Update when search is added val isLocalSearchEnabled = false - for (mediaFilter in mediaFilters) { - coroutineScope.launch { - lazyEvents - .get() - .dispatch( - Event.ReportPhotopickerApiInfo( - dispatcherToken = dispatcherToken, - sessionId = sessionId, - pickerIntentAction = pickerIntentAction, - pickerSize = pickerSize, - mediaFilter = mediaFilter, - maxPickedItemsCount = maxPickedItemsCount, - selectedTab = selectedTab, - selectedAlbum = selectedAlbum, - isOrderedSelectionSet = isOrderedSelectionSet, - isAccentColorSet = isAccentColorSet, - isDefaultTabSet = isDefaultTabSet, - isCloudSearchEnabled = isCloudSearchEnabled, - isLocalSearchEnabled = isLocalSearchEnabled, - ) + val isTranscodingRequested: Boolean = + photopickerConfiguration.callingPackageMediaCapabilities != null + coroutineScope.launch { + lazyEvents + .get() + .dispatch( + Event.ReportPhotopickerApiInfo( + dispatcherToken = dispatcherToken, + sessionId = sessionId, + pickerIntentAction = pickerIntentAction, + pickerSize = pickerSize, + mediaFilter = mediaFilter, + maxPickedItemsCount = maxPickedItemsCount, + selectedTab = selectedTab, + selectedAlbum = selectedAlbum, + isOrderedSelectionSet = isOrderedSelectionSet, + isAccentColorSet = isAccentColorSet, + isDefaultTabSet = isDefaultTabSet, + isCloudSearchEnabled = isCloudSearchEnabled, + isLocalSearchEnabled = isLocalSearchEnabled, + isTranscodingRequested = isTranscodingRequested, ) + ) + } +} + +/** Dispatches an event to log App transcoding media capabilities if advertised by the app */ +fun dispatchReportPickerAppMediaCapabilities( + coroutineScope: CoroutineScope, + lazyEvents: Lazy<Events>, + photopickerConfiguration: PhotopickerConfiguration, +) { + val dispatcherToken = FeatureToken.CORE.token + val sessionId = photopickerConfiguration.sessionId + val appMediaCapabilities: ApplicationMediaCapabilities? = + photopickerConfiguration.callingPackageMediaCapabilities + if (appMediaCapabilities != null) { + with(appMediaCapabilities) { + val supportedHdrTypes: IntArray = getEnumsForTypes(true, getSupportedHdrTypes()) + val unsupportedHdrTypes: IntArray = getEnumsForTypes(false, getUnsupportedHdrTypes()) + coroutineScope.launch { + lazyEvents + .get() + .dispatch( + Event.ReportPickerAppMediaCapabilities( + dispatcherToken = dispatcherToken, + sessionId = sessionId, + supportedHdrTypes = supportedHdrTypes, + unsupportedHdrTypes = unsupportedHdrTypes, + ) + ) + } + } + } +} + +private fun getEnumsForTypes(supported: Boolean, hdrTypesList: List<String>): IntArray { + var array: MutableList<Int> = mutableListOf() + for (type in hdrTypesList) { + when (type) { + MediaFeature.HdrType.DOLBY_VISION -> { + if (supported) array.add(Telemetry.HdrTypes.DOLBY_SUPPORTED.type) + else array.add(Telemetry.HdrTypes.DOLBY_UNSUPPORTED.type) + } + MediaFeature.HdrType.HDR10 -> { + if (supported) array.add(Telemetry.HdrTypes.HDR10_SUPPORTED.type) + else array.add(Telemetry.HdrTypes.HDR10_UNSUPPORTED.type) + } + MediaFeature.HdrType.HDR10_PLUS -> { + if (supported) array.add(Telemetry.HdrTypes.HDR10PLUS_SUPPORTED.type) + else array.add(Telemetry.HdrTypes.HDR10PLUS_UNSUPPORTED.type) + } + MediaFeature.HdrType.HLG -> { + if (supported) array.add(Telemetry.HdrTypes.HLG_SUPPORTED.type) + else array.add(Telemetry.HdrTypes.HLG_UNSUPPORTED.type) + } } } + return array.toIntArray() } /** Dispatches an event to log all the final state details of the picker */ diff --git a/photopicker/src/com/android/photopicker/core/events/Event.kt b/photopicker/src/com/android/photopicker/core/events/Event.kt index 00d7b0ca5..c6b674c90 100644 --- a/photopicker/src/com/android/photopicker/core/events/Event.kt +++ b/photopicker/src/com/android/photopicker/core/events/Event.kt @@ -96,6 +96,7 @@ interface Event { val isDefaultTabSet: Boolean, val isCloudSearchEnabled: Boolean, val isLocalSearchEnabled: Boolean, + val isTranscodingRequested: Boolean, ) : Event /** @@ -225,6 +226,24 @@ interface Event { val surfacePackageDeliveryStartTime: Int, val surfacePackageDeliveryEndTime: Int, ) : Event + + /** Logs media capabilities of the App requesting transcoding */ + data class ReportPickerAppMediaCapabilities( + override val dispatcherToken: String, + val sessionId: Int, + val supportedHdrTypes: IntArray, + val unsupportedHdrTypes: IntArray, + ) : Event + + /** Logs information about the transcoding video */ + data class ReportTranscodingVideoDetails( + override val dispatcherToken: String, + val sessionId: Int, + val duration: Int, + val colorTransfer: Int, + val colorStandard: Int, + val mimeType: Int, + ) : Event } /** @@ -383,6 +402,57 @@ interface Telemetry { } /* + Different supported and unsupported HDR types + */ + enum class HdrTypes(val type: Int) { + HDR10_SUPPORTED( + MediaProviderStatsLog + .PHOTOPICKER_APP_MEDIA_CAPABILITIES_REPORTED__SUPPORTED_HDR_TYPES__TYPE_HDR10 + ), + HDR10PLUS_SUPPORTED( + MediaProviderStatsLog + .PHOTOPICKER_APP_MEDIA_CAPABILITIES_REPORTED__SUPPORTED_HDR_TYPES__TYPE_HDR10_PLUS + ), + HLG_SUPPORTED( + MediaProviderStatsLog + .PHOTOPICKER_APP_MEDIA_CAPABILITIES_REPORTED__SUPPORTED_HDR_TYPES__TYPE_HLG + ), + DOLBY_SUPPORTED( + MediaProviderStatsLog + .PHOTOPICKER_APP_MEDIA_CAPABILITIES_REPORTED__SUPPORTED_HDR_TYPES__TYPE_DOLBY_VISION + ), + HDR10_UNSUPPORTED( + MediaProviderStatsLog + .PHOTOPICKER_APP_MEDIA_CAPABILITIES_REPORTED__UNSUPPORTED_HDR_TYPES__TYPE_HDR10 + ), + HDR10PLUS_UNSUPPORTED( + MediaProviderStatsLog + .PHOTOPICKER_APP_MEDIA_CAPABILITIES_REPORTED__UNSUPPORTED_HDR_TYPES__TYPE_HDR10_PLUS + ), + HLG_UNSUPPORTED( + MediaProviderStatsLog + .PHOTOPICKER_APP_MEDIA_CAPABILITIES_REPORTED__UNSUPPORTED_HDR_TYPES__TYPE_HLG + ), + DOLBY_UNSUPPORTED( + MediaProviderStatsLog + .PHOTOPICKER_APP_MEDIA_CAPABILITIES_REPORTED__UNSUPPORTED_HDR_TYPES__TYPE_DOLBY_VISION + ), + } + + /* + Different Video mime types + */ + enum class VideoMimeType(val type: Int) { + DOLBY( + MediaProviderStatsLog + .PHOTOPICKER_VIDEO_TRANSCODING_DETAILS_LOGGED__MIME_TYPE__MIME_DOLBY + ), + HEVC( + MediaProviderStatsLog.PHOTOPICKER_VIDEO_TRANSCODING_DETAILS_LOGGED__MIME_TYPE__MIME_HEVC + ), + } + + /* Different picker tabs */ enum class SelectedTab(val tab: Int) { @@ -556,6 +626,15 @@ interface Telemetry { MediaProviderStatsLog.PHOTOPICKER_UIEVENT_LOGGED__UI_EVENT__UI_LOADED_EMPTY_STATE ), UNSET_UI_EVENT(MediaProviderStatsLog.PHOTOPICKER_UIEVENT_LOGGED__UI_EVENT__UNSET_UI_EVENT), + PICKER_TRANSCODING_START( + MediaProviderStatsLog.PHOTOPICKER_UIEVENT_LOGGED__UI_EVENT__PICKER_TRANSCODING_STARTED + ), + PICKER_TRANSCODING_SUCCESS( + MediaProviderStatsLog.PHOTOPICKER_UIEVENT_LOGGED__UI_EVENT__PICKER_TRANSCODING_FINISHED + ), + PICKER_TRANSCODING_FAILED( + MediaProviderStatsLog.PHOTOPICKER_UIEVENT_LOGGED__UI_EVENT__PICKER_TRANSCODING_FAILED + ), } /* diff --git a/photopicker/src/com/android/photopicker/core/events/PhotopickerEventLogger.kt b/photopicker/src/com/android/photopicker/core/events/PhotopickerEventLogger.kt index c6bc6445f..5f3787d76 100644 --- a/photopicker/src/com/android/photopicker/core/events/PhotopickerEventLogger.kt +++ b/photopicker/src/com/android/photopicker/core/events/PhotopickerEventLogger.kt @@ -100,6 +100,7 @@ class PhotopickerEventLogger(val dataService: Lazy<DataService>) { /* is_search_enabled */ false, event.isCloudSearchEnabled, event.isLocalSearchEnabled, + event.isTranscodingRequested, ) } is Event.LogPhotopickerUIEvent -> { @@ -256,6 +257,24 @@ class PhotopickerEventLogger(val dataService: Lazy<DataService>) { event.surfacePackageDeliveryEndTime, ) } + is Event.ReportPickerAppMediaCapabilities -> { + MediaProviderStatsLog.write( + MediaProviderStatsLog.PHOTOPICKER_APP_MEDIA_CAPABILITIES_REPORTED, + event.sessionId, + event.supportedHdrTypes, + event.unsupportedHdrTypes, + ) + } + is Event.ReportTranscodingVideoDetails -> { + MediaProviderStatsLog.write( + MediaProviderStatsLog.PHOTOPICKER_VIDEO_TRANSCODING_DETAILS_LOGGED, + event.sessionId, + event.duration, + event.colorStandard, + event.colorTransfer, + event.mimeType, + ) + } } } } diff --git a/photopicker/src/com/android/photopicker/core/features/FeatureManager.kt b/photopicker/src/com/android/photopicker/core/features/FeatureManager.kt index 0fd821eb6..c1ff94c45 100644 --- a/photopicker/src/com/android/photopicker/core/features/FeatureManager.kt +++ b/photopicker/src/com/android/photopicker/core/features/FeatureManager.kt @@ -121,6 +121,8 @@ class FeatureManager( Event.ReportPhotopickerSearchInfo::class.java, Event.ReportSearchDataExtractionDetails::class.java, Event.ReportEmbeddedPhotopickerInfo::class.java, + Event.ReportPickerAppMediaCapabilities::class.java, + Event.ReportTranscodingVideoDetails::class.java, ) } diff --git a/photopicker/src/com/android/photopicker/core/user/UserMonitor.kt b/photopicker/src/com/android/photopicker/core/user/UserMonitor.kt index 15c1b403c..3c6402697 100644 --- a/photopicker/src/com/android/photopicker/core/user/UserMonitor.kt +++ b/photopicker/src/com/android/photopicker/core/user/UserMonitor.kt @@ -100,7 +100,17 @@ class UserMonitor( properties.getShowInSharingSurfaces() == UserProperties.SHOW_IN_SHARING_SURFACES_SEPARATE } else { - true + when { + processOwnerUserHandle.identifier == it.identifier -> true + // For SDK < V, accept all managed profiles, and the parent + // of the current process owner. Ignore all others. + userManager.isManagedProfile(it.identifier) -> true + it.identifier == + userManager + .getProfileParent(processOwnerUserHandle) + ?.identifier -> true + else -> false + } } } .map { getUserProfileFromHandle(it, context) }, @@ -255,8 +265,7 @@ class UserMonitor( ) { Log.i( TAG, - "The active profile is no longer enabled, transitioning back to the process" + - " owner's profile.", + "The active profile is no longer enabled, transitioning back to the process owner's profile.", ) // The current profile is disabled, we need to transition back to the process @@ -281,8 +290,7 @@ class UserMonitor( ?: run { Log.w( TAG, - "Could not find the process owner's profile to switch to when the" + - " active profile was disabled.", + "Could not find the process owner's profile to switch to when the active profile was disabled.", ) // Still attempt to update the list of profiles. @@ -307,42 +315,103 @@ class UserMonitor( /** * Determines if the current handle supports CrossProfile content sharing. * + * This method accepts a pair of user handles (from/to) and determines if CrossProfile access is + * permitted between those two profiles. + * + * There are differences is on how the access is determined based on the platform SDK: + * - For Platform SDK < V: + * + * A check for CrossProfileIntentForwarders in the origin (from) profile that target the + * destination (to) profile. If such a forwarder exists, then access is allowed, and denied + * otherwise. + * - For Platform SDK >= V: + * + * The method now takes into account access delegation, which was first added in Android V. + * + * For profiles that set the [CROSS_PROFILE_CONTENT_SHARING_DELEGATE_FROM_PARENT] property in + * its [UserProperties], its parent profile will be substituted in for its side of the check. + * + * ex. For access checks between a Managed (from) and Private (to) profile, where: + * - Managed does not delegate to its parent + * - Private delegates to its parent + * + * The following logic is performed: Managed -> parent(Private) + * + * The same check in the other direction would yield: parent(Private) -> Managed + * + * Note how the private profile is never actually used for either side of the check, since it + * is delegating its access check to the parent. And thus, if Managed can access the parent, + * it can also access the private. + * + * @param context Current context object, for switching user contexts. + * @param fromUser The Origin profile, where the user is coming from + * @param toUser The destination profile, where the user is attempting to go to. * @return Whether CrossProfile content sharing is supported in this handle. */ - private fun getIsCrossProfileAllowedForHandle(handle: UserHandle): Boolean { + private fun getIsCrossProfileAllowedForHandle( + context: Context, + fromUser: UserHandle, + toUser: UserHandle, + ): Boolean { + + /** + * Determine if the provided [UserHandle] delegates its cross profile content sharing (both + * to / from this profile) to its parent's access. + * + * @return True if the profile delegates to its parent, false otherwise. + */ + fun profileDelegatesToParent(handle: UserHandle): Boolean { + + // Early exit, this check only exists on V+ + if (!SdkLevel.isAtLeastV()) { + return false + } - // Early exit conditions - if (handle == processOwnerUserHandle) { - return true + val props = userManager.getUserProperties(handle) + return props.getCrossProfileContentSharingStrategy() == + UserProperties.CROSS_PROFILE_CONTENT_SHARING_DELEGATE_FROM_PARENT } - // First, check if cross profile is delegated to parent profile - if (SdkLevel.isAtLeastV()) { - val properties: UserProperties = userManager.getUserProperties(handle) - if ( - /* - * All user profiles with user property - * [UserProperties.CROSS_PROFILE_CONTENT_SHARING_DELEGATE_FROM_PARENT] - * can access each other including its parent. - */ - properties.getCrossProfileContentSharingStrategy() == - UserProperties.CROSS_PROFILE_CONTENT_SHARING_DELEGATE_FROM_PARENT - ) { - val parent = userManager.getProfileParent(handle) + // Early exit conditions, accessing self. + // NOTE: It is also possible to reach this state if this method is recursively checking + // from: parent(A) to:parent(B) where A and B are both children of the same parent. + if (fromUser.identifier == toUser.identifier) { + return true + } - parent?.let { - return getIsCrossProfileAllowedForHandle(it) - } + // Decide if we should use actual from or parent(from) + val currentFromUser: UserHandle = + if (profileDelegatesToParent(fromUser)) { + userManager.getProfileParent(fromUser) ?: fromUser + } else { + fromUser + } - // Couldn't resolve parent, fail closed. - return false + // Decide if we should use actual to or parent(to) + val currentToUser: UserHandle = + if (profileDelegatesToParent(toUser)) { + userManager.getProfileParent(toUser) ?: toUser + } else { + toUser } + + // When the from/to has changed from the original parameters, recursively restart the checks + // with the new from/to handles. + if ( + fromUser.identifier != currentFromUser.identifier || + toUser.identifier != currentToUser.identifier + ) { + return getIsCrossProfileAllowedForHandle(context, currentFromUser, currentToUser) } // As a last resort, no applicable cross profile information found, so inspect the current // configuration and if there is an intent set, try to see // if there is a matching CrossProfileIntentForwarder - return configuration.value.doesCrossProfileIntentForwarderExists(packageManager, handle) + return configuration.value.doesCrossProfileIntentForwarderExists( + packageManager, + fromUser, + toUser, + ) } /** @@ -355,7 +424,8 @@ class UserMonitor( val isParentProfile = userManager.getProfileParent(handle) == null val isManaged = userManager.isManagedProfile(handle.getIdentifier()) val isQuietModeEnabled = userManager.isQuietModeEnabled(handle) - var isCrossProfileSupported = getIsCrossProfileAllowedForHandle(handle) + var isCrossProfileSupported = + getIsCrossProfileAllowedForHandle(context, processOwnerUserHandle, handle) val (icon, label) = try { @@ -410,8 +480,6 @@ class UserMonitor( processOwnerUserHandle -> emptySet() else -> buildSet { - if (isParentProfile) - return@buildSet // Parent profile can always be accessed by children if (isQuietModeEnabled) { add(UserProfile.DisabledReason.QUIET_MODE) diff --git a/photopicker/src/com/android/photopicker/features/categorygrid/CategoryGrid.kt b/photopicker/src/com/android/photopicker/features/categorygrid/CategoryGrid.kt index 7d77ffc51..f6703268c 100644 --- a/photopicker/src/com/android/photopicker/features/categorygrid/CategoryGrid.kt +++ b/photopicker/src/com/android/photopicker/features/categorygrid/CategoryGrid.kt @@ -22,6 +22,7 @@ import androidx.compose.foundation.layout.PaddingValues import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.grid.GridCells import androidx.compose.foundation.lazy.grid.rememberLazyGridState @@ -237,6 +238,7 @@ fun CategoryButton(modifier: Modifier) { imageVector = ImageVector.vectorResource(R.drawable.photopicker_category_icon), contentDescription = null, + modifier = Modifier.size(18.dp), ) Spacer(Modifier.width(8.dp)) Text( diff --git a/photopicker/src/com/android/photopicker/features/photogrid/PhotoGrid.kt b/photopicker/src/com/android/photopicker/features/photogrid/PhotoGrid.kt index f5dfa8eec..b61675737 100644 --- a/photopicker/src/com/android/photopicker/features/photogrid/PhotoGrid.kt +++ b/photopicker/src/com/android/photopicker/features/photogrid/PhotoGrid.kt @@ -28,12 +28,12 @@ import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.grid.GridCells import androidx.compose.foundation.lazy.grid.rememberLazyGridState import androidx.compose.material.icons.Icons import androidx.compose.material.icons.outlined.Image -import androidx.compose.material.icons.outlined.PhotoAlbum import androidx.compose.material3.Icon import androidx.compose.material3.Text import androidx.compose.material3.windowsizeclass.WindowWidthSizeClass @@ -385,7 +385,11 @@ fun PhotoGridNavButton(modifier: Modifier) { when { categoryFeatureEnabled && searchFeatureEnabled -> { Row(verticalAlignment = Alignment.CenterVertically) { - Icon(imageVector = Icons.Outlined.PhotoAlbum, contentDescription = null) + Icon( + imageVector = Icons.Outlined.Image, + contentDescription = null, + modifier = Modifier.size(18.dp), + ) Spacer(Modifier.width(8.dp)) Text( stringResource(R.string.photopicker_photos_nav_button_label), diff --git a/photopicker/src/com/android/photopicker/features/preparemedia/MediaPreparerViewModel.kt b/photopicker/src/com/android/photopicker/features/preparemedia/MediaPreparerViewModel.kt index 41753b265..df626b1a9 100644 --- a/photopicker/src/com/android/photopicker/features/preparemedia/MediaPreparerViewModel.kt +++ b/photopicker/src/com/android/photopicker/features/preparemedia/MediaPreparerViewModel.kt @@ -474,6 +474,31 @@ constructor( // Trigger transcoding. val transcodeStatus = if (transcoder.isTranscodeRequired(context, mediaCapabilities, item)) { + val transcodingVideoInfo: Transcoder.VideoInfo? = + transcoder.getTranscodingVideoInfo() + scope.launch { + val configuration = configurationManager.configuration.value + if (transcodingVideoInfo != null) { + events.dispatch( + Event.ReportTranscodingVideoDetails( + dispatcherToken = FeatureToken.CORE.token, + sessionId = configuration.sessionId, + duration = transcodingVideoInfo.duration, + colorTransfer = transcodingVideoInfo.colorTransfer, + colorStandard = transcodingVideoInfo.colorStandard, + mimeType = transcodingVideoInfo.mimeType, + ) + ) + } + events.dispatch( + Event.LogPhotopickerUIEvent( + FeatureToken.CORE.token, + configuration.sessionId, + configuration.callingPackageUid ?: -1, + Telemetry.UiEvent.PICKER_TRANSCODING_START, + ) + ) + } val uri = item.mediaUri val resultBundle = contentResolver.call( @@ -485,9 +510,31 @@ constructor( if (resultBundle?.getBoolean(PICKER_TRANSCODE_RESULT, false) == true) { Log.v(PrepareMediaFeature.TAG, "Transcode successful: $item") + scope.launch { + val configuration = configurationManager.configuration.value + events.dispatch( + Event.LogPhotopickerUIEvent( + FeatureToken.CORE.token, + configuration.sessionId, + configuration.callingPackageUid ?: -1, + Telemetry.UiEvent.PICKER_TRANSCODING_SUCCESS, + ) + ) + } TranscodeStatus.SUCCEED } else { Log.w(PrepareMediaFeature.TAG, "Not able to transcode: $item") + scope.launch { + val configuration = configurationManager.configuration.value + events.dispatch( + Event.LogPhotopickerUIEvent( + FeatureToken.CORE.token, + configuration.sessionId, + configuration.callingPackageUid ?: -1, + Telemetry.UiEvent.PICKER_TRANSCODING_FAILED, + ) + ) + } TranscodeStatus.NOT_APPLIED } } else { diff --git a/photopicker/src/com/android/photopicker/features/preparemedia/Transcoder.kt b/photopicker/src/com/android/photopicker/features/preparemedia/Transcoder.kt index f2f82e89e..5b1bb38fd 100644 --- a/photopicker/src/com/android/photopicker/features/preparemedia/Transcoder.kt +++ b/photopicker/src/com/android/photopicker/features/preparemedia/Transcoder.kt @@ -26,6 +26,14 @@ import com.android.photopicker.data.model.Media /** Provides methods to help video transcode. */ interface Transcoder { + /** Data class to hold details of the transcoding video. */ + data class VideoInfo( + val duration: Int, + val colorStandard: Int, + val colorTransfer: Int, + val mimeType: Int, + ) + /** * Checks if a transcode is required for the given video. * @@ -39,6 +47,9 @@ interface Transcoder { video: Media.Video, ): Boolean + /** Returns details of the video that needs transcoding */ + fun getTranscodingVideoInfo(): VideoInfo? + companion object { /** diff --git a/photopicker/src/com/android/photopicker/features/preparemedia/TranscoderImpl.kt b/photopicker/src/com/android/photopicker/features/preparemedia/TranscoderImpl.kt index 547440037..bf48ed34f 100644 --- a/photopicker/src/com/android/photopicker/features/preparemedia/TranscoderImpl.kt +++ b/photopicker/src/com/android/photopicker/features/preparemedia/TranscoderImpl.kt @@ -31,11 +31,14 @@ import androidx.annotation.VisibleForTesting import androidx.media3.common.util.MediaFormatUtil.createFormatFromMediaFormat import androidx.media3.common.util.MediaFormatUtil.isVideoFormat import androidx.media3.exoplayer.MediaExtractorCompat +import com.android.photopicker.core.events.Telemetry import com.android.photopicker.data.model.Media /** A class that help video transcode. */ class TranscoderImpl : Transcoder { + private var needsTranscodingVideoInfo: Transcoder.VideoInfo? = null + override fun isTranscodeRequired( context: Context, mediaCapabilities: ApplicationMediaCapabilities?, @@ -53,7 +56,7 @@ class TranscoderImpl : Transcoder { // Check if any video tracks need to be transcoded. val videoTrackMediaFormats = getVideoTrackMediaFormats(context, video) for (mediaFormat in videoTrackMediaFormats) { - if (isTranscodeRequired(mediaFormat, mediaCapabilities)) { + if (isTranscodeRequired(mediaFormat, mediaCapabilities, video.duration)) { return true } } @@ -98,6 +101,7 @@ class TranscoderImpl : Transcoder { fun isTranscodeRequired( mediaFormat: MediaFormat, mediaCapabilities: ApplicationMediaCapabilities, + duration: Int = 0, ): Boolean { val format = createFormatFromMediaFormat(mediaFormat) val mimeType = format.sampleMimeType @@ -111,6 +115,13 @@ class TranscoderImpl : Transcoder { // what the caller intended. if (isHlg10(colorStandard, colorTransfer) && !isHdrTypeSupported(HdrType.HLG)) { + needsTranscodingVideoInfo = + Transcoder.VideoInfo( + duration, + colorStandard ?: 0, + colorTransfer ?: 0, + Telemetry.VideoMimeType.HEVC.type, + ) return true } @@ -119,6 +130,13 @@ class TranscoderImpl : Transcoder { (!isHdrTypeSupported(HdrType.HDR10) || !isHdrTypeSupported(HdrType.HDR10_PLUS)) ) { + needsTranscodingVideoInfo = + Transcoder.VideoInfo( + duration, + colorStandard ?: 0, + colorTransfer ?: 0, + Telemetry.VideoMimeType.HEVC.type, + ) return true } } @@ -127,13 +145,24 @@ class TranscoderImpl : Transcoder { isHdrDolbyVision(mimeType, colorStandard, colorTransfer) && !isHdrTypeSupported(HdrType.DOLBY_VISION) ) { + needsTranscodingVideoInfo = + Transcoder.VideoInfo( + duration, + colorStandard ?: 0, + colorTransfer ?: 0, + Telemetry.VideoMimeType.DOLBY.type, + ) return true } } - return false } + /** Returns details of the video that needs transcoding */ + override fun getTranscodingVideoInfo(): Transcoder.VideoInfo? { + return needsTranscodingVideoInfo + } + companion object { private const val TAG = "Transcoder" @VisibleForTesting const val DURATION_LIMIT_MS = 60_000L // 1 min diff --git a/photopicker/src/com/android/photopicker/features/search/Search.kt b/photopicker/src/com/android/photopicker/features/search/Search.kt index 1e6cb0a6c..9cd927e09 100644 --- a/photopicker/src/com/android/photopicker/features/search/Search.kt +++ b/photopicker/src/com/android/photopicker/features/search/Search.kt @@ -39,6 +39,7 @@ import androidx.compose.foundation.shape.CircleShape import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.ArrowBack +import androidx.compose.material.icons.filled.Close import androidx.compose.material.icons.filled.Today import androidx.compose.material.icons.outlined.HideImage import androidx.compose.material.icons.outlined.History @@ -118,6 +119,7 @@ import com.android.photopicker.core.theme.LocalWindowSizeClass import com.android.photopicker.extensions.navigateToPreviewMedia import com.android.photopicker.extensions.transferScrollableTouchesToHostInEmbedded import com.android.photopicker.features.preview.PreviewFeature +import com.android.photopicker.features.search.SearchViewModel.Companion.ZERO_STATE_SEARCH_QUERY import com.android.photopicker.features.search.model.SearchSuggestion import com.android.photopicker.features.search.model.SearchSuggestionType import com.android.photopicker.features.search.model.UserSearchState @@ -428,16 +430,12 @@ fun SearchInputContent( * @param searchQuery The current text entered in search bar input field. * @param focused A boolean value indicating whether the search input field is currently focused. * @param onSearchQueryChanged A callback function that is invoked when the search query text - * changes. - * * This function receives the updated search query as a parameter. - * + * changes. This function receives the updated search query as a parameter. * @param onFocused A callback function that is invoked when the focus state of the search field - * changes. - * * This function receives a boolean value indicating the new focus state. - * + * changes. This function receives a boolean value indicating the new focus state. * @param onSearch A callback function to be invoked when a text is searched. * @param modifier A Modifier that can be applied to the SearchInput composable to customize its - * * appearance and behavior. + * appearance and behavior. */ @Composable @OptIn(ExperimentalMaterial3Api::class) @@ -473,16 +471,53 @@ private fun SearchInput( expanded = focused, onExpandedChange = onFocused, leadingIcon = { SearchBarIcon(focused, onFocused, onSearchQueryChanged) }, + trailingIcon = { + SearchBarTrailingIcon( + focused && !searchQuery.equals(ZERO_STATE_SEARCH_QUERY), + onSearchQueryChanged, + ) + }, modifier = modifier.focusRequester(focusRequester), ) RequestFocusOnResume(focusRequester = focusRequester, focused) } +/** + * A composable function that displays the trailing icon in a SearchBar. The icon is shown when + * query is typed clicking on which clears the typed text. + * + * @param showClearIcon A boolean value indicating whether clear icon is to be shown + * @param onSearchQueryChanged A callback function that is invoked when the search query text + * changes. This function receives the updated search query as a parameter. + * @param viewModel The `SearchViewModel` providing the search logic and state. + */ @Composable -private fun RequestFocusOnResume(focusRequester: FocusRequester, focused: Boolean) { +private fun SearchBarTrailingIcon( + showClearIcon: Boolean, + onSearchQueryChanged: (String) -> Unit, + viewModel: SearchViewModel = obtainViewModel(), +) { + val searchState by viewModel.searchState.collectAsStateWithLifecycle() + if (showClearIcon && searchState is SearchState.Inactive) { + IconButton(onClick = { onSearchQueryChanged("") }) { + Icon( + Icons.Filled.Close, + contentDescription = stringResource(R.string.photopicker_search_clear_text), + ) + } + } +} + +@Composable +private fun RequestFocusOnResume( + focusRequester: FocusRequester, + focused: Boolean, + viewModel: SearchViewModel = obtainViewModel(), +) { val lifecycleOwner = LocalLifecycleOwner.current + val searchState by viewModel.searchState.collectAsStateWithLifecycle() LaunchedEffect(Unit) { - when (focused) { + when (focused && searchState is SearchState.Inactive) { true -> lifecycleOwner.repeatOnLifecycle(state = Lifecycle.State.RESUMED) { focusRequester.requestFocus() @@ -789,7 +824,11 @@ fun SuggestionItem(suggestion: SearchSuggestion) { } val text = suggestion.displayText ?: "" Text(text = text, modifier = Modifier.padding(start = MEASUREMENT_LARGE_PADDING).weight(1f)) - if (suggestion.type != SearchSuggestionType.FACE && suggestion.icon != null) { + if ( + suggestion.type != SearchSuggestionType.FACE && + suggestion.type != SearchSuggestionType.HISTORY && + suggestion.icon != null + ) { ShowSuggestionIcon(suggestion, Modifier.size(MEASUREMENT_OTHER_ICON).clip(CircleShape)) } } diff --git a/photopicker/tests/src/com/android/photopicker/core/banners/BannerManagerImplTest.kt b/photopicker/tests/src/com/android/photopicker/core/banners/BannerManagerImplTest.kt index 5b40fa827..fb47d8a1a 100644 --- a/photopicker/tests/src/com/android/photopicker/core/banners/BannerManagerImplTest.kt +++ b/photopicker/tests/src/com/android/photopicker/core/banners/BannerManagerImplTest.kt @@ -65,6 +65,7 @@ import org.mockito.ArgumentMatchers.anyInt import org.mockito.Mock import org.mockito.Mockito.anyInt import org.mockito.Mockito.anyString +import org.mockito.Mockito.eq import org.mockito.Mockito.isNull import org.mockito.Mockito.mock import org.mockito.Mockito.never @@ -161,7 +162,13 @@ class BannerManagerImplTest { whenever(mockUserManager.getProfileParent(USER_HANDLE_MANAGED)) { USER_HANDLE_PRIMARY } val mockResolveInfo = ReflectedResolveInfo(USER_ID_MANAGED) - whenever(mockPackageManager.queryIntentActivities(any(Intent::class.java), anyInt())) { + whenever( + mockPackageManager.queryIntentActivitiesAsUser( + any(Intent::class.java), + anyInt(), + eq(USER_HANDLE_PRIMARY), + ) + ) { listOf(mockResolveInfo) } diff --git a/photopicker/tests/src/com/android/photopicker/core/configuration/TestPhotopickerConfiguration.kt b/photopicker/tests/src/com/android/photopicker/core/configuration/TestPhotopickerConfiguration.kt index e444adabb..4382acead 100644 --- a/photopicker/tests/src/com/android/photopicker/core/configuration/TestPhotopickerConfiguration.kt +++ b/photopicker/tests/src/com/android/photopicker/core/configuration/TestPhotopickerConfiguration.kt @@ -17,6 +17,7 @@ package com.android.photopicker.core.configuration import android.content.Intent +import android.media.ApplicationMediaCapabilities import com.android.photopicker.core.events.generatePickerSessionId import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.flow.SharingStarted @@ -69,6 +70,7 @@ class TestPhotopickerConfiguration { private var sessionId: Int = generatePickerSessionId() private var flags: PhotopickerFlags = PhotopickerFlags() private var mimeTypes: ArrayList<String> = arrayListOf("image/*", "video/*") + private var appMediaCapabilities: ApplicationMediaCapabilities? = null fun action(value: String) = apply { this.action = value } @@ -92,6 +94,10 @@ class TestPhotopickerConfiguration { fun mimeTypes(value: ArrayList<String>) = apply { this.mimeTypes = value } + fun appMediaCapabilities(value: ApplicationMediaCapabilities) = apply { + this.appMediaCapabilities = value + } + fun build(): PhotopickerConfiguration { return PhotopickerConfiguration( action = action, @@ -105,6 +111,7 @@ class TestPhotopickerConfiguration { sessionId = sessionId, flags = flags, mimeTypes = mimeTypes, + callingPackageMediaCapabilities = appMediaCapabilities, ) } } diff --git a/photopicker/tests/src/com/android/photopicker/core/events/DispatchersTest.kt b/photopicker/tests/src/com/android/photopicker/core/events/DispatchersTest.kt index 0b99adf27..b63ebc444 100644 --- a/photopicker/tests/src/com/android/photopicker/core/events/DispatchersTest.kt +++ b/photopicker/tests/src/com/android/photopicker/core/events/DispatchersTest.kt @@ -20,11 +20,15 @@ import android.content.ContentResolver import android.content.Context import android.content.pm.PackageManager import android.content.pm.UserProperties +import android.media.ApplicationMediaCapabilities +import android.media.MediaFeature.HdrType import android.net.Uri +import android.os.Build import android.os.Parcel import android.os.UserHandle import android.os.UserManager import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.filters.SdkSuppress import androidx.test.filters.SmallTest import androidx.test.platform.app.InstrumentationRegistry import com.android.modules.utils.build.SdkLevel @@ -39,6 +43,7 @@ import com.android.photopicker.core.events.dispatchPhotopickerExpansionStateChan import com.android.photopicker.core.events.dispatchReportPhotopickerApiInfoEvent import com.android.photopicker.core.events.dispatchReportPhotopickerMediaItemStatusEvent import com.android.photopicker.core.events.dispatchReportPhotopickerSessionInfoEvent +import com.android.photopicker.core.events.dispatchReportPickerAppMediaCapabilities import com.android.photopicker.core.events.generatePickerSessionId import com.android.photopicker.core.features.FeatureManager import com.android.photopicker.core.features.FeatureToken @@ -154,10 +159,9 @@ class DispatchersTest { mockSystemService(mockContext, UserManager::class.java) { mockUserManager } if (SdkLevel.isAtLeastV()) { - whenever(mockUserManager.getUserProperties(any(UserHandle::class.java))) - @JvmSerializableLambda { - UserProperties.Builder().build() - } + whenever( + mockUserManager.getUserProperties(any(UserHandle::class.java)) + ) @JvmSerializableLambda { UserProperties.Builder().build() } whenever(mockUserManager.getUserBadge()) { InstrumentationRegistry.getInstrumentation() .context @@ -358,12 +362,127 @@ class DispatchersTest { } @Test - fun testDispatchReportPhotopickerApiInfoEvent() = runTest { + fun testDispatchReportPhotopickerApiInfoEventWithPhotoMimeType() = runTest { + // Setup + setup(testScope = this) + + val mimeTypeList = arrayListOf("image/jpg") + val telemetryMimeTypeMapping = Telemetry.MediaType.PHOTO + + val pickerIntentAction = Telemetry.PickerIntentAction.ACTION_PICK_IMAGES + val cloudSearch = lazyFeatureManager.get().isFeatureEnabled(SearchFeature::class.java) + val photopickerConfiguration = + TestPhotopickerConfiguration.build { + action(value = "") + sessionId(value = sessionId) + callingPackageUid(value = packageUid) + runtimeEnv(value = PhotopickerRuntimeEnv.EMBEDDED) + mimeTypes(mimeTypeList) + } + + val expectedEvent = + Event.ReportPhotopickerApiInfo( + dispatcherToken = FeatureToken.CORE.token, + sessionId = sessionId, + pickerIntentAction = pickerIntentAction, + pickerSize = Telemetry.PickerSize.COLLAPSED, + mediaFilter = telemetryMimeTypeMapping, + maxPickedItemsCount = 1, + selectedTab = Telemetry.SelectedTab.UNSET_SELECTED_TAB, + selectedAlbum = Telemetry.SelectedAlbum.UNSET_SELECTED_ALBUM, + isOrderedSelectionSet = false, + isAccentColorSet = false, + isDefaultTabSet = false, + isCloudSearchEnabled = cloudSearch, + isLocalSearchEnabled = false, + isTranscodingRequested = false, + ) + + // Action + dispatchReportPhotopickerApiInfoEvent( + coroutineScope = backgroundScope, + lazyEvents = lazyEvents, + photopickerConfiguration = photopickerConfiguration, + pickerIntentAction = pickerIntentAction, + lazyFeatureManager = lazyFeatureManager, + ) + advanceTimeBy(delayTimeMillis = 50) + + // Assert + assertThat(eventsDispatched).contains(expectedEvent) + assertThat(expectedEvent.mediaFilter).isEqualTo(telemetryMimeTypeMapping) + } + + @Test + fun testDispatchReportPhotopickerApiInfoEventWithVideoMimeType() = runTest { + // Setup + setup(testScope = this) + + val mimeTypeList = arrayListOf("video/jpg") + val telemetryMimeTypeMapping = Telemetry.MediaType.VIDEO + + val pickerIntentAction = Telemetry.PickerIntentAction.ACTION_PICK_IMAGES + val cloudSearch = lazyFeatureManager.get().isFeatureEnabled(SearchFeature::class.java) + val photopickerConfiguration = + TestPhotopickerConfiguration.build { + action(value = "") + sessionId(value = sessionId) + callingPackageUid(value = packageUid) + runtimeEnv(value = PhotopickerRuntimeEnv.EMBEDDED) + mimeTypes(mimeTypeList) + } + + val expectedEvent = + Event.ReportPhotopickerApiInfo( + dispatcherToken = FeatureToken.CORE.token, + sessionId = sessionId, + pickerIntentAction = pickerIntentAction, + pickerSize = Telemetry.PickerSize.COLLAPSED, + mediaFilter = telemetryMimeTypeMapping, + maxPickedItemsCount = 1, + selectedTab = Telemetry.SelectedTab.UNSET_SELECTED_TAB, + selectedAlbum = Telemetry.SelectedAlbum.UNSET_SELECTED_ALBUM, + isOrderedSelectionSet = false, + isAccentColorSet = false, + isDefaultTabSet = false, + isCloudSearchEnabled = cloudSearch, + isLocalSearchEnabled = false, + isTranscodingRequested = false, + ) + + // Action + dispatchReportPhotopickerApiInfoEvent( + coroutineScope = backgroundScope, + lazyEvents = lazyEvents, + photopickerConfiguration = photopickerConfiguration, + pickerIntentAction = pickerIntentAction, + lazyFeatureManager = lazyFeatureManager, + ) + advanceTimeBy(delayTimeMillis = 50) + + // Assert + assertThat(eventsDispatched).contains(expectedEvent) + assertThat(expectedEvent.mediaFilter).isEqualTo(telemetryMimeTypeMapping) + } + + @Test + fun testDispatchReportPhotopickerApiInfoEventWithBothPhotoAndVideoMimeType() = runTest { // Setup setup(testScope = this) + val mimeTypeList = arrayListOf("image/jpg", "video/mp4") + val telemetryMimeTypeMapping = Telemetry.MediaType.PHOTO_VIDEO + val pickerIntentAction = Telemetry.PickerIntentAction.ACTION_PICK_IMAGES val cloudSearch = lazyFeatureManager.get().isFeatureEnabled(SearchFeature::class.java) + val photopickerConfiguration = + TestPhotopickerConfiguration.build { + action(value = "") + sessionId(value = sessionId) + callingPackageUid(value = packageUid) + runtimeEnv(value = PhotopickerRuntimeEnv.EMBEDDED) + mimeTypes(mimeTypeList) + } val expectedEvent = Event.ReportPhotopickerApiInfo( @@ -371,7 +490,7 @@ class DispatchersTest { sessionId = sessionId, pickerIntentAction = pickerIntentAction, pickerSize = Telemetry.PickerSize.COLLAPSED, - mediaFilter = Telemetry.MediaType.PHOTO, + mediaFilter = telemetryMimeTypeMapping, maxPickedItemsCount = 1, selectedTab = Telemetry.SelectedTab.UNSET_SELECTED_TAB, selectedAlbum = Telemetry.SelectedAlbum.UNSET_SELECTED_ALBUM, @@ -380,6 +499,7 @@ class DispatchersTest { isDefaultTabSet = false, isCloudSearchEnabled = cloudSearch, isLocalSearchEnabled = false, + isTranscodingRequested = false, ) // Action @@ -394,5 +514,97 @@ class DispatchersTest { // Assert assertThat(eventsDispatched).contains(expectedEvent) + assertThat(expectedEvent.mediaFilter).isEqualTo(telemetryMimeTypeMapping) + } + + @Test + fun testDispatchReportPhotopickerApiInfoEventWithDefaultPhotoAndVideoMimeType() = runTest { + // Setup + setup(testScope = this) + + val mimeTypeList = arrayListOf("image/*", "video/*") + val telemetryMimeTypeMapping = Telemetry.MediaType.PHOTO_VIDEO + + val pickerIntentAction = Telemetry.PickerIntentAction.ACTION_PICK_IMAGES + val cloudSearch = lazyFeatureManager.get().isFeatureEnabled(SearchFeature::class.java) + val photopickerConfiguration = + TestPhotopickerConfiguration.build { + action(value = "") + sessionId(value = sessionId) + callingPackageUid(value = packageUid) + runtimeEnv(value = PhotopickerRuntimeEnv.EMBEDDED) + mimeTypes(mimeTypeList) + } + + val expectedEvent = + Event.ReportPhotopickerApiInfo( + dispatcherToken = FeatureToken.CORE.token, + sessionId = sessionId, + pickerIntentAction = pickerIntentAction, + pickerSize = Telemetry.PickerSize.COLLAPSED, + mediaFilter = telemetryMimeTypeMapping, + maxPickedItemsCount = 1, + selectedTab = Telemetry.SelectedTab.UNSET_SELECTED_TAB, + selectedAlbum = Telemetry.SelectedAlbum.UNSET_SELECTED_ALBUM, + isOrderedSelectionSet = false, + isAccentColorSet = false, + isDefaultTabSet = false, + isCloudSearchEnabled = cloudSearch, + isLocalSearchEnabled = false, + isTranscodingRequested = false, + ) + + // Action + dispatchReportPhotopickerApiInfoEvent( + coroutineScope = backgroundScope, + lazyEvents = lazyEvents, + photopickerConfiguration = photopickerConfiguration, + pickerIntentAction = pickerIntentAction, + lazyFeatureManager = lazyFeatureManager, + ) + advanceTimeBy(delayTimeMillis = 50) + + // Assert + assertThat(eventsDispatched).contains(expectedEvent) + assertThat(expectedEvent.mediaFilter).isEqualTo(telemetryMimeTypeMapping) + } + + @SdkSuppress(minSdkVersion = Build.VERSION_CODES.S) + @Test + fun testDispatchReportPickerAppMediaCapabilities() = runTest { + // Setup + setup(testScope = this) + + val capabilities = + ApplicationMediaCapabilities.Builder().addUnsupportedHdrType(HdrType.HDR10).build() + + val photopickerConfiguration = + TestPhotopickerConfiguration.build { + action(value = "") + sessionId(value = sessionId) + callingPackageUid(value = packageUid) + runtimeEnv(value = PhotopickerRuntimeEnv.EMBEDDED) + appMediaCapabilities(capabilities) + } + + val expectedEvent = + Event.ReportPickerAppMediaCapabilities( + dispatcherToken = FeatureToken.CORE.token, + sessionId = sessionId, + supportedHdrTypes = intArrayOf(), + unsupportedHdrTypes = intArrayOf(Telemetry.HdrTypes.HDR10_UNSUPPORTED.type), + ) + + // Action + dispatchReportPickerAppMediaCapabilities( + coroutineScope = backgroundScope, + lazyEvents = lazyEvents, + photopickerConfiguration = photopickerConfiguration, + ) + advanceTimeBy(delayTimeMillis = 50) + + // Assert + assertThat(eventsDispatched.size).isEqualTo(1) + assertThat(eventsDispatched.get(0).toString()).isEqualTo(expectedEvent.toString()) } } diff --git a/photopicker/tests/src/com/android/photopicker/core/user/UserMonitorTest.kt b/photopicker/tests/src/com/android/photopicker/core/user/UserMonitorTest.kt index adca148b4..aca667018 100644 --- a/photopicker/tests/src/com/android/photopicker/core/user/UserMonitorTest.kt +++ b/photopicker/tests/src/com/android/photopicker/core/user/UserMonitorTest.kt @@ -55,6 +55,7 @@ import org.junit.runner.RunWith import org.mockito.ArgumentCaptor import org.mockito.ArgumentMatchers.any import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.eq import org.mockito.Captor import org.mockito.Mock import org.mockito.Mockito.mock @@ -88,6 +89,10 @@ class UserMonitorTest { private val USER_ID_MANAGED: Int = 10 private val MANAGED_PROFILE_BASE: UserProfile + private val USER_HANDLE_PRIVATE: UserHandle + private val USER_ID_PRIVATE: Int = 11 + private val PRIVATE_PROFILE_BASE: UserProfile + private val initialExpectedStatus: UserStatus private val mockContentResolver: ContentResolver = mock(ContentResolver::class.java) @@ -128,6 +133,19 @@ class UserMonitorTest { label = PLATFORM_PROVIDED_PROFILE_LABEL, ) + val parcel3 = Parcel.obtain() + parcel2.writeInt(USER_ID_PRIVATE) + parcel2.setDataPosition(0) + USER_HANDLE_PRIVATE = UserHandle(parcel3) + parcel3.recycle() + + PRIVATE_PROFILE_BASE = + UserProfile( + handle = USER_HANDLE_PRIVATE, + profileType = UserProfile.ProfileType.UNKNOWN, + label = PLATFORM_PROVIDED_PROFILE_LABEL, + ) + initialExpectedStatus = UserStatus( activeUserProfile = PRIMARY_PROFILE_BASE, @@ -161,7 +179,13 @@ class UserMonitorTest { // Fake for a CrossProfileIntentForwarderActivity for the managed profile val resolveInfoForManagedUser = ReflectedResolveInfo(USER_HANDLE_MANAGED.getIdentifier()) - whenever(mockPackageManager.queryIntentActivities(any(Intent::class.java), anyInt())) { + whenever( + mockPackageManager.queryIntentActivitiesAsUser( + any(Intent::class.java), + anyInt(), + eq(USER_HANDLE_PRIMARY), + ) + ) { listOf(resolveInfoForManagedUser) } @@ -214,6 +238,242 @@ class UserMonitorTest { } } + @Test + fun testProfilesForCrossProfileNoDelegationVPlus() { + assumeTrue(SdkLevel.isAtLeastV()) + + // Add a third profile (private) to the list of profiles + whenever(mockUserManager.userProfiles) { + listOf(USER_HANDLE_PRIMARY, USER_HANDLE_MANAGED, USER_HANDLE_PRIVATE) + } + whenever(mockUserManager.isQuietModeEnabled(USER_HANDLE_PRIVATE)) { false } + whenever(mockUserManager.isManagedProfile(USER_ID_PRIVATE)) { false } + whenever(mockUserManager.getProfileParent(USER_HANDLE_PRIVATE)) { USER_HANDLE_PRIMARY } + + // The private profile should delegate its access to the parent + whenever(mockUserManager.getUserProperties(USER_HANDLE_PRIVATE)) @JvmSerializableLambda { + UserProperties.Builder() + .setCrossProfileContentSharingStrategy( + UserProperties.CROSS_PROFILE_CONTENT_SHARING_NO_DELEGATION + ) + .build() + } + + runTest { // this: TestScope + + // When the primary profile is the process owner + userMonitor = + UserMonitor( + mockContext, + provideTestConfigurationFlow( + scope = this.backgroundScope, + defaultConfiguration = + TestPhotopickerConfiguration.build { + action(MediaStore.ACTION_PICK_IMAGES) + intent(Intent(MediaStore.ACTION_PICK_IMAGES)) + }, + ), + this.backgroundScope, + StandardTestDispatcher(this.testScheduler), + USER_HANDLE_PRIMARY, + ) + + var reportedStatus = userMonitor.userStatus.first() + var expectedStatus = + UserStatus( + activeUserProfile = PRIMARY_PROFILE_BASE, + allProfiles = + listOf( + PRIMARY_PROFILE_BASE, + MANAGED_PROFILE_BASE, + PRIVATE_PROFILE_BASE.copy( + disabledReasons = + setOf(UserProfile.DisabledReason.CROSS_PROFILE_NOT_ALLOWED) + ), + ), + activeContentResolver = mockContentResolver, + ) + assertUserStatusIsEqualIgnoringFields(reportedStatus, expectedStatus) + + // Reset user monitor, private user is now the process owner + userMonitor = + UserMonitor( + mockContext, + provideTestConfigurationFlow( + scope = this.backgroundScope, + defaultConfiguration = + TestPhotopickerConfiguration.build { + action(MediaStore.ACTION_PICK_IMAGES) + intent(Intent(MediaStore.ACTION_PICK_IMAGES)) + }, + ), + this.backgroundScope, + StandardTestDispatcher(this.testScheduler), + USER_HANDLE_PRIVATE, + ) + + reportedStatus = userMonitor.userStatus.first() + expectedStatus = + UserStatus( + activeUserProfile = PRIVATE_PROFILE_BASE, + allProfiles = + listOf( + PRIMARY_PROFILE_BASE.copy( + disabledReasons = + setOf(UserProfile.DisabledReason.CROSS_PROFILE_NOT_ALLOWED) + ), + MANAGED_PROFILE_BASE.copy( + disabledReasons = + setOf(UserProfile.DisabledReason.CROSS_PROFILE_NOT_ALLOWED) + ), + PRIVATE_PROFILE_BASE, + ), + activeContentResolver = mockContentResolver, + ) + assertUserStatusIsEqualIgnoringFields(reportedStatus, expectedStatus) + // + // Reset user monitor, managed user is now the process owner + userMonitor = + UserMonitor( + mockContext, + provideTestConfigurationFlow( + scope = this.backgroundScope, + defaultConfiguration = + TestPhotopickerConfiguration.build { + action(MediaStore.ACTION_PICK_IMAGES) + intent(Intent(MediaStore.ACTION_PICK_IMAGES)) + }, + ), + this.backgroundScope, + StandardTestDispatcher(this.testScheduler), + USER_HANDLE_MANAGED, + ) + + reportedStatus = userMonitor.userStatus.first() + expectedStatus = + UserStatus( + activeUserProfile = MANAGED_PROFILE_BASE, + allProfiles = + listOf( + PRIMARY_PROFILE_BASE, + MANAGED_PROFILE_BASE, + PRIVATE_PROFILE_BASE.copy( + disabledReasons = + setOf(UserProfile.DisabledReason.CROSS_PROFILE_NOT_ALLOWED) + ), + ), + activeContentResolver = mockContentResolver, + ) + assertUserStatusIsEqualIgnoringFields(reportedStatus, expectedStatus) + } + } + + @Test + fun testProfilesForCrossProfileDelegationVPlus() { + assumeTrue(SdkLevel.isAtLeastV()) + + // Add a third profile (private) to the list of profiles + whenever(mockUserManager.userProfiles) { + listOf(USER_HANDLE_PRIMARY, USER_HANDLE_MANAGED, USER_HANDLE_PRIVATE) + } + whenever(mockUserManager.isQuietModeEnabled(USER_HANDLE_PRIVATE)) { false } + whenever(mockUserManager.isManagedProfile(USER_ID_PRIVATE)) { false } + whenever(mockUserManager.getProfileParent(USER_HANDLE_PRIVATE)) { USER_HANDLE_PRIMARY } + + // The private profile should delegate its access to the parent + whenever(mockUserManager.getUserProperties(USER_HANDLE_PRIVATE)) @JvmSerializableLambda { + UserProperties.Builder() + .setCrossProfileContentSharingStrategy( + UserProperties.CROSS_PROFILE_CONTENT_SHARING_DELEGATE_FROM_PARENT + ) + .build() + } + + runTest { // this: TestScope + + // When the primary profile is the process owner + userMonitor = + UserMonitor( + mockContext, + provideTestConfigurationFlow( + scope = this.backgroundScope, + defaultConfiguration = + TestPhotopickerConfiguration.build { + action(MediaStore.ACTION_PICK_IMAGES) + intent(Intent(MediaStore.ACTION_PICK_IMAGES)) + }, + ), + this.backgroundScope, + StandardTestDispatcher(this.testScheduler), + USER_HANDLE_PRIMARY, + ) + + var reportedStatus = userMonitor.userStatus.first() + var expectedStatus = + UserStatus( + activeUserProfile = PRIMARY_PROFILE_BASE, + allProfiles = + listOf(PRIMARY_PROFILE_BASE, MANAGED_PROFILE_BASE, PRIVATE_PROFILE_BASE), + activeContentResolver = mockContentResolver, + ) + assertUserStatusIsEqualIgnoringFields(reportedStatus, expectedStatus) + + // Reset user monitor, private user is now the process owner + userMonitor = + UserMonitor( + mockContext, + provideTestConfigurationFlow( + scope = this.backgroundScope, + defaultConfiguration = + TestPhotopickerConfiguration.build { + action(MediaStore.ACTION_PICK_IMAGES) + intent(Intent(MediaStore.ACTION_PICK_IMAGES)) + }, + ), + this.backgroundScope, + StandardTestDispatcher(this.testScheduler), + USER_HANDLE_PRIVATE, + ) + + reportedStatus = userMonitor.userStatus.first() + expectedStatus = + UserStatus( + activeUserProfile = PRIVATE_PROFILE_BASE, + allProfiles = + listOf(PRIMARY_PROFILE_BASE, MANAGED_PROFILE_BASE, PRIVATE_PROFILE_BASE), + activeContentResolver = mockContentResolver, + ) + assertUserStatusIsEqualIgnoringFields(reportedStatus, expectedStatus) + // + // Reset user monitor, managed user is now the process owner + userMonitor = + UserMonitor( + mockContext, + provideTestConfigurationFlow( + scope = this.backgroundScope, + defaultConfiguration = + TestPhotopickerConfiguration.build { + action(MediaStore.ACTION_PICK_IMAGES) + intent(Intent(MediaStore.ACTION_PICK_IMAGES)) + }, + ), + this.backgroundScope, + StandardTestDispatcher(this.testScheduler), + USER_HANDLE_MANAGED, + ) + + reportedStatus = userMonitor.userStatus.first() + expectedStatus = + UserStatus( + activeUserProfile = MANAGED_PROFILE_BASE, + allProfiles = + listOf(PRIMARY_PROFILE_BASE, MANAGED_PROFILE_BASE, PRIVATE_PROFILE_BASE), + activeContentResolver = mockContentResolver, + ) + assertUserStatusIsEqualIgnoringFields(reportedStatus, expectedStatus) + } + } + /** Ensures profiles with a cross profile forwarding intent are active */ @Test fun testProfilesForCrossProfileIntentForwardingVPlus() { @@ -302,7 +562,13 @@ class UserMonitorTest { ) .build() } - whenever(mockPackageManager.queryIntentActivities(any(Intent::class.java), anyInt())) { + whenever( + mockPackageManager.queryIntentActivitiesAsUser( + any(Intent::class.java), + anyInt(), + any(UserHandle::class.java), + ) + ) { emptyList<ResolveInfo>() } @@ -350,7 +616,13 @@ class UserMonitorTest { assumeFalse(SdkLevel.isAtLeastV()) - whenever(mockPackageManager.queryIntentActivities(any(Intent::class.java), anyInt())) { + whenever( + mockPackageManager.queryIntentActivitiesAsUser( + any(Intent::class.java), + anyInt(), + any(UserHandle::class.java), + ) + ) { emptyList<ResolveInfo>() } @@ -441,7 +713,13 @@ class UserMonitorTest { .build() } - whenever(mockPackageManager.queryIntentActivities(any(Intent::class.java), anyInt())) { + whenever( + mockPackageManager.queryIntentActivitiesAsUser( + any(Intent::class.java), + anyInt(), + eq(USER_HANDLE_PRIMARY), + ) + ) { listOf(ReflectedResolveInfo(USER_HANDLE_MANAGED.getIdentifier())) } @@ -517,7 +795,13 @@ class UserMonitorTest { whenever(mockUserManager.getProfileParent(userHandleUnknownManaged)) { USER_HANDLE_PRIMARY } whenever(mockUserManager.isQuietModeEnabled(userHandleUnknownManaged)) { false } - whenever(mockPackageManager.queryIntentActivities(any(Intent::class.java), anyInt())) { + whenever( + mockPackageManager.queryIntentActivitiesAsUser( + any(Intent::class.java), + anyInt(), + eq(USER_HANDLE_PRIMARY), + ) + ) { listOf(ReflectedResolveInfo(USER_HANDLE_MANAGED.getIdentifier())) } @@ -613,7 +897,13 @@ class UserMonitorTest { .build() } - whenever(mockPackageManager.queryIntentActivities(any(Intent::class.java), anyInt())) { + whenever( + mockPackageManager.queryIntentActivitiesAsUser( + any(Intent::class.java), + anyInt(), + any(UserHandle::class.java), + ) + ) { emptyList<ResolveInfo>() } @@ -696,7 +986,13 @@ class UserMonitorTest { whenever(mockUserManager.getProfileParent(userHandleUnknownManaged)) { USER_HANDLE_PRIMARY } whenever(mockUserManager.isQuietModeEnabled(userHandleUnknownManaged)) { false } - whenever(mockPackageManager.queryIntentActivities(any(Intent::class.java), anyInt())) { + whenever( + mockPackageManager.queryIntentActivitiesAsUser( + any(Intent::class.java), + anyInt(), + any(UserHandle::class.java), + ) + ) { emptyList<ResolveInfo>() } diff --git a/photopicker/tests/src/com/android/photopicker/features/profileselector/ProfileSelectorViewModelTest.kt b/photopicker/tests/src/com/android/photopicker/features/profileselector/ProfileSelectorViewModelTest.kt index 7d776d813..5f52fe3d6 100644 --- a/photopicker/tests/src/com/android/photopicker/features/profileselector/ProfileSelectorViewModelTest.kt +++ b/photopicker/tests/src/com/android/photopicker/features/profileselector/ProfileSelectorViewModelTest.kt @@ -62,6 +62,7 @@ import org.junit.runner.RunWith import org.mockito.Mock import org.mockito.Mockito.any import org.mockito.Mockito.anyInt +import org.mockito.Mockito.eq import org.mockito.MockitoAnnotations @SmallTest @@ -170,7 +171,13 @@ class ProfileSelectorViewModelTest { whenever(mockUserManager.getProfileLabel()) { "label" } } val mockResolveInfo = ReflectedResolveInfo(USER_ID_MANAGED) - whenever(mockPackageManager.queryIntentActivities(any(Intent::class.java), anyInt())) { + whenever( + mockPackageManager.queryIntentActivitiesAsUser( + any(Intent::class.java), + anyInt(), + eq(USER_HANDLE_PRIMARY), + ) + ) { listOf(mockResolveInfo) } } diff --git a/photopicker/tests/src/com/android/photopicker/features/profileselector/SwitchProfileBannerTest.kt b/photopicker/tests/src/com/android/photopicker/features/profileselector/SwitchProfileBannerTest.kt index e69173012..b30286412 100644 --- a/photopicker/tests/src/com/android/photopicker/features/profileselector/SwitchProfileBannerTest.kt +++ b/photopicker/tests/src/com/android/photopicker/features/profileselector/SwitchProfileBannerTest.kt @@ -18,9 +18,12 @@ package com.android.photopicker.features.profileselector import android.content.ContentResolver import android.content.Context +import android.content.Intent import android.content.pm.PackageManager +import android.content.pm.ResolveInfo import android.os.UserHandle import android.os.UserManager +import android.provider.MediaStore import android.test.mock.MockContentResolver import androidx.compose.ui.test.ExperimentalTestApi import androidx.compose.ui.test.assert @@ -73,6 +76,9 @@ import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Rule import org.junit.Test +import org.mockito.ArgumentMatchers.any +import org.mockito.ArgumentMatchers.anyInt +import org.mockito.ArgumentMatchers.eq import org.mockito.Mock import org.mockito.MockitoAnnotations @@ -87,6 +93,17 @@ import org.mockito.MockitoAnnotations @OptIn(ExperimentalCoroutinesApi::class, ExperimentalTestApi::class) class SwitchProfileBannerTest : PhotopickerFeatureBaseTest() { + /** + * Class that exposes the @hide api [targetUserId] in order to supply proper values for + * reflection based code that is inspecting this field. + * + * @property targetUserId + */ + private class ReflectedResolveInfo(@JvmField val targetUserId: Int) : ResolveInfo() { + + override fun isCrossProfileIntentForwarderActivity(): Boolean = true + } + companion object { val USER_ID_PRIMARY: Int = 0 val USER_HANDLE_PRIMARY: UserHandle = UserHandle.of(USER_ID_PRIMARY) @@ -147,6 +164,18 @@ class SwitchProfileBannerTest : PhotopickerFeatureBaseTest() { whenever(mockUserManager.getProfileParent(USER_HANDLE_MANAGED)) { USER_HANDLE_PRIMARY } whenever(mockUserManager.getProfileParent(USER_HANDLE_PRIMARY)) { null } + // Fake for a CrossProfileIntentForwarderActivity for the managed profile + val resolveInfoForPrimaryUser = ReflectedResolveInfo(USER_HANDLE_PRIMARY.getIdentifier()) + whenever( + mockPackageManager.queryIntentActivitiesAsUser( + any(Intent::class.java), + anyInt(), + eq(USER_HANDLE_MANAGED), + ) + ) { + listOf(resolveInfoForPrimaryUser) + } + val resources = getTestableContext().getResources() if (SdkLevel.isAtLeastV()) { whenever(mockUserManager.getProfileLabel()) @@ -159,6 +188,9 @@ class SwitchProfileBannerTest : PhotopickerFeatureBaseTest() { resources.getString(R.string.photopicker_profile_managed_label), ) } + + // Ensure an intent is set for cross profile checking + configurationManager.get().setIntent(Intent(MediaStore.ACTION_PICK_IMAGES)) } @Test diff --git a/src/com/android/providers/media/LocalCallingIdentity.java b/src/com/android/providers/media/LocalCallingIdentity.java index ef98544ec..2a5e452ce 100644 --- a/src/com/android/providers/media/LocalCallingIdentity.java +++ b/src/com/android/providers/media/LocalCallingIdentity.java @@ -29,12 +29,13 @@ import static com.android.providers.media.util.PermissionUtils.checkPermissionIn import static com.android.providers.media.util.PermissionUtils.checkPermissionManager; import static com.android.providers.media.util.PermissionUtils.checkPermissionQueryAllPackages; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadAudio; +import static com.android.providers.media.util.PermissionUtils.checkPermissionReadForLegacyStorage; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadImages; -import static com.android.providers.media.util.PermissionUtils.checkPermissionReadStorage; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadVideo; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadVisualUserSelected; import static com.android.providers.media.util.PermissionUtils.checkPermissionSelf; import static com.android.providers.media.util.PermissionUtils.checkPermissionShell; +import static com.android.providers.media.util.PermissionUtils.checkPermissionUpdateOemMetadata; import static com.android.providers.media.util.PermissionUtils.checkPermissionWriteAudio; import static com.android.providers.media.util.PermissionUtils.checkPermissionWriteImages; import static com.android.providers.media.util.PermissionUtils.checkPermissionWriteStorage; @@ -353,6 +354,7 @@ public class LocalCallingIdentity { public static final int PERMISSION_QUERY_ALL_PACKAGES = 1 << 28; public static final int PERMISSION_ACCESS_MEDIA_OWNER_PACKAGE_NAME = 1 << 29; public static final int PERMISSION_ACCESS_OEM_METADATA = 1 << 30; + public static final int PERMISSION_UPDATE_OEM_METADATA = 1 << 31; private volatile int hasPermission; private volatile int hasPermissionResolved; @@ -430,7 +432,7 @@ public class LocalCallingIdentity { context, pid, uid, getPackageName(), attributionTag, forDataDelivery); case PERMISSION_IS_SYSTEM_GALLERY: return checkWriteImagesOrVideoAppOps( - context, uid, getPackageName(), attributionTag); + context, uid, getPackageName(), attributionTag, forDataDelivery); case PERMISSION_INSTALL_PACKAGES: return checkPermissionInstallPackages( context, pid, uid, getPackageName(), attributionTag); @@ -452,6 +454,9 @@ public class LocalCallingIdentity { case PERMISSION_ACCESS_OEM_METADATA: return checkPermissionAccessOemMetadata(context, pid, uid, getPackageName(), attributionTag); + case PERMISSION_UPDATE_OEM_METADATA: + return checkPermissionUpdateOemMetadata(context, pid, uid, getPackageName(), + attributionTag); default: return false; } @@ -475,8 +480,7 @@ public class LocalCallingIdentity { // To address b/338519249, we will check for sdk version V+ boolean targetSdkIsAtLeastV = getTargetSdkVersion() >= Build.VERSION_CODES.VANILLA_ICE_CREAM; - return checkIsLegacyStorageGranted(context, uid, getPackageName(), attributionTag, - targetSdkIsAtLeastV); + return checkIsLegacyStorageGranted(context, uid, getPackageName(), targetSdkIsAtLeastV); } private volatile boolean shouldBypass; @@ -572,8 +576,14 @@ public class LocalCallingIdentity { } private boolean isLegacyReadInternal() { - return hasPermission(PERMISSION_IS_LEGACY_GRANTED) - && checkPermissionReadStorage(context, pid, uid, getPackageName(), attributionTag); + boolean isLegacyStorageGranted = hasPermission(PERMISSION_IS_LEGACY_GRANTED); + if (!isLegacyStorageGranted) { + return false; + } + + boolean isTargetSdkAtleastT = getTargetSdkVersion() >= Build.VERSION_CODES.TIRAMISU; + return checkPermissionReadForLegacyStorage(context, pid, uid, getPackageName(), + attributionTag, isTargetSdkAtleastT); } /** System internals or callers holding permission have no redaction */ @@ -749,6 +759,13 @@ public class LocalCallingIdentity { } /** + * Returns {@code true} if this package has permission to update oem_metadata of any media. + */ + public boolean checkCallingPermissionToUpdateOemMetadata() { + return hasPermission(PERMISSION_UPDATE_OEM_METADATA); + } + + /** * Returns {@code true} if this package is a legacy app and has read permission */ public boolean isCallingPackageLegacyRead() { diff --git a/src/com/android/providers/media/MediaProvider.java b/src/com/android/providers/media/MediaProvider.java index f5edd912c..85227f7d6 100644 --- a/src/com/android/providers/media/MediaProvider.java +++ b/src/com/android/providers/media/MediaProvider.java @@ -34,6 +34,7 @@ import static android.provider.CloudMediaProviderContract.METHOD_GET_ASYNC_CONTE import static android.provider.MediaStore.EXTRA_IS_STABLE_URIS_ENABLED; import static android.provider.MediaStore.EXTRA_OPEN_ASSET_FILE_REQUEST; import static android.provider.MediaStore.EXTRA_OPEN_FILE_REQUEST; +import static android.provider.MediaStore.EXTRA_URI_LIST; import static android.provider.MediaStore.Files.FileColumns.MEDIA_TYPE; import static android.provider.MediaStore.Files.FileColumns.MEDIA_TYPE_IMAGE; import static android.provider.MediaStore.Files.FileColumns.MEDIA_TYPE_VIDEO; @@ -513,6 +514,17 @@ public class MediaProvider extends ContentProvider { @EnabledAfter(targetSdkVersion = Build.VERSION_CODES.VANILLA_ICE_CREAM) public static final long ENABLE_OWNED_PHOTOS = 310703690L; + + /** + * Excludes unreliable storage volumes from being included in + * {@link MediaStore#getExternalVolumeNames(Context)}. + */ + @ChangeId + @EnabledSince(targetSdkVersion = Build.VERSION_CODES.CUR_DEVELOPMENT) + @VisibleForTesting + // TODO: b/402623169 Set CUR_DEVELOPMENT as the latest version once available + static final long EXCLUDE_UNRELIABLE_STORAGE_VOLUMES = 391360514L; + /** * Set of {@link Cursor} columns that refer to raw filesystem paths. */ @@ -5849,6 +5861,12 @@ public class MediaProvider extends ContentProvider { } } + + // Enforce oem_metadata permission if caller is not MediaProvider + if (Flags.enableOemMetadataUpdate() && initialValues.containsKey(OEM_METADATA)) { + enforcePermissionCheckForOemMetadataUpdate(); + } + long rowId = -1; Uri newUri = null; @@ -7306,11 +7324,80 @@ public class MediaProvider extends ContentProvider { removeRecoveryData(); return new Bundle(); } + case MediaStore.BULK_UPDATE_OEM_METADATA_CALL: { + callForBulkUpdateOemMetadataColumn(); + return new Bundle(); + } default: throw new UnsupportedOperationException("Unsupported call: " + method); } } + private void callForBulkUpdateOemMetadataColumn() { + if (!Flags.enableOemMetadataUpdate()) { + return; + } + + enforcePermissionCheckForOemMetadataUpdate(); + Set<String> oemSupportedMimeTypes = mMediaScanner.getOemSupportedMimeTypes(); + if (oemSupportedMimeTypes == null || oemSupportedMimeTypes.isEmpty()) { + // Nothing to update + return; + } + + // Get media types to update rows based on media type + Set<Integer> mediaTypesToBeUpdated = new HashSet<>(); + for (String mimeType : oemSupportedMimeTypes) { + // Convert to media type to avoid using like clause on mime types to protect against + // SQL injection + mediaTypesToBeUpdated.add(MimeUtils.resolveMediaType(mimeType)); + } + + if (mediaTypesToBeUpdated.isEmpty()) { + // For invalid mime types, do not bother + return; + } + + final LocalCallingIdentity token = clearLocalCallingIdentity(); + try { + ContentValues values = new ContentValues(); + values.putNull(OEM_METADATA); + // Mark _modifier as _MODIFIER_CR to allow metadata update on next scan. This + // is explicitly required when calling update with MediaProvider identity + values.put(FileColumns._MODIFIER, FileColumns._MODIFIER_CR); + Bundle extras = new Bundle(); + extras.putString(ContentResolver.QUERY_ARG_SQL_SELECTION, + appendMediaTypeClause(mediaTypesToBeUpdated)); + Log.v(TAG, "Trigger bulk update of OEM metadata"); + update(MediaStore.Files.getContentUri(MediaStore.VOLUME_EXTERNAL), values, extras); + } finally { + restoreLocalCallingIdentity(token); + } + } + + private String appendMediaTypeClause(Set<Integer> mediaTypesToBeUpdated) { + List<String> whereMediaTypesCondition = new ArrayList<String>(); + for (Integer mediaType : mediaTypesToBeUpdated) { + whereMediaTypesCondition.add( + String.format(Locale.ROOT, "%s=%d", MEDIA_TYPE, mediaType)); + } + + StringBuilder sb = new StringBuilder(); + sb.append("("); + sb.append(TextUtils.join(" OR ", whereMediaTypesCondition)); + sb.append(")"); + return sb.toString(); + } + + @VisibleForTesting + protected void enforcePermissionCheckForOemMetadataUpdate() { + if (!isCallingPackageSelf() + && !mCallingIdentity.get().checkCallingPermissionToUpdateOemMetadata()) { + throw new SecurityException( + "Calling package does not have permission to update OEM metadata"); + } + } + @Nullable private Bundle getResultForRevokeReadGrantForPackage(Bundle extras) { final int caller = Binder.getCallingUid(); @@ -7353,7 +7440,7 @@ public class MediaProvider extends ContentProvider { userId = uidToUserId(packageUid); // Uris are not a requirement for revoke all call if (!isCallForRevokeAll) { - uris = extras.getParcelableArrayList(MediaStore.EXTRA_URI_LIST); + uris = extras.getParcelableArrayList(EXTRA_URI_LIST); } } else { // All other callers are unauthorized. @@ -7630,14 +7717,14 @@ public class MediaProvider extends ContentProvider { @NotNull private Bundle getResultForGetRedactedMediaUriList(Bundle extras) { - final List<Uri> uris = extras.getParcelableArrayList(MediaStore.EXTRA_URI_LIST); + final List<Uri> uris = extras.getParcelableArrayList(EXTRA_URI_LIST); // NOTE: It is ok to update the DB and return a redacted URI for the cases when // the user code only has read access, hence we don't check for write permission. enforceCallingPermission(uris, false); final LocalCallingIdentity token = clearLocalCallingIdentity(); try { final Bundle res = new Bundle(); - res.putParcelableArrayList(MediaStore.EXTRA_URI_LIST, + res.putParcelableArrayList(EXTRA_URI_LIST, (ArrayList<? extends Parcelable>) getRedactedUri(uris)); return res; } finally { @@ -7668,12 +7755,12 @@ public class MediaProvider extends ContentProvider { } else if (checkPermissionSelf(caller) || isCallerPhotoPicker()) { // If the caller is MediaProvider the accepted parameters are EXTRA_URI_LIST // and EXTRA_UID. - if (!extras.containsKey(MediaStore.EXTRA_URI_LIST) + if (!extras.containsKey(EXTRA_URI_LIST) && !extras.containsKey(Intent.EXTRA_UID)) { throw new IllegalArgumentException( "Missing required extras arguments: EXTRA_URI_LIST or" + " EXTRA_UID"); } - uris = extras.getParcelableArrayList(MediaStore.EXTRA_URI_LIST); + uris = extras.getParcelableArrayList(EXTRA_URI_LIST); final PackageManager pm = getContext().getPackageManager(); final int packageUid = extras.getInt(Intent.EXTRA_UID); final String[] packages = pm.getPackagesForUid(packageUid); @@ -7773,7 +7860,8 @@ public class MediaProvider extends ContentProvider { String packageName = arg; int uid = extras.getInt(MediaStore.EXTRA_IS_SYSTEM_GALLERY_UID); boolean isSystemGallery = PermissionUtils.checkWriteImagesOrVideoAppOps( - getContext(), uid, packageName, getContext().getAttributionTag()); + getContext(), uid, packageName, getContext().getAttributionTag(), + /*forDataDelivery*/ false); Bundle res = new Bundle(); res.putBoolean(MediaStore.EXTRA_IS_SYSTEM_GALLERY_RESPONSE, isSystemGallery); return res; @@ -8752,6 +8840,11 @@ public class MediaProvider extends ContentProvider { initialValues.remove(FileColumns.GENERATION_MODIFIED); } + // Enforce oem_metadata permission if caller is not MediaProvider + if (Flags.enableOemMetadataUpdate() && initialValues.containsKey(OEM_METADATA)) { + enforcePermissionCheckForOemMetadataUpdate(); + } + if (!isCallingPackageSelf()) { Trace.beginSection("MP.filter"); @@ -11888,14 +11981,15 @@ public class MediaProvider extends ContentProvider { final ContentResolver resolver = getContext().getContentResolver(); final Uri uri = getBaseContentUri(volumeName); // TODO(b/182396009) we probably also want to notify clone profile (and vice versa) - resolver.notifyChange(getBaseContentUri(volumeName), null); + ForegroundThread.getExecutor().execute(() -> { + resolver.notifyChange(getBaseContentUri(volumeName), null); + }); if (LOGV) Log.v(TAG, "Attached volume: " + volume); if (!MediaStore.VOLUME_INTERNAL.equals(volumeName)) { - // Also notify on synthetic view of all devices - resolver.notifyChange(getBaseContentUri(MediaStore.VOLUME_EXTERNAL), null); - ForegroundThread.getExecutor().execute(() -> { + // Also notify on synthetic view of all devices + resolver.notifyChange(getBaseContentUri(MediaStore.VOLUME_EXTERNAL), null); mExternalDatabase.runWithTransaction((db) -> { ensureNecessaryFolders(volume, db); return null; @@ -11956,11 +12050,15 @@ public class MediaProvider extends ContentProvider { } final ContentResolver resolver = getContext().getContentResolver(); - resolver.notifyChange(getBaseContentUri(volumeName), null); + ForegroundThread.getExecutor().execute(() -> { + resolver.notifyChange(getBaseContentUri(volumeName), null); + }); if (!MediaStore.VOLUME_INTERNAL.equals(volumeName)) { - // Also notify on synthetic view of all devices - resolver.notifyChange(getBaseContentUri(MediaStore.VOLUME_EXTERNAL), null); + ForegroundThread.getExecutor().execute(() -> { + // Also notify on synthetic view of all devices + resolver.notifyChange(getBaseContentUri(MediaStore.VOLUME_EXTERNAL), null); + }); } if (LOGV) Log.v(TAG, "Detached volume: " + volumeName); @@ -12052,6 +12150,8 @@ public class MediaProvider extends ContentProvider { sMutableColumns.add(MediaStore.Files.FileColumns.MIME_TYPE); sMutableColumns.add(MediaStore.Files.FileColumns.MEDIA_TYPE); + + sMutableColumns.add(MediaStore.Files.FileColumns.OEM_METADATA); } /** diff --git a/src/com/android/providers/media/photopicker/data/PickerDbFacade.java b/src/com/android/providers/media/photopicker/data/PickerDbFacade.java index 23124ca48..3145ab73b 100644 --- a/src/com/android/providers/media/photopicker/data/PickerDbFacade.java +++ b/src/com/android/providers/media/photopicker/data/PickerDbFacade.java @@ -35,6 +35,7 @@ import android.content.ContentUris; import android.content.ContentValues; import android.content.Context; import android.database.Cursor; +import android.database.DatabaseUtils; import android.database.MatrixCursor; import android.database.MergeCursor; import android.database.sqlite.SQLiteConstraintException; @@ -1167,11 +1168,31 @@ public class PickerDbFacade { } private Cursor queryMediaIdForAppsLocked(@NonNull SQLiteQueryBuilder qb, - @NonNull String[] projection, @NonNull String[] selectionArgs, + @NonNull String[] columns, @NonNull String[] selectionArgs, String pickerSegmentType) { - return qb.query(getDatabase(), getMediaStoreProjectionLocked(projection, pickerSegmentType), - /* selection */ null, selectionArgs, /* groupBy */ null, /* having */ null, - /* orderBy */ null, /* limitStr */ null); + final Cursor cursor = + qb.query(getDatabase(), getMediaStoreProjectionLocked(columns, pickerSegmentType), + /* selection */ null, selectionArgs, /* groupBy */ null, /* having */ null, + /* orderBy */ null, /* limitStr */ null); + + if (columns == null || columns.length == 0 || cursor.getColumnCount() == columns.length) { + return cursor; + } else { + // An unknown column was encountered. Populate it will null for backwards compatibility. + final MatrixCursor result = new MatrixCursor(columns); + if (cursor.moveToFirst()) { + do { + final ContentValues contentValues = new ContentValues(); + DatabaseUtils.cursorRowToContentValues(cursor, contentValues); + final MatrixCursor.RowBuilder rowBuilder = result.newRow(); + for (String column : columns) { + rowBuilder.add(column, contentValues.get(column)); + } + } while (cursor.moveToNext()); + } + cursor.close(); + return result; + } } /** @@ -1325,54 +1346,51 @@ public class PickerDbFacade { } private String[] getMediaStoreProjectionLocked(String[] columns, String pickerSegmentType) { - final String[] projection = new String[columns.length]; + final List<String> projection = new ArrayList<>(); - for (int i = 0; i < projection.length; i++) { + for (int i = 0; i < columns.length; i++) { switch (columns[i]) { case PickerMediaColumns.DATA: - projection[i] = getProjectionDataLocked(PickerMediaColumns.DATA, - pickerSegmentType); + projection.add(getProjectionDataLocked(PickerMediaColumns.DATA, + pickerSegmentType)); break; case PickerMediaColumns.DISPLAY_NAME: - projection[i] = - getProjectionSimple( - getDisplayNameSql(), PickerMediaColumns.DISPLAY_NAME); + projection.add(getProjectionSimple( + getDisplayNameSql(), PickerMediaColumns.DISPLAY_NAME)); break; case PickerMediaColumns.MIME_TYPE: - projection[i] = - getProjectionSimple(KEY_MIME_TYPE, PickerMediaColumns.MIME_TYPE); + projection.add(getProjectionSimple( + KEY_MIME_TYPE, PickerMediaColumns.MIME_TYPE)); break; case PickerMediaColumns.DATE_TAKEN: - projection[i] = - getProjectionSimple(KEY_DATE_TAKEN_MS, PickerMediaColumns.DATE_TAKEN); + projection.add(getProjectionSimple( + KEY_DATE_TAKEN_MS, PickerMediaColumns.DATE_TAKEN)); break; case PickerMediaColumns.SIZE: - projection[i] = getProjectionSimple(KEY_SIZE_BYTES, PickerMediaColumns.SIZE); + projection.add(getProjectionSimple(KEY_SIZE_BYTES, PickerMediaColumns.SIZE)); break; case PickerMediaColumns.DURATION_MILLIS: - projection[i] = - getProjectionSimple( - KEY_DURATION_MS, PickerMediaColumns.DURATION_MILLIS); + projection.add(getProjectionSimple( + KEY_DURATION_MS, PickerMediaColumns.DURATION_MILLIS)); break; case PickerMediaColumns.HEIGHT: - projection[i] = getProjectionSimple(KEY_HEIGHT, PickerMediaColumns.HEIGHT); + projection.add(getProjectionSimple(KEY_HEIGHT, PickerMediaColumns.HEIGHT)); break; case PickerMediaColumns.WIDTH: - projection[i] = getProjectionSimple(KEY_WIDTH, PickerMediaColumns.WIDTH); + projection.add(getProjectionSimple(KEY_WIDTH, PickerMediaColumns.WIDTH)); break; case PickerMediaColumns.ORIENTATION: - projection[i] = - getProjectionSimple(KEY_ORIENTATION, PickerMediaColumns.ORIENTATION); + projection.add(getProjectionSimple( + KEY_ORIENTATION, PickerMediaColumns.ORIENTATION)); break; default: - projection[i] = getProjectionSimple("NULL", columns[i]); // Ignore unsupported columns; we do not throw error here to support - // backward compatibility + // backward compatibility for ACTION_GET_CONTENT takeover. Log.w(TAG, "Unexpected Picker column: " + columns[i]); } } - return projection; + return projection.toArray(new String[0]); } private String getProjectionAuthorityLocked() { diff --git a/src/com/android/providers/media/photopicker/data/UserManagerState.java b/src/com/android/providers/media/photopicker/data/UserManagerState.java index de6af1b0c..8df364c3c 100644 --- a/src/com/android/providers/media/photopicker/data/UserManagerState.java +++ b/src/com/android/providers/media/photopicker/data/UserManagerState.java @@ -21,14 +21,11 @@ import static androidx.core.util.Preconditions.checkNotNull; import android.annotation.NonNull; import android.annotation.Nullable; import android.annotation.RequiresApi; -import android.annotation.SuppressLint; -import android.app.ActivityManager; import android.content.Context; import android.content.Intent; import android.content.pm.PackageManager; import android.content.pm.ResolveInfo; import android.content.pm.UserProperties; -import android.content.res.Resources; import android.graphics.drawable.Drawable; import android.os.Build; import android.os.Handler; @@ -243,48 +240,40 @@ public interface UserManagerState { setUserIds(); } - private UserId getSystemUser() { - return UserId.of(UserHandle.of(ActivityManager.getCurrentUser())); - } - private void setUserIds() { - setUserIdsInternal(); - mIsMultiUserProfiles.postValue(isMultiUserProfiles()); - } - - private void setUserIdsInternal() { mUserProfileIds.clear(); - mUserProfileIds.add(getSystemUser()); - if (mUserManager == null) { - Log.e(TAG, "Cannot obtain user manager"); - return; - } - // Here there could be other profiles too , that we do not want to show anywhere - // in photo picker at all. - final List<UserHandle> userProfiles = mUserManager.getUserProfiles(); - if (SdkLevel.isAtLeastV()) { - for (UserHandle userHandle : userProfiles) { - UserProperties userProperties = mUserManager.getUserProperties(userHandle); - UserId userId = UserId.of(userHandle); - - // Check if we want to show this profile data in PhotoPicker or if it is - // an owner profile itself. - if (getSystemUser().getIdentifier() != userHandle.getIdentifier() - && userProperties.getShowInSharingSurfaces() - == userProperties.SHOW_IN_SHARING_SURFACES_SEPARATE) { - mUserProfileIds.add(userId); + mUserProfileIds.add(mCurrentUser); + boolean currentUserIsManaged = + mUserManager.isManagedProfile(mCurrentUser.getIdentifier()); + + for (UserHandle handle : mUserManager.getUserProfiles()) { + + // For >= Android V, check if the profile wants to be shown + if (SdkLevel.isAtLeastV()) { + + UserProperties properties = mUserManager.getUserProperties(handle); + if (properties.getShowInSharingSurfaces() + != UserProperties.SHOW_IN_SHARING_SURFACES_SEPARATE) { + continue; } - } - } else { - // if sdk version is less than V, then maximum two profiles with separate tab - // could only be available - for (UserHandle userHandle : userProfiles) { - if (mUserManager.isManagedProfile(userHandle.getIdentifier())) { - mUserProfileIds.add(UserId.of(userHandle)); + } else { + // Only allow managed profiles + the parent user on lower than V. + if (currentUserIsManaged + && mUserManager.getProfileParent(mCurrentUser.getUserHandle()) + == handle) { + // Intentionally empty so that this profile gets added. + } else if (!mUserManager.isManagedProfile(handle.getIdentifier())) { + continue; } } + + // Ensure the system user doesn't get added twice. + if (mUserProfileIds.contains(UserId.of(handle))) continue; + mUserProfileIds.add(UserId.of(handle)); } + + mIsMultiUserProfiles.postValue(isMultiUserProfiles()); } @Override @@ -313,12 +302,162 @@ public interface UserManagerState { return crossProfileAllowedStatusForAll; } + + /** + * External method that allows quick checking from the current user to a target user. + * + * Takes into account the On/Off state of the profile, as well as cross profile content + * sharing policies. + * + * @param targetUser the target of the access. Current User is the "from" user. + * @return If the target user currently is eligible for cross profile content sharing. + */ @Override - public boolean isCrossProfileAllowedToUser(UserId otherUser) { + public boolean isCrossProfileAllowedToUser(UserId targetUser) { assertMainThread(); - return !isProfileOff(otherUser) && !isBlockedByAdmin(otherUser); + return !isProfileOff(targetUser) && !isBlockedByAdmin(targetUser); + } + + /** + * Determines if the provided UserIds support CrossProfile content sharing. + * + * <p>This method accepts a pair of user handles (from/to) and determines if CrossProfile + * access is permitted between those two profiles. + * + * <p>There are differences is on how the access is determined based on the platform SDK: + * + * <p>For Platform SDK < V: + * + * <p>A check for CrossProfileIntentForwarders in the origin (from) profile that target the + * destination (to) profile. If such a forwarder exists, then access is allowed, and denied + * otherwise. + * + * <p>For Platform SDK >= V: + * + * <p>The method now takes into account access delegation, which was first added in Android + * V. + * + * <p>For profiles that set the [CROSS_PROFILE_CONTENT_SHARING_DELEGATE_FROM_PARENT] + * property in its [UserProperties], its parent profile will be substituted in for its side + * of the check. + * + * <p>ex. For access checks between a Managed (from) and Private (to) profile, where: - + * Managed does not delegate to its parent - Private delegates to its parent + * + * <p>The following logic is performed: Managed -> parent(Private) + * + * <p>The same check in the other direction would yield: parent(Private) -> Managed + * + * <p>Note how the private profile is never actually used for either side of the check, + * since it is delegating its access check to the parent. And thus, if Managed can access + * the parent, it can also access the private. + * + * @param context Current context object, for switching user contexts. + * @param intent The current intent the Photopicker is running under. + * @param fromUser The Origin profile, where the user is coming from + * @param toUser The destination profile, where the user is attempting to go to. + * @return Whether CrossProfile content sharing is supported in this handle. + */ + private boolean isCrossProfileAllowedToUser( + Context context, Intent intent, UserId fromUser, UserId toUser) { + + // Early exit conditions, accessing self. + // NOTE: It is also possible to reach this state if this method is recursively checking + // from: parent(A) to:parent(B) where A and B are both children of the same parent. + if (fromUser.getIdentifier() == toUser.getIdentifier()) { + return true; + } + + // Decide if we should use actual from or parent(from) + UserHandle currentFromUser = + getProfileToCheckCrossProfileAccess(fromUser.getUserHandle()); + + // Decide if we should use actual to or parent(to) + UserHandle currentToUser = getProfileToCheckCrossProfileAccess(toUser.getUserHandle()); + + // When the from/to has changed from the original parameters, recursively restart the + // checks with the new from/to handles. + if (fromUser.getIdentifier() != currentFromUser.getIdentifier() + || toUser.getIdentifier() != currentToUser.getIdentifier()) { + return isCrossProfileAllowedToUser( + context, intent, UserId.of(currentFromUser), UserId.of(currentToUser)); + } + + return doesCrossProfileIntentForwarderExist( + intent, + mContext.getPackageManager(), + fromUser.getUserHandle(), + toUser.getUserHandle()); } + /** + * Checks the Intent to see if it can be resolved as a CrossProfileIntentForwarderActivity + * for the target user. + * + * @param intent The current intent the photopicker is running under. + * @param pm the PM which will be used for querying. + * @param fromUser the [UserHandle] of the origin user + * @param targetUserHandle the [UserHandle] of the target user + * @return Whether the current Intent Photopicker may be running under has a matching + * CrossProfileIntentForwarderActivity + */ + private boolean doesCrossProfileIntentForwarderExist( + Intent intent, + PackageManager pm, + UserHandle fromUser, + UserHandle targetUserHandle) { + + // Clear out the component & package before attempting to match + Intent intentToCheck = (Intent) intent.clone(); + intentToCheck.setComponent(null); + intentToCheck.setPackage(null); + + for (ResolveInfo resolveInfo : + pm.queryIntentActivitiesAsUser( + intentToCheck, PackageManager.MATCH_DEFAULT_ONLY, fromUser)) { + + // If the activity is a CrossProfileIntentForwardingActivity, inspect its + // targetUserId to see if it targets the user we are currently checking for. + if (resolveInfo.isCrossProfileIntentForwarderActivity()) { + + /* + * IMPORTANT: This is a reflection based hack to ensure the profile is + * actually the installer of the CrossProfileIntentForwardingActivity. + * + * ResolveInfo.targetUserId exists, but is a hidden API not available to + * mainline modules, and no such API exists, so it is accessed via + * reflection below. All exceptions are caught to protect against + * reflection related issues such as: + * NoSuchFieldException / IllegalAccessException / SecurityException. + * + * In the event of an exception, the code fails "closed" for the current + * profile to avoid showing content that should not be visible. + */ + try { + Field targetUserIdField = + resolveInfo.getClass().getDeclaredField("targetUserId"); + targetUserIdField.setAccessible(true); + int targetUserId = (int) targetUserIdField.get(resolveInfo); + + if (targetUserId == targetUserHandle.getIdentifier()) { + // Don't need to look further, exit the loop. + return true; + } + + } catch (NoSuchFieldException | IllegalAccessException | SecurityException ex) { + // Couldn't check the targetUserId via reflection, so fail without + // further iterations. + Log.e(TAG, "Could not access targetUserId via reflection.", ex); + return false; + } catch (Exception ex) { + Log.e(TAG, "Exception occurred during cross profile checks", ex); + } + } + } + return false; + } + + @Override public MutableLiveData<Boolean> getIsMultiUserProfiles() { return mIsMultiUserProfiles; @@ -332,8 +471,7 @@ public interface UserManagerState { @Override public void resetUserIdsAndSetCrossProfileValues(Intent intent) { - assertMainThread(); - setUserIdsInternal(); + resetUserIds(); setCrossProfileValues(intent); mIsMultiUserProfiles.postValue(isMultiUserProfiles()); } @@ -511,6 +649,12 @@ public interface UserManagerState { || !CrossProfileUtils.isMediaProviderAvailable(userId, mContext); } + /** + * Determines if the target UserHandle delegates its content sharing to its parent. + * + * @param userHandle The target handle to check delegation for. + * @return TRUE if V+ and the handle delegates to parent. False otherwise. + */ private boolean isCrossProfileStrategyDelegatedToParent(UserHandle userHandle) { if (SdkLevel.isAtLeastV()) { if (mUserManager == null) { @@ -526,6 +670,15 @@ public interface UserManagerState { return false; } + /** + * Acquires the correct {@link UserHandle} which should be used for CrossProfile access + * checks. + * + * @param userHandle the origin handle. + * @return The UserHandle that should be used for cross profile access checks. In the event + * the origin handle delegates its access, this may not be the same handle as the origin + * handle. + */ private UserHandle getProfileToCheckCrossProfileAccess(UserHandle userHandle) { if (mUserManager == null) { Log.e(TAG, "Cannot obtain user manager"); @@ -552,7 +705,6 @@ public interface UserManagerState { * @param intent The intent Photopicker is currently running under, for * CrossProfileForwardActivity checking. */ - @SuppressLint("DiscouragedPrivateApi") private void setBlockedByAdminValue(Intent intent) { if (intent == null) { Log.e( @@ -562,95 +714,6 @@ public interface UserManagerState { return; } - Map<UserId, Boolean> profileIsAccessibleToProcessOwner = new HashMap<>(); - List<UserId> delegatedFromParent = new ArrayList<>(); - - final PackageManager pm = mContext.getPackageManager(); - - // Resolve CrossProfile activities for all user profiles that Photopicker is - // aware of. - for (UserId userId : mUserProfileIds) { - - // If the UserId is the system user, exit early. - if (userId.getIdentifier() == mCurrentUser.getIdentifier()) { - profileIsAccessibleToProcessOwner.put(userId, true); - continue; - } - - // This UserId delegates its strategy to the parent profile - if (isCrossProfileStrategyDelegatedToParent(userId.getUserHandle())) { - delegatedFromParent.add(userId); - continue; - } - - // Clear out the component & package before attempting to match - Intent intentToCheck = (Intent) intent.clone(); - intentToCheck.setComponent(null); - intentToCheck.setPackage(null); - - for (ResolveInfo resolveInfo : - pm.queryIntentActivities( - intentToCheck, PackageManager.MATCH_DEFAULT_ONLY)) { - - // If the activity is a CrossProfileIntentForwardingActivity, inspect its - // targetUserId to - // see if it targets the user we are currently checking for. - if (resolveInfo.isCrossProfileIntentForwarderActivity()) { - - /* - * IMPORTANT: This is a reflection based hack to ensure the profile is - * actually the installer of the CrossProfileIntentForwardingActivity. - * - * ResolveInfo.targetUserId exists, but is a hidden API not available to - * mainline modules, and no such API exists, so it is accessed via - * reflection below. All exceptions are caught to protect against - * reflection related issues such as: - * NoSuchFieldException / IllegalAccessException / SecurityException. - * - * In the event of an exception, the code fails "closed" for the current - * profile to avoid showing content that should not be visible. - */ - try { - Field targetUserIdField = - resolveInfo.getClass().getDeclaredField("targetUserId"); - targetUserIdField.setAccessible(true); - int targetUserId = (int) targetUserIdField.get(resolveInfo); - - if (targetUserId == userId.getIdentifier()) { - profileIsAccessibleToProcessOwner.put(userId, true); - - // Don't need to look further, exit the loop. - break; - } - - } catch (NoSuchFieldException - | IllegalAccessException - | SecurityException ex) { - // Couldn't check the targetUserId via reflection, so fail without - // further - // iterations. - Log.e(TAG, "Could not access targetUserId via reflection.", ex); - break; - } catch (Exception ex) { - Log.e(TAG, "Exception occurred during cross profile checks", ex); - } - } - } - // Fail case, was unable to find a suitable Activity for this user. - profileIsAccessibleToProcessOwner.putIfAbsent(userId, false); - } - - // For profiles that delegate their access to the parent, set the access for - // those profiles equal to the same as their parent. - for (UserId userId : delegatedFromParent) { - UserHandle parent = - mUserManager.getProfileParent(UserHandle.of(userId.getIdentifier())); - profileIsAccessibleToProcessOwner.put( - userId, - profileIsAccessibleToProcessOwner.getOrDefault( - UserId.of(parent), /* default= */ false)); - } - mIsProfileBlockedByAdminMap.clear(); for (UserId userId : mUserProfileIds) { mIsProfileBlockedByAdminMap.put( @@ -658,8 +721,8 @@ public interface UserManagerState { // calculated, (which are blocked, rather than which are accessible) so the // boolean needs to be inverted. userId, - !profileIsAccessibleToProcessOwner.getOrDefault( - userId, /* default= */ false)); + !isCrossProfileAllowedToUser(mContext, intent, UserId.CURRENT_USER, userId) + ); } } @@ -667,15 +730,9 @@ public interface UserManagerState { public Map<UserId, String> getProfileLabelsForAll() { assertMainThread(); Map<UserId, String> profileLabels = new HashMap<>(); - String personalTabLabel = mContext.getString(R.string.picker_personal_profile_label); - profileLabels.put(getSystemUser(), personalTabLabel); - if (SdkLevel.isAtLeastV()) { - for (UserId userId : mUserProfileIds) { - UserHandle userHandle = userId.getUserHandle(); - if (userHandle.getIdentifier() != getSystemUser().getIdentifier()) { - profileLabels.put(userId, getProfileLabel(userHandle)); - } - } + for (UserId userId : mUserProfileIds) { + UserHandle userHandle = userId.getUserHandle(); + profileLabels.put(userId, getProfileLabel(userHandle)); } return profileLabels; @@ -683,55 +740,69 @@ public interface UserManagerState { private String getProfileLabel(UserHandle userHandle) { if (SdkLevel.isAtLeastV()) { - Context userContext = mContext.createContextAsUser(userHandle, 0 /* flags */); try { + Context userContext = mContext.createContextAsUser(userHandle, 0 /* flags */); UserManager userManager = userContext.getSystemService(UserManager.class); if (userManager == null) { Log.e(TAG, "Cannot obtain user manager"); return null; } return userManager.getProfileLabel(); - } catch (Resources.NotFoundException e) { - // Todo(b/318530691): Handle the label for the profile that is not defined - // already + } catch (IllegalStateException e) { + Log.e(TAG, "could not create user context for user.", e); + } catch (Exception e) { + Log.e(TAG, "Exception while fetching profile badge", e); } } - return null; + + // Fall back case if not V, or an error encountered above, return hard coded strings. + boolean isPrimaryProfile = mUserManager.getProfileParent(userHandle) == null; + boolean isManagedProfile = mUserManager.isManagedProfile(userHandle.getIdentifier()); + + int resId; + if (isPrimaryProfile) { + resId = R.string.photopicker_profile_primary_label; + } else if (isManagedProfile) { + resId = R.string.photopicker_profile_managed_label; + } else { + resId = R.string.photopicker_profile_unknown_label; + } + + return mContext.getString(resId); } @Override public Map<UserId, Drawable> getProfileBadgeForAll() { assertMainThread(); Map<UserId, Drawable> profileBadges = new HashMap<>(); - profileBadges.put(getSystemUser(), mContext.getDrawable(R.drawable.ic_personal_mode)); - if (SdkLevel.isAtLeastV()) { - for (UserId userId : mUserProfileIds) { - UserHandle userHandle = userId.getUserHandle(); - if (userHandle.getIdentifier() != getSystemUser().getIdentifier()) { - profileBadges.put(userId, getProfileBadge(userHandle)); - } - } + for (UserId userId : mUserProfileIds) { + profileBadges.put(userId, getProfileBadge(userId.getUserHandle())); } return profileBadges; } private Drawable getProfileBadge(UserHandle userHandle) { if (SdkLevel.isAtLeastV()) { - Context userContext = mContext.createContextAsUser(userHandle, 0 /* flags */); try { + Context userContext = mContext.createContextAsUser(userHandle, 0 /* flags */); UserManager userManager = userContext.getSystemService(UserManager.class); if (userManager == null) { Log.e(TAG, "Cannot obtain user manager"); return null; } return userManager.getUserBadge(); - } catch (Resources.NotFoundException e) { - // Todo(b/318530691): Handle the icon for the profile that is not defined - // already - Log.i(TAG, "failed to find resource"); + } catch (IllegalStateException e) { + Log.e(TAG, "could not create user context for user.", e); + } catch (Exception e) { + Log.e(TAG, "Exception while fetching profile badge", e); } } - return null; + + // Fall back case if not V, or an error encountered above, return hard coded icons. + boolean isManagedProfile = mUserManager.isManagedProfile(userHandle.getIdentifier()); + int drawable = + isManagedProfile ? R.drawable.ic_work_outline : R.drawable.ic_personal_mode; + return mContext.getDrawable(drawable); } @Override diff --git a/src/com/android/providers/media/scan/LegacyMediaScanner.java b/src/com/android/providers/media/scan/LegacyMediaScanner.java index d73dda584..39bffe2f5 100644 --- a/src/com/android/providers/media/scan/LegacyMediaScanner.java +++ b/src/com/android/providers/media/scan/LegacyMediaScanner.java @@ -22,6 +22,7 @@ import android.net.Uri; import com.android.providers.media.MediaVolume; import java.io.File; +import java.util.Set; @Deprecated public class LegacyMediaScanner implements MediaScanner { @@ -60,4 +61,9 @@ public class LegacyMediaScanner implements MediaScanner { public void onDirectoryDirty(File file) { throw new UnsupportedOperationException(); } + + @Override + public Set<String> getOemSupportedMimeTypes() { + throw new UnsupportedOperationException(); + } } diff --git a/src/com/android/providers/media/scan/MediaScanner.java b/src/com/android/providers/media/scan/MediaScanner.java index 8999542bf..2ce537409 100644 --- a/src/com/android/providers/media/scan/MediaScanner.java +++ b/src/com/android/providers/media/scan/MediaScanner.java @@ -33,6 +33,7 @@ import com.android.providers.media.MediaVolume; import java.io.File; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; +import java.util.Set; public interface MediaScanner { int REASON_UNKNOWN = MEDIA_PROVIDER_SCAN_OCCURRED__REASON__UNKNOWN; @@ -62,4 +63,9 @@ public interface MediaScanner { void onIdleScanStopped(); void onDirectoryDirty(@NonNull File file); + + /** + * Returns OEM supported mime types for OEM metadata. + */ + Set<String> getOemSupportedMimeTypes(); } diff --git a/src/com/android/providers/media/scan/ModernMediaScanner.java b/src/com/android/providers/media/scan/ModernMediaScanner.java index 3b9f3c4bb..da6b30b5f 100644 --- a/src/com/android/providers/media/scan/ModernMediaScanner.java +++ b/src/com/android/providers/media/scan/ModernMediaScanner.java @@ -287,7 +287,8 @@ public class ModernMediaScanner implements MediaScanner { } } - private Set<String> getOemSupportedMimeTypes() { + @Override + public Set<String> getOemSupportedMimeTypes() { try { // Return if no package implements OemMetadataService if (!mDefaultOemMetadataServicePackage.isPresent()) { diff --git a/src/com/android/providers/media/util/MimeTypeFixHandler.java b/src/com/android/providers/media/util/MimeTypeFixHandler.java index bf0aa535e..8bdd88ef1 100644 --- a/src/com/android/providers/media/util/MimeTypeFixHandler.java +++ b/src/com/android/providers/media/util/MimeTypeFixHandler.java @@ -44,7 +44,10 @@ public final class MimeTypeFixHandler { private static final String TAG = "MimeTypeFixHandler"; private static final Map<String, String> sExtToMimeType = new HashMap<>(); + private static final Map<String, String> sMimeTypeToExt = new HashMap<>(); + private static final Map<String, String> sCorruptedExtToMimeType = new HashMap<>(); + private static final Map<String, String> sCorruptedMimeTypeToExt = new HashMap<>(); /** * Loads MIME type mappings from the classpath resource if not already loaded. @@ -58,13 +61,14 @@ public final class MimeTypeFixHandler { } if (sExtToMimeType.isEmpty()) { - parseTypes(context, R.raw.mime_types, sExtToMimeType); + parseTypes(context, R.raw.mime_types, sExtToMimeType, sMimeTypeToExt); // this will add or override the extension to mime type mapping - parseTypes(context, R.raw.android_mime_types, sExtToMimeType); + parseTypes(context, R.raw.android_mime_types, sExtToMimeType, sMimeTypeToExt); Log.v(TAG, "MIME types loaded"); } if (sCorruptedExtToMimeType.isEmpty()) { - parseTypes(context, R.raw.corrupted_mime_types, sCorruptedExtToMimeType); + parseTypes(context, R.raw.corrupted_mime_types, sCorruptedExtToMimeType, + sCorruptedMimeTypeToExt); Log.v(TAG, "Corrupted MIME types loaded"); } @@ -77,7 +81,8 @@ public final class MimeTypeFixHandler { * @param resource the mime.type resource * @param mapping the map to populate with file extension (key) to MIME type (value) mappings */ - private static void parseTypes(Context context, int resource, Map<String, String> mapping) { + private static void parseTypes(Context context, int resource, Map<String, String> extToMimeType, + Map<String, String> mimeTypeToExt) { try (InputStream inputStream = context.getResources().openRawResource(resource)) { try (BufferedReader reader = new BufferedReader(new InputStreamReader(inputStream))) { String line; @@ -92,10 +97,24 @@ public final class MimeTypeFixHandler { if (tokens.length < 2) { continue; } - String mimeType = tokens[0]; + String mimeType = tokens[0].toLowerCase(Locale.ROOT); + String firstExt = tokens[1].toLowerCase(Locale.ROOT); + if (firstExt.startsWith("?")) { + firstExt = firstExt.substring(1); + if (firstExt.isEmpty()) { + continue; + } + } + // ?mime ext1 ?ext2 ext3 if (mimeType.toLowerCase(Locale.ROOT).startsWith("?")) { mimeType = mimeType.substring(1); // Remove the "?" + if (mimeType.isEmpty()) { + continue; + } + mimeTypeToExt.putIfAbsent(mimeType, firstExt); + } else { + mimeTypeToExt.put(mimeType, firstExt); } for (int i = 1; i < tokens.length; i++) { @@ -103,11 +122,9 @@ public final class MimeTypeFixHandler { boolean putIfAbsent = extension.startsWith("?"); if (putIfAbsent) { extension = extension.substring(1); // Remove the "?" - if (!mapping.containsKey(extension)) { - mapping.put(extension, mimeType); - } + extToMimeType.putIfAbsent(extension, mimeType); } else { - mapping.put(extension, mimeType); + extToMimeType.put(extension, mimeType); } } } @@ -139,6 +156,35 @@ public final class MimeTypeFixHandler { return Optional.empty(); } + /** + * Gets file extension from MIME type. + * + * @param mimeType The MIME type. + * @return Optional file extension, or empty. + */ + static Optional<String> getExtFromMimeType(String mimeType) { + if (mimeType == null) { + return Optional.empty(); + } + + mimeType = mimeType.toLowerCase(Locale.ROOT); + return Optional.ofNullable(sMimeTypeToExt.get(mimeType)); + } + + /** + * Checks if a MIME type is corrupted. + * + * @param mimeType The MIME type. + * @return {@code true} if corrupted, {@code false} otherwise. + */ + static boolean isCorruptedMimeType(String mimeType) { + if (sMimeTypeToExt.containsKey(mimeType)) { + return false; + } + + return sCorruptedMimeTypeToExt.containsKey(mimeType); + } + /** * Scans the database for files with unsupported or mismatched MIME types and updates them. diff --git a/src/com/android/providers/media/util/MimeUtils.java b/src/com/android/providers/media/util/MimeUtils.java index 354ceb193..5698a83aa 100644 --- a/src/com/android/providers/media/util/MimeUtils.java +++ b/src/com/android/providers/media/util/MimeUtils.java @@ -276,6 +276,11 @@ public class MimeUtils { */ @NonNull public static String getExtensionFromMimeType(@Nullable String mimeType) { + Optional<String> android15Extension = getExtFromMimeTypeForAndroid15(mimeType); + if (android15Extension.isPresent()) { + return android15Extension.get(); + } + final String extension = MimeTypeMap.getSingleton().getExtensionFromMimeType(mimeType); if (extension != null) { return "." + extension; @@ -302,4 +307,28 @@ public class MimeUtils { } return Optional.empty(); } + + /** + * Gets file extension from MIME type for Android 15. + * Handles Android 15 specific MIME type to extension mapping. If the mime-type is corrupted, + * then return the default one with respect to mime type. + * + * @param mimeType The MIME type. + * @return Optional file extension (with dot), or empty. + */ + private static Optional<String> getExtFromMimeTypeForAndroid15(String mimeType) { + if (Flags.enableMimeTypeFixForAndroid15() + && Build.VERSION.SDK_INT == Build.VERSION_CODES.VANILLA_ICE_CREAM) { + Optional<String> value = MimeTypeFixHandler.getExtFromMimeType(mimeType); + if (value.isPresent()) { + return Optional.of("." + value.get()); + } else if (MimeTypeFixHandler.isCorruptedMimeType(mimeType)) { + if (isImageMimeType(mimeType)) return Optional.of(DEFAULT_IMAGE_FILE_EXTENSION); + if (isVideoMimeType(mimeType)) return Optional.of(DEFAULT_VIDEO_FILE_EXTENSION); + return Optional.of(""); + } + } + return Optional.empty(); + } + } diff --git a/src/com/android/providers/media/util/PermissionUtils.java b/src/com/android/providers/media/util/PermissionUtils.java index 3dcdc1150..7dbfd23da 100644 --- a/src/com/android/providers/media/util/PermissionUtils.java +++ b/src/com/android/providers/media/util/PermissionUtils.java @@ -149,6 +149,41 @@ public class PermissionUtils { } /** + * Check for read permission when legacy storage is granted. + * There is a bug in AppOpsManager that keeps legacy storage granted even + * when an app updates its targetSdkVersion from value <30 to >=30. + * If an app upgrades from targetSdk 29 to targetSdk 33, legacy storage + * remains granted and in targetSdk 33, app are required to replace R_E_S + * with R_M_*. If an app updates its manifest with R_M_*, permission check + * in MediaProvider will look for R_E_S and will not grant read access as + * the app would be still treated as legacy. Ensure that legacy app either has + * R_E_S or all of R_M_* to get read permission. Since this is a fix for legacy + * app op bug, we are avoiding granular permission checks based on media type. + */ + public static boolean checkPermissionReadForLegacyStorage(@NonNull Context context, + int pid, int uid, @NonNull String packageName, @Nullable String attributionTag, + boolean isTargetSdkAtleastT) { + if (isTargetSdkAtleastT) { + return checkPermissionForDataDelivery(context, READ_EXTERNAL_STORAGE, pid, uid, + packageName, attributionTag, + generateAppOpMessage(packageName, sOpDescription.get())) || ( + checkPermissionForDataDelivery(context, READ_MEDIA_IMAGES, pid, uid, + packageName, attributionTag, + generateAppOpMessage(packageName, sOpDescription.get())) + && checkPermissionForDataDelivery(context, READ_MEDIA_VIDEO, pid, uid, + packageName, attributionTag, + generateAppOpMessage(packageName, sOpDescription.get())) + && checkPermissionForDataDelivery(context, READ_MEDIA_AUDIO, pid, uid, + packageName, attributionTag, + generateAppOpMessage(packageName, sOpDescription.get()))); + } else { + return checkPermissionForDataDelivery(context, READ_EXTERNAL_STORAGE, pid, uid, + packageName, attributionTag, + generateAppOpMessage(packageName, sOpDescription.get())); + } + } + + /** * Check if the given package has been granted the * android.Manifest.permission#ACCESS_MEDIA_LOCATION permission. */ @@ -209,13 +244,13 @@ public class PermissionUtils { } public static boolean checkIsLegacyStorageGranted(@NonNull Context context, int uid, - String packageName, @Nullable String attributionTag, boolean isTargetSdkAtLeastV) { + String packageName, boolean isTargetSdkAtLeastV) { if (!isTargetSdkAtLeastV && context.getSystemService(AppOpsManager.class) .unsafeCheckOp(OPSTR_LEGACY_STORAGE, uid, packageName) == MODE_ALLOWED) { return true; } // Check OPSTR_NO_ISOLATED_STORAGE app op. - return checkNoIsolatedStorageGranted(context, uid, packageName, attributionTag); + return checkNoIsolatedStorageGranted(context, uid, packageName); } public static boolean checkPermissionReadAudio( @@ -360,7 +395,7 @@ public class PermissionUtils { /** * Check if the given package has been granted the - * android.provider.MediaStore#ACCESS_OEM_METADATA_PERMISSION permission. + * {@link android.provider.MediaStore#ACCESS_OEM_METADATA_PERMISSION} permission. */ public static boolean checkPermissionAccessOemMetadata(@NonNull Context context, int pid, int uid, @NonNull String packageName, @Nullable String attributionTag) { @@ -368,6 +403,16 @@ public class PermissionUtils { pid, uid, packageName, attributionTag, null); } + /** + * Check if the given package has been granted the + * {@link android.provider.MediaStore#UPDATE_OEM_METADATA_PERMISSION} permission. + */ + public static boolean checkPermissionUpdateOemMetadata(@NonNull Context context, + int pid, int uid, @NonNull String packageName, @Nullable String attributionTag) { + return checkPermissionForPreflight(context, MediaStore.UPDATE_OEM_METADATA_PERMISSION, + pid, uid, packageName); + } + public static boolean checkPermissionInstallPackages(@NonNull Context context, int pid, int uid, @NonNull String packageName, @Nullable String attributionTag) { return checkPermissionForDataDelivery(context, INSTALL_PACKAGES, pid, @@ -385,13 +430,13 @@ public class PermissionUtils { * indicates the package is a system gallery. */ public static boolean checkWriteImagesOrVideoAppOps(@NonNull Context context, int uid, - @NonNull String packageName, @Nullable String attributionTag) { + @NonNull String packageName, @Nullable String attributionTag, boolean forDataDelivery) { return checkAppOp( context, OPSTR_WRITE_MEDIA_IMAGES, uid, packageName, attributionTag, - generateAppOpMessage(packageName, sOpDescription.get())) + generateAppOpMessage(packageName, sOpDescription.get()), forDataDelivery) || checkAppOp( context, OPSTR_WRITE_MEDIA_VIDEO, uid, packageName, attributionTag, - generateAppOpMessage(packageName, sOpDescription.get())); + generateAppOpMessage(packageName, sOpDescription.get()), forDataDelivery); } /** @@ -401,7 +446,8 @@ public class PermissionUtils { int uid, @NonNull String[] sharedPackageNames, @Nullable String attributionTag) { for (String packageName : sharedPackageNames) { if (checkAppOp(context, OPSTR_REQUEST_INSTALL_PACKAGES, uid, packageName, - attributionTag, generateAppOpMessage(packageName, sOpDescription.get()))) { + attributionTag, generateAppOpMessage(packageName, sOpDescription.get()), + /*forDataDelivery*/ false)) { return true; } } @@ -426,10 +472,9 @@ public class PermissionUtils { @VisibleForTesting static boolean checkNoIsolatedStorageGranted(@NonNull Context context, int uid, - @NonNull String packageName, @Nullable String attributionTag) { + @NonNull String packageName) { final AppOpsManager appOps = context.getSystemService(AppOpsManager.class); - int ret = appOps.noteOpNoThrow(OPSTR_NO_ISOLATED_STORAGE, uid, packageName, attributionTag, - generateAppOpMessage(packageName, "am instrument --no-isolated-storage")); + int ret = appOps.unsafeCheckOpNoThrow(OPSTR_NO_ISOLATED_STORAGE, uid, packageName); return ret == AppOpsManager.MODE_ALLOWED; } @@ -466,9 +511,10 @@ public class PermissionUtils { */ private static boolean checkAppOp(@NonNull Context context, @NonNull String op, int uid, @NonNull String packageName, - @Nullable String attributionTag, @Nullable String opMessage) { + @Nullable String attributionTag, @Nullable String opMessage, boolean forDataDelivery) { final AppOpsManager appOps = context.getSystemService(AppOpsManager.class); - final int mode = appOps.noteOpNoThrow(op, uid, packageName, attributionTag, opMessage); + final int mode = forDataDelivery ? appOps.noteOpNoThrow(op, uid, packageName, + attributionTag, opMessage) : appOps.unsafeCheckOpNoThrow(op, uid, packageName); switch (mode) { case AppOpsManager.MODE_ALLOWED: return true; diff --git a/tests/Android.bp b/tests/Android.bp index f3c98e530..3de35a077 100644 --- a/tests/Android.bp +++ b/tests/Android.bp @@ -155,6 +155,25 @@ android_test_helper_app { ], } +android_test_helper_app { + name: "LegacyMediaProviderTestAppFor33", + manifest: "test_app/LegacyTestAppWithTargetSdk33.xml", + srcs: [ + "test_app/src/**/*.java", + "src/com/android/providers/media/util/TestUtils.java", + ], + static_libs: [ + "cts-install-lib", + ], + sdk_version: "test_current", + target_sdk_version: "33", + min_sdk_version: "30", + test_suites: [ + "general-tests", + "mts-mediaprovider", + ], +} + // This looks a bit awkward, but we need our tests to run against either // MediaProvider or MediaProviderGoogle, and we don't know which one is // on the device being tested, so we can't sign our tests with a key that @@ -173,6 +192,7 @@ android_test { resource_dirs: [ "main_res", "res", + "photopicker_res", ], srcs: [ @@ -249,6 +269,7 @@ android_test { data: [ ":LegacyMediaProviderTestApp", + ":LegacyMediaProviderTestAppFor33", ":LegacyMediaProviderTestAppFor35", ":MediaProviderTestAppForPermissionActivity", ":MediaProviderTestAppForPermissionActivity33", diff --git a/tests/AndroidManifest.xml b/tests/AndroidManifest.xml index acd1a21af..2e7d9348c 100644 --- a/tests/AndroidManifest.xml +++ b/tests/AndroidManifest.xml @@ -13,6 +13,7 @@ <package android:name="com.android.providers.media.testapp.withuserselectedperms" /> <package android:name="com.android.providers.media.testapp.legacy" /> <package android:name="com.android.providers.media.testapp.legacywithtargetsdk35" /> + <package android:name="com.android.providers.media.testapp.legacywithtargetsdk33" /> </queries> <uses-permission android:name="android.permission.READ_EXTERNAL_STORAGE" /> @@ -31,7 +32,8 @@ <uses-permission android:name="com.android.providers.media.permission.BIND_OEM_METADATA_SERVICE"/> - <application android:label="MediaProvider Tests"> + <application android:label="MediaProvider Tests" + android:debuggable="true"> <uses-library android:name="android.test.runner" /> <activity android:name="com.android.providers.media.GetResultActivity" /> diff --git a/tests/AndroidTest.xml b/tests/AndroidTest.xml index 31d9e1535..44fb27930 100644 --- a/tests/AndroidTest.xml +++ b/tests/AndroidTest.xml @@ -30,6 +30,7 @@ <option name="test-file-name" value="MediaProviderTestAppWithUserSelectedPerms.apk" /> <option name="test-file-name" value="MediaProviderTestAppWithoutPerms.apk" /> <option name="test-file-name" value="LegacyMediaProviderTestApp.apk" /> + <option name="test-file-name" value="LegacyMediaProviderTestAppFor33.apk" /> <option name="test-file-name" value="LegacyMediaProviderTestAppFor35.apk" /> <option name="install-arg" value="-g" /> </target_preparer> diff --git a/tests/photopicker_res b/tests/photopicker_res new file mode 120000 index 000000000..c1f73810f --- /dev/null +++ b/tests/photopicker_res @@ -0,0 +1 @@ +../photopicker/res
\ No newline at end of file diff --git a/tests/src/com/android/providers/media/GetExternalVolumesBehaviorModificationTest.java b/tests/src/com/android/providers/media/GetExternalVolumesBehaviorModificationTest.java new file mode 100644 index 000000000..743e00f8a --- /dev/null +++ b/tests/src/com/android/providers/media/GetExternalVolumesBehaviorModificationTest.java @@ -0,0 +1,168 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.providers.media; + +import android.compat.testing.PlatformCompatChangeRule; +import android.content.Context; +import android.os.Build; +import android.os.Environment; +import android.os.Parcel; +import android.os.UserHandle; +import android.os.storage.StorageManager; +import android.os.storage.StorageVolume; +import android.platform.test.annotations.RequiresFlagsEnabled; +import android.platform.test.flag.junit.CheckFlagsRule; +import android.platform.test.flag.junit.DeviceFlagsValueProvider; +import android.provider.MediaStore; + +import androidx.test.ext.junit.runners.AndroidJUnit4; +import androidx.test.filters.SdkSuppress; +import androidx.test.platform.app.InstrumentationRegistry; + +import com.android.providers.media.flags.Flags; + +import libcore.junit.util.compat.CoreCompatChangeRule.DisableCompatChanges; +import libcore.junit.util.compat.CoreCompatChangeRule.EnableCompatChanges; + +import junit.framework.Assert; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.TestRule; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.MockitoAnnotations; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Set; + +@RunWith(AndroidJUnit4.class) +@RequiresFlagsEnabled({Flags.FLAG_EXCLUDE_UNRELIABLE_VOLUMES}) +@SdkSuppress(minSdkVersion = Build.VERSION_CODES.TIRAMISU) +public class GetExternalVolumesBehaviorModificationTest { + @Rule + public TestRule compatChangeRule = new PlatformCompatChangeRule(); + + @Rule + public final CheckFlagsRule mCheckFlagsRule = + DeviceFlagsValueProvider.createCheckFlagsRule(); + + @Mock + private Context mContext; + + private static final String RELIABLE_STORAGE = "reliable"; + private static final String UNRELIABLE_STORAGE = "unreliable"; + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + + // create a testing storage volume which behaves as a reliable storage and hence have a + // directory starting with storage/. Naming this volume as reliable. + Parcel parcel = Parcel.obtain(); + parcel.writeString8("1"); // id + parcel.writeString8("Storage/emulated/testDir"); // path + parcel.writeString8("Storage/emulated/testDir"); // internalPath + parcel.writeString8(""); // description + parcel.writeInt(0); // removable (boolean) + parcel.writeInt(1); // primary (boolean) + parcel.writeInt(0); // emulated (boolean) + parcel.writeInt(0); // allowMassStorage (boolean) + parcel.writeInt(0); // allowFullBackup (boolean) + parcel.writeLong(1000); // maxFileSize + parcel.writeParcelable(UserHandle.CURRENT, 0); // owner (UserHandle) + parcel.writeInt(0); // uuid + parcel.writeString8(RELIABLE_STORAGE); // name + parcel.writeString8(Environment.MEDIA_MOUNTED); // state + + parcel.setDataPosition(0); + + StorageVolume reliableStorage = StorageVolume.CREATOR.createFromParcel(parcel); + + // create a testing storage volume which behaves as a unreliable storage and hence have a + // directory starting with mnt/. Naming this volume as unreliable. + Parcel parcel2 = Parcel.obtain(); + parcel2.writeString8("2"); // id + parcel2.writeString8("mnt/testDir"); // path + parcel2.writeString8("mnt/testDir"); // internalPath + parcel2.writeString8(""); // description + parcel2.writeInt(0); // removable (boolean) + parcel2.writeInt(1); // primary (boolean) + parcel2.writeInt(0); // emulated (boolean) + parcel2.writeInt(0); // allowMassStorage (boolean) + parcel2.writeInt(0); // allowFullBackup (boolean) + parcel2.writeLong(1000); // maxFileSize + parcel2.writeParcelable(UserHandle.CURRENT, 0); // owner (UserHandle) + parcel2.writeInt(0); // uuid + parcel2.writeString8(UNRELIABLE_STORAGE); // name + parcel2.writeString8(Environment.MEDIA_MOUNTED); // state + + parcel2.setDataPosition(0); + + StorageVolume unreliableStorage = StorageVolume.CREATOR.createFromParcel(parcel2); + + // Creating a mock storage manager which on being queried for storage volumes return the + // list of both reliable and unreliable storage. + StorageManager mockedStorageManager = Mockito.mock(StorageManager.class); + Mockito.when(mockedStorageManager.getStorageVolumes()).thenReturn(new ArrayList<>( + Arrays.asList(reliableStorage, unreliableStorage))); + + // Creating a mock for context so that it returns the mocked storage manager. + mContext = Mockito.mock(Context.class); + Mockito.when(mContext.getSystemServiceName(StorageManager.class)).thenReturn( + Context.STORAGE_SERVICE); + Mockito.when(mContext.getApplicationInfo()).thenReturn( + InstrumentationRegistry.getInstrumentation().getContext().getApplicationInfo()); + Mockito.when(mContext.getSystemService(StorageManager.class)).thenReturn( + mockedStorageManager); + } + + /** + * This test verifies the behaviour of MediaStore.getExternalVolumeNames() before enabling the + * EXCLUDE_UNRELIABLE_STORAGE_VOLUMES appcompat flag. + */ + @Test + @DisableCompatChanges({MediaProvider.EXCLUDE_UNRELIABLE_STORAGE_VOLUMES}) + public void test_getExternalVolumes_returnsAllVolumes() { + Set<String> result = MediaStore.getExternalVolumeNames(mContext); + + // Verify result is not null and both unreliable and reliable storage is returned. + Assert.assertNotNull(result); + Assert.assertEquals(2, result.size()); + Assert.assertTrue(result.contains(RELIABLE_STORAGE)); + Assert.assertTrue(result.contains(UNRELIABLE_STORAGE)); + } + + /** + * This test verifies the behaviour of MediaStore.getExternalVolumeNames() before enabling the + * EXCLUDE_UNRELIABLE_STORAGE_VOLUMES appcompat flag. + */ + @Test + @EnableCompatChanges({MediaProvider.EXCLUDE_UNRELIABLE_STORAGE_VOLUMES}) + public void test_getExternalVolumes_returnsFilteredVolumes() { + Set<String> result = MediaStore.getExternalVolumeNames(mContext); + + // Verify result is not null and only reliable storage is returned. + Assert.assertNotNull(result); + Assert.assertEquals(1, result.size()); + Assert.assertTrue(result.contains(RELIABLE_STORAGE)); + Assert.assertFalse(result.contains(UNRELIABLE_STORAGE)); + } +} + diff --git a/tests/src/com/android/providers/media/IsolatedContext.java b/tests/src/com/android/providers/media/IsolatedContext.java index fa9a103ba..1b0f1f31e 100644 --- a/tests/src/com/android/providers/media/IsolatedContext.java +++ b/tests/src/com/android/providers/media/IsolatedContext.java @@ -152,6 +152,11 @@ public class IsolatedContext extends ContextWrapper { protected boolean shouldCheckForMaliciousActivity() { return Flags.enableMaliciousAppDetector(); } + + @Override + protected void enforcePermissionCheckForOemMetadataUpdate(){ + + } }; } diff --git a/tests/src/com/android/providers/media/leveldb/LevelDBManagerTest.java b/tests/src/com/android/providers/media/leveldb/LevelDBManagerTest.java index f684e8ce5..191f11a59 100644 --- a/tests/src/com/android/providers/media/leveldb/LevelDBManagerTest.java +++ b/tests/src/com/android/providers/media/leveldb/LevelDBManagerTest.java @@ -21,8 +21,8 @@ import static com.google.common.truth.Truth.assertThat; import android.content.Context; +import androidx.test.InstrumentationRegistry; import androidx.test.ext.junit.runners.AndroidJUnit4; -import androidx.test.platform.app.InstrumentationRegistry; import org.junit.Test; import org.junit.runner.RunWith; @@ -39,7 +39,7 @@ public class LevelDBManagerTest { @Test public void testLevelDbOperations() { - final Context context = InstrumentationRegistry.getInstrumentation().getContext(); + final Context context = InstrumentationRegistry.getTargetContext(); String levelDBFile = "test-leveldb"; final String levelDBPath = context.getFilesDir().getPath() + "/" + levelDBFile; LevelDBInstance levelDBInstance = LevelDBManager.getInstance(levelDBPath); diff --git a/tests/src/com/android/providers/media/oemmetadataservices/OemMetadataServiceTest.java b/tests/src/com/android/providers/media/oemmetadataservices/OemMetadataServiceTest.java index 774635be4..d8001c6c4 100644 --- a/tests/src/com/android/providers/media/oemmetadataservices/OemMetadataServiceTest.java +++ b/tests/src/com/android/providers/media/oemmetadataservices/OemMetadataServiceTest.java @@ -26,6 +26,7 @@ import android.Manifest; import android.app.Instrumentation; import android.content.ComponentName; import android.content.ContentUris; +import android.content.ContentValues; import android.content.Context; import android.content.Intent; import android.content.ServiceConnection; @@ -63,6 +64,7 @@ import org.junit.runner.RunWith; import java.io.File; import java.util.Arrays; +import java.util.HashMap; import java.util.Map; import java.util.Set; import java.util.concurrent.CountDownLatch; @@ -71,7 +73,6 @@ import java.util.concurrent.TimeoutException; import java.util.function.Supplier; @RunWith(AndroidJUnit4.class) -@EnableFlags(com.android.providers.media.flags.Flags.FLAG_ENABLE_OEM_METADATA) @SdkSuppress(minSdkVersion = Build.VERSION_CODES.S) public class OemMetadataServiceTest { @@ -105,6 +106,7 @@ public class OemMetadataServiceTest { } @Test + @EnableFlags(com.android.providers.media.flags.Flags.FLAG_ENABLE_OEM_METADATA) public void testGetSupportedMimeTypes() throws Exception { bindService(); assertNotNull(mOemMetadataServiceWrapper); @@ -116,6 +118,7 @@ public class OemMetadataServiceTest { } @Test + @EnableFlags(com.android.providers.media.flags.Flags.FLAG_ENABLE_OEM_METADATA) public void testGetOemCustomData() throws Exception { bindService(); assertNotNull(mOemMetadataServiceWrapper); @@ -141,6 +144,7 @@ public class OemMetadataServiceTest { } @Test + @EnableFlags(com.android.providers.media.flags.Flags.FLAG_ENABLE_OEM_METADATA) public void testScanOfOemMetadataAndFilterOnReadWithoutPermission() throws Exception { IsolatedContext isolatedContext = new IsolatedContext(mContext, "modern", /* asFuseThread */ false); @@ -181,6 +185,130 @@ public class OemMetadataServiceTest { } @Test + @EnableFlags({com.android.providers.media.flags.Flags.FLAG_ENABLE_OEM_METADATA, + com.android.providers.media.flags.Flags.FLAG_ENABLE_OEM_METADATA_UPDATE}) + public void testTriggerOemMetadataUpdateWithPermission() throws Exception { + IsolatedContext isolatedContext = new IsolatedContext(mContext, "modern", + /* asFuseThread */ false); + ModernMediaScanner modernMediaScanner = new ModernMediaScanner(isolatedContext, + new TestConfigStore()); + final File downloads = new File(Environment.getExternalStorageDirectory(), + Environment.DIRECTORY_DOWNLOADS); + final File audioFile = new File(downloads, "audio.mp3"); + try { + stage(R.raw.test_audio, audioFile); + Uri uri = modernMediaScanner.scanFile(audioFile, MediaScanner.REASON_UNKNOWN); + DatabaseHelper databaseHelper = isolatedContext.getExternalDatabase(); + // Direct query on DB returns stored value of oem_metadata + try (Cursor c = databaseHelper.runWithoutTransaction(db -> db.query( + "files", new String[]{FileColumns.OEM_METADATA, FileColumns._MODIFIER}, + "_id=?", new String[]{String.valueOf(ContentUris.parseId(uri))}, + null, null, null))) { + assertThat(c.getCount()).isEqualTo(1); + c.moveToNext(); + byte[] oemData = c.getBlob(0); + int modifier = c.getInt(1); + assertThat(oemData).isNotNull(); + Map<String, String> map = convertStringToOemMetadataMap(new String(oemData)); + assertThat(map.keySet()).containsExactly("a", "b", "c", "d", "e"); + assertThat(modifier).isEqualTo(FileColumns._MODIFIER_MEDIA_SCAN); + } + + ContentValues contentValues = new ContentValues(); + Map<String, String> updatedData = Map.of("a1", "b1", "a2", "b2"); + contentValues.put(FileColumns.OEM_METADATA, updatedData.toString()); + isolatedContext.getContentResolver().update(uri, contentValues, null); + + try (Cursor c = databaseHelper.runWithoutTransaction(db -> db.query( + "files", new String[]{FileColumns.OEM_METADATA, FileColumns._MODIFIER}, + "_id=?", new String[]{String.valueOf(ContentUris.parseId(uri))}, + null, null, null))) { + assertThat(c.getCount()).isEqualTo(1); + c.moveToNext(); + byte[] oemData = c.getBlob(0); + int modifier = c.getInt(1); + assertThat(modifier).isEqualTo(FileColumns._MODIFIER_MEDIA_SCAN); + assertThat(oemData).isNotNull(); + Map<String, String> map = convertStringToOemMetadataMap(new String(oemData)); + assertThat(map.keySet()).containsExactly("a1", "a2"); + } + } finally { + audioFile.delete(); + } + } + + @Test + @EnableFlags({com.android.providers.media.flags.Flags.FLAG_ENABLE_OEM_METADATA, + com.android.providers.media.flags.Flags.FLAG_ENABLE_OEM_METADATA_UPDATE}) + public void testTriggerBulkUpdateOemMetadataInNextScan() throws Exception { + IsolatedContext isolatedContext = new IsolatedContext(mContext, "modern", + /* asFuseThread */ false); + ModernMediaScanner modernMediaScanner = new ModernMediaScanner(isolatedContext, + new TestConfigStore()); + final File downloads = new File(Environment.getExternalStorageDirectory(), + Environment.DIRECTORY_DOWNLOADS); + final File audioFile = new File(downloads, "audio.mp3"); + try { + stage(R.raw.test_audio, audioFile); + + Uri uri = modernMediaScanner.scanFile(audioFile, MediaScanner.REASON_UNKNOWN); + + DatabaseHelper databaseHelper = isolatedContext.getExternalDatabase(); + // Direct query on DB returns stored value of oem_metadata + try (Cursor c = databaseHelper.runWithoutTransaction(db -> db.query( + "files", new String[]{FileColumns.OEM_METADATA, FileColumns._MODIFIER}, + "_id=?", new String[]{String.valueOf(ContentUris.parseId(uri))}, + null, null, null))) { + assertThat(c.getCount()).isEqualTo(1); + c.moveToNext(); + byte[] oemData = c.getBlob(0); + int modifier = c.getInt(1); + assertThat(oemData).isNotNull(); + Map<String, String> map = convertStringToOemMetadataMap(new String(oemData)); + assertThat(map.keySet()).containsExactly("a", "b", "c", "d", "e"); + assertThat(modifier).isEqualTo(FileColumns._MODIFIER_MEDIA_SCAN); + } + + // Change service behavior to verify updated results. Add new key "f". + TestOemMetadataService.updateOemMetadataServiceData(); + // OEM metadata should be allowed to update to null and modifier + // should now be set to _MODIFIER_CR as scan has not happened yet + MediaStore.bulkUpdateOemMetadataInNextScan(isolatedContext); + try (Cursor c = databaseHelper.runWithoutTransaction(db -> db.query( + "files", new String[]{FileColumns.OEM_METADATA, FileColumns._MODIFIER}, + "_id=?", new String[]{String.valueOf(ContentUris.parseId(uri))}, + null, null, null))) { + assertThat(c.getCount()).isEqualTo(1); + c.moveToNext(); + byte[] oemData = c.getBlob(0); + int modifier = c.getInt(1); + assertThat(oemData).isNull(); + assertThat(modifier).isEqualTo(FileColumns._MODIFIER_CR); + } + + // Trigger scan to allow OEM metadata update + MediaStore.scanFile(isolatedContext.getContentResolver(), audioFile); + + try (Cursor c = databaseHelper.runWithoutTransaction(db -> db.query( + "files", new String[]{FileColumns.OEM_METADATA, FileColumns._MODIFIER}, + "_id=?", new String[]{String.valueOf(ContentUris.parseId(uri))}, + null, null, null))) { + assertThat(c.getCount()).isEqualTo(1); + c.moveToNext(); + byte[] oemData = c.getBlob(0); + int modifier = c.getInt(1); + assertThat(modifier).isEqualTo(FileColumns._MODIFIER_MEDIA_SCAN); + assertThat(oemData).isNotNull(); + Map<String, String> map = convertStringToOemMetadataMap(new String(oemData)); + assertThat(map.keySet()).containsExactly("a", "b", "c", "d", "e", "f"); + } + } finally { + audioFile.delete(); + TestOemMetadataService.resetOemMetadataServiceData(); + } + } + + @Test public void testNoServiceBindingWithoutPermission() throws Exception { updateStateOfServiceWithPermission(PackageManager.COMPONENT_ENABLED_STATE_DISABLED); IsolatedContext isolatedContext = new IsolatedContext(mContext, "modern", @@ -272,4 +400,24 @@ public class OemMetadataServiceTest { mContext.bindService(intent, mServiceConnection, Context.BIND_AUTO_CREATE); mServiceLatch.await(3, TimeUnit.SECONDS); } + + public static Map<String, String> convertStringToOemMetadataMap(String stringMapping) { + Map<String, String> map = new HashMap<>(); + if (stringMapping == null || stringMapping.isEmpty()) { + return map; + } + stringMapping = stringMapping.substring(1, stringMapping.length() - 1); + // Split into key-value pairs + String[] pairs = stringMapping.split(", "); + + for (String pair : pairs) { + String[] keyValue = pair.split("="); + String key = keyValue[0]; + String value = keyValue[1]; + if (key != null) { + map.put(key, value); + } + } + return map; + } } diff --git a/tests/src/com/android/providers/media/oemmetadataservices/TestOemMetadataService.java b/tests/src/com/android/providers/media/oemmetadataservices/TestOemMetadataService.java index cdc4f71ce..ae1dcf8a2 100644 --- a/tests/src/com/android/providers/media/oemmetadataservices/TestOemMetadataService.java +++ b/tests/src/com/android/providers/media/oemmetadataservices/TestOemMetadataService.java @@ -27,6 +27,15 @@ import java.util.Set; public class TestOemMetadataService extends OemMetadataService { + static Map<String, String> sOemMetadata = new HashMap<>(); + + static { + sOemMetadata.put("a", "1"); + sOemMetadata.put("b", "2"); + sOemMetadata.put("c", "3"); + sOemMetadata.put("d", "4"); + sOemMetadata.put("e", "5"); + } @Override public Set<String> onGetSupportedMimeTypes() { @@ -35,12 +44,14 @@ public class TestOemMetadataService extends OemMetadataService { @Override public Map<String, String> onGetOemCustomData(@NonNull ParcelFileDescriptor pfd) { - Map<String, String> oemMetadata = new HashMap<>(); - oemMetadata.put("a", "1"); - oemMetadata.put("b", "2"); - oemMetadata.put("c", "3"); - oemMetadata.put("d", "4"); - oemMetadata.put("e", "5"); - return oemMetadata; + return sOemMetadata; + } + + public static void updateOemMetadataServiceData() { + sOemMetadata.put("f", "6"); + } + + public static void resetOemMetadataServiceData() { + sOemMetadata.remove("f"); } } diff --git a/tests/src/com/android/providers/media/photopicker/data/PickerDbFacadeTest.java b/tests/src/com/android/providers/media/photopicker/data/PickerDbFacadeTest.java index 68438b176..45069df81 100644 --- a/tests/src/com/android/providers/media/photopicker/data/PickerDbFacadeTest.java +++ b/tests/src/com/android/providers/media/photopicker/data/PickerDbFacadeTest.java @@ -1459,7 +1459,7 @@ public class PickerDbFacadeTest { } // Assert invalid projection column - final String invalidColumn = "testInvalidColumn"; + final String invalidColumn = "test invalid column"; final String[] invalidProjection = new String[]{ PickerMediaColumns.DATE_TAKEN, invalidColumn @@ -1471,12 +1471,17 @@ public class PickerDbFacadeTest { "Unexpected number of rows when asserting invalid projection column with " + "cloud provider.") .that(cr.getCount()).isEqualTo(1); + assertWithMessage("Unexpected number of columns in cursor") + .that(cr.getColumnCount()) + .isEqualTo(2); cr.moveToFirst(); - assertWithMessage( - "Unexpected value of the invalidColumn with cloud provider.") + assertWithMessage("Unexpected value of the invalidColumn with cloud provider.") .that(cr.getLong(cr.getColumnIndexOrThrow(invalidColumn))) .isEqualTo(0); + assertWithMessage("Unexpected value of the invalidColumn with cloud provider.") + .that(cr.getString(cr.getColumnIndexOrThrow(invalidColumn))) + .isEqualTo(null); assertWithMessage( "Unexpected value of PickerMediaColumns.DATE_TAKEN with cloud provider.") .that(cr.getLong(cr.getColumnIndexOrThrow(PickerMediaColumns.DATE_TAKEN))) diff --git a/tests/src/com/android/providers/media/photopicker/data/UserManagerStateTest.java b/tests/src/com/android/providers/media/photopicker/data/UserManagerStateTest.java index dda66c673..f2fc38d9d 100644 --- a/tests/src/com/android/providers/media/photopicker/data/UserManagerStateTest.java +++ b/tests/src/com/android/providers/media/photopicker/data/UserManagerStateTest.java @@ -23,6 +23,7 @@ import static org.junit.Assume.assumeFalse; import static org.junit.Assume.assumeTrue; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyInt; +import static org.mockito.Mockito.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -111,6 +112,7 @@ public class UserManagerStateTest { // set Managed Profile identification when(mMockUserManager.isManagedProfile(mManagedUser.getIdentifier())).thenReturn(true); + when(mMockUserManager.isManagedProfile(mManagedUser2.getIdentifier())).thenReturn(true); when(mMockUserManager.isManagedProfile(mPersonalUser.getIdentifier())).thenReturn(false); when(mMockUserManager.isManagedProfile(mOtherUser1.getIdentifier())).thenReturn(false); when(mMockUserManager.isManagedProfile(mOtherUser2.getIdentifier())).thenReturn(false); @@ -171,6 +173,62 @@ public class UserManagerStateTest { } @Test + public void testCrossProfileAccessWithDelegationVPlus() { + assumeTrue(SdkLevel.isAtLeastV()); + + // Return a ResolveInfo for the correct managed profile. + when(mMockPackageManager.queryIntentActivitiesAsUser( + any(Intent.class), anyInt(), any(UserHandle.class))) + .thenReturn(List.of()); + + initializeUserManagerState( + UserId.of(mPersonalUser), + Arrays.asList(mPersonalUser, mManagedUser, mOtherUser1)); + + InstrumentationRegistry.getInstrumentation() + .runOnMainSync( + () -> { + mUserManagerState.setIntentAndCheckRestrictions(new Intent()); + assertThat( + mUserManagerState.isCrossProfileAllowedToUser( + UserId.of(mManagedUser))) + .isFalse(); + assertThat( + mUserManagerState.isCrossProfileAllowedToUser( + UserId.of(mOtherUser1))) + .isTrue(); + }); + } + + @Test + public void testCrossProfileAccessWithDelegationManagedToPrivateVPlus() { + assumeTrue(SdkLevel.isAtLeastV()); + + // Return a ResolveInfo for the personal profile only. + when(mMockPackageManager.queryIntentActivitiesAsUser( + any(Intent.class), anyInt(), eq(mManagedUser))) + .thenReturn(List.of(new ReflectedResolveInfo(mPersonalUser.getIdentifier()))); + + initializeUserManagerState( + UserId.of(mManagedUser), + Arrays.asList(mPersonalUser, mManagedUser, mOtherUser1)); + + InstrumentationRegistry.getInstrumentation() + .runOnMainSync( + () -> { + mUserManagerState.setIntentAndCheckRestrictions(new Intent()); + assertThat( + mUserManagerState.isCrossProfileAllowedToUser( + UserId.of(mPersonalUser))) + .isTrue(); + assertThat( + mUserManagerState.isCrossProfileAllowedToUser( + UserId.of(mOtherUser1))) + .isTrue(); + }); + } + + @Test public void testCrossProfileAccessWithMultipleManagedProfilesIsAllowedVPlus() { assumeTrue(SdkLevel.isAtLeastV()); @@ -186,7 +244,8 @@ public class UserManagerStateTest { when(mMockUserManager.getUserProperties(mManagedUser2)).thenReturn(mManagedUser2Properties); // Return a ResolveInfo for the correct managed profile. - when(mMockPackageManager.queryIntentActivities(any(Intent.class), anyInt())) + when(mMockPackageManager.queryIntentActivitiesAsUser( + any(Intent.class), anyInt(), eq(mPersonalUser))) .thenReturn(List.of(new ReflectedResolveInfo(mManagedUser2.getIdentifier()))); initializeUserManagerState( @@ -224,7 +283,8 @@ public class UserManagerStateTest { when(mMockUserManager.getUserProperties(mManagedUser2)).thenReturn(mManagedUser2Properties); // Return a ResolveInfo for the OTHER managed profile. - when(mMockPackageManager.queryIntentActivities(any(Intent.class), anyInt())) + when(mMockPackageManager.queryIntentActivitiesAsUser( + any(Intent.class), anyInt(), eq(mPersonalUser))) .thenReturn(List.of(new ReflectedResolveInfo(mManagedUser.getIdentifier()))); initializeUserManagerState( @@ -254,7 +314,8 @@ public class UserManagerStateTest { when(mMockUserManager.isManagedProfile(mManagedUser2.getIdentifier())).thenReturn(true); // Return a ResolveInfo for the correct managed profile. - when(mMockPackageManager.queryIntentActivities(any(Intent.class), anyInt())) + when(mMockPackageManager.queryIntentActivitiesAsUser( + any(Intent.class), anyInt(), eq(mPersonalUser))) .thenReturn(List.of(new ReflectedResolveInfo(mManagedUser2.getIdentifier()))); initializeUserManagerState( @@ -280,7 +341,8 @@ public class UserManagerStateTest { when(mMockUserManager.isManagedProfile(mManagedUser2.getIdentifier())).thenReturn(true); // Return a ResolveInfo for the OTHER managed profile. - when(mMockPackageManager.queryIntentActivities(any(Intent.class), anyInt())) + when(mMockPackageManager.queryIntentActivitiesAsUser( + any(Intent.class), anyInt(), eq(mPersonalUser))) .thenReturn(List.of(new ReflectedResolveInfo(mManagedUser.getIdentifier()))); initializeUserManagerState( @@ -349,19 +411,14 @@ public class UserManagerStateTest { public void testGetAllUserProfileIdsThatNeedToShowInPhotoPicker_currentUserIsPersonalUser() { initializeUserManagerState( UserId.of(mPersonalUser), - Arrays.asList(mPersonalUser, mManagedUser, mOtherUser1, mOtherUser2)); + Arrays.asList(mPersonalUser, mManagedUser)); InstrumentationRegistry.getInstrumentation() .runOnMainSync( () -> { List<UserId> userIdList = - SdkLevel.isAtLeastV() - ? Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser), - UserId.of(mOtherUser1)) - : Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser)); + Arrays.asList( + UserId.of(mPersonalUser), + UserId.of(mManagedUser)); assertThat(mUserManagerState.getAllUserProfileIds()) .containsExactlyElementsIn(userIdList); @@ -372,19 +429,14 @@ public class UserManagerStateTest { public void testGetAllUserProfileIdsThatNeedToShowInPhotoPicker_currentUserIsManagedUser() { initializeUserManagerState( UserId.of(mManagedUser), - Arrays.asList(mPersonalUser, mManagedUser, mOtherUser1, mOtherUser2)); + Arrays.asList(mPersonalUser, mManagedUser)); InstrumentationRegistry.getInstrumentation() .runOnMainSync( () -> { List<UserId> userIdList = - SdkLevel.isAtLeastV() - ? Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser), - UserId.of(mOtherUser1)) - : Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser)); + Arrays.asList( + UserId.of(mPersonalUser), + UserId.of(mManagedUser)); assertThat(mUserManagerState.getAllUserProfileIds()) .containsExactlyElementsIn(userIdList); @@ -392,22 +444,21 @@ public class UserManagerStateTest { } @Test - public void testGetAllUserProfileIdsThatNeedToShowInPhotoPicker_currentUserIsOtherUser1() { + public void + testGetAllUserProfileIdsThatNeedToShowInPhotoPicker_currentUserIsOtherUser1VPlus() { + assumeTrue(SdkLevel.isAtLeastV()); initializeUserManagerState( UserId.of(mOtherUser1), - Arrays.asList(mPersonalUser, mManagedUser, mOtherUser1, mOtherUser2)); + Arrays.asList(mPersonalUser, mManagedUser, mOtherUser1)); InstrumentationRegistry.getInstrumentation() .runOnMainSync( () -> { List<UserId> userIdList = - SdkLevel.isAtLeastV() - ? Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser), - UserId.of(mOtherUser1)) - : Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser)); + Arrays.asList( + UserId.of(mPersonalUser), + UserId.of(mManagedUser), + UserId.of(mOtherUser1)); + assertThat(mUserManagerState.getAllUserProfileIds()) .containsExactlyElementsIn(userIdList); }); @@ -433,6 +484,7 @@ public class UserManagerStateTest { @Test public void testUserIds_multiUserProfilesAvailable_currentUserIsPersonalUser() { + assumeFalse(SdkLevel.isAtLeastV()); UserId currentUser = UserId.of(mPersonalUser); // if available user profiles are {personal , managed, otherUser1 } @@ -446,13 +498,7 @@ public class UserManagerStateTest { assertThat(mUserManagerState.getCurrentUserProfileId()) .isEqualTo(UserId.of(mPersonalUser)); - List<UserId> userIdList = - SdkLevel.isAtLeastV() - ? Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser), - UserId.of(mOtherUser1)) - : Arrays.asList( + List<UserId> userIdList = Arrays.asList( UserId.of(mPersonalUser), UserId.of(mManagedUser)); assertThat(mUserManagerState.getAllUserProfileIds()) @@ -463,77 +509,36 @@ public class UserManagerStateTest { mUserManagerState.getCurrentUserProfileId())) .isFalse(); }); - - // if available user profiles are {personal , otherUser1 } - initializeUserManagerState(currentUser, Arrays.asList(mPersonalUser, mOtherUser1)); - InstrumentationRegistry.getInstrumentation() - .runOnMainSync( - () -> { - if (SdkLevel.isAtLeastV()) { - assertThat(mUserManagerState.isMultiUserProfiles()).isTrue(); - } else { - assertThat(mUserManagerState.isMultiUserProfiles()).isFalse(); - } - - List<UserId> userIdList = - SdkLevel.isAtLeastV() - ? Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mOtherUser1)) - : Arrays.asList(UserId.of(mPersonalUser)); - assertThat(mUserManagerState.getAllUserProfileIds()) - .containsExactlyElementsIn(userIdList); - }); - - // if available user profiles are {personal , otherUser2 } - initializeUserManagerState(currentUser, Arrays.asList(mPersonalUser, mOtherUser2)); - InstrumentationRegistry.getInstrumentation() - .runOnMainSync( - () -> { - assertThat(mUserManagerState.isMultiUserProfiles()).isFalse(); - - assertThat(mUserManagerState.getCurrentUserProfileId()) - .isEqualTo(UserId.of(mPersonalUser)); - assertThat(mUserManagerState.getAllUserProfileIds()) - .containsExactlyElementsIn( - Arrays.asList(UserId.of(mPersonalUser))); - }); } @Test - public void testUserIds_multiUserProfilesAvailable_currentUserIsOtherUser2() { - UserId currentUser = UserId.of(mOtherUser2); + public void testUserIds_multiUserProfilesAvailable_currentUserIsPersonalUserVPlus() { + assumeTrue(SdkLevel.isAtLeastV()); + UserId currentUser = UserId.of(mPersonalUser); + // if available user profiles are {personal , managed, otherUser1 } initializeUserManagerState( - currentUser, Arrays.asList(mPersonalUser, mManagedUser, mOtherUser1, mOtherUser2)); + currentUser, Arrays.asList(mPersonalUser, mManagedUser, mOtherUser1)); InstrumentationRegistry.getInstrumentation() .runOnMainSync( () -> { assertThat(mUserManagerState.isMultiUserProfiles()).isTrue(); + assertThat(mUserManagerState.getCurrentUserProfileId()) - .isEqualTo(UserId.of(mOtherUser2)); + .isEqualTo(UserId.of(mPersonalUser)); List<UserId> userIdList = - SdkLevel.isAtLeastV() - ? Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser), - UserId.of(mOtherUser1)) - : Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser)); + Arrays.asList( + UserId.of(mPersonalUser), + UserId.of(mManagedUser), + UserId.of(mOtherUser1)); assertThat(mUserManagerState.getAllUserProfileIds()) .containsExactlyElementsIn(userIdList); - }); - initializeUserManagerState(currentUser, Arrays.asList(mPersonalUser, mOtherUser2)); - InstrumentationRegistry.getInstrumentation() - .runOnMainSync( - () -> { - assertThat(mUserManagerState.isMultiUserProfiles()).isFalse(); - assertThat(mUserManagerState.getAllUserProfileIds()) - .containsExactlyElementsIn( - Arrays.asList(UserId.of(mPersonalUser))); + assertThat( + mUserManagerState.isManagedUserProfile( + mUserManagerState.getCurrentUserProfileId())) + .isFalse(); }); } @@ -556,56 +561,6 @@ public class UserManagerStateTest { mUserManagerState.getCurrentUserProfileId())) .isTrue(); }); - - // set current user as otherUser2 - InstrumentationRegistry.getInstrumentation() - .runOnMainSync( - () -> { - mUserManagerState.setUserAsCurrentUserProfile(UserId.of(mOtherUser2)); - assertThat(mUserManagerState.getCurrentUserProfileId()) - .isEqualTo(UserId.of(mManagedUser)); - }); - - // set current user as otherUser1 - InstrumentationRegistry.getInstrumentation() - .runOnMainSync( - () -> { - mUserManagerState.setUserAsCurrentUserProfile(UserId.of(mOtherUser1)); - UserHandle currentUserProfile = - SdkLevel.isAtLeastV() ? mOtherUser1 : mManagedUser; - assertThat(mUserManagerState.getCurrentUserProfileId()) - .isEqualTo(UserId.of(currentUserProfile)); - }); - - // set current user as personalUser - InstrumentationRegistry.getInstrumentation() - .runOnMainSync( - () -> { - mUserManagerState.setUserAsCurrentUserProfile(UserId.of(mPersonalUser)); - assertThat(mUserManagerState.getCurrentUserProfileId()) - .isEqualTo(UserId.of(mPersonalUser)); - }); - - // set current user otherUser3 - InstrumentationRegistry.getInstrumentation() - .runOnMainSync( - () -> { - mUserManagerState.setUserAsCurrentUserProfile(UserId.of(mOtherUser3)); - assertThat(mUserManagerState.getCurrentUserProfileId()) - .isEqualTo(UserId.of(mPersonalUser)); - - List<UserId> userIdList = - SdkLevel.isAtLeastV() - ? Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser), - UserId.of(mOtherUser1)) - : Arrays.asList( - UserId.of(mPersonalUser), - UserId.of(mManagedUser)); - assertThat(mUserManagerState.getAllUserProfileIds()) - .containsExactlyElementsIn(userIdList); - }); } private void initializeUserManagerState(UserId current, List<UserHandle> usersOnDevice) { diff --git a/tests/src/com/android/providers/media/util/MimeTypeFixHandlerTest.java b/tests/src/com/android/providers/media/util/MimeTypeFixHandlerTest.java index de86f97d7..738d2008e 100644 --- a/tests/src/com/android/providers/media/util/MimeTypeFixHandlerTest.java +++ b/tests/src/com/android/providers/media/util/MimeTypeFixHandlerTest.java @@ -17,6 +17,7 @@ package com.android.providers.media.util; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @@ -98,6 +99,35 @@ public class MimeTypeFixHandlerTest { @Test @EnableFlags(Flags.FLAG_ENABLE_MIME_TYPE_FIX_FOR_ANDROID_15) + public void testIsCorruptedMimeType() { + // jpeg present in mime.types mapping + assertFalse(MimeTypeFixHandler.isCorruptedMimeType("image/jpeg")); + + // avif present in android.mime.types mapping + assertFalse(MimeTypeFixHandler.isCorruptedMimeType("image/avif")); + + // dwg in corrupted mapping + assertTrue(MimeTypeFixHandler.isCorruptedMimeType("image/vnd.dwg")); + } + + @Test + @EnableFlags(Flags.FLAG_ENABLE_MIME_TYPE_FIX_FOR_ANDROID_15) + public void testGetExtFromMimeType() { + // jpeg present in mime.types mapping + Optional<String> jpegExtension = MimeTypeFixHandler.getExtFromMimeType("image/jpeg"); + assertTrue(jpegExtension.isPresent()); + + // avif present in android.mime.types mapping + Optional<String> avifExtension = MimeTypeFixHandler.getExtFromMimeType("image/avif"); + assertTrue(avifExtension.isPresent()); + + // dwg in corrupted mapping + Optional<String> dwgExtension = MimeTypeFixHandler.getExtFromMimeType("image/vnd.dwg"); + assertFalse(dwgExtension.isPresent()); + } + + @Test + @EnableFlags(Flags.FLAG_ENABLE_MIME_TYPE_FIX_FOR_ANDROID_15) public void testUpdateUnsupportedMimeTypesForWrongEntries() { createEntriesInFilesTable(); diff --git a/tests/src/com/android/providers/media/util/PermissionUtilsTest.java b/tests/src/com/android/providers/media/util/PermissionUtilsTest.java index eafc383a4..7eb6c667b 100644 --- a/tests/src/com/android/providers/media/util/PermissionUtilsTest.java +++ b/tests/src/com/android/providers/media/util/PermissionUtilsTest.java @@ -21,6 +21,7 @@ import static android.Manifest.permission.MANAGE_EXTERNAL_STORAGE; import static android.Manifest.permission.MANAGE_MEDIA; import static android.Manifest.permission.UPDATE_APP_OPS_STATS; import static android.app.AppOpsManager.OPSTR_ACCESS_MEDIA_LOCATION; +import static android.app.AppOpsManager.OPSTR_LEGACY_STORAGE; import static android.app.AppOpsManager.OPSTR_NO_ISOLATED_STORAGE; import static android.app.AppOpsManager.OPSTR_READ_MEDIA_AUDIO; import static android.app.AppOpsManager.OPSTR_READ_MEDIA_IMAGES; @@ -44,6 +45,7 @@ import static com.android.providers.media.util.PermissionUtils.checkPermissionIn import static com.android.providers.media.util.PermissionUtils.checkPermissionManageMedia; import static com.android.providers.media.util.PermissionUtils.checkPermissionManager; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadAudio; +import static com.android.providers.media.util.PermissionUtils.checkPermissionReadForLegacyStorage; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadImages; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadStorage; import static com.android.providers.media.util.PermissionUtils.checkPermissionReadVideo; @@ -105,6 +107,10 @@ public class PermissionUtilsTest { "com.android.providers.media.testapp.legacy", 1, false, "LegacyMediaProviderTestApp.apk"); + private static final TestApp LEGACY_TEST_APP_33 = new TestApp("LegacyTestAppWithTargetSdk33", + "com.android.providers.media.testapp.legacywithtargetsdk33", 1, false, + "LegacyMediaProviderTestAppFor33.apk"); + private static final TestApp LEGACY_TEST_APP_35 = new TestApp("LegacyTestAppWithTargetSdk35", "com.android.providers.media.testapp.legacywithtargetsdk35", 1, false, "LegacyMediaProviderTestAppFor35.apk"); @@ -162,7 +168,7 @@ public class PermissionUtilsTest { final Context context = getContext(); final int uid = android.os.Process.myUid(); final String packageName = context.getPackageName(); - assertThat(checkNoIsolatedStorageGranted(context, uid, packageName, null)).isFalse(); + assertThat(checkNoIsolatedStorageGranted(context, uid, packageName)).isFalse(); } @Test @@ -177,7 +183,7 @@ public class PermissionUtilsTest { assertThat(checkPermissionShell(testAppUid)).isFalse(); assertThat( checkIsLegacyStorageGranted(getContext(), testAppUid, packageName, - null, /* isTargetSdkAtLeastS */ false)).isFalse(); + /* isTargetSdkAtLeastS */ false)).isFalse(); assertThat( checkPermissionInstallPackages(getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); @@ -212,7 +218,7 @@ public class PermissionUtilsTest { assertThat(checkPermissionSelf(getContext(), TEST_APP_PID, testAppUid)).isFalse(); assertThat(checkPermissionShell(testAppUid)).isFalse(); assertThat(checkIsLegacyStorageGranted(getContext(), testAppUid, packageName, - null, /* isTargetSdkAtLeastV */ false)).isFalse(); + /* isTargetSdkAtLeastV */ false)).isFalse(); assertThat(checkPermissionInstallPackages( getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); assertThat(checkPermissionAccessMtp( @@ -242,7 +248,7 @@ public class PermissionUtilsTest { assertThat(checkPermissionSelf(getContext(), TEST_APP_PID, testAppUid)).isFalse(); assertThat(checkPermissionShell(testAppUid)).isFalse(); assertThat(checkIsLegacyStorageGranted(getContext(), testAppUid, packageName, - null, /* isTargetSdkAtLeastV */ true)).isFalse(); + /* isTargetSdkAtLeastV */ true)).isFalse(); assertThat(checkPermissionInstallPackages( getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); assertThat(checkPermissionAccessMtp( @@ -251,6 +257,44 @@ public class PermissionUtilsTest { getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); assertThat(checkPermissionReadStorage( getContext(), TEST_APP_PID, testAppUid, packageName, null)).isTrue(); + assertThat(checkPermissionReadForLegacyStorage( + getContext(), TEST_APP_PID, testAppUid, packageName, + null, /* isTargetSdkAtLeastT */ true)).isTrue(); + } finally { + dropShellPermission(); + } + } + + @Test + @SdkSuppress(minSdkVersion = Build.VERSION_CODES.TIRAMISU, codeName = "Tiramisu") + public void testDefaultPermissionsOnLegacyTestAppWithTargetSdk33() throws Exception { + String packageName = LEGACY_TEST_APP_33.getPackageName(); + int testAppUid = getContext().getPackageManager().getPackageUid(packageName, 0); + adoptShellPermission(UPDATE_APP_OPS_STATS, MANAGE_APP_OPS_MODES); + + try { + assertThat(checkPermissionSelf(getContext(), TEST_APP_PID, testAppUid)).isFalse(); + assertThat(checkPermissionShell(testAppUid)).isFalse(); + assertThat(checkPermissionInstallPackages( + getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); + assertThat(checkPermissionAccessMtp( + getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); + assertThat(checkPermissionWriteStorage( + getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); + + modifyAppOp(testAppUid, OPSTR_READ_MEDIA_IMAGES, AppOpsManager.MODE_ALLOWED); + modifyAppOp(testAppUid, OPSTR_READ_MEDIA_VIDEO, AppOpsManager.MODE_ALLOWED); + modifyAppOp(testAppUid, OPSTR_READ_MEDIA_AUDIO, AppOpsManager.MODE_ALLOWED); + modifyAppOp(testAppUid, OPSTR_LEGACY_STORAGE, AppOpsManager.MODE_ALLOWED); + + assertThat(checkIsLegacyStorageGranted(getContext(), testAppUid, + packageName, /* isTargetSdkAtLeastV */ false)).isTrue(); + // Since R_E_S is not granted, this is should return false + assertThat(checkPermissionReadStorage( + getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); + assertThat(checkPermissionReadForLegacyStorage( + getContext(), TEST_APP_PID, testAppUid, packageName, + null, /* isTargetSdkAtLeastT */ true)).isTrue(); } finally { dropShellPermission(); } @@ -279,7 +323,7 @@ public class PermissionUtilsTest { assertThat( checkIsLegacyStorageGranted(getContext(), testAppUid, packageName, - null, /* isTargetSdkAtLeastS */ false)).isFalse(); + /* isTargetSdkAtLeastS */ false)).isFalse(); assertThat( checkPermissionInstallPackages(getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); @@ -328,7 +372,12 @@ public class PermissionUtilsTest { assertThat( checkIsLegacyStorageGranted(getContext(), testAppUid, packageName, - null, /* isTargetSdkAtLeastS */ false)).isTrue(); + /* isTargetSdkAtLeastS */ false)).isTrue(); + assertThat( + checkPermissionReadForLegacyStorage(getContext(), TEST_APP_PID, + testAppUid, packageName, + null, /* isTargetSdkAtLeastT */ false)).isTrue(); + assertThat( checkPermissionInstallPackages(getContext(), TEST_APP_PID, testAppUid, packageName, null)).isFalse(); @@ -429,18 +478,15 @@ public class PermissionUtilsTest { try { assertThat( - checkNoIsolatedStorageGranted(getContext(), testAppUid, packageName, - null)).isFalse(); + checkNoIsolatedStorageGranted(getContext(), testAppUid, packageName)).isFalse(); modifyAppOp(testAppUid, OPSTR_NO_ISOLATED_STORAGE, AppOpsManager.MODE_ALLOWED); assertThat( - checkNoIsolatedStorageGranted(getContext(), testAppUid, packageName, - null)).isTrue(); + checkNoIsolatedStorageGranted(getContext(), testAppUid, packageName)).isTrue(); modifyAppOp(testAppUid, OPSTR_NO_ISOLATED_STORAGE, AppOpsManager.MODE_ERRORED); assertThat( - checkNoIsolatedStorageGranted(getContext(), testAppUid, packageName, - null)).isFalse(); + checkNoIsolatedStorageGranted(getContext(), testAppUid, packageName)).isFalse(); } finally { dropShellPermission(); } @@ -689,7 +735,8 @@ public class PermissionUtilsTest { static private void checkPermissionsForGallery(int uid, int pid, String packageName, boolean expected) { assertEquals(expected, - checkWriteImagesOrVideoAppOps(getContext(), uid, packageName, null)); + checkWriteImagesOrVideoAppOps(getContext(), uid, packageName, null, + /* forDataDelivery */ true)); assertEquals(expected, checkPermissionWriteImages(getContext(), pid, uid, packageName, null, /* forDataDelivery */ true)); diff --git a/tests/test_app/LegacyTestAppWithTargetSdk33.xml b/tests/test_app/LegacyTestAppWithTargetSdk33.xml new file mode 100644 index 000000000..541e5abcf --- /dev/null +++ b/tests/test_app/LegacyTestAppWithTargetSdk33.xml @@ -0,0 +1,40 @@ +<?xml version="1.0" encoding="utf-8"?> +<!-- + ~ Copyright (C) 2025 The Android Open Source Project + ~ + ~ Licensed under the Apache License, Version 2.0 (the "License"); + ~ you may not use this file except in compliance with the License. + ~ You may obtain a copy of the License at + ~ + ~ http://www.apache.org/licenses/LICENSE-2.0 + ~ + ~ Unless required by applicable law or agreed to in writing, software + ~ distributed under the License is distributed on an "AS IS" BASIS, + ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + ~ See the License for the specific language governing permissions and + ~ limitations under the License. + --> + +<manifest xmlns:android="http://schemas.android.com/apk/res/android" + package="com.android.providers.media.testapp.legacywithtargetsdk33" + android:versionCode="1" + android:versionName="1.0"> + + <uses-sdk android:minSdkVersion="30" android:targetSdkVersion="33" /> + + <uses-permission android:name="android.permission.READ_MEDIA_IMAGES" /> + <uses-permission android:name="android.permission.READ_MEDIA_VIDEO" /> + <uses-permission android:name="android.permission.READ_MEDIA_AUDIO" /> + + <application android:label="LegacyTestAppWithTargetSdk33" + android:requestLegacyExternalStorage="true"> + <activity android:name="com.android.providers.media.util.TestAppActivity" + android:exported="true"> + <intent-filter> + <action android:name="android.intent.action.MAIN"/> + <category android:name="android.intent.category.DEFAULT"/> + <category android:name="android.intent.category.LAUNCHER"/> + </intent-filter> + </activity> + </application> +</manifest> |