From 07d55f86a85c440d2414b2e16a433c177244cfc8 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sat, 12 Sep 2026 01:48:55 +0300 Subject: [PATCH 01/21] Fix predictive back --- .../PredictiveBackTransitionTest.kt | 63 +++++++++++++++++ .../components/PredictiveBackTransition.kt | 69 ++++++++++--------- 2 files changed, 100 insertions(+), 32 deletions(-) create mode 100644 app/src/test/kotlin/com/android/messaging/ui/common/components/PredictiveBackTransitionTest.kt diff --git a/app/src/test/kotlin/com/android/messaging/ui/common/components/PredictiveBackTransitionTest.kt b/app/src/test/kotlin/com/android/messaging/ui/common/components/PredictiveBackTransitionTest.kt new file mode 100644 index 000000000..91b5fe948 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/common/components/PredictiveBackTransitionTest.kt @@ -0,0 +1,63 @@ +package com.android.messaging.ui.common.components + +import androidx.compose.animation.core.AnimationVector1D +import androidx.compose.animation.core.VectorConverter +import androidx.navigationevent.NavigationEvent +import kotlin.math.roundToLong +import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue +import org.junit.Test + +class PredictiveBackTransitionTest { + + @Test + fun predictiveBackTranslation_pushesAwayFromTheSwipedEdge() { + val fromLeft = translationFrom(NavigationEvent.EDGE_LEFT) + val fromRight = translationFrom(NavigationEvent.EDGE_RIGHT) + + assertTrue("left swipe should push right, was $fromLeft", fromLeft > 0) + assertEquals(-fromLeft, fromRight) + } + + @Test + fun predictiveBackTranslation_keepsTheScaledScreenInsideTheFarEdge() { + val scaledHalfWidth = FULL_WIDTH * PREDICTIVE_BACK_TARGET_SCALE / 2f + val farEdge = FULL_WIDTH / 2f + scaledHalfWidth + translationFrom(NavigationEvent.EDGE_LEFT) + + val expectedMargin = FULL_WIDTH * PREDICTIVE_BACK_EDGE_MARGIN_FRACTION + assertEquals(FULL_WIDTH - expectedMargin, farEdge, 1f) + } + + @Test + fun predictiveBackTranslation_withoutAnEdgeOnlyShrinks() { + assertEquals(0, translationFrom(NavigationEvent.EDGE_NONE)) + } + + /** + * The gesture seeks the transition by play time, so a screen only follows the finger while + * half the play time means half the movement. An eased spec would run ahead of the finger. + */ + @Test + fun predictiveBackSpec_movesEvenlyAcrossItsPlayTime() { + val spec = predictiveBackSpec().vectorize(Float.VectorConverter) + val start = AnimationVector1D(0f) + val end = AnimationVector1D(1f) + val durationNanos = spec.getDurationNanos(start, end, start) + + for (fraction in listOf(0.25f, 0.5f, 0.75f)) { + val playTimeNanos = (fraction * durationNanos).roundToLong() + val moved = spec.getValueFromNanos(playTimeNanos, start, end, start).value + + assertEquals("at $fraction of the play time", fraction, moved, TOLERANCE) + } + } + + private fun translationFrom(swipeEdge: Int): Int { + return predictiveBackTranslation(swipeEdge = swipeEdge, fullWidth = FULL_WIDTH) + } + + private companion object { + const val FULL_WIDTH = 1080 + const val TOLERANCE = 0.01f + } +} diff --git a/src/com/android/messaging/ui/common/components/PredictiveBackTransition.kt b/src/com/android/messaging/ui/common/components/PredictiveBackTransition.kt index 744d71784..df78ea85e 100644 --- a/src/com/android/messaging/ui/common/components/PredictiveBackTransition.kt +++ b/src/com/android/messaging/ui/common/components/PredictiveBackTransition.kt @@ -4,52 +4,57 @@ import androidx.compose.animation.ContentTransform import androidx.compose.animation.EnterTransition import androidx.compose.animation.ExitTransition import androidx.compose.animation.core.FiniteAnimationSpec -import androidx.compose.animation.core.spring -import androidx.compose.animation.fadeIn +import androidx.compose.animation.core.LinearEasing +import androidx.compose.animation.core.tween import androidx.compose.animation.scaleOut +import androidx.compose.animation.slideOutHorizontally import androidx.compose.animation.togetherWith -import androidx.compose.ui.graphics.TransformOrigin import androidx.navigationevent.NavigationEvent +import androidx.navigationevent.NavigationEvent.SwipeEdge +import kotlin.math.roundToInt -internal fun predictiveBackContentTransform(swipeEdge: Int): ContentTransform { - return predictiveBackEnter() togetherWith predictiveBackExit(swipeEdge = swipeEdge) +internal fun predictiveBackContentTransform( + @SwipeEdge swipeEdge: Int, +): ContentTransform { + return EnterTransition.None togetherWith predictiveBackExit(swipeEdge = swipeEdge) } -private fun predictiveBackEnter(): EnterTransition { - return fadeIn(animationSpec = predictiveBackSpec()) -} - -private fun predictiveBackExit(swipeEdge: Int): ExitTransition { - return scaleOut( +private fun predictiveBackExit( + @SwipeEdge swipeEdge: Int, +): ExitTransition { + val shrink = scaleOut( animationSpec = predictiveBackSpec(), targetScale = PREDICTIVE_BACK_TARGET_SCALE, - transformOrigin = predictiveBackTransformOrigin(swipeEdge = swipeEdge), ) + val push = slideOutHorizontally(animationSpec = predictiveBackSpec()) { fullWidth -> + predictiveBackTranslation(swipeEdge = swipeEdge, fullWidth = fullWidth) + } + + return shrink + push } -private fun predictiveBackTransformOrigin(swipeEdge: Int): TransformOrigin { - val pivotFractionX = when (swipeEdge) { - NavigationEvent.EDGE_LEFT -> TRAILING_PIVOT_FRACTION - NavigationEvent.EDGE_RIGHT -> LEADING_PIVOT_FRACTION - else -> CENTER_PIVOT_FRACTION +internal fun predictiveBackTranslation( + @SwipeEdge swipeEdge: Int, + fullWidth: Int, +): Int { + val slack = fullWidth * (1f - PREDICTIVE_BACK_TARGET_SCALE) / 2f + val travel = (slack - fullWidth * PREDICTIVE_BACK_EDGE_MARGIN_FRACTION).roundToInt() + + return when (swipeEdge) { + NavigationEvent.EDGE_LEFT -> travel + NavigationEvent.EDGE_RIGHT -> -travel + else -> 0 } - - return TransformOrigin( - pivotFractionX = pivotFractionX, - pivotFractionY = CENTER_PIVOT_FRACTION, - ) } -private fun predictiveBackSpec(): FiniteAnimationSpec { - return spring( - dampingRatio = PREDICTIVE_BACK_DAMPING_RATIO, - stiffness = PREDICTIVE_BACK_STIFFNESS, +internal fun predictiveBackSpec(): FiniteAnimationSpec { + return tween( + durationMillis = PREDICTIVE_BACK_DURATION_MILLIS, + easing = LinearEasing, ) } -private const val PREDICTIVE_BACK_TARGET_SCALE = 0.9f -private const val PREDICTIVE_BACK_DAMPING_RATIO = 1.0f -private const val PREDICTIVE_BACK_STIFFNESS = 1600.0f -private const val LEADING_PIVOT_FRACTION = 0f -private const val CENTER_PIVOT_FRACTION = 0.5f -private const val TRAILING_PIVOT_FRACTION = 1f +internal const val PREDICTIVE_BACK_TARGET_SCALE = 0.75f +internal const val PREDICTIVE_BACK_EDGE_MARGIN_FRACTION = 0.025f + +private const val PREDICTIVE_BACK_DURATION_MILLIS = 100 From 4cd7581baff5f86a87a21087ab5617892e752dea Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sat, 12 Sep 2026 20:42:51 +0300 Subject: [PATCH 02/21] Run one notification pass at a time Overlapping passes all saw the same "nothing posted yet" state and repeated each other's attachment decodes, which is what put the SMS_DELIVER broadcast over its deadline. --- .../BugleNotificationsConcurrentPassTest.kt | 78 +++++++++++++++++++ .../datamodel/BugleNotifications.java | 9 +++ 2 files changed, 87 insertions(+) create mode 100644 app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsConcurrentPassTest.kt diff --git a/app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsConcurrentPassTest.kt b/app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsConcurrentPassTest.kt new file mode 100644 index 000000000..6ce32d77d --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsConcurrentPassTest.kt @@ -0,0 +1,78 @@ +package com.android.messaging.datamodel + +import com.android.messaging.FactoryTestAccess +import com.android.messaging.testutil.installTestFactory +import com.android.messaging.util.PhoneUtils +import io.mockk.every +import io.mockk.mockk +import io.mockk.unmockkAll +import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit.MILLISECONDS +import java.util.concurrent.TimeUnit.SECONDS +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicInteger +import kotlin.concurrent.thread +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment + +@RunWith(RobolectricTestRunner::class) +class BugleNotificationsConcurrentPassTest { + + private val phoneUtils = mockk(relaxed = true) + + @Before + fun setUp() { + installTestFactory( + context = RuntimeEnvironment.getApplication().applicationContext, + phoneUtils = phoneUtils, + ) + } + + @After + fun tearDown() { + unmockkAll() + FactoryTestAccess.reset() + } + + @Test + fun update_whileAnotherPassIsRunning_waitsForIt() { + val firstIsInside = CountDownLatch(1) + val secondGotInside = CountDownLatch(1) + val overlapped = AtomicBoolean(false) + val passes = AtomicInteger() + + // The first thing a pass does once it is under way. Returning false makes it stop here, so + // the test observes the pass boundary and nothing else. + every { phoneUtils.isDefaultSmsApp() } answers { + when (passes.getAndIncrement()) { + 0 -> { + firstIsInside.countDown() + overlapped.set(secondGotInside.await(OVERLAP_TIMEOUT_MILLIS, MILLISECONDS)) + } + + else -> secondGotInside.countDown() + } + false + } + + val first = thread { BugleNotifications.update("1", BugleNotifications.UPDATE_MESSAGES) } + assertTrue("the first pass never started", firstIsInside.await(5, SECONDS)) + val second = thread { BugleNotifications.update("2", BugleNotifications.UPDATE_MESSAGES) } + first.join() + second.join() + + assertEquals("both passes should have run", 2, passes.get()) + assertFalse("a second notification pass ran inside the first one", overlapped.get()) + } + + private companion object { + const val OVERLAP_TIMEOUT_MILLIS = 500L + } +} diff --git a/src/com/android/messaging/datamodel/BugleNotifications.java b/src/com/android/messaging/datamodel/BugleNotifications.java index 9e7197682..1dab49d11 100644 --- a/src/com/android/messaging/datamodel/BugleNotifications.java +++ b/src/com/android/messaging/datamodel/BugleNotifications.java @@ -132,6 +132,9 @@ public class BugleNotifications { private static final AtomicLong sLastNotificationImageSweep = new AtomicLong(); + /** Guards a notification pass, so that concurrent callers do not repeat each other's work */ + private static final Object sNotificationLock = new Object(); + /** * This is the volume at which to play the observable-conversation notification sound, * expressed as a fraction of the system notification volume. @@ -162,6 +165,12 @@ public static void update(final String conversationId, final int coverage) { } Assert.isNotMainThread(); + synchronized (sNotificationLock) { + updateSerialized(conversationId, coverage); + } + } + + private static void updateSerialized(final String conversationId, final int coverage) { final long passStart = System.currentTimeMillis(); try { if (!PhoneUtils.getDefault().isDefaultSmsApp()) { From 8c5768eba9a44a9e9391e89b55f907bf03c89c1e Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sat, 12 Sep 2026 20:43:10 +0300 Subject: [PATCH 03/21] Transcode each notification image once The file was named with a fresh random id per call, so every pass decoded and re-compressed every image again. Name it after parts._id, which is digits-only and never reused, and drop the cache whenever the database is created, since that restarts those ids. --- .../MessageNotificationAttachmentUriTest.kt | 40 +++++- .../BugleNotificationsImageCacheTest.kt | 117 ++++++++++++++++++ .../BugleNotificationsMessageLimitTest.kt | 1 + .../datamodel/DatabaseUpgradeHelperTest.kt | 46 +++++++ .../datamodel/BugleNotifications.java | 18 ++- .../messaging/datamodel/DatabaseHelper.java | 9 ++ .../messaging/datamodel/FileProvider.java | 17 ++- .../datamodel/MessageNotificationState.java | 11 +- .../datamodel/NotificationImageProvider.kt | 28 ++--- 9 files changed, 261 insertions(+), 26 deletions(-) create mode 100644 app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsImageCacheTest.kt diff --git a/app/src/androidTest/java/com/android/messaging/datamodel/MessageNotificationAttachmentUriTest.kt b/app/src/androidTest/java/com/android/messaging/datamodel/MessageNotificationAttachmentUriTest.kt index 14d2445af..cf6b8c873 100644 --- a/app/src/androidTest/java/com/android/messaging/datamodel/MessageNotificationAttachmentUriTest.kt +++ b/app/src/androidTest/java/com/android/messaging/datamodel/MessageNotificationAttachmentUriTest.kt @@ -179,12 +179,34 @@ class MessageNotificationAttachmentUriTest { ) } + /** + * Re-minting the file per pass is what put the `SMS_DELIVER` broadcast over its deadline: every + * pass decoded and re-compressed the same attachment again. The file is named after the part, + * so a pass that has already transcoded it reuses it. + */ @Test - fun createStyledMessage_calledTwice_doesNotReuseTheSameFile() { + fun createStyledMessage_calledTwiceForTheSamePart_reusesTheTranscodedFile() { val first = messageLineInfo(attachmentUri).createStyledMessage(SENDER).dataUri val second = messageLineInfo(attachmentUri).createStyledMessage(SENDER).dataUri - assertNotEquals("the same file was handed to two notifications", first, second) + assertNotNull("no image was attached to the notification", first) + assertEquals("the attachment was transcoded a second time", first, second) + assertEquals( + "reusing the transcode still left a second file behind", + 1, + NotificationImageProvider.listImageFiles().size, + ) + } + + /** Parts must not share a file, or a conversation gets handed another one's photo. */ + @Test + fun createStyledMessage_forDifferentParts_usesDifferentFiles() { + val first = messageLineInfo(attachmentUri, PART_ID).createStyledMessage(SENDER).dataUri + val second = + messageLineInfo(attachmentUri, OTHER_PART_ID).createStyledMessage(SENDER).dataUri + + assertNotNull("no image was attached to the notification", first) + assertNotEquals("two parts were handed the same file", first, second) } /** @@ -193,7 +215,9 @@ class MessageNotificationAttachmentUriTest { */ @Test fun sweepNotificationImages_keepsPostedImagesAndDeletesOrphans() { - val orphan = imageFileOf(NotificationImageProvider.buildNotificationImageUri()) + val orphan = imageFileOf( + messageLineInfo(attachmentUri, OTHER_PART_ID).createStyledMessage(SENDER).dataUri, + ) val message = messageLineInfo(attachmentUri).createStyledMessage(SENDER) val posted = imageFileOf(message.dataUri) assertTrue("the platform refused to grant access to the attached image", post(message)) @@ -268,16 +292,20 @@ class MessageNotificationAttachmentUriTest { ) } - private fun messageLineInfo(uri: Uri): MessageNotificationState.MessageLineInfo { + private fun messageLineInfo( + uri: Uri, + partId: String = PART_ID, + ): MessageNotificationState.MessageLineInfo { return MessageNotificationState.MessageLineInfo( // authorId, authorFullName, authorFirstName "author", "Sender", "Sender", - // text, attachmentUri, attachmentType + // text, attachmentUri, attachmentType, attachmentPartId "Check out this photo!", uri, ContentType.IMAGE_JPEG, + partId, // isManualDownloadNeeded, avatarUri, messageId, timestamp, contactUriString false, null, @@ -297,6 +325,8 @@ class MessageNotificationAttachmentUriTest { const val POST_POLL_MILLIS = 50L const val NOTIFICATION_ID = 0x7103 const val NOTIFICATION_TAG = "BUG-015" + const val PART_ID = "7" + const val OTHER_PART_ID = "8" val UNGRANTABLE_URI: Uri = Uri.parse("content://call_log/calls/1") val SELF: Person = Person.Builder().setName("Me").build() val SENDER: Person = Person.Builder().setName("Sender").setKey("author").build() diff --git a/app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsImageCacheTest.kt b/app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsImageCacheTest.kt new file mode 100644 index 000000000..22581d0e3 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsImageCacheTest.kt @@ -0,0 +1,117 @@ +package com.android.messaging.datamodel + +import android.graphics.Bitmap +import android.net.Uri +import com.android.messaging.FactoryTestAccess +import com.android.messaging.datamodel.media.ImageResource +import com.android.messaging.datamodel.media.MediaResourceManager +import com.android.messaging.testutil.FakeBuglePrefs +import com.android.messaging.testutil.createTestFactory +import io.mockk.every +import io.mockk.mockk +import io.mockk.unmockkAll +import io.mockk.verify +import kotlin.time.Duration.Companion.minutes +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment + +@RunWith(RobolectricTestRunner::class) +class BugleNotificationsImageCacheTest { + + private val context = RuntimeEnvironment.getApplication().applicationContext + private val mediaResourceManager = mockk() + + @Before + fun setUp() { + val factory = createTestFactory( + context = context, + dataModel = mockk(relaxed = true), + prefs = FakeBuglePrefs(), + ) + every { factory.mediaResourceManager } returns mediaResourceManager + FactoryTestAccess.install(factory) + + every { mediaResourceManager.requestMediaResourceSync(any()) } answers { + mockk(relaxed = true) { + every { bitmap } returns Bitmap.createBitmap(4, 4, Bitmap.Config.ARGB_8888) + } + } + } + + @After + fun tearDown() { + unmockkAll() + FactoryTestAccess.reset() + } + + @Test + fun getNotificationImageUri_forSameAttachmentPart_transcodesOnce() { + val first = BugleNotifications.getNotificationImageUri(context, ATTACHMENT, "7") + val second = BugleNotifications.getNotificationImageUri(context, ATTACHMENT, "7") + + assertNotNull(first) + assertEquals(first, second) + verify(exactly = 1) { mediaResourceManager.requestMediaResourceSync(any()) } + } + + @Test + fun getNotificationImageUri_forDifferentAttachmentParts_transcodesEach() { + val first = BugleNotifications.getNotificationImageUri(context, ATTACHMENT, "7") + val second = BugleNotifications.getNotificationImageUri(context, ATTACHMENT, "8") + + assertNotNull(first) + assertNotNull(second) + assertNotEquals(first, second) + verify(exactly = 2) { mediaResourceManager.requestMediaResourceSync(any()) } + } + + /** The sweep reclaims images once their notification is gone; the next pass must rebuild them. */ + @Test + fun getNotificationImageUri_afterCachedImageSwept_transcodesAgain() { + val first = BugleNotifications.getNotificationImageUri(context, ATTACHMENT, "7") + NotificationImageProvider.getFileFromUri(first)!!.delete() + + val second = BugleNotifications.getNotificationImageUri(context, ATTACHMENT, "7") + + assertNotNull(second) + assertEquals(first, second) + verify(exactly = 2) { mediaResourceManager.requestMediaResourceSync(any()) } + } + + /** + * A cache hit has to touch the file, or [BugleNotifications.sweepNotificationImages] reclaims an + * image that a notification posted in this pass still points at. + */ + @Test + fun getNotificationImageUri_onCacheHit_touchesFileSoTheSweepSpareIt() { + val uri = BugleNotifications.getNotificationImageUri(context, ATTACHMENT, "7") + val file = NotificationImageProvider.getFileFromUri(uri)!! + val stale = System.currentTimeMillis() - 10.minutes.inWholeMilliseconds + assertTrue(file.setLastModified(stale)) + + BugleNotifications.getNotificationImageUri(context, ATTACHMENT, "7") + + assertTrue(file.lastModified() > stale) + } + + /** Parts that are not persisted yet have no id, and must still get an image. */ + @Test + fun getNotificationImageUri_withoutPartId_stillTranscodes() { + val uri = BugleNotifications.getNotificationImageUri(context, ATTACHMENT, null) + + assertNotNull(uri) + assertTrue(NotificationImageProvider.isNotificationImageUri(uri)) + } + + private companion object { + val ATTACHMENT: Uri = Uri.parse("content://mms/part/1") + } +} diff --git a/app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsMessageLimitTest.kt b/app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsMessageLimitTest.kt index 8ab4059d9..771af23ae 100644 --- a/app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsMessageLimitTest.kt +++ b/app/src/test/kotlin/com/android/messaging/datamodel/BugleNotificationsMessageLimitTest.kt @@ -85,6 +85,7 @@ class BugleNotificationsMessageLimitTest { "Message $messageId", null, null, + null, false, null, messageId, diff --git a/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt b/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt index 266335523..05541ddbb 100644 --- a/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt +++ b/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt @@ -11,6 +11,7 @@ import com.android.messaging.testutil.installTestFactory import io.mockk.unmockkAll import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -68,6 +69,51 @@ class DatabaseUpgradeHelperTest { } } + /** + * rebuildTables() drops the parts table, and that takes its sqlite_sequence row along, so + * parts._id restarts at 1. Notification images are named after the part they were transcoded + * from and outlive the database, so a leftover image for part 1 would be served as the image + * of whatever part next takes that id - showing an unrelated photo in a notification. + */ + @Test + fun rebuildTables_discardsCachedNotificationImages() { + val stale = checkNotNull( + NotificationImageProvider.buildNotificationImageUri("1") + ?.let(NotificationImageProvider::getFileFromUri) + ) + stale.writeBytes(byteArrayOf(1, 2, 3)) + + SQLiteDatabase.create(null).use(DatabaseHelper::rebuildTables) + + assertFalse( + "a cached notification image outlived the part id space it was named after", + stale.exists(), + ) + } + + /** + * A corrupt database is deleted and recreated underneath us, and SQLiteOpenHelper answers the + * resulting version zero with onCreate() rather than rebuildTables(). The cleanup has to sit + * where both paths meet, or images cached against the old part ids survive to be served as + * some unrelated part's picture. + */ + @Test + fun onCreate_discardsCachedNotificationImages() { + val context = RuntimeEnvironment.getApplication().applicationContext + val stale = checkNotNull( + NotificationImageProvider.buildNotificationImageUri("1") + ?.let(NotificationImageProvider::getFileFromUri) + ) + stale.writeBytes(byteArrayOf(1, 2, 3)) + + SQLiteDatabase.create(null).use { DatabaseHelper.getInstance(context).onCreate(it) } + + assertFalse( + "a cached notification image outlived the database it was named against", + stale.exists(), + ) + } + @Test fun upgradeToVersion3_createsPinnedColumnAndIndex() { val table = DatabaseHelper.CONVERSATIONS_TABLE diff --git a/src/com/android/messaging/datamodel/BugleNotifications.java b/src/com/android/messaging/datamodel/BugleNotifications.java index 1dab49d11..f1783bf20 100644 --- a/src/com/android/messaging/datamodel/BugleNotifications.java +++ b/src/com/android/messaging/datamodel/BugleNotifications.java @@ -742,12 +742,21 @@ public static Bitmap getAvatarBitmap(Context context, Uri avatarUri) { return null; } - static Uri getNotificationImageUri(final Context context, final Uri imageUri) { + /** + * Transcodes the attachment at {@code imageUri} into a file named after {@code partId}, or + * returns the file an earlier call already wrote for that part. Call it from a notification + * pass only: the deterministic name makes the temporary file it writes through deterministic + * too, so two concurrent callers for one part would trample each other's write. + */ + static Uri getNotificationImageUri(final Context context, final Uri imageUri, + final String partId) { if (imageUri == null) { return null; } - final Uri notificationImageUri = NotificationImageProvider.buildNotificationImageUri(); + final Uri notificationImageUri = NotificationImageProvider + .buildNotificationImageUri(partId); + if (notificationImageUri == null) { return null; } @@ -756,6 +765,11 @@ static Uri getNotificationImageUri(final Context context, final Uri imageUri) { return null; } + if (imageFile.length() > 0) { + imageFile.setLastModified(System.currentTimeMillis()); + return notificationImageUri; + } + final ImageRequestDescriptor descriptor = new UriImageRequestDescriptor( imageUri, NOTIFICATION_IMAGE_MAX_SIZE, diff --git a/src/com/android/messaging/datamodel/DatabaseHelper.java b/src/com/android/messaging/datamodel/DatabaseHelper.java index 0dde72c1b..7c5a0c418 100644 --- a/src/com/android/messaging/datamodel/DatabaseHelper.java +++ b/src/com/android/messaging/datamodel/DatabaseHelper.java @@ -33,6 +33,7 @@ import com.android.messaging.util.LogUtil; import com.google.common.annotations.VisibleForTesting; +import java.io.File; import java.util.ArrayList; import java.util.List; @@ -812,6 +813,14 @@ private static void dropAllIndexes(final SQLiteDatabase db) { } private static void createDatabase(final SQLiteDatabase db) { + // A fresh parts table starts its _id at 1 again, whether this is a rebuild, a first + // run, or the recreation that follows corruption. The notification images are named + // after those ids and outlive the database, so leaving them behind would serve one + // part's image as the image of whatever part next takes its id + for (final File image : NotificationImageProvider.listImageFiles()) { + image.delete(); + } + for (final String sql : CREATE_TABLE_SQLS) { db.execSQL(sql); } diff --git a/src/com/android/messaging/datamodel/FileProvider.java b/src/com/android/messaging/datamodel/FileProvider.java index ee332cd11..5556068a7 100644 --- a/src/com/android/messaging/datamodel/FileProvider.java +++ b/src/com/android/messaging/datamodel/FileProvider.java @@ -80,10 +80,23 @@ protected static boolean ensureFileExists(final File file) { * @return unique uri that can be used to write temporary files */ protected static Uri buildFileUri(final String authority, final String extension) { - final long fileId = Math.abs(RANDOM_ID.nextLong()); + return buildFileUri(authority, extension, + String.valueOf(Math.abs(RANDOM_ID.nextLong()))); + } + + /** + * Build uri for a file with a caller-chosen id, so that the same logical content maps to the + * same file on every call. The id must satisfy {@link #isValidFileId}. + * @param authority authority with which to populate uri + * @param extension optional file extension + * @param fileId the file id to use + * @return uri naming that file + */ + protected static Uri buildFileUri(final String authority, final String extension, + final String fileId) { final Uri.Builder builder = (new Uri.Builder()).authority(authority).scheme( ContentResolver.SCHEME_CONTENT); - builder.appendPath(String.valueOf(fileId)); + builder.appendPath(fileId); if (!TextUtils.isEmpty(extension)) { builder.appendQueryParameter(FILE_EXTENSION_PARAM_KEY, extension); } diff --git a/src/com/android/messaging/datamodel/MessageNotificationState.java b/src/com/android/messaging/datamodel/MessageNotificationState.java index 7407ed09d..87845101f 100644 --- a/src/com/android/messaging/datamodel/MessageNotificationState.java +++ b/src/com/android/messaging/datamodel/MessageNotificationState.java @@ -122,11 +122,12 @@ public static class MessageLineInfo { final Uri mAvatarUri; final Uri mAttachmentUri; final String mAttachmentType; + final String mAttachmentPartId; final String mContactUriString; MessageLineInfo(final String authorId, final String authorFullName, final String authorFirstName, final CharSequence text, final Uri attachmentUri, - final String attachmentType, + final String attachmentType, final String attachmentPartId, final boolean isManualDownloadNeeded, final Uri avatarUri, final String messageId, final long timestamp, final String contactUriString) { mAuthorId = authorId; @@ -150,6 +151,7 @@ public static class MessageLineInfo { mAvatarUri = avatarUri; mAttachmentUri = attachmentUri; mAttachmentType = attachmentType; + mAttachmentPartId = attachmentPartId; mContactUriString = contactUriString; } @@ -195,7 +197,8 @@ public MessagingStyle.Message createStyledMessage(Person person) { new MessagingStyle.Message(mText, mTimestamp, person); if (mAttachmentUri != null && ContentType.isImageType(mAttachmentType)) { final Uri notificationImageUri = BugleNotifications.getNotificationImageUri( - Factory.get().getApplicationContext(), mAttachmentUri); + Factory.get().getApplicationContext(), mAttachmentUri, + mAttachmentPartId); if (notificationImageUri != null) { message.setData(ContentType.IMAGE_JPEG, notificationImageUri); } @@ -552,11 +555,13 @@ private static ConversationsList createConversationsList() { // Otherwise, show the first one we find. Uri attachmentUri = null; String attachmentType = null; + String attachmentPartId = null; final MessagePartData messagePartData = getMostInterestingAttachment(convMessageData); if (messagePartData != null) { attachmentUri = messagePartData.getContentUri(); attachmentType = messagePartData.getContentType(); + attachmentPartId = messagePartData.getPartId(); } Uri contactUri = convMessageData.getSenderContactLookupUri(); @@ -567,7 +572,7 @@ private static ConversationsList createConversationsList() { conversation.mLineInfos.add(new MessageLineInfo(authorId, authorFullName, authorFirstName, text, - attachmentUri, attachmentType, + attachmentUri, attachmentType, attachmentPartId, isManualDownloadNeeded, avatarUri, messageId, timestamp, contactUriString)); messageCount++; diff --git a/src/com/android/messaging/datamodel/NotificationImageProvider.kt b/src/com/android/messaging/datamodel/NotificationImageProvider.kt index eeb3f283d..6d9549879 100644 --- a/src/com/android/messaging/datamodel/NotificationImageProvider.kt +++ b/src/com/android/messaging/datamodel/NotificationImageProvider.kt @@ -31,21 +31,21 @@ class NotificationImageProvider : FileProvider() { } @JvmStatic - fun buildNotificationImageUri(): Uri? { - val uri = buildFileUri(AUTHORITY, NOTIFICATION_IMAGE_EXTENSION) - val file = getFileFromUri(uri) - - return when { - file == null -> null - ensureFileExists(file) -> uri - else -> { - LogUtil.e( - LogUtil.BUGLE_TAG, - "Failed to create notification image ${file.absolutePath}", - ) - null - } + fun buildNotificationImageUri(fileId: String?): Uri? { + val directory = getDirectory() + if (!directory.exists() && !directory.mkdirs()) { + LogUtil.e(LogUtil.BUGLE_TAG, "Failed to create ${directory.absolutePath}") + return null } + + val uri = when { + !fileId.isNullOrEmpty() && isValidFileId(fileId) -> + buildFileUri(AUTHORITY, NOTIFICATION_IMAGE_EXTENSION, fileId) + + else -> buildFileUri(AUTHORITY, NOTIFICATION_IMAGE_EXTENSION) + } + + return uri.takeIf { getFileFromUri(it) != null } } @JvmStatic From 4d8edba076a936392fc5d00576f58a1e275e28ae Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sat, 12 Sep 2026 20:43:18 +0300 Subject: [PATCH 04/21] Stop holding the SMS_DELIVER broadcast open past its deadline Wait at most 8s for the import, then finish the broadcast. The import runs in the application scope either way, and the message commits before the notification pass that takes the time. --- .../android/messaging/receiver/SmsDeliverReceiver.kt | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/com/android/messaging/receiver/SmsDeliverReceiver.kt b/src/com/android/messaging/receiver/SmsDeliverReceiver.kt index 5303ab89c..076e12914 100644 --- a/src/com/android/messaging/receiver/SmsDeliverReceiver.kt +++ b/src/com/android/messaging/receiver/SmsDeliverReceiver.kt @@ -7,7 +7,9 @@ import android.provider.Telephony.Sms import android.telephony.SmsMessage import com.android.messaging.di.receiver.IncomingSmsEntryPoint import dagger.hilt.android.EntryPointAccessors +import kotlin.time.Duration.Companion.seconds import kotlinx.coroutines.launch +import kotlinx.coroutines.withTimeoutOrNull class SmsDeliverReceiver : BroadcastReceiver() { @@ -21,8 +23,14 @@ class SmsDeliverReceiver : BroadcastReceiver() { val appContext = context.applicationContext val entryPoint = entryPoint(appContext) entryPoint.applicationScope().launch(entryPoint.ioDispatcher()) { - try { + val delivery = launch { entryPoint.incomingSmsDeliverer().deliverFromIntent(appContext, intent) + } + try { + // The import keeps running in the application scope either way; stop holding the + // broadcast open for it once the window is nearly spent, or the system kills us + // with an ANR and the import does not finish at all. + withTimeoutOrNull(BROADCAST_BUDGET) { delivery.join() } } finally { pendingResult.finish() } @@ -31,6 +39,8 @@ class SmsDeliverReceiver : BroadcastReceiver() { companion object { + private val BROADCAST_BUDGET = 8.seconds + @JvmStatic fun deliverSmsMessages( context: Context, From d8f406e4c2de7f8ff31cfc5ccb7669ef7b072de5 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sat, 12 Sep 2026 21:40:36 +0300 Subject: [PATCH 05/21] Fix new chat background color --- .../conversation/addparticipants/AddParticipantsScreen.kt | 6 ++---- .../messaging/ui/conversation/entry/NewChatScreen.kt | 5 +---- .../recipientpicker/component/RecipientSelectionContent.kt | 2 +- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/com/android/messaging/ui/conversation/addparticipants/AddParticipantsScreen.kt b/src/com/android/messaging/ui/conversation/addparticipants/AddParticipantsScreen.kt index b2dc720b2..5c206d1b5 100644 --- a/src/com/android/messaging/ui/conversation/addparticipants/AddParticipantsScreen.kt +++ b/src/com/android/messaging/ui/conversation/addparticipants/AddParticipantsScreen.kt @@ -74,13 +74,11 @@ internal fun AddParticipantsScreen( Scaffold( modifier = modifier, - containerColor = MaterialTheme.colorScheme.surfaceVariant, + containerColor = MaterialTheme.colorScheme.surfaceContainer, topBar = { TopAppBar( colors = TopAppBarDefaults.topAppBarColors( - containerColor = MaterialTheme.colorScheme.surfaceVariant, - navigationIconContentColor = MaterialTheme.colorScheme.onSurface, - titleContentColor = MaterialTheme.colorScheme.onSurface, + containerColor = MaterialTheme.colorScheme.surfaceContainer, ), navigationIcon = { IconButton( diff --git a/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt b/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt index 232d362fd..f8b647e85 100644 --- a/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt +++ b/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt @@ -134,7 +134,7 @@ private fun NewChatScreenContent( selectedGroupRecipients: ImmutableList = persistentListOf(), simSelectorUiState: ConversationSimSelectorUiState = ConversationSimSelectorUiState(), ) { - val screenContainerColor = MaterialTheme.colorScheme.surfaceVariant + val screenContainerColor = MaterialTheme.colorScheme.surfaceContainer Scaffold( modifier = modifier, @@ -177,9 +177,6 @@ private fun NewChatTopAppBar( TopAppBar( colors = TopAppBarDefaults.topAppBarColors( containerColor = containerColor, - navigationIconContentColor = MaterialTheme.colorScheme.onSurface, - titleContentColor = MaterialTheme.colorScheme.onSurface, - actionIconContentColor = MaterialTheme.colorScheme.onSurfaceVariant, ), navigationIcon = { IconButton( diff --git a/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt b/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt index 7d8002250..4b20d6dda 100644 --- a/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt +++ b/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt @@ -104,7 +104,7 @@ private fun RecipientSelectionContentLayout( ) { Surface( modifier = modifier, - color = MaterialTheme.colorScheme.surfaceVariant, + color = MaterialTheme.colorScheme.surfaceContainer, ) { Column( modifier = Modifier From 92048f0d4622c9427af34681560ef0428860d8ba Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sat, 12 Sep 2026 23:01:48 +0300 Subject: [PATCH 06/21] Capitalize sentences in the message and caption fields --- .../MessageComposeBarInputTypeTest.kt | 82 +++++++++++++++++++ .../components/composer/MessageComposeBar.kt | 5 ++ .../review/ConversationMediaPickerReview.kt | 5 ++ 3 files changed, 92 insertions(+) create mode 100644 app/src/test/kotlin/com/android/messaging/ui/common/components/composer/MessageComposeBarInputTypeTest.kt diff --git a/app/src/test/kotlin/com/android/messaging/ui/common/components/composer/MessageComposeBarInputTypeTest.kt b/app/src/test/kotlin/com/android/messaging/ui/common/components/composer/MessageComposeBarInputTypeTest.kt new file mode 100644 index 000000000..baca24fc8 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/common/components/composer/MessageComposeBarInputTypeTest.kt @@ -0,0 +1,82 @@ +package com.android.messaging.ui.common.components.composer + +import android.text.InputType +import android.view.inputmethod.EditorInfo +import androidx.compose.runtime.Composable +import androidx.compose.ui.platform.InterceptPlatformTextInput +import androidx.compose.ui.test.junit4.v2.createComposeRule +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.performClick +import com.android.messaging.testutil.TEST_WAIT_TIMEOUT_MILLIS +import com.android.messaging.ui.core.AppTheme +import kotlinx.coroutines.awaitCancellation +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +internal class MessageComposeBarInputTypeTest { + + @get:Rule + val composeTestRule = createComposeRule() + + @Test + fun messageComposeField_reportsSentenceCapitalizationToTheKeyboard() { + val inputType = focusedFieldInputType() + + assertTrue(inputType and InputType.TYPE_TEXT_FLAG_CAP_SENTENCES != 0) + } + + private fun focusedFieldInputType(): Int { + var editorInfo: EditorInfo? = null + + composeTestRule.setContent { + RecordingEditorInfo(onEditorInfo = { editorInfo = it }) { + AppTheme { + MessageComposeBar( + text = "", + onTextChange = {}, + isFieldEnabled = true, + isFieldContentHidden = false, + fieldFocusRequester = null, + fieldStateDescription = null, + fieldTestTag = MESSAGE_COMPOSE_FIELD_TEST_TAG, + sendAction = {}, + ) + } + } + } + + composeTestRule + .onNodeWithTag(testTag = MESSAGE_COMPOSE_FIELD_TEST_TAG) + .performClick() + + composeTestRule.waitUntil(timeoutMillis = TEST_WAIT_TIMEOUT_MILLIS) { + editorInfo != null + } + + return requireNotNull(editorInfo).inputType + } +} + +/** + * Captures the [EditorInfo] the focused text field below would hand to the keyboard, instead of + * letting the request reach the real input method. + */ +@Composable +private fun RecordingEditorInfo( + onEditorInfo: (EditorInfo) -> Unit, + content: @Composable () -> Unit, +) { + InterceptPlatformTextInput( + interceptor = { request, _ -> + val editorInfo = EditorInfo() + request.createInputConnection(outAttributes = editorInfo) + onEditorInfo(editorInfo) + awaitCancellation() + }, + content = content, + ) +} diff --git a/src/com/android/messaging/ui/common/components/composer/MessageComposeBar.kt b/src/com/android/messaging/ui/common/components/composer/MessageComposeBar.kt index 80918577f..f56f923f8 100644 --- a/src/com/android/messaging/ui/common/components/composer/MessageComposeBar.kt +++ b/src/com/android/messaging/ui/common/components/composer/MessageComposeBar.kt @@ -13,6 +13,7 @@ import androidx.compose.foundation.layout.heightIn import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.foundation.shape.CircleShape +import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.rounded.Send import androidx.compose.material3.Icon @@ -35,6 +36,7 @@ import androidx.compose.ui.semantics.Role import androidx.compose.ui.semantics.clearAndSetSemantics import androidx.compose.ui.semantics.semantics import androidx.compose.ui.semantics.stateDescription +import androidx.compose.ui.text.input.KeyboardCapitalization import androidx.compose.ui.tooling.preview.PreviewLightDark import androidx.compose.ui.unit.dp import com.android.messaging.R @@ -161,6 +163,9 @@ private fun MessageComposeField( trailingIcon = trailingContent, minLines = 1, maxLines = 4, + keyboardOptions = KeyboardOptions( + capitalization = KeyboardCapitalization.Sentences, + ), ) overlay?.invoke(this) diff --git a/src/com/android/messaging/ui/conversation/mediapicker/component/review/ConversationMediaPickerReview.kt b/src/com/android/messaging/ui/conversation/mediapicker/component/review/ConversationMediaPickerReview.kt index 1ca9f78f5..89cac6b04 100644 --- a/src/com/android/messaging/ui/conversation/mediapicker/component/review/ConversationMediaPickerReview.kt +++ b/src/com/android/messaging/ui/conversation/mediapicker/component/review/ConversationMediaPickerReview.kt @@ -16,6 +16,7 @@ import androidx.compose.foundation.layout.statusBarsPadding import androidx.compose.foundation.pager.HorizontalPager import androidx.compose.foundation.pager.PageSize import androidx.compose.foundation.pager.PagerState +import androidx.compose.foundation.text.KeyboardOptions import androidx.compose.material.icons.Icons import androidx.compose.material.icons.rounded.AddAPhoto import androidx.compose.material.icons.rounded.Close @@ -37,6 +38,7 @@ import androidx.compose.ui.graphics.Color import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.TextRange +import androidx.compose.ui.text.input.KeyboardCapitalization import androidx.compose.ui.text.input.TextFieldValue import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.PreviewLightDark @@ -450,6 +452,9 @@ private fun ReviewCaptionTextField( ) }, singleLine = true, + keyboardOptions = KeyboardOptions( + capitalization = KeyboardCapitalization.Sentences, + ), ) } From 9d637dd6d5450032829f49163b50707a2352c264 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sat, 12 Sep 2026 23:01:48 +0300 Subject: [PATCH 07/21] Restore the short message keyboard type --- .../composer/MessageComposeBarInputTypeTest.kt | 18 ++++++++++++++++++ .../components/composer/MessageComposeBar.kt | 2 ++ .../review/ConversationMediaPickerReview.kt | 6 +++--- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/app/src/test/kotlin/com/android/messaging/ui/common/components/composer/MessageComposeBarInputTypeTest.kt b/app/src/test/kotlin/com/android/messaging/ui/common/components/composer/MessageComposeBarInputTypeTest.kt index baca24fc8..d41cbef1e 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/common/components/composer/MessageComposeBarInputTypeTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/common/components/composer/MessageComposeBarInputTypeTest.kt @@ -10,6 +10,7 @@ import androidx.compose.ui.test.performClick import com.android.messaging.testutil.TEST_WAIT_TIMEOUT_MILLIS import com.android.messaging.ui.core.AppTheme import kotlinx.coroutines.awaitCancellation +import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Rule import org.junit.Test @@ -29,6 +30,23 @@ internal class MessageComposeBarInputTypeTest { assertTrue(inputType and InputType.TYPE_TEXT_FLAG_CAP_SENTENCES != 0) } + @Test + fun messageComposeField_reportsShortMessageVariationToTheKeyboard() { + val inputType = focusedFieldInputType() + + assertEquals( + InputType.TYPE_TEXT_VARIATION_SHORT_MESSAGE, + inputType and InputType.TYPE_MASK_VARIATION, + ) + } + + @Test + fun messageComposeField_staysMultiLine() { + val inputType = focusedFieldInputType() + + assertTrue(inputType and InputType.TYPE_TEXT_FLAG_MULTI_LINE != 0) + } + private fun focusedFieldInputType(): Int { var editorInfo: EditorInfo? = null diff --git a/src/com/android/messaging/ui/common/components/composer/MessageComposeBar.kt b/src/com/android/messaging/ui/common/components/composer/MessageComposeBar.kt index f56f923f8..cf76d5240 100644 --- a/src/com/android/messaging/ui/common/components/composer/MessageComposeBar.kt +++ b/src/com/android/messaging/ui/common/components/composer/MessageComposeBar.kt @@ -37,6 +37,7 @@ import androidx.compose.ui.semantics.clearAndSetSemantics import androidx.compose.ui.semantics.semantics import androidx.compose.ui.semantics.stateDescription import androidx.compose.ui.text.input.KeyboardCapitalization +import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.tooling.preview.PreviewLightDark import androidx.compose.ui.unit.dp import com.android.messaging.R @@ -165,6 +166,7 @@ private fun MessageComposeField( maxLines = 4, keyboardOptions = KeyboardOptions( capitalization = KeyboardCapitalization.Sentences, + keyboardType = KeyboardType.ShortMessage, ), ) diff --git a/src/com/android/messaging/ui/conversation/mediapicker/component/review/ConversationMediaPickerReview.kt b/src/com/android/messaging/ui/conversation/mediapicker/component/review/ConversationMediaPickerReview.kt index 89cac6b04..d625f716d 100644 --- a/src/com/android/messaging/ui/conversation/mediapicker/component/review/ConversationMediaPickerReview.kt +++ b/src/com/android/messaging/ui/conversation/mediapicker/component/review/ConversationMediaPickerReview.kt @@ -39,6 +39,7 @@ import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.TextRange import androidx.compose.ui.text.input.KeyboardCapitalization +import androidx.compose.ui.text.input.KeyboardType import androidx.compose.ui.text.input.TextFieldValue import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.PreviewLightDark @@ -401,9 +402,7 @@ private fun ReviewCaptionTextField( mutableStateOf(value = false) } var fieldValue by remember(attachmentContentUri) { - mutableStateOf( - value = captionText.toCaptionTextFieldValue(), - ) + mutableStateOf(value = captionText.toCaptionTextFieldValue()) } LaunchedEffect(attachmentContentUri, captionText) { @@ -454,6 +453,7 @@ private fun ReviewCaptionTextField( singleLine = true, keyboardOptions = KeyboardOptions( capitalization = KeyboardCapitalization.Sentences, + keyboardType = KeyboardType.ShortMessage, ), ) } From b2e35efc86ec90ec4ac4e85a2d81af60e098b302 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sun, 13 Sep 2026 14:16:37 +0300 Subject: [PATCH 08/21] Resolve the settings SIM the same way the composer does --- .../SubscriptionSelectionResolverTest.kt | 123 ++++++++++++++ .../ConversationSettingsDelegateImplTest.kt | 159 ++++++++++++++++++ ...nversationSettingsUiStateMapperImplTest.kt | 72 +++++++- .../delegate/ConversationSettingsDelegate.kt | 8 +- .../ConversationSettingsUiStateMapper.kt | 20 ++- 5 files changed, 367 insertions(+), 15 deletions(-) create mode 100644 app/src/test/kotlin/com/android/messaging/data/subscription/SubscriptionSelectionResolverTest.kt create mode 100644 app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/delegate/ConversationSettingsDelegateImplTest.kt diff --git a/app/src/test/kotlin/com/android/messaging/data/subscription/SubscriptionSelectionResolverTest.kt b/app/src/test/kotlin/com/android/messaging/data/subscription/SubscriptionSelectionResolverTest.kt new file mode 100644 index 000000000..f332035f2 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/data/subscription/SubscriptionSelectionResolverTest.kt @@ -0,0 +1,123 @@ +package com.android.messaging.data.subscription + +import com.android.messaging.data.conversation.model.ParticipantId +import com.android.messaging.data.conversation.model.metadata.ConversationSubscriptionLabel +import com.android.messaging.data.subscription.model.SubId +import com.android.messaging.data.subscription.model.Subscription +import com.android.messaging.datamodel.data.ParticipantData +import kotlinx.collections.immutable.persistentListOf +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +internal class SubscriptionSelectionResolverTest { + + private val firstSubscription = createSubscription( + selfParticipantId = FIRST_SELF_PARTICIPANT_ID, + subId = FIRST_SUB_ID, + slotId = 1, + ) + private val secondSubscription = createSubscription( + selfParticipantId = SECOND_SELF_PARTICIPANT_ID, + subId = SECOND_SUB_ID, + slotId = 2, + ) + private val subscriptions = persistentListOf(firstSubscription, secondSubscription) + + @Test + fun resolve_selfParticipantIdMatches_selectsThatSubscription() { + val selectedSubscription = resolveSelectedSubscription( + subscriptions = subscriptions, + selectedSelfParticipantId = ParticipantId(SECOND_SELF_PARTICIPANT_ID), + defaultSmsSubscriptionId = SubId(FIRST_SUB_ID), + ) + + assertEquals(secondSubscription, selectedSubscription) + } + + @Test + fun resolve_selfParticipantIdMatchesNothing_selectsDefaultSmsSubscription() { + val selectedSubscription = resolveSelectedSubscription( + subscriptions = subscriptions, + selectedSelfParticipantId = ParticipantId(UNKNOWN_SELF_PARTICIPANT_ID), + defaultSmsSubscriptionId = SubId(SECOND_SUB_ID), + ) + + assertEquals(secondSubscription, selectedSubscription) + } + + @Test + fun resolve_selfParticipantIdIsNull_selectsDefaultSmsSubscription() { + val selectedSubscription = resolveSelectedSubscription( + subscriptions = subscriptions, + selectedSelfParticipantId = null, + defaultSmsSubscriptionId = SubId(SECOND_SUB_ID), + ) + + assertEquals(secondSubscription, selectedSubscription) + } + + @Test + fun resolve_defaultSmsSubscriptionIsNotActive_selectsFirstSubscription() { + val selectedSubscription = resolveSelectedSubscription( + subscriptions = subscriptions, + selectedSelfParticipantId = null, + defaultSmsSubscriptionId = SubId(REMOVED_SUB_ID), + ) + + assertEquals(firstSubscription, selectedSubscription) + } + + @Test + fun resolve_defaultSmsSubscriptionIdIsUnset_ignoresEmulatedSubscription() { + val emulatedSubscription = createSubscription( + selfParticipantId = EMULATED_SELF_PARTICIPANT_ID, + subId = ParticipantData.DEFAULT_SELF_SUB_ID, + slotId = 2, + ) + + val selectedSubscription = resolveSelectedSubscription( + subscriptions = persistentListOf(firstSubscription, emulatedSubscription), + selectedSelfParticipantId = null, + defaultSmsSubscriptionId = SubId(ParticipantData.DEFAULT_SELF_SUB_ID), + ) + + assertEquals(firstSubscription, selectedSubscription) + } + + @Test + fun resolve_noSubscriptions_returnsNull() { + val selectedSubscription = resolveSelectedSubscription( + subscriptions = persistentListOf(), + selectedSelfParticipantId = ParticipantId(FIRST_SELF_PARTICIPANT_ID), + defaultSmsSubscriptionId = SubId(FIRST_SUB_ID), + ) + + assertNull(selectedSubscription) + } + + private fun createSubscription( + selfParticipantId: String, + subId: Int, + slotId: Int, + ): Subscription { + return Subscription( + selfParticipantId = ParticipantId(selfParticipantId), + subId = SubId(subId), + label = ConversationSubscriptionLabel.Slot(slotId = slotId), + displayDestination = null, + displaySlotId = slotId, + color = 0, + ) + } + + private companion object { + private const val FIRST_SELF_PARTICIPANT_ID = "self-participant-1" + private const val SECOND_SELF_PARTICIPANT_ID = "self-participant-2" + private const val UNKNOWN_SELF_PARTICIPANT_ID = "self-participant-default" + private const val EMULATED_SELF_PARTICIPANT_ID = "debug_sim_emulated_2" + private const val FIRST_SUB_ID = 11 + private const val SECOND_SUB_ID = 12 + private const val REMOVED_SUB_ID = 13 + } +} diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/delegate/ConversationSettingsDelegateImplTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/delegate/ConversationSettingsDelegateImplTest.kt new file mode 100644 index 000000000..92dea5d72 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/delegate/ConversationSettingsDelegateImplTest.kt @@ -0,0 +1,159 @@ +package com.android.messaging.ui.conversationsettings.screen.delegate + +import androidx.lifecycle.SavedStateHandle +import com.android.messaging.data.blockedparticipants.repository.BlockedParticipantsRepository +import com.android.messaging.data.conversation.model.ConversationId +import com.android.messaging.data.conversation.model.ParticipantId +import com.android.messaging.data.conversation.model.metadata.ConversationSubscriptionLabel +import com.android.messaging.data.conversation.repository.ConversationsRepository +import com.android.messaging.data.conversationsettings.model.ConversationSettingsData +import com.android.messaging.data.conversationsettings.repository.ConversationNotificationRepository +import com.android.messaging.data.conversationsettings.repository.ConversationSettingsRepository +import com.android.messaging.data.subscription.model.SubId +import com.android.messaging.data.subscription.model.Subscription +import com.android.messaging.data.subscription.repository.SubscriptionsRepository +import com.android.messaging.domain.conversationsettings.usecase.SetConversationSelfParticipantId +import com.android.messaging.ui.conversationsettings.screen.CONVERSATION_SETTINGS_CONVERSATION_ID_ARG +import com.android.messaging.ui.conversationsettings.screen.mapper.ConversationSettingsUiStateMapperImpl +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import kotlinx.collections.immutable.persistentListOf +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.assertEquals +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +internal class ConversationSettingsDelegateImplTest { + + private val settingsRepository = mockk() + private val subscriptionsRepository = mockk() + private val setConversationSelfParticipantId = + mockk(relaxed = true) + + @Test + fun bind_conversationBoundToDefaultSelf_showsDefaultSmsSubscription() { + runTest { + stubConversationSelfParticipantId(ParticipantId(DEFAULT_SELF_PARTICIPANT_ID)) + val delegate = createDelegate(applicationScope = backgroundScope) + + delegate.bind(backgroundScope) + runCurrent() + + assertEquals(SECOND_SUBSCRIPTION, delegate.state.value.selectedSubscription) + } + } + + @Test + fun bind_conversationBoundToSubscription_showsThatSubscription() { + runTest { + stubConversationSelfParticipantId(ParticipantId(FIRST_SELF_PARTICIPANT_ID)) + val delegate = createDelegate(applicationScope = backgroundScope) + + delegate.bind(backgroundScope) + runCurrent() + + assertEquals(FIRST_SUBSCRIPTION, delegate.state.value.selectedSubscription) + } + } + + @Test + fun setSelfParticipantId_tappingTheImplicitlySelectedSubscription_pinsIt() { + runTest { + stubConversationSelfParticipantId(ParticipantId(DEFAULT_SELF_PARTICIPANT_ID)) + val delegate = createDelegate(applicationScope = backgroundScope) + + delegate.bind(backgroundScope) + runCurrent() + delegate.setSelfParticipantId(ParticipantId(SECOND_SELF_PARTICIPANT_ID)) + runCurrent() + + coVerify(exactly = 1) { + setConversationSelfParticipantId( + conversationId = CONVERSATION_ID, + selfParticipantId = ParticipantId(SECOND_SELF_PARTICIPANT_ID), + ) + } + } + } + + @Test + fun setSelfParticipantId_tappingTheAlreadySelectedSubscription_isIgnored() { + runTest { + stubConversationSelfParticipantId(ParticipantId(FIRST_SELF_PARTICIPANT_ID)) + val delegate = createDelegate(applicationScope = backgroundScope) + + delegate.bind(backgroundScope) + runCurrent() + delegate.setSelfParticipantId(ParticipantId(FIRST_SELF_PARTICIPANT_ID)) + runCurrent() + + coVerify(exactly = 0) { setConversationSelfParticipantId(any(), any()) } + } + } + + private fun stubConversationSelfParticipantId(selfParticipantId: ParticipantId) { + every { settingsRepository.getConversationSettings(CONVERSATION_ID) } returns flowOf( + ConversationSettingsData( + conversationId = CONVERSATION_ID, + dbSelfParticipantId = selfParticipantId, + ), + ) + every { subscriptionsRepository.observeActiveSubscriptions() } returns flowOf( + persistentListOf(FIRST_SUBSCRIPTION, SECOND_SUBSCRIPTION), + ) + every { subscriptionsRepository.observeDefaultSmsSubscriptionId() } returns flowOf( + SubId(SECOND_SUB_ID), + ) + } + + private fun createDelegate(applicationScope: CoroutineScope): ConversationSettingsDelegateImpl { + return ConversationSettingsDelegateImpl( + repository = settingsRepository, + notificationRepository = mockk(relaxed = true), + subscriptionsRepository = subscriptionsRepository, + mapper = ConversationSettingsUiStateMapperImpl( + canPlacePhoneCall = { false }, + canShowOrAddContact = { _, _, _, _ -> false }, + isContactSavedUseCase = { _, _ -> false }, + ), + conversationsRepository = mockk(relaxed = true), + blockedParticipantsRepository = mockk(relaxed = true), + setConversationSelfParticipantId = setConversationSelfParticipantId, + applicationScope = applicationScope, + savedStateHandle = SavedStateHandle( + mapOf(CONVERSATION_SETTINGS_CONVERSATION_ID_ARG to CONVERSATION_ID.value), + ), + ) + } + + private companion object { + private val CONVERSATION_ID = ConversationId("conversation-1") + private const val DEFAULT_SELF_PARTICIPANT_ID = "self-participant-default" + private const val FIRST_SELF_PARTICIPANT_ID = "self-participant-1" + private const val SECOND_SELF_PARTICIPANT_ID = "self-participant-2" + private const val FIRST_SUB_ID = 1 + private const val SECOND_SUB_ID = 2 + + private val FIRST_SUBSCRIPTION = Subscription( + selfParticipantId = ParticipantId(FIRST_SELF_PARTICIPANT_ID), + subId = SubId(FIRST_SUB_ID), + label = ConversationSubscriptionLabel.Slot(slotId = 1), + displayDestination = null, + displaySlotId = 1, + color = 0, + ) + private val SECOND_SUBSCRIPTION = Subscription( + selfParticipantId = ParticipantId(SECOND_SELF_PARTICIPANT_ID), + subId = SubId(SECOND_SUB_ID), + label = ConversationSubscriptionLabel.Slot(slotId = 2), + displayDestination = null, + displaySlotId = 2, + color = 0, + ) + } +} diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/mapper/ConversationSettingsUiStateMapperImplTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/mapper/ConversationSettingsUiStateMapperImplTest.kt index ab4bf3da3..84e8d514a 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/mapper/ConversationSettingsUiStateMapperImplTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/mapper/ConversationSettingsUiStateMapperImplTest.kt @@ -1,7 +1,11 @@ package com.android.messaging.ui.conversationsettings.screen.mapper import com.android.messaging.data.conversation.model.ConversationId +import com.android.messaging.data.conversation.model.ParticipantId +import com.android.messaging.data.conversation.model.metadata.ConversationSubscriptionLabel import com.android.messaging.data.conversationsettings.model.ConversationSettingsData +import com.android.messaging.data.subscription.model.SubId +import com.android.messaging.data.subscription.model.Subscription import com.android.messaging.datamodel.data.ParticipantData import com.android.messaging.ui.conversationsettings.screen.model.ParticipantUiState import io.mockk.every @@ -41,6 +45,48 @@ internal class ConversationSettingsUiStateMapperImplTest { assertEquals(DISPLAY_DESTINATION, participantUiState.details) } + @Test + fun map_conversationBoundToDefaultSelf_selectsDefaultSmsSubscription() { + val uiState = mapper.map( + data = ConversationSettingsData( + conversationId = CONVERSATION_ID, + dbSelfParticipantId = ParticipantId(DEFAULT_SELF_PARTICIPANT_ID), + ), + subscriptions = persistentListOf(FIRST_SUBSCRIPTION, SECOND_SUBSCRIPTION), + defaultSmsSubscriptionId = SubId(SECOND_SUB_ID), + ) + + assertEquals(SECOND_SUBSCRIPTION, uiState.selectedSubscription) + } + + @Test + fun map_conversationBoundToSubscription_selectsThatSubscription() { + val uiState = mapper.map( + data = ConversationSettingsData( + conversationId = CONVERSATION_ID, + dbSelfParticipantId = ParticipantId(FIRST_SELF_PARTICIPANT_ID), + ), + subscriptions = persistentListOf(FIRST_SUBSCRIPTION, SECOND_SUBSCRIPTION), + defaultSmsSubscriptionId = SubId(SECOND_SUB_ID), + ) + + assertEquals(FIRST_SUBSCRIPTION, uiState.selectedSubscription) + } + + @Test + fun map_conversationBoundToDefaultSelf_keepsUnresolvedSelfParticipantId() { + val uiState = mapper.map( + data = ConversationSettingsData( + conversationId = CONVERSATION_ID, + dbSelfParticipantId = ParticipantId(DEFAULT_SELF_PARTICIPANT_ID), + ), + subscriptions = persistentListOf(FIRST_SUBSCRIPTION, SECOND_SUBSCRIPTION), + defaultSmsSubscriptionId = SubId(SECOND_SUB_ID), + ) + + assertEquals(ParticipantId(DEFAULT_SELF_PARTICIPANT_ID), uiState.selfParticipantId) + } + private fun mapParticipant( name: String?, unknownSender: Boolean, @@ -54,10 +100,12 @@ internal class ConversationSettingsUiStateMapperImplTest { return mapper .map( - ConversationSettingsData( + data = ConversationSettingsData( conversationId = CONVERSATION_ID, participants = persistentListOf(participant), ), + subscriptions = persistentListOf(), + defaultSmsSubscriptionId = SubId(ParticipantData.DEFAULT_SELF_SUB_ID), ) .participants .single() @@ -68,5 +116,27 @@ internal class ConversationSettingsUiStateMapperImplTest { private const val SEND_DESTINATION = "+15550123" private const val DISPLAY_DESTINATION = "+1 555-0123" private const val FULL_NAME = "Ada Lovelace" + private const val DEFAULT_SELF_PARTICIPANT_ID = "self-participant-default" + private const val FIRST_SELF_PARTICIPANT_ID = "self-participant-1" + private const val SECOND_SELF_PARTICIPANT_ID = "self-participant-2" + private const val FIRST_SUB_ID = 1 + private const val SECOND_SUB_ID = 2 + + private val FIRST_SUBSCRIPTION = Subscription( + selfParticipantId = ParticipantId(FIRST_SELF_PARTICIPANT_ID), + subId = SubId(FIRST_SUB_ID), + label = ConversationSubscriptionLabel.Slot(slotId = 1), + displayDestination = null, + displaySlotId = 1, + color = 0, + ) + private val SECOND_SUBSCRIPTION = Subscription( + selfParticipantId = ParticipantId(SECOND_SELF_PARTICIPANT_ID), + subId = SubId(SECOND_SUB_ID), + label = ConversationSubscriptionLabel.Slot(slotId = 2), + displayDestination = null, + displaySlotId = 2, + color = 0, + ) } } diff --git a/src/com/android/messaging/ui/conversationsettings/screen/delegate/ConversationSettingsDelegate.kt b/src/com/android/messaging/ui/conversationsettings/screen/delegate/ConversationSettingsDelegate.kt index f995d76fd..c6bcc1d7d 100644 --- a/src/com/android/messaging/ui/conversationsettings/screen/delegate/ConversationSettingsDelegate.kt +++ b/src/com/android/messaging/ui/conversationsettings/screen/delegate/ConversationSettingsDelegate.kt @@ -8,7 +8,6 @@ import com.android.messaging.data.conversation.repository.ConversationsRepositor import com.android.messaging.data.conversationsettings.model.SnoozeOption import com.android.messaging.data.conversationsettings.repository.ConversationNotificationRepository import com.android.messaging.data.conversationsettings.repository.ConversationSettingsRepository -import com.android.messaging.data.subscription.repository.ConversationSimSelectionRepository import com.android.messaging.data.subscription.repository.SubscriptionsRepository import com.android.messaging.datamodel.ParticipantRefresh import com.android.messaging.di.core.ApplicationCoroutineScope @@ -45,7 +44,6 @@ internal class ConversationSettingsDelegateImpl @Inject constructor( private val repository: ConversationSettingsRepository, private val notificationRepository: ConversationNotificationRepository, private val subscriptionsRepository: SubscriptionsRepository, - private val simSelectionRepository: ConversationSimSelectionRepository, private val mapper: ConversationSettingsUiStateMapper, private val conversationsRepository: ConversationsRepository, private val blockedParticipantsRepository: BlockedParticipantsRepository, @@ -91,12 +89,12 @@ internal class ConversationSettingsDelegateImpl @Inject constructor( return combine( settings, subscriptionsRepository.observeActiveSubscriptions(), - simSelectionRepository.observe(id), - ) { data, subscriptions, selfIdOverride -> + subscriptionsRepository.observeDefaultSmsSubscriptionId(), + ) { data, subscriptions, defaultSmsSubscriptionId -> mapper.map( data = data, subscriptions = subscriptions, - selfIdOverride = selfIdOverride, + defaultSmsSubscriptionId = defaultSmsSubscriptionId, ) } } diff --git a/src/com/android/messaging/ui/conversationsettings/screen/mapper/ConversationSettingsUiStateMapper.kt b/src/com/android/messaging/ui/conversationsettings/screen/mapper/ConversationSettingsUiStateMapper.kt index ba103a69c..545a1c4e7 100644 --- a/src/com/android/messaging/ui/conversationsettings/screen/mapper/ConversationSettingsUiStateMapper.kt +++ b/src/com/android/messaging/ui/conversationsettings/screen/mapper/ConversationSettingsUiStateMapper.kt @@ -2,7 +2,9 @@ package com.android.messaging.ui.conversationsettings.screen.mapper import com.android.messaging.data.conversation.model.ParticipantId import com.android.messaging.data.conversationsettings.model.ConversationSettingsData +import com.android.messaging.data.subscription.model.SubId import com.android.messaging.data.subscription.model.Subscription +import com.android.messaging.data.subscription.resolveSelectedSubscription import com.android.messaging.datamodel.data.ParticipantData import com.android.messaging.domain.conversation.usecase.participant.CanShowOrAddContact import com.android.messaging.domain.conversation.usecase.participant.IsContactSaved @@ -11,14 +13,13 @@ import com.android.messaging.ui.conversationsettings.screen.model.ConversationSe import com.android.messaging.ui.conversationsettings.screen.model.ParticipantUiState import javax.inject.Inject import kotlinx.collections.immutable.ImmutableList -import kotlinx.collections.immutable.persistentListOf import kotlinx.collections.immutable.toImmutableList internal interface ConversationSettingsUiStateMapper { fun map( data: ConversationSettingsData, - subscriptions: ImmutableList = persistentListOf(), - selfIdOverride: ParticipantId? = null, + subscriptions: ImmutableList, + defaultSmsSubscriptionId: SubId, ): ConversationSettingsUiState } @@ -31,16 +32,17 @@ internal class ConversationSettingsUiStateMapperImpl @Inject constructor( override fun map( data: ConversationSettingsData, subscriptions: ImmutableList, - selfIdOverride: ParticipantId?, + defaultSmsSubscriptionId: SubId, ): ConversationSettingsUiState { val participants = data.participants .map(::toParticipantUiState) .toImmutableList() val otherParticipant = participants.singleOrNull() - val effectiveSelfId = selfIdOverride ?: data.dbSelfParticipantId - val selectedSubscription = subscriptions - .firstOrNull { it.selfParticipantId == effectiveSelfId } - ?: subscriptions.firstOrNull() + val selectedSubscription = resolveSelectedSubscription( + subscriptions = subscriptions, + selectedSelfParticipantId = data.dbSelfParticipantId, + defaultSmsSubscriptionId = defaultSmsSubscriptionId, + ) val canShowContact = otherParticipant?.let { participant -> canShowOrAddContact( @@ -58,7 +60,7 @@ internal class ConversationSettingsUiStateMapperImpl @Inject constructor( isSnoozed = data.isSnoozed, participants = participants, otherParticipant = otherParticipant, - selfParticipantId = effectiveSelfId, + selfParticipantId = data.dbSelfParticipantId, availableSubscriptions = subscriptions, selectedSubscription = selectedSubscription, isSimSwitchAvailable = subscriptions.size > 1, From c2f1199e1a36b891d014916cb1f024be3a05b0a2 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sun, 13 Sep 2026 14:54:57 +0300 Subject: [PATCH 09/21] Pin the new group button above the contact list --- .../conversation/entry/NewChatScreenTest.kt | 84 +++++++++++++++---- .../ui/conversation/entry/NewChatScreen.kt | 12 +-- .../ui/conversation/entry/NewGroupButton.kt | 6 +- .../component/RecipientSelectionContent.kt | 15 ++-- 4 files changed, 86 insertions(+), 31 deletions(-) diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt index a4fc9ffb9..8e5685eb3 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/entry/NewChatScreenTest.kt @@ -24,10 +24,12 @@ import com.android.messaging.ui.recipientselection.component.row.CONTACT_ID import com.android.messaging.ui.recipientselection.component.row.MOBILE_NORMALIZED_DESTINATION import com.android.messaging.ui.recipientselection.component.row.contactItem import com.android.messaging.ui.recipientselection.component.row.selectedRecipient +import com.android.messaging.ui.recipientselection.model.picker.RecipientPickerListItem import com.android.messaging.ui.recipientselection.model.picker.RecipientPickerUiState import io.mockk.every import io.mockk.mockk import io.mockk.verify +import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.flow.MutableStateFlow @@ -74,20 +76,7 @@ class NewChatScreenTest { val screenModel = createScreenModel( initialUiState = NewChatUiState( recipientPickerUiState = RecipientPickerUiState( - items = persistentListOf( - *Array(size = 30) { index -> - contactItem( - id = index.toLong(), - displayName = "Contact $index", - destination = "+1 555 ${ - index.toString().padStart(length = 4, padChar = '0') - }", - normalizedDestination = "+1555${ - index.toString().padStart(length = 4, padChar = '0') - }", - ) - }, - ), + items = contactItems(count = 30), canLoadMore = true, ), ), @@ -220,6 +209,50 @@ class NewChatScreenTest { .assertCountEquals(expectedSize = 1) } + @Test + fun createGroupButton_staysVisibleWhileTheContactListScrolls() { + val screenModel = createScreenModel( + initialUiState = NewChatUiState( + recipientPickerUiState = RecipientPickerUiState( + items = contactItems(count = 30), + ), + ), + ) + + setContent(screenModel = screenModel) + + composeTestRule + .onNode(matcher = hasScrollToIndexAction()) + .performScrollToIndex(index = 29) + composeTestRule.waitForIdle() + + composeTestRule + .onNodeWithTag(testTag = NEW_CHAT_CREATE_GROUP_BUTTON_TEST_TAG) + .assertIsDisplayed() + } + + @Test + fun leavingCreateGroupMode_showsTheCreateGroupButtonAgain() { + val uiState = MutableStateFlow( + NewChatUiState( + recipientPickerUiState = RecipientPickerUiState( + items = contactItems(count = 30), + ), + ), + ) + + setContent(screenModel = createScreenModel(uiStateFlow = uiState)) + + uiState.value = uiState.value.copy(isCreatingGroup = true) + composeTestRule.waitForIdle() + uiState.value = uiState.value.copy(isCreatingGroup = false) + composeTestRule.waitForIdle() + + composeTestRule + .onNodeWithTag(testTag = NEW_CHAT_CREATE_GROUP_BUTTON_TEST_TAG) + .assertIsDisplayed() + } + private fun setContent(screenModel: NewChatScreenModel) { composeTestRule.setContent { AppTheme { @@ -233,11 +266,32 @@ class NewChatScreenTest { } } + private fun contactItems(count: Int): ImmutableList { + return persistentListOf( + *Array(size = count) { index -> + val suffix = index.toString().padStart(length = 4, padChar = '0') + + contactItem( + id = index.toLong(), + displayName = "Contact $index", + destination = "+1 555 $suffix", + normalizedDestination = "+1555$suffix", + ) + }, + ) + } + private fun createScreenModel(initialUiState: NewChatUiState): NewChatScreenModel { + return createScreenModel(uiStateFlow = MutableStateFlow(value = initialUiState)) + } + + private fun createScreenModel( + uiStateFlow: MutableStateFlow, + ): NewChatScreenModel { return mockk(relaxed = true) { every { effects } returns MutableSharedFlow() every { navigationEvents } returns MutableSharedFlow() - every { uiState } returns MutableStateFlow(value = initialUiState) + every { uiState } returns uiStateFlow } } } diff --git a/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt b/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt index f8b647e85..6b1e8279c 100644 --- a/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt +++ b/src/com/android/messaging/ui/conversation/entry/NewChatScreen.kt @@ -255,18 +255,18 @@ private fun NewChatRecipientSelectionContent( onContactRecipientClick = onContactLongClick, ), onSelectedRecipientClick = onCreateGroupRecipientClick, + pinnedTopContent = { + NewChatCreateGroupHeader( + isCreatingGroup = isCreatingGroup, + onCreateGroupClick = onCreateGroupClick, + ) + }, simSelectorSlot = { NewChatSimSelectorRow( uiState = simSelectorUiState, onSimSelected = onSimSelected, ) }, - topListContent = { - NewChatRecipientSelectionTopListContent( - isCreatingGroup = isCreatingGroup, - onCreateGroupClick = onCreateGroupClick, - ) - }, ) } diff --git a/src/com/android/messaging/ui/conversation/entry/NewGroupButton.kt b/src/com/android/messaging/ui/conversation/entry/NewGroupButton.kt index 12feb4f93..83e7f26dd 100644 --- a/src/com/android/messaging/ui/conversation/entry/NewGroupButton.kt +++ b/src/com/android/messaging/ui/conversation/entry/NewGroupButton.kt @@ -40,7 +40,7 @@ import com.android.messaging.ui.conversation.NEW_CHAT_CREATE_GROUP_BUTTON_TEST_T import com.android.messaging.ui.core.MessagingPreviewColumn @Composable -internal fun NewChatRecipientSelectionTopListContent( +internal fun NewChatCreateGroupHeader( isCreatingGroup: Boolean, onCreateGroupClick: () -> Unit, ) { @@ -139,9 +139,9 @@ private fun newChatSpatialAnimationSpec(): FiniteAnimationSpec { @PreviewLightDark @Composable -private fun NewChatRecipientSelectionTopListContentPreview() { +private fun NewChatCreateGroupHeaderPreview() { MessagingPreviewColumn { - NewChatRecipientSelectionTopListContent( + NewChatCreateGroupHeader( isCreatingGroup = false, onCreateGroupClick = {}, ) diff --git a/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt b/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt index 4b20d6dda..316508f26 100644 --- a/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt +++ b/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionContent.kt @@ -52,8 +52,8 @@ internal fun RecipientSelectionContent( onQueryChanged: (String) -> Unit = {}, onRecipientDestinationLongClick: OnRecipientDestinationAction? = null, onSelectedRecipientClick: (SelectedRecipient) -> Unit = {}, + pinnedTopContent: (@Composable () -> Unit)? = null, simSelectorSlot: (@Composable () -> Unit)? = null, - topListContent: (@Composable () -> Unit)? = null, ) { val queryFocusRequester = remember { FocusRequester() } val armedDestination = rememberSaveable { mutableStateOf(null) } @@ -69,6 +69,7 @@ internal fun RecipientSelectionContent( RecipientSelectionContentLayout( modifier = modifier, + pinnedTopContent = pinnedTopContent, queryArea = { RecipientSelectionArmedQueryArea( uiState = uiState, @@ -86,7 +87,6 @@ internal fun RecipientSelectionContent( uiState = uiState, rowDecorators = rowDecorators, armedDestination = armedDestination, - topListContent = topListContent, onLoadMore = onLoadMore, onPrimaryActionClick = onPrimaryActionClick, onRecipientDestinationClick = onRecipientDestinationClick, @@ -101,6 +101,7 @@ private fun RecipientSelectionContentLayout( queryArea: @Composable () -> Unit, contactsArea: @Composable () -> Unit, modifier: Modifier = Modifier, + pinnedTopContent: (@Composable () -> Unit)? = null, ) { Surface( modifier = modifier, @@ -115,6 +116,8 @@ private fun RecipientSelectionContentLayout( queryArea() Spacer(modifier = Modifier.height(12.dp)) + pinnedTopContent?.invoke() + Box(modifier = Modifier.weight(weight = 1f)) { contactsArea() } @@ -194,7 +197,6 @@ private fun RecipientSelectionArmedContactsArea( onLoadMore: () -> Unit, onPrimaryActionClick: () -> Unit, modifier: Modifier = Modifier, - topListContent: (@Composable () -> Unit)? = null, ) { val currentOnPrimaryActionClick = rememberUpdatedState(onPrimaryActionClick) val currentOnRecipientDestinationClick = rememberUpdatedState(onRecipientDestinationClick) @@ -237,7 +239,6 @@ private fun RecipientSelectionArmedContactsArea( onRecipientDestinationLongClick = onRecipientDestinationLongClickWrapped .takeIf { onRecipientDestinationLongClick != null }, emptyStateText = R.string.contact_list_empty_text, - topListContent = topListContent, ) } @@ -290,7 +291,7 @@ private fun RecipientSelectionContentSimSelectorAndTopContentPreview() { onSimSelected = { _ -> }, ) }, - topListContent = { + pinnedTopContent = { PreviewRecipientSelectionContactsTopListContent() }, ) @@ -309,8 +310,8 @@ private fun PreviewRecipientSelectionContent( uiState: RecipientSelectionContentUiState, modifier: Modifier = Modifier.height(height = 560.dp), onRecipientDestinationLongClick: OnRecipientDestinationAction? = { _, _ -> }, + pinnedTopContent: (@Composable () -> Unit)? = null, simSelectorSlot: (@Composable () -> Unit)? = null, - topListContent: (@Composable () -> Unit)? = null, ) { MessagingPreviewTheme { RecipientSelectionContent( @@ -322,8 +323,8 @@ private fun PreviewRecipientSelectionContent( onRecipientDestinationLongClick = onRecipientDestinationLongClick, onSelectedRecipientClick = { _ -> }, onQueryChanged = { _ -> }, + pinnedTopContent = pinnedTopContent, simSelectorSlot = simSelectorSlot, - topListContent = topListContent, ) } } From 8b1f76287e1c028380c364af6f60b5e43a9c6bb7 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sun, 13 Sep 2026 17:13:32 +0300 Subject: [PATCH 10/21] Collapse the settings header only when the list can scroll far enough --- .../screen/HeaderCollapseProgressTest.kt | 93 +++++++++++++++++++ .../screen/ConversationSettingsScreen.kt | 46 +++++++-- 2 files changed, 131 insertions(+), 8 deletions(-) create mode 100644 app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/HeaderCollapseProgressTest.kt diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/HeaderCollapseProgressTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/HeaderCollapseProgressTest.kt new file mode 100644 index 000000000..c9065e60b --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/conversationsettings/screen/HeaderCollapseProgressTest.kt @@ -0,0 +1,93 @@ +package com.android.messaging.ui.conversationsettings.screen + +import org.junit.Assert.assertEquals +import org.junit.Test + +internal class HeaderCollapseProgressTest { + + @Test + fun headerCollapseProgress_listScrollsFurtherThanHeader_followsScrollOffset() { + val progress = headerCollapseProgress( + scrollOffset = COLLAPSE_DISTANCE / 4, + headerSize = HEADER_SIZE, + maxScrollDistance = HEADER_SIZE * 2, + ) + + assertEquals(0.25f, progress, 0f) + } + + @Test + fun headerCollapseProgress_listScrollsExactlyOneHeader_collapsesFully() { + val progress = headerCollapseProgress( + scrollOffset = HEADER_SIZE, + headerSize = HEADER_SIZE, + maxScrollDistance = HEADER_SIZE, + ) + + assertEquals(1f, progress, 0f) + } + + @Test + fun headerCollapseProgress_listScrollsJustShortOfHeader_collapsesFully() { + val scrollDistance = COLLAPSE_DISTANCE + 1 + + val progress = headerCollapseProgress( + scrollOffset = scrollDistance, + headerSize = HEADER_SIZE, + maxScrollDistance = scrollDistance, + ) + + assertEquals(1f, progress, 0f) + } + + @Test + fun headerCollapseProgress_listScrollsExactlyTheCollapseDistance_collapsesFully() { + val progress = headerCollapseProgress( + scrollOffset = COLLAPSE_DISTANCE, + headerSize = HEADER_SIZE, + maxScrollDistance = COLLAPSE_DISTANCE, + ) + + assertEquals(1f, progress, 0f) + } + + @Test + fun headerCollapseProgress_listScrollsLessThanHeaderCollapseDistance_staysExpanded() { + val progress = headerCollapseProgress( + scrollOffset = 12, + headerSize = HEADER_SIZE, + maxScrollDistance = COLLAPSE_DISTANCE - 1, + ) + + assertEquals(0f, progress, 0f) + } + + @Test + fun headerCollapseProgress_endOfListNotLaidOut_followsScrollOffset() { + val progress = headerCollapseProgress( + scrollOffset = COLLAPSE_DISTANCE / 2, + headerSize = HEADER_SIZE, + maxScrollDistance = null, + ) + + assertEquals(0.5f, progress, 0f) + } + + @Test + fun headerCollapseProgress_headerNotMeasured_staysExpanded() { + val progress = headerCollapseProgress( + scrollOffset = 12, + headerSize = 0, + maxScrollDistance = null, + ) + + assertEquals(0f, progress, 0f) + } + + private companion object { + const val HEADER_SIZE = 400 + + /** Scroll distance over which the cross-fade of a [HEADER_SIZE] header completes. */ + const val COLLAPSE_DISTANCE = 360 + } +} diff --git a/src/com/android/messaging/ui/conversationsettings/screen/ConversationSettingsScreen.kt b/src/com/android/messaging/ui/conversationsettings/screen/ConversationSettingsScreen.kt index 3a3801898..bde1adcb5 100644 --- a/src/com/android/messaging/ui/conversationsettings/screen/ConversationSettingsScreen.kt +++ b/src/com/android/messaging/ui/conversationsettings/screen/ConversationSettingsScreen.kt @@ -14,6 +14,7 @@ import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.lazy.LazyColumn +import androidx.compose.foundation.lazy.LazyListLayoutInfo import androidx.compose.foundation.lazy.LazyListScope import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.rememberLazyListState @@ -88,6 +89,8 @@ import com.android.messaging.ui.core.MessagingPreviewTheme import kotlinx.collections.immutable.ImmutableList import kotlinx.collections.immutable.persistentListOf +private const val HEADER_COLLAPSE_DISTANCE_FRACTION = 0.9f + @Composable internal fun ConversationSettingsScreen( screenModel: ConversationSettingsScreenModel, @@ -245,17 +248,44 @@ private fun rememberCollapseProgress( ): ComposeState { return remember { derivedStateOf { - if (listState.firstVisibleItemIndex > 0) { - return@derivedStateOf 1f + val layoutInfo = listState.layoutInfo + val scrollOffset = listState.firstVisibleItemScrollOffset + + when { + listState.firstVisibleItemIndex > 0 -> 1f + else -> headerCollapseProgress( + scrollOffset = scrollOffset, + headerSize = layoutInfo.visibleItemsInfo.firstOrNull()?.size ?: 0, + maxScrollDistance = layoutInfo.maxScrollDistance(scrollOffset = scrollOffset), + ) } + } + } +} - val headerInfo = listState.layoutInfo.visibleItemsInfo.firstOrNull() - if (headerInfo == null || headerInfo.size == 0) { - return@derivedStateOf 0f - } +internal fun headerCollapseProgress( + scrollOffset: Int, + headerSize: Int, + maxScrollDistance: Int?, +): Float { + val collapseDistance = headerSize * HEADER_COLLAPSE_DISTANCE_FRACTION + val collapsesFully = collapseDistance > 0f && + (maxScrollDistance == null || maxScrollDistance >= collapseDistance) + + return when { + collapsesFully -> (scrollOffset / collapseDistance).coerceIn(0f, 1f) + else -> 0f + } +} + +private fun LazyListLayoutInfo.maxScrollDistance(scrollOffset: Int): Int? { + val lastItem = visibleItemsInfo.lastOrNull() - val scrollOffset = listState.firstVisibleItemScrollOffset.toFloat() - (scrollOffset / headerInfo.size.toFloat()).coerceIn(0f, 1f) + return when { + lastItem == null || lastItem.index != totalItemsCount - 1 -> null + else -> { + val contentEnd = lastItem.offset + lastItem.size + afterContentPadding + scrollOffset + (contentEnd - viewportEndOffset).coerceAtLeast(0) } } } From 4e5386ffe2e17ccd0b47ecb7d12cb515713fd5b7 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Sun, 13 Sep 2026 17:13:32 +0300 Subject: [PATCH 11/21] Fix the settings top bar title never fading in --- .../common/ConversationSettingsComponents.kt | 30 ++++++++++++------- .../common/ConversationSettingsTopAppBar.kt | 28 +++++++++++------ 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/src/com/android/messaging/ui/conversationsettings/common/ConversationSettingsComponents.kt b/src/com/android/messaging/ui/conversationsettings/common/ConversationSettingsComponents.kt index b40d1a6e8..7aec91d91 100644 --- a/src/com/android/messaging/ui/conversationsettings/common/ConversationSettingsComponents.kt +++ b/src/com/android/messaging/ui/conversationsettings/common/ConversationSettingsComponents.kt @@ -28,6 +28,7 @@ import androidx.compose.runtime.remember import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.alpha import androidx.compose.ui.graphics.Color import androidx.compose.ui.graphics.Shape import androidx.compose.ui.graphics.graphicsLayer @@ -88,16 +89,9 @@ internal fun ConversationHeader( if (title.isNotEmpty()) { Spacer(modifier = Modifier.height(16.dp)) - Text( - text = title, - style = MaterialTheme.typography.headlineSmall, - color = MaterialTheme.colorScheme.onSurface, - textAlign = TextAlign.Center, - maxLines = 3, - overflow = TextOverflow.Ellipsis, - modifier = Modifier.graphicsLayer { - alpha = 1f - collapseProgress() - }, + ExpandedTitle( + title = title, + collapseProgress = collapseProgress, ) } } @@ -181,6 +175,22 @@ internal fun ParticipantItem( ) } +@Composable +private fun ExpandedTitle( + title: String, + collapseProgress: () -> Float, +) { + Text( + text = title, + style = MaterialTheme.typography.headlineSmall, + color = MaterialTheme.colorScheme.onSurface, + textAlign = TextAlign.Center, + maxLines = 3, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.alpha(alpha = 1f - collapseProgress()), + ) +} + @Composable private fun ParticipantRow( participant: ParticipantUiState, diff --git a/src/com/android/messaging/ui/conversationsettings/common/ConversationSettingsTopAppBar.kt b/src/com/android/messaging/ui/conversationsettings/common/ConversationSettingsTopAppBar.kt index 3a7f73f44..8daf101a0 100644 --- a/src/com/android/messaging/ui/conversationsettings/common/ConversationSettingsTopAppBar.kt +++ b/src/com/android/messaging/ui/conversationsettings/common/ConversationSettingsTopAppBar.kt @@ -20,6 +20,7 @@ import androidx.compose.material3.TopAppBarDefaults import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.ui.draw.alpha import androidx.compose.ui.graphics.graphicsLayer import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.style.TextOverflow @@ -71,15 +72,9 @@ internal fun ConversationSettingsTopAppBar( Spacer(modifier = Modifier.width(12.dp)) - Text( - text = title, - style = MaterialTheme.typography.titleMedium, - color = MaterialTheme.colorScheme.onSurface, - maxLines = 1, - overflow = TextOverflow.Ellipsis, - modifier = Modifier.graphicsLayer { - alpha = collapseProgress() - }, + CollapsedTitle( + title = title, + collapseProgress = collapseProgress, ) } }, @@ -96,3 +91,18 @@ internal fun ConversationSettingsTopAppBar( ), ) } + +@Composable +private fun CollapsedTitle( + title: String, + collapseProgress: () -> Float, +) { + Text( + text = title, + style = MaterialTheme.typography.titleMedium, + color = MaterialTheme.colorScheme.onSurface, + maxLines = 1, + overflow = TextOverflow.Ellipsis, + modifier = Modifier.alpha(alpha = collapseProgress()), + ) +} From db2106f167c93149721d0b40ba3778b30202eec8 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Mon, 14 Sep 2026 19:23:19 +0300 Subject: [PATCH 12/21] Fail an MMS send cleanly when an attachment URI is no longer readable --- .../messaging/sms/MmsUtilsSendPathTest.kt | 88 +++++++++++++++++++ .../messaging/mmslib/pdu/PduPersister.java | 3 + src/com/android/messaging/sms/MmsUtils.java | 4 + 3 files changed, 95 insertions(+) create mode 100644 app/src/test/kotlin/com/android/messaging/sms/MmsUtilsSendPathTest.kt diff --git a/app/src/test/kotlin/com/android/messaging/sms/MmsUtilsSendPathTest.kt b/app/src/test/kotlin/com/android/messaging/sms/MmsUtilsSendPathTest.kt new file mode 100644 index 000000000..e3d232a29 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/sms/MmsUtilsSendPathTest.kt @@ -0,0 +1,88 @@ +package com.android.messaging.sms + +import android.content.ContentResolver +import android.content.Context +import android.net.Uri +import com.android.messaging.FactoryTestAccess +import com.android.messaging.datamodel.data.MessageData +import com.android.messaging.datamodel.data.MessagePartData +import com.android.messaging.testutil.TEST_CONVERSATION_ID_VALUE as CONVERSATION_ID +import com.android.messaging.testutil.installTestFactory +import io.mockk.every +import io.mockk.mockk +import io.mockk.verify +import org.junit.After +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * The app holds no media read permissions, so an attachment uri carried by an old message can be + * unreadable by the time that message is sent or resent. The send path has to fail the message + * instead of letting the SecurityException escape the action service and kill the process. + */ +@RunWith(RobolectricTestRunner::class) +class MmsUtilsSendPathTest { + + private val contentResolver = mockk() + private val context = mockk() + + @Before + fun setUp() { + every { context.contentResolver } returns contentResolver + installTestFactory(context = context) + } + + @After + fun tearDown() { + FactoryTestAccess.reset() + } + + @Test + fun insertSendingMmsMessage_returnsNullWhenAnAttachmentIsNoLongerReadable() { + every { contentResolver.openInputStream(any()) } throws SecurityException( + "Permission Denial: opening provider com.android.providers.media.MediaProvider", + ) + + val messageUri = MmsUtils.insertSendingMmsMessage( + context, + listOf(RECIPIENT), + createMmsDraftWithImageAttachment(), + SUB_ID, + SELF_PHONE_NUMBER, + TIMESTAMP, + ) + + assertNull(messageUri) + verify { contentResolver.openInputStream(ATTACHMENT_URI) } + } + + private fun createMmsDraftWithImageAttachment(): MessageData { + val messageData = MessageData.createDraftMmsMessage( + CONVERSATION_ID, + "self-1", + "Hello", + "Subject", + ) + messageData.addPart( + MessagePartData.createMediaMessagePart( + "image/jpeg", + ATTACHMENT_URI, + 320, + 240, + ), + ) + + return messageData + } + + private companion object { + private const val RECIPIENT = "+15555550100" + private const val SELF_PHONE_NUMBER = "+15555550101" + private const val SUB_ID = 1 + private const val TIMESTAMP = 1_700_000_000_000L + private val ATTACHMENT_URI: Uri = Uri.parse("content://media/external/images/media/1") + } +} diff --git a/src/com/android/messaging/mmslib/pdu/PduPersister.java b/src/com/android/messaging/mmslib/pdu/PduPersister.java index e4f4d7127..83d7d33b2 100644 --- a/src/com/android/messaging/mmslib/pdu/PduPersister.java +++ b/src/com/android/messaging/mmslib/pdu/PduPersister.java @@ -973,6 +973,9 @@ private void persistData(final PduPart part, final Uri uri, } catch (final FileNotFoundException e) { Log.e(TAG, "Failed to open Input/Output stream.", e); throw new MmsException(e); + } catch (final SecurityException e) { + Log.e(TAG, "Not allowed to read " + dataUri, e); + throw new MmsException(e); } catch (final IOException e) { Log.e(TAG, "Failed to read/write data.", e); throw new MmsException(e); diff --git a/src/com/android/messaging/sms/MmsUtils.java b/src/com/android/messaging/sms/MmsUtils.java index 5a71dc42d..9cb3bd01e 100644 --- a/src/com/android/messaging/sms/MmsUtils.java +++ b/src/com/android/messaging/sms/MmsUtils.java @@ -2175,6 +2175,10 @@ private static SendReq createMmsSendReq(final Context context, final int subId, requireReadReport, expiryTime, priority, timestampMillis); } catch (final InvalidHeaderValueException e) { LogUtil.e(TAG, "InvalidHeaderValue creating sendReq PDU"); + } catch (final SecurityException e) { + // An attachment uri we can no longer read, e.g. a MediaStore uri left in an old + // draft. Fail the send cleanly instead of taking the process down. + LogUtil.e(TAG, "Not allowed to read attachment creating sendReq PDU", e); } catch (final OutOfMemoryError e) { LogUtil.e(TAG, "Out of memory error creating sendReq PDU"); } From a5084d6c0b85c06b9304496e7d803e8dd4823ccc Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Mon, 14 Sep 2026 01:41:40 +0300 Subject: [PATCH 13/21] Drop legacy MediaStore attachments from restored drafts --- ...versationMessageDataDraftMapperImplTest.kt | 66 +++++++++++++------ .../ConversationDraftsRepositoryImplTest.kt | 5 +- .../ConversationMessageDataDraftMapper.kt | 18 +++-- 3 files changed, 63 insertions(+), 26 deletions(-) diff --git a/app/src/test/kotlin/com/android/messaging/data/conversation/mapper/ConversationMessageDataDraftMapperImplTest.kt b/app/src/test/kotlin/com/android/messaging/data/conversation/mapper/ConversationMessageDataDraftMapperImplTest.kt index af8d2df5a..1e967c5e0 100644 --- a/app/src/test/kotlin/com/android/messaging/data/conversation/mapper/ConversationMessageDataDraftMapperImplTest.kt +++ b/app/src/test/kotlin/com/android/messaging/data/conversation/mapper/ConversationMessageDataDraftMapperImplTest.kt @@ -3,6 +3,7 @@ package com.android.messaging.data.conversation.mapper import android.net.Uri import com.android.messaging.data.conversation.model.ParticipantId import com.android.messaging.data.conversation.model.draft.ConversationDraftAttachment +import com.android.messaging.datamodel.MediaScratchFileProvider import com.android.messaging.datamodel.data.MessageData import com.android.messaging.datamodel.data.MessagePartData import com.android.messaging.testutil.TEST_CONVERSATION_ID as CONVERSATION_ID @@ -30,7 +31,7 @@ class ConversationMessageDataDraftMapperImplTest { MessagePartData.createMediaMessagePart( "Caption", "image/jpeg", - Uri.parse("content://media/image/1"), + scratchUri("1.jpg"), 640, 480, ), @@ -48,7 +49,7 @@ class ConversationMessageDataDraftMapperImplTest { listOf( createAttachment( contentType = "image/jpeg", - contentUri = "content://media/image/1", + contentUri = scratchUri("1.jpg").toString(), captionText = "Caption", width = 640, height = 480, @@ -100,7 +101,7 @@ class ConversationMessageDataDraftMapperImplTest { messageData.addPart( MessagePartData.createMediaMessagePart( "image/png", - Uri.parse("content://media/image/2"), + scratchUri("2.png"), MessagePartData.UNSPECIFIED_SIZE, MessagePartData.UNSPECIFIED_SIZE, ), @@ -110,7 +111,7 @@ class ConversationMessageDataDraftMapperImplTest { val attachment = draft.attachments.single() assertEquals("image/png", attachment.contentType) - assertEquals("content://media/image/2", attachment.contentUri) + assertEquals(scratchUri("2.png").toString(), attachment.contentUri) assertEquals("", attachment.captionText) assertNull(attachment.width) assertNull(attachment.height) @@ -126,7 +127,7 @@ class ConversationMessageDataDraftMapperImplTest { messageData.addPart( MessagePartData.createMediaMessagePart( "", - Uri.parse("content://media/image/3"), + scratchUri("3.jpg"), 320, 240, ), @@ -142,7 +143,7 @@ class ConversationMessageDataDraftMapperImplTest { messageData.addPart( MessagePartData.createMediaMessagePart( "audio/mp3", - Uri.parse("content://media/audio/4"), + scratchUri("4.mp3"), 0, 0, ), @@ -154,7 +155,7 @@ class ConversationMessageDataDraftMapperImplTest { listOf( createAttachment( contentType = "audio/mp3", - contentUri = "content://media/audio/4", + contentUri = scratchUri("4.mp3").toString(), width = 0, height = 0, ), @@ -164,27 +165,54 @@ class ConversationMessageDataDraftMapperImplTest { } @Test - fun map_dropsAttachmentsBackedByPhotoPickerUris() { + fun map_dropsAttachmentsBackedByMediaStoreUris() { val messageData = MessageData.createDraftSmsMessage( CONVERSATION_ID.value, "self-1", "Hello", ) - messageData.addPart( - MessagePartData.createMediaMessagePart( - "image/jpeg", - Uri.parse( - "content://media/picker/0/" + - "com.android.providers.media.photopicker/media/1", + listOf( + "content://media/picker/0/com.android.providers.media.photopicker/media/1", + "content://media/external/images/media/1", + "content://com.android.providers.media.documents/document/image%3A1", + // An authority that merely starts with "media" belongs to somebody else. + "content://media.example/images/1", + // Forwarding seeds a raw mms part uri, readable as the default SMS app. + "content://mms/part/1", + ).forEach { contentUri -> + messageData.addPart( + MessagePartData.createMediaMessagePart( + "image/jpeg", + Uri.parse(contentUri), + 320, + 240, ), - 320, - 240, - ), - ) + ) + } val draft = mapper.map(messageData = messageData) - assertEquals(emptyList(), draft.attachments) + assertEquals( + listOf( + createAttachment( + contentType = "image/jpeg", + contentUri = "content://media.example/images/1", + width = 320, + height = 240, + ), + createAttachment( + contentType = "image/jpeg", + contentUri = "content://mms/part/1", + width = 320, + height = 240, + ), + ), + draft.attachments, + ) + } + + private fun scratchUri(name: String): Uri { + return MediaScratchFileProvider.getUriBuilder().appendPath(name).build() } private fun createAttachment( diff --git a/app/src/test/kotlin/com/android/messaging/data/conversation/repository/ConversationDraftsRepositoryImplTest.kt b/app/src/test/kotlin/com/android/messaging/data/conversation/repository/ConversationDraftsRepositoryImplTest.kt index bb1663c91..026ea0e9b 100644 --- a/app/src/test/kotlin/com/android/messaging/data/conversation/repository/ConversationDraftsRepositoryImplTest.kt +++ b/app/src/test/kotlin/com/android/messaging/data/conversation/repository/ConversationDraftsRepositoryImplTest.kt @@ -10,6 +10,7 @@ import com.android.messaging.data.conversation.mapper.ConversationMessageDataDra import com.android.messaging.data.conversation.model.ParticipantId import com.android.messaging.data.conversation.model.draft.ConversationDraft import com.android.messaging.data.conversation.store.ConversationDraftStore +import com.android.messaging.datamodel.MediaScratchFileProvider import com.android.messaging.datamodel.MessagingContentProvider import com.android.messaging.datamodel.data.MessageData import com.android.messaging.datamodel.data.MessagePartData @@ -413,7 +414,7 @@ class ConversationDraftsRepositoryImplTest { addPart( MessagePartData.createMediaMessagePart( "audio/3gpp", - Uri.parse("content://media/audio/1"), + MediaScratchFileProvider.getUriBuilder().appendPath("1.3gpp").build(), 0, 0, ), @@ -431,7 +432,7 @@ class ConversationDraftsRepositoryImplTest { addPart( MessagePartData.createMediaMessagePart( "image/jpeg", - Uri.parse("content://media/image/1"), + MediaScratchFileProvider.getUriBuilder().appendPath("1.jpg").build(), 640, 480, ), diff --git a/src/com/android/messaging/data/conversation/mapper/ConversationMessageDataDraftMapper.kt b/src/com/android/messaging/data/conversation/mapper/ConversationMessageDataDraftMapper.kt index 86fe25645..dc6514ec0 100644 --- a/src/com/android/messaging/data/conversation/mapper/ConversationMessageDataDraftMapper.kt +++ b/src/com/android/messaging/data/conversation/mapper/ConversationMessageDataDraftMapper.kt @@ -1,11 +1,13 @@ package com.android.messaging.data.conversation.mapper +import androidx.core.net.toUri import com.android.messaging.data.conversation.model.ParticipantId import com.android.messaging.data.conversation.model.draft.ConversationDraft import com.android.messaging.data.conversation.model.draft.ConversationDraftAttachment import com.android.messaging.datamodel.data.MessageData import com.android.messaging.datamodel.data.MessagePartData import com.android.messaging.util.LogUtil +import com.android.messaging.util.UriUtil import javax.inject.Inject import kotlinx.collections.immutable.toImmutableList @@ -43,8 +45,8 @@ internal class ConversationMessageDataDraftMapperImpl @Inject constructor() : val contentUri = part.contentUri?.toString()?.takeIf { it.isNotBlank() } return when { - isPhotoPickerUri(contentUri) -> { - LogUtil.w(TAG, "Dropping draft attachment backed by photo picker URI") + isMediaStoreUri(contentUri) -> { + LogUtil.w(TAG, "Dropping draft attachment backed by MediaStore URI") null } @@ -73,12 +75,18 @@ internal class ConversationMessageDataDraftMapperImpl @Inject constructor() : return size.takeIf { it != MessagePartData.UNSPECIFIED_SIZE } } - private fun isPhotoPickerUri(uri: String?): Boolean { - return uri?.startsWith(prefix = PHOTO_PICKER_URI_PREFIX) == true + /** + * The app holds no media read permissions, so a MediaStore URI is unreadable unless a grant + * came with it, and a grant never survives into a persisted draft. No live path stores one + * anyway - every picker copies its selection into scratch space first - so these only turn up + * in drafts written by the pre-Compose gallery picker. Drop them rather than restore an + * attachment that can only fail. + */ + private fun isMediaStoreUri(uri: String?): Boolean { + return uri != null && UriUtil.isMediaStoreUri(uri.toUri()) } private companion object { private const val TAG = "ConversationMsgDataDraftMapper" - private const val PHOTO_PICKER_URI_PREFIX = "content://media/picker/" } } From 35a1a6de6f6a5b572703b91c22ca051083251611 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Mon, 14 Sep 2026 01:18:17 +0300 Subject: [PATCH 14/21] Drop the media read permissions and the dead gallery picker --- AndroidManifest.xml | 3 - .../datamodel/ActionSyncTestDataModel.java | 12 - .../ConversationMediaRepositoryImplTest.kt | 230 ------------------ .../data/media/model/ConversationMediaItem.kt | 10 - .../repository/ConversationMediaRepository.kt | 130 ---------- .../messaging/datamodel/DataModel.java | 6 - .../messaging/datamodel/DataModelImpl.java | 12 - .../datamodel/GalleryBoundCursorLoader.java | 53 ---- .../datamodel/data/GalleryGridItemData.java | 157 ------------ .../datamodel/data/MediaPickerData.java | 175 ------------- .../VideoThumbnailRequestDescriptor.java | 37 --- .../conversation/ConversationBindsModule.kt | 8 - .../android/messaging/util/ImageUtils.java | 35 --- src/com/android/messaging/util/UriUtil.java | 13 - 14 files changed, 881 deletions(-) delete mode 100644 app/src/test/kotlin/com/android/messaging/data/media/repository/ConversationMediaRepositoryImplTest.kt delete mode 100644 src/com/android/messaging/data/media/model/ConversationMediaItem.kt delete mode 100644 src/com/android/messaging/data/media/repository/ConversationMediaRepository.kt delete mode 100644 src/com/android/messaging/datamodel/GalleryBoundCursorLoader.java delete mode 100644 src/com/android/messaging/datamodel/data/GalleryGridItemData.java delete mode 100644 src/com/android/messaging/datamodel/data/MediaPickerData.java delete mode 100644 src/com/android/messaging/datamodel/media/VideoThumbnailRequestDescriptor.java diff --git a/AndroidManifest.xml b/AndroidManifest.xml index 6fda90c3d..81d97de25 100644 --- a/AndroidManifest.xml +++ b/AndroidManifest.xml @@ -40,9 +40,6 @@ - - - diff --git a/app/src/test/java/com/android/messaging/datamodel/ActionSyncTestDataModel.java b/app/src/test/java/com/android/messaging/datamodel/ActionSyncTestDataModel.java index 65d6a4356..b8b11c0b8 100644 --- a/app/src/test/java/com/android/messaging/datamodel/ActionSyncTestDataModel.java +++ b/app/src/test/java/com/android/messaging/datamodel/ActionSyncTestDataModel.java @@ -8,9 +8,7 @@ import com.android.messaging.datamodel.data.ConversationData; import com.android.messaging.datamodel.data.ConversationListData; import com.android.messaging.datamodel.data.DraftMessageData; -import com.android.messaging.datamodel.data.GalleryGridItemData; import com.android.messaging.datamodel.data.LaunchConversationData; -import com.android.messaging.datamodel.data.MediaPickerData; import com.android.messaging.util.Assert; public class ActionSyncTestDataModel extends DataModel { @@ -37,16 +35,6 @@ public ConversationData createConversationData(final Context context, throw new UnsupportedOperationException(); } - @Override - public MediaPickerData createMediaPickerData(final Context context) { - throw new UnsupportedOperationException(); - } - - @Override - public GalleryGridItemData createGalleryGridItemData() { - throw new UnsupportedOperationException(); - } - @Override public LaunchConversationData createLaunchConversationData( final LaunchConversationData.LaunchConversationDataListener listener) { diff --git a/app/src/test/kotlin/com/android/messaging/data/media/repository/ConversationMediaRepositoryImplTest.kt b/app/src/test/kotlin/com/android/messaging/data/media/repository/ConversationMediaRepositoryImplTest.kt deleted file mode 100644 index 48f985773..000000000 --- a/app/src/test/kotlin/com/android/messaging/data/media/repository/ConversationMediaRepositoryImplTest.kt +++ /dev/null @@ -1,230 +0,0 @@ -package com.android.messaging.data.media.repository - -import android.content.ContentResolver -import android.database.Cursor -import android.database.MatrixCursor -import android.net.Uri -import android.os.Bundle -import android.provider.MediaStore -import com.android.messaging.data.media.model.ConversationMediaItem -import com.android.messaging.testutil.MainDispatcherRule -import com.android.messaging.util.ContentType -import com.android.messaging.util.UriUtil -import io.mockk.every -import io.mockk.mockk -import io.mockk.slot -import io.mockk.verify -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.single -import kotlinx.coroutines.test.runTest -import org.junit.Assert.assertEquals -import org.junit.Rule -import org.junit.Test -import org.junit.runner.RunWith -import org.robolectric.RobolectricTestRunner - -@OptIn(ExperimentalCoroutinesApi::class) -@RunWith(RobolectricTestRunner::class) -class ConversationMediaRepositoryImplTest { - - @get:Rule - val mainDispatcherRule = MainDispatcherRule() - - @Test - fun getRecentMedia_queriesMediaStoreWithExpectedProjectionAndArgs() { - runTest( - context = mainDispatcherRule.testDispatcher - ) { - val contentResolver = mockk() - val capturedUri = slot() - val capturedProjection = slot>() - val capturedQueryArgs = slot() - - every { - contentResolver.query( - capture(capturedUri), - capture(capturedProjection), - capture(capturedQueryArgs), - null, - ) - } returns createCursor() - - val repository = createRepository(contentResolver = contentResolver) - - repository.getRecentMedia(limit = 37).single() - - verify(exactly = 1) { - contentResolver.query( - any(), - any>(), - any(), - null, - ) - } - - assertRecentMediaQuery( - uri = capturedUri.captured, - projection = capturedProjection.captured, - queryArgs = capturedQueryArgs.captured, - limit = 37, - ) - } - } - - @Test - fun getRecentMedia_mapsCursorRows() { - runTest(context = mainDispatcherRule.testDispatcher) { - val contentResolver = mockk() - stubQuery( - contentResolver = contentResolver, - result = createCursor(), - ) - val repository = createRepository(contentResolver = contentResolver) - - val items = repository.getRecentMedia(limit = 37).single() - - assertEquals( - listOf( - ConversationMediaItem( - mediaId = "10", - contentUri = UriUtil.getContentUriForMediaStoreId(10L).toString(), - contentType = ContentType.IMAGE_UNSPECIFIED, - width = null, - height = 720, - durationMillis = null, - ), - ConversationMediaItem( - mediaId = "11", - contentUri = UriUtil.getContentUriForMediaStoreId(11L).toString(), - contentType = ContentType.VIDEO_UNSPECIFIED, - width = 1920, - height = 1080, - durationMillis = 1234L, - ), - ), - items, - ) - } - } - - @Test - fun getRecentMedia_returnsEmptyListForNullCursor() { - runTest( - context = mainDispatcherRule.testDispatcher - ) { - val contentResolver = mockk() - stubQuery( - contentResolver = contentResolver, - result = null, - ) - val repository = createRepository(contentResolver = contentResolver) - - val items = repository.getRecentMedia(limit = 7).single() - - assertEquals(emptyList(), items) - } - } - - private fun createRepository( - contentResolver: ContentResolver, - ): ConversationMediaRepositoryImpl { - return ConversationMediaRepositoryImpl( - contentResolver = contentResolver, - ioDispatcher = mainDispatcherRule.testDispatcher, - ) - } - - private fun stubQuery( - contentResolver: ContentResolver, - result: Cursor?, - ) { - every { - contentResolver.query( - any(), - any>(), - any(), - null, - ) - } returns result - } - - @Suppress("SameParameterValue") - private fun assertRecentMediaQuery( - uri: Uri, - projection: Array, - queryArgs: Bundle, - limit: Int, - ) { - assertEquals( - MediaStore.Files.getContentUri(MediaStore.VOLUME_EXTERNAL), - uri, - ) - assertEquals( - listOf( - MediaStore.Files.FileColumns._ID, - MediaStore.Files.FileColumns.MEDIA_TYPE, - MediaStore.Files.FileColumns.MIME_TYPE, - MediaStore.Files.FileColumns.DATE_ADDED, - MediaStore.Files.FileColumns.WIDTH, - MediaStore.Files.FileColumns.HEIGHT, - MediaStore.Video.VideoColumns.DURATION, - ), - projection.toList(), - ) - assertEquals( - "${MediaStore.Files.FileColumns.MEDIA_TYPE} IN " + - "(${MediaStore.Files.FileColumns.MEDIA_TYPE_IMAGE}," + - "${MediaStore.Files.FileColumns.MEDIA_TYPE_VIDEO})", - queryArgs.getString(ContentResolver.QUERY_ARG_SQL_SELECTION), - ) - assertEquals( - listOf(MediaStore.Files.FileColumns.DATE_ADDED), - queryArgs.getStringArray(ContentResolver.QUERY_ARG_SORT_COLUMNS)?.toList(), - ) - assertEquals( - ContentResolver.QUERY_SORT_DIRECTION_DESCENDING, - queryArgs.getInt(ContentResolver.QUERY_ARG_SORT_DIRECTION), - ) - assertEquals( - limit, - queryArgs.getInt(ContentResolver.QUERY_ARG_LIMIT), - ) - } - - private fun createCursor(): Cursor { - return MatrixCursor( - arrayOf( - MediaStore.Files.FileColumns._ID, - MediaStore.Files.FileColumns.MEDIA_TYPE, - MediaStore.Files.FileColumns.MIME_TYPE, - MediaStore.Files.FileColumns.DATE_ADDED, - MediaStore.Files.FileColumns.WIDTH, - MediaStore.Files.FileColumns.HEIGHT, - MediaStore.Video.VideoColumns.DURATION, - ), - ).apply { - addRow( - arrayOf( - 10L, - MediaStore.Files.FileColumns.MEDIA_TYPE_IMAGE, - "", - 1L, - 0, - 720, - 0L, - ), - ) - addRow( - arrayOf( - 11L, - MediaStore.Files.FileColumns.MEDIA_TYPE_VIDEO, - "", - 2L, - 1920, - 1080, - 1234L, - ), - ) - } - } -} diff --git a/src/com/android/messaging/data/media/model/ConversationMediaItem.kt b/src/com/android/messaging/data/media/model/ConversationMediaItem.kt deleted file mode 100644 index 7f5fcea62..000000000 --- a/src/com/android/messaging/data/media/model/ConversationMediaItem.kt +++ /dev/null @@ -1,10 +0,0 @@ -package com.android.messaging.data.media.model - -internal data class ConversationMediaItem( - val mediaId: String, - val contentUri: String, - val contentType: String, - val width: Int?, - val height: Int?, - val durationMillis: Long?, -) diff --git a/src/com/android/messaging/data/media/repository/ConversationMediaRepository.kt b/src/com/android/messaging/data/media/repository/ConversationMediaRepository.kt deleted file mode 100644 index 3f8a16665..000000000 --- a/src/com/android/messaging/data/media/repository/ConversationMediaRepository.kt +++ /dev/null @@ -1,130 +0,0 @@ -package com.android.messaging.data.media.repository - -import android.content.ContentResolver -import android.os.Bundle -import android.provider.MediaStore -import com.android.messaging.data.media.model.ConversationMediaItem -import com.android.messaging.di.core.IoDispatcher -import com.android.messaging.util.ContentType -import com.android.messaging.util.UriUtil -import com.android.messaging.util.core.extension.typedFlow -import javax.inject.Inject -import kotlinx.coroutines.CoroutineDispatcher -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.flowOn - -internal interface ConversationMediaRepository { - fun getRecentMedia(limit: Int = DEFAULT_RECENT_MEDIA_LIMIT): Flow> - - private companion object { - private const val DEFAULT_RECENT_MEDIA_LIMIT = 200 - } -} - -internal class ConversationMediaRepositoryImpl @Inject constructor( - private val contentResolver: ContentResolver, - @param:IoDispatcher - private val ioDispatcher: CoroutineDispatcher, -) : ConversationMediaRepository { - - override fun getRecentMedia(limit: Int): Flow> { - return typedFlow { - queryRecentMedia(limit = limit) - }.flowOn(context = ioDispatcher) - } - - private fun queryRecentMedia(limit: Int): List { - return contentResolver.query( - MediaStore.Files.getContentUri(MediaStore.VOLUME_EXTERNAL), - RECENT_MEDIA_PROJECTION, - createRecentMediaQueryArgs(limit = limit), - null, - )?.use { cursor -> - val idIndex = cursor.getColumnIndexOrThrow(MediaStore.Files.FileColumns._ID) - val mediaTypeIndex = cursor.getColumnIndexOrThrow( - MediaStore.Files.FileColumns.MEDIA_TYPE, - ) - val mimeTypeIndex = cursor.getColumnIndexOrThrow( - MediaStore.Files.FileColumns.MIME_TYPE, - ) - val widthIndex = cursor.getColumnIndexOrThrow(MediaStore.Files.FileColumns.WIDTH) - val heightIndex = cursor.getColumnIndexOrThrow( - MediaStore.Files.FileColumns.HEIGHT, - ) - val durationIndex = cursor.getColumnIndexOrThrow( - MediaStore.Video.VideoColumns.DURATION, - ) - - buildList(capacity = cursor.count) { - while (cursor.moveToNext()) { - val mediaStoreId = cursor.getLong(idIndex) - val mediaTypeValue = cursor.getInt(mediaTypeIndex) - - val item = ConversationMediaItem( - mediaId = mediaStoreId.toString(), - contentUri = UriUtil - .getContentUriForMediaStoreId(mediaStoreId) - .toString(), - contentType = cursor - .getString(mimeTypeIndex) - ?.takeIf { it.isNotBlank() } - ?: fallbackContentType(mediaTypeValue = mediaTypeValue), - width = cursor.getInt(widthIndex).takeIf { it > 0 }, - height = cursor.getInt(heightIndex).takeIf { it > 0 }, - durationMillis = cursor.getLong(durationIndex).takeIf { it > 0 }, - ) - - add(item) - } - } - }.orEmpty() - } - - private fun createRecentMediaQueryArgs(limit: Int): Bundle { - return Bundle().apply { - putString( - ContentResolver.QUERY_ARG_SQL_SELECTION, - RECENT_MEDIA_SELECTION, - ) - putStringArray( - ContentResolver.QUERY_ARG_SORT_COLUMNS, - arrayOf(MediaStore.Files.FileColumns.DATE_ADDED), - ) - putInt( - ContentResolver.QUERY_ARG_SORT_DIRECTION, - ContentResolver.QUERY_SORT_DIRECTION_DESCENDING, - ) - putInt(ContentResolver.QUERY_ARG_LIMIT, limit) - } - } - - private fun fallbackContentType(mediaTypeValue: Int): String { - return when (mediaTypeValue) { - MediaStore.Files.FileColumns.MEDIA_TYPE_VIDEO -> ContentType.VIDEO_UNSPECIFIED - else -> ContentType.IMAGE_UNSPECIFIED - } - } - - private companion object { - private val RECENT_MEDIA_PROJECTION: Array = arrayOf( - MediaStore.Files.FileColumns._ID, - MediaStore.Files.FileColumns.MEDIA_TYPE, - MediaStore.Files.FileColumns.MIME_TYPE, - MediaStore.Files.FileColumns.DATE_ADDED, - MediaStore.Files.FileColumns.WIDTH, - MediaStore.Files.FileColumns.HEIGHT, - MediaStore.Video.VideoColumns.DURATION, - ) - - private val RECENT_MEDIA_SELECTION: String by lazy { - buildString { - append(MediaStore.Files.FileColumns.MEDIA_TYPE) - append(" IN (") - append(MediaStore.Files.FileColumns.MEDIA_TYPE_IMAGE) - append(",") - append(MediaStore.Files.FileColumns.MEDIA_TYPE_VIDEO) - append(")") - } - } - } -} diff --git a/src/com/android/messaging/datamodel/DataModel.java b/src/com/android/messaging/datamodel/DataModel.java index d2879c2fc..51548d82f 100644 --- a/src/com/android/messaging/datamodel/DataModel.java +++ b/src/com/android/messaging/datamodel/DataModel.java @@ -29,10 +29,8 @@ import com.android.messaging.datamodel.data.ConversationListData; import com.android.messaging.datamodel.data.ConversationListData.ConversationListDataListener; import com.android.messaging.datamodel.data.DraftMessageData; -import com.android.messaging.datamodel.data.GalleryGridItemData; import com.android.messaging.datamodel.data.LaunchConversationData; import com.android.messaging.datamodel.data.LaunchConversationData.LaunchConversationDataListener; -import com.android.messaging.datamodel.data.MediaPickerData; import com.android.messaging.util.Assert.DoesNotRunOnMainThread; public abstract class DataModel { @@ -63,10 +61,6 @@ public abstract ConversationListData createConversationListData(final Context co public abstract ConversationData createConversationData(final Context context, final ConversationDataListener listener, final String conversationId); - public abstract MediaPickerData createMediaPickerData(final Context context); - - public abstract GalleryGridItemData createGalleryGridItemData(); - public abstract LaunchConversationData createLaunchConversationData( LaunchConversationDataListener listener); diff --git a/src/com/android/messaging/datamodel/DataModelImpl.java b/src/com/android/messaging/datamodel/DataModelImpl.java index a35cb2db3..4c9f21708 100644 --- a/src/com/android/messaging/datamodel/DataModelImpl.java +++ b/src/com/android/messaging/datamodel/DataModelImpl.java @@ -29,10 +29,8 @@ import com.android.messaging.datamodel.data.ConversationListData; import com.android.messaging.datamodel.data.ConversationListData.ConversationListDataListener; import com.android.messaging.datamodel.data.DraftMessageData; -import com.android.messaging.datamodel.data.GalleryGridItemData; import com.android.messaging.datamodel.data.LaunchConversationData; import com.android.messaging.datamodel.data.LaunchConversationData.LaunchConversationDataListener; -import com.android.messaging.datamodel.data.MediaPickerData; import com.android.messaging.datamodel.data.ParticipantData; import com.android.messaging.sms.MmsConfig; import com.android.messaging.util.Assert; @@ -75,16 +73,6 @@ public ConversationData createConversationData(final Context context, return new ConversationData(context, listener, conversationId); } - @Override - public MediaPickerData createMediaPickerData(final Context context) { - return new MediaPickerData(context); - } - - @Override - public GalleryGridItemData createGalleryGridItemData() { - return new GalleryGridItemData(); - } - @Override public LaunchConversationData createLaunchConversationData( final LaunchConversationDataListener listener) { diff --git a/src/com/android/messaging/datamodel/GalleryBoundCursorLoader.java b/src/com/android/messaging/datamodel/GalleryBoundCursorLoader.java deleted file mode 100644 index 1974a5e7a..000000000 --- a/src/com/android/messaging/datamodel/GalleryBoundCursorLoader.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright (C) 2015 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.messaging.datamodel; - -import android.content.Context; -import android.net.Uri; -import android.provider.MediaStore.Files; -import android.provider.MediaStore.Files.FileColumns; -import android.provider.MediaStore.MediaColumns; - -import com.android.messaging.datamodel.data.GalleryGridItemData; -import com.android.messaging.datamodel.data.MessagePartData; -import com.google.common.base.Joiner; - -/** - * A BoundCursorLoader that reads local media on the device. - */ -public class GalleryBoundCursorLoader extends BoundCursorLoader { - public static final String MEDIA_SCANNER_VOLUME_EXTERNAL = "external"; - private static final Uri STORAGE_URI = Files.getContentUri(MEDIA_SCANNER_VOLUME_EXTERNAL); - private static final String SORT_ORDER = MediaColumns.DATE_MODIFIED + " DESC"; - private static final String SELECTION = createSelection( - MessagePartData.ACCEPTABLE_GALLERY_MEDIA_TYPES, - new Integer[] { - FileColumns.MEDIA_TYPE_IMAGE, - FileColumns.MEDIA_TYPE_VIDEO, - FileColumns.MEDIA_TYPE_AUDIO - }); - - public GalleryBoundCursorLoader(final String bindingId, final Context context) { - super(bindingId, context, STORAGE_URI, GalleryGridItemData.MEDIA_PROJECTION, SELECTION, - null, SORT_ORDER); - } - - private static String createSelection(final String[] mimeTypes, Integer[] mediaTypes) { - return MediaColumns.MIME_TYPE + " IN ('" + Joiner.on("','").join(mimeTypes) + "') AND " - + FileColumns.MEDIA_TYPE + " IN (" + Joiner.on(',').join(mediaTypes) + ")"; - } -} diff --git a/src/com/android/messaging/datamodel/data/GalleryGridItemData.java b/src/com/android/messaging/datamodel/data/GalleryGridItemData.java deleted file mode 100644 index 941d38d4e..000000000 --- a/src/com/android/messaging/datamodel/data/GalleryGridItemData.java +++ /dev/null @@ -1,157 +0,0 @@ -/* - * Copyright (C) 2015 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.messaging.datamodel.data; - -import android.database.Cursor; -import android.graphics.Rect; -import android.net.Uri; -import android.provider.BaseColumns; -import android.provider.MediaStore.MediaColumns; -import android.text.TextUtils; - -import com.android.messaging.datamodel.media.FileImageRequestDescriptor; -import com.android.messaging.datamodel.media.ImageRequest; -import com.android.messaging.datamodel.media.UriImageRequestDescriptor; -import com.android.messaging.datamodel.media.VideoThumbnailRequestDescriptor; -import com.android.messaging.util.Assert; -import com.android.messaging.util.ContentType; -import com.android.messaging.util.UriUtil; - -/** - * Provides data for GalleryGridItemView - */ -public class GalleryGridItemData { - public static final String[] MEDIA_PROJECTION = new String[] { - MediaColumns._ID, - MediaColumns.DATA, - MediaColumns.WIDTH, - MediaColumns.HEIGHT, - MediaColumns.MIME_TYPE, - MediaColumns.DATE_MODIFIED, - MediaColumns.DISPLAY_NAME}; - - public static final String[] SPECIAL_ITEM_COLUMNS = new String[] { - BaseColumns._ID - }; - - private static final int INDEX_ID = 0; - - // For local media gallery. - private static final int INDEX_DATA_PATH = 1; - private static final int INDEX_WIDTH = 2; - private static final int INDEX_HEIGHT = 3; - private static final int INDEX_MIME_TYPE = 4; - private static final int INDEX_DATE_MODIFIED = 5; - private static final int INDEX_DISPLAY_NAME = 6; - - /** A special item's id for picking a media from document picker */ - public static final String ID_DOCUMENT_PICKER_ITEM = "-1"; - - private UriImageRequestDescriptor mImageData; - private String mContentType; - private boolean mIsDocumentPickerItem; - private long mDateSeconds; - private String mFileName; - private Uri mAudioUri; - - public GalleryGridItemData() { - } - - public void bind(final Cursor cursor, final int desiredWidth, final int desiredHeight) { - mIsDocumentPickerItem = TextUtils.equals(cursor.getString(INDEX_ID), - ID_DOCUMENT_PICKER_ITEM); - if (mIsDocumentPickerItem) { - mImageData = null; - mContentType = null; - } else { - mContentType = cursor.getString(INDEX_MIME_TYPE); - final String filePath = cursor.getString(INDEX_DATA_PATH); - final String dateModified = cursor.getString(INDEX_DATE_MODIFIED); - mDateSeconds = !TextUtils.isEmpty(dateModified) ? Long.parseLong(dateModified) : -1; - if (ContentType.isAudioType(mContentType)) { - mImageData = null; - mAudioUri = UriUtil.getUriForResourceFile(filePath); - mFileName = cursor.getString(INDEX_DISPLAY_NAME); - } else { // For image and video types - int sourceWidth = cursor.getInt(INDEX_WIDTH); - int sourceHeight = cursor.getInt(INDEX_HEIGHT); - - // Guard against bad data - if (sourceWidth <= 0) { - sourceWidth = ImageRequest.UNSPECIFIED_SIZE; - } - if (sourceHeight <= 0) { - sourceHeight = ImageRequest.UNSPECIFIED_SIZE; - } - - if (ContentType.isVideoType(mContentType)) { - mImageData = new VideoThumbnailRequestDescriptor( - cursor.getLong(INDEX_ID), - desiredWidth, - desiredHeight, - sourceWidth, - sourceHeight); - } else { - mImageData = new FileImageRequestDescriptor( - filePath, - desiredWidth, - desiredHeight, - sourceWidth, - sourceHeight, - true /* canUseThumbnail */, - true /* allowCompression */, - true /* isStatic */); - } - } - } - } - - public boolean isDocumentPickerItem() { - return mIsDocumentPickerItem; - } - - public Uri getImageUri() { - return ContentType.isAudioType(mContentType) ? mAudioUri : mImageData.uri; - } - - public UriImageRequestDescriptor getImageRequestDescriptor() { - return mImageData; - } - - public MessagePartData constructMessagePartData(final Rect startRect) { - Assert.isTrue(!mIsDocumentPickerItem); - return ContentType.isAudioType(mContentType) - ? new MediaPickerMessagePartData(startRect, mContentType, mAudioUri, 0, 0) - : new MediaPickerMessagePartData(startRect, mContentType, mImageData.uri, - mImageData.sourceWidth, mImageData.sourceHeight); - } - - /** - * @return The date in seconds. This can be negative if we could not retreive date info - */ - public long getDateSeconds() { - return mDateSeconds; - } - - public String getContentType() { - return mContentType; - } - - public String getFileName() { - return mFileName; - } -} diff --git a/src/com/android/messaging/datamodel/data/MediaPickerData.java b/src/com/android/messaging/datamodel/data/MediaPickerData.java deleted file mode 100644 index 015d2e740..000000000 --- a/src/com/android/messaging/datamodel/data/MediaPickerData.java +++ /dev/null @@ -1,175 +0,0 @@ -/* - * Copyright (C) 2015 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.messaging.datamodel.data; - -import android.content.Context; -import android.database.Cursor; -import android.os.Bundle; -import androidx.annotation.Nullable; -import androidx.loader.app.LoaderManager; -import androidx.loader.content.Loader; - -import com.android.messaging.datamodel.BoundCursorLoader; -import com.android.messaging.datamodel.GalleryBoundCursorLoader; -import com.android.messaging.datamodel.binding.BindableData; -import com.android.messaging.datamodel.binding.BindingBase; -import com.android.messaging.util.Assert; -import com.android.messaging.util.BuglePrefs; -import com.android.messaging.util.BuglePrefsKeys; -import com.android.messaging.util.LogUtil; - -/** - * Services data needs for MediaPicker. - */ -public class MediaPickerData extends BindableData { - public interface MediaPickerDataListener { - void onMediaPickerDataUpdated(MediaPickerData mediaPickerData, Object data, int loaderId); - } - - private static final String BINDING_ID = "bindingId"; - private final Context mContext; - private LoaderManager mLoaderManager; - private final GalleryLoaderCallbacks mGalleryLoaderCallbacks; - private MediaPickerDataListener mListener; - - public MediaPickerData(final Context context) { - mContext = context; - mGalleryLoaderCallbacks = new GalleryLoaderCallbacks(); - } - - public static final int GALLERY_MEDIA_LOADER = 1; - - /** - * A trampoline class so that we can inherit from LoaderManager.LoaderCallbacks multiple times. - */ - private class GalleryLoaderCallbacks extends CursorLoaderCallbacks { - @Override - public Loader onCreateLoader(final int id, final Bundle args) { - final String bindingId = args.getString(BINDING_ID); - // Check if data still bound to the requesting ui element - if (isBound(bindingId)) { - switch (id) { - case GALLERY_MEDIA_LOADER: - return new GalleryBoundCursorLoader(bindingId, mContext); - - default: - Assert.fail("Unknown loader id for gallery picker!"); - break; - } - } else { - LogUtil.w(LogUtil.BUGLE_TAG, "Loader created after unbinding the media picker"); - } - return null; - } - - /** - * {@inheritDoc} - */ - @Override - public void onLoadFinish(final Loader loader, final Cursor data) { - final BoundCursorLoader cursorLoader = (BoundCursorLoader) loader; - if (isBound(cursorLoader.getBindingId())) { - switch (loader.getId()) { - case GALLERY_MEDIA_LOADER: - mListener.onMediaPickerDataUpdated(MediaPickerData.this, data, - GALLERY_MEDIA_LOADER); - break; - - default: - Assert.fail("Unknown loader id for gallery picker!"); - break; - } - } else { - LogUtil.w(LogUtil.BUGLE_TAG, "Loader finished after unbinding the media picker"); - } - } - - /** - * {@inheritDoc} - */ - @Override - public void onLoaderReset(final Loader loader) { - final BoundCursorLoader cursorLoader = (BoundCursorLoader) loader; - if (isBound(cursorLoader.getBindingId())) { - switch (loader.getId()) { - case GALLERY_MEDIA_LOADER: - mListener.onMediaPickerDataUpdated(MediaPickerData.this, null, - GALLERY_MEDIA_LOADER); - break; - - default: - Assert.fail("Unknown loader id for media picker!"); - break; - } - } else { - LogUtil.w(LogUtil.BUGLE_TAG, "Loader reset after unbinding the media picker"); - } - } - } - - - - public void startLoader(final int loaderId, final BindingBase binding, - @Nullable Bundle args, final MediaPickerDataListener listener) { - if (args == null) { - args = new Bundle(); - } - args.putString(BINDING_ID, binding.getBindingId()); - if (loaderId == GALLERY_MEDIA_LOADER) { - mLoaderManager.initLoader(loaderId, args, mGalleryLoaderCallbacks).forceLoad(); - } else { - Assert.fail("Unsupported loader id for media picker!"); - } - mListener = listener; - } - - public void destroyLoader(final int loaderId) { - mLoaderManager.destroyLoader(loaderId); - } - - public void init(final LoaderManager loaderManager) { - mLoaderManager = loaderManager; - } - - @Override - protected void unregisterListeners() { - // This could be null if we bind but the caller doesn't init the BindableData - if (mLoaderManager != null) { - mLoaderManager.destroyLoader(GALLERY_MEDIA_LOADER); - mLoaderManager = null; - } - } - - /** - * Gets the last selected chooser index, or -1 if no selection has been saved. - */ - public int getSelectedChooserIndex() { - return BuglePrefs.getApplicationPrefs().getInt( - BuglePrefsKeys.SELECTED_MEDIA_PICKER_CHOOSER_INDEX, - BuglePrefsKeys.SELECTED_MEDIA_PICKER_CHOOSER_INDEX_DEFAULT); - } - - /** - * Saves the selected media chooser index. - * @param selectedIndex the selected media chooser index. - */ - public void saveSelectedChooserIndex(final int selectedIndex) { - BuglePrefs.getApplicationPrefs().putInt(BuglePrefsKeys.SELECTED_MEDIA_PICKER_CHOOSER_INDEX, - selectedIndex); - } - -} diff --git a/src/com/android/messaging/datamodel/media/VideoThumbnailRequestDescriptor.java b/src/com/android/messaging/datamodel/media/VideoThumbnailRequestDescriptor.java deleted file mode 100644 index 22f187131..000000000 --- a/src/com/android/messaging/datamodel/media/VideoThumbnailRequestDescriptor.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * Copyright (C) 2015 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.messaging.datamodel.media; - -import android.content.Context; - -import com.android.messaging.util.ImageUtils; -import com.android.messaging.util.UriUtil; - -public class VideoThumbnailRequestDescriptor extends UriImageRequestDescriptor { - public VideoThumbnailRequestDescriptor(final long id, int desiredWidth, int desiredHeight, - int sourceWidth, int sourceHeight) { - super(UriUtil.getContentUriForMediaStoreId(id), desiredWidth, desiredHeight, sourceWidth, - sourceHeight, false /* canCompress */, false /* isStatic */, - false /* cropToCircle */, - ImageUtils.DEFAULT_CIRCLE_BACKGROUND_COLOR /* circleBackgroundColor */, - ImageUtils.DEFAULT_CIRCLE_STROKE_COLOR /* circleStrokeColor */); - } - - @Override - public MediaRequest buildSyncMediaRequest(Context context) { - return new VideoThumbnailRequest(context, this); - } -} diff --git a/src/com/android/messaging/di/conversation/ConversationBindsModule.kt b/src/com/android/messaging/di/conversation/ConversationBindsModule.kt index e662f526c..dea6d39bf 100644 --- a/src/com/android/messaging/di/conversation/ConversationBindsModule.kt +++ b/src/com/android/messaging/di/conversation/ConversationBindsModule.kt @@ -34,8 +34,6 @@ import com.android.messaging.data.conversation.store.ConversationSelfIdStore import com.android.messaging.data.conversation.store.ConversationSelfIdStoreImpl import com.android.messaging.data.media.repository.ConversationAttachmentsRepository import com.android.messaging.data.media.repository.ConversationAttachmentsRepositoryImpl -import com.android.messaging.data.media.repository.ConversationMediaRepository -import com.android.messaging.data.media.repository.ConversationMediaRepositoryImpl import com.android.messaging.data.subscription.repository.ConversationSimSelectionRepository import com.android.messaging.data.subscription.repository.ConversationSimSelectionRepositoryImpl import com.android.messaging.data.subscription.repository.SubscriptionsRepository @@ -367,12 +365,6 @@ internal abstract class ConversationBindsModule { impl: ConversationVCardMetadataMapperImpl, ): ConversationVCardMetadataMapper - @Binds - @Reusable - abstract fun bindConversationMediaRepository( - impl: ConversationMediaRepositoryImpl, - ): ConversationMediaRepository - @Binds abstract fun bindConversationMetadataUiStateMapper( impl: ConversationMetadataUiStateMapperImpl, diff --git a/src/com/android/messaging/util/ImageUtils.java b/src/com/android/messaging/util/ImageUtils.java index 0fcea874d..e04d67b9f 100644 --- a/src/com/android/messaging/util/ImageUtils.java +++ b/src/com/android/messaging/util/ImageUtils.java @@ -18,7 +18,6 @@ import android.app.ActivityManager; import android.content.ContentResolver; import android.content.Context; -import android.database.Cursor; import android.graphics.Bitmap; import android.graphics.BitmapFactory; import android.graphics.BitmapShader; @@ -29,14 +28,12 @@ import android.graphics.RectF; import android.graphics.Shader.TileMode; import android.net.Uri; -import android.provider.MediaStore; import android.text.TextUtils; import com.android.messaging.Factory; import com.android.messaging.datamodel.MediaScratchFileProvider; import com.android.messaging.datamodel.MessagingContentProvider; import com.android.messaging.datamodel.media.ImageRequest; -import com.android.messaging.util.Assert.DoesNotRunOnMainThread; import com.android.messaging.util.exif.ExifInterface; import com.google.common.annotations.VisibleForTesting; @@ -202,38 +199,6 @@ public int calculateInSampleSize( return inSampleSize; } - private static final String[] MEDIA_CONTENT_PROJECTION = new String[] { - MediaStore.MediaColumns.MIME_TYPE - }; - - private static final int INDEX_CONTENT_TYPE = 0; - - @DoesNotRunOnMainThread - public static String getContentType(final ContentResolver cr, final Uri uri) { - // Figure out the content type of media. - String contentType = null; - Cursor cursor = null; - if (UriUtil.isMediaStoreUri(uri)) { - try { - cursor = cr.query(uri, MEDIA_CONTENT_PROJECTION, null, null, null); - - if (cursor != null && cursor.moveToFirst()) { - contentType = cursor.getString(INDEX_CONTENT_TYPE); - } - } finally { - if (cursor != null) { - cursor.close(); - } - } - } - if (contentType == null) { - // Last ditch effort to get the content type. Look at the file extension. - contentType = ContentType.getContentTypeFromExtension(uri.toString(), - ContentType.IMAGE_UNSPECIFIED); - } - return contentType; - } - /** * @param context Android context * @param uri Uri to the image data diff --git a/src/com/android/messaging/util/UriUtil.java b/src/com/android/messaging/util/UriUtil.java index 508ff4e5c..f3c7414c6 100644 --- a/src/com/android/messaging/util/UriUtil.java +++ b/src/com/android/messaging/util/UriUtil.java @@ -24,7 +24,6 @@ import android.text.TextUtils; import com.android.messaging.Factory; -import com.android.messaging.datamodel.GalleryBoundCursorLoader; import com.android.messaging.datamodel.MediaScratchFileProvider; import com.android.messaging.util.Assert.DoesNotRunOnMainThread; @@ -124,18 +123,6 @@ public static boolean isMediaStoreUri(final Uri uri) { TextUtils.equals(MEDIA_STORE_URI_KLP, uriAuthority)); } - /** - * Gets the content:// style URI for the given MediaStore row Id in the files table on the - * external volume. - * - * @param id the MediaStore row Id to get the URI for - * @return the URI to the files table on the external storage. - */ - public static Uri getContentUriForMediaStoreId(final long id) { - return MediaStore.Files.getContentUri( - GalleryBoundCursorLoader.MEDIA_SCANNER_VOLUME_EXTERNAL, id); - } - /** * Gets the size in bytes for the content uri. Currently we only support content in the * scratch space. From ace13f9abfa7eaf2d40c37e353400a531bc4f55f Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Mon, 14 Sep 2026 22:56:31 +0300 Subject: [PATCH 15/21] Merge the message details row semantics so TalkBack offers its copy action --- .../MessageDetailsCopyActionA11yTest.kt | 109 ++++++++++++++++++ .../MessageDetailsCopyActionTest.kt | 73 ++++++++++++ .../MessageDetailsComponents.kt | 2 +- 3 files changed, 183 insertions(+), 1 deletion(-) create mode 100644 app/src/androidTest/kotlin/com/android/messaging/ui/conversation/messagedetails/MessageDetailsCopyActionA11yTest.kt create mode 100644 app/src/test/kotlin/com/android/messaging/ui/conversation/messagedetails/MessageDetailsCopyActionTest.kt diff --git a/app/src/androidTest/kotlin/com/android/messaging/ui/conversation/messagedetails/MessageDetailsCopyActionA11yTest.kt b/app/src/androidTest/kotlin/com/android/messaging/ui/conversation/messagedetails/MessageDetailsCopyActionA11yTest.kt new file mode 100644 index 000000000..a0c099a5b --- /dev/null +++ b/app/src/androidTest/kotlin/com/android/messaging/ui/conversation/messagedetails/MessageDetailsCopyActionA11yTest.kt @@ -0,0 +1,109 @@ +package com.android.messaging.ui.conversation.messagedetails + +import android.view.accessibility.AccessibilityNodeInfo +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.junit4.v2.createAndroidComposeRule +import androidx.test.platform.app.InstrumentationRegistry +import com.android.messaging.R +import com.android.messaging.ui.core.AppTheme +import org.junit.Assert.assertTrue +import org.junit.Rule +import org.junit.Test + +internal class MessageDetailsCopyActionA11yTest { + + @get:Rule + val composeTestRule = createAndroidComposeRule() + + @Test + fun copyActionSitsOnAScreenReaderFocusableNode() { + composeTestRule.setContent { + AppTheme { + MessageDetailsStatusSection( + sentTimestamp = null, + receivedTimestamp = RECEIVED_TIMESTAMP, + onCopy = {}, + ) + } + } + composeTestRule.waitForIdle() + + val context = InstrumentationRegistry.getInstrumentation().targetContext + val copyLabel = context.getString(R.string.copy_to_clipboard) + val receivedLabel = context.getString(R.string.message_details_received_label) + + val nodesOfferingCopy = awaitNodesOfferingCopy(copyLabel = copyLabel) + + assertTrue( + "No accessibility node exposes the \"$copyLabel\" custom action at all", + nodesOfferingCopy.isNotEmpty(), + ) + + val focusableNodes = nodesOfferingCopy.filter { it.isScreenReaderFocusable } + + assertTrue( + "The \"$copyLabel\" action exists on ${nodesOfferingCopy.size} node(s) but none is " + + "screen reader focusable, so TalkBack never offers it: " + + nodesOfferingCopy.joinToString { it.describe() }, + focusableNodes.isNotEmpty(), + ) + + val receivedValue = formatMessageDetailsTimestamp(timestampMillis = RECEIVED_TIMESTAMP) + + assertTrue( + "The focusable node offering \"$copyLabel\" announces nothing about the row it " + + "copies: " + focusableNodes.joinToString { it.dumpSubtree() }, + focusableNodes.any { node -> + node.subtreeText().let { + it.contains(receivedLabel) && it.contains(receivedValue) + } + }, + ) + } + + private fun AccessibilityNodeInfo.subtreeText(): String { + return collectNodes().joinToString(separator = " ") { node -> + listOfNotNull(node.text, node.contentDescription).joinToString(separator = " ") + } + } + + private fun AccessibilityNodeInfo.dumpSubtree(): String { + return collectNodes().joinToString(separator = " / ") { it.describe() } + } + + private fun awaitNodesOfferingCopy(copyLabel: String): List { + val uiAutomation = InstrumentationRegistry.getInstrumentation().uiAutomation + repeat(A11Y_TREE_POLL_ATTEMPTS) { + val root = uiAutomation.rootInActiveWindow + val matches = root?.collectNodes().orEmpty().filter { node -> + node.actionList.any { it.label == copyLabel } + } + if (matches.isNotEmpty()) { + return matches + } + Thread.sleep(A11Y_TREE_POLL_INTERVAL_MILLIS) + } + return emptyList() + } + + private fun AccessibilityNodeInfo.collectNodes(): List { + return buildList { + add(this@collectNodes) + repeat(childCount) { index -> + getChild(index)?.let { addAll(it.collectNodes()) } + } + } + } + + private fun AccessibilityNodeInfo.describe(): String { + return "[text=$text, contentDescription=$contentDescription, " + + "screenReaderFocusable=$isScreenReaderFocusable, " + + "actions=${actionList.mapNotNull { it.label }}]" + } + + private companion object { + private const val RECEIVED_TIMESTAMP = 1_700_000_000_000L + private const val A11Y_TREE_POLL_ATTEMPTS = 20 + private const val A11Y_TREE_POLL_INTERVAL_MILLIS = 250L + } +} diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/messagedetails/MessageDetailsCopyActionTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/messagedetails/MessageDetailsCopyActionTest.kt new file mode 100644 index 000000000..9b474e399 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/messagedetails/MessageDetailsCopyActionTest.kt @@ -0,0 +1,73 @@ +package com.android.messaging.ui.conversation.messagedetails + +import androidx.compose.ui.semantics.SemanticsActions +import androidx.compose.ui.semantics.getOrNull +import androidx.compose.ui.test.junit4.v2.createComposeRule +import androidx.compose.ui.test.onNodeWithText +import com.android.common.test.helpers.targetContext +import com.android.messaging.FactoryTestAccess +import com.android.messaging.R +import com.android.messaging.testutil.installTestFactory +import com.android.messaging.ui.core.AppTheme +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +internal class MessageDetailsCopyActionTest { + + @get:Rule + val composeTestRule = createComposeRule() + + @Before + fun setUp() { + installTestFactory(context = targetContext) + } + + @After + fun tearDown() { + FactoryTestAccess.reset() + } + + @Test + fun copyActionIsOfferedOnTheNodeAScreenReaderFocuses() { + val copied = mutableListOf() + + composeTestRule.setContent { + AppTheme { + MessageDetailsStatusSection( + sentTimestamp = null, + receivedTimestamp = RECEIVED_TIMESTAMP, + onCopy = { copied += it }, + ) + } + } + + val label = targetContext.getString(R.string.message_details_received_label) + val focusedNode = composeTestRule.onNodeWithText(label).fetchSemanticsNode() + + val customActions = focusedNode.config.getOrNull(SemanticsActions.CustomActions) + assertNotNull("No custom action on the node holding the row text", customActions) + + assertEquals( + targetContext.getString(R.string.copy_to_clipboard), + customActions!!.single().label, + ) + + customActions.single().action() + + assertEquals( + listOf(formatMessageDetailsTimestamp(timestampMillis = RECEIVED_TIMESTAMP)), + copied, + ) + } + + private companion object { + private const val RECEIVED_TIMESTAMP = 1_700_000_000_000L + } +} diff --git a/src/com/android/messaging/ui/conversation/messagedetails/MessageDetailsComponents.kt b/src/com/android/messaging/ui/conversation/messagedetails/MessageDetailsComponents.kt index 1a48d8838..a623074e9 100644 --- a/src/com/android/messaging/ui/conversation/messagedetails/MessageDetailsComponents.kt +++ b/src/com/android/messaging/ui/conversation/messagedetails/MessageDetailsComponents.kt @@ -298,7 +298,7 @@ private fun Modifier.copyOnLongPress( .pointerInput(value) { detectTapGestures(onLongPress = { onCopy(value) }) } - .semantics { + .semantics(mergeDescendants = true) { customActions = listOf( CustomAccessibilityAction(label = copyLabel) { onCopy(value) From 4dde9b0f656d06458a43679cd1d15dbdc09928c2 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Mon, 14 Sep 2026 22:56:38 +0300 Subject: [PATCH 16/21] Announce who sent a message so screen readers can tell incoming from outgoing --- .../ConversationMessageSenderA11yTest.kt | 123 ++++++++++++++++ .../ConversationMessageSenderSemanticsTest.kt | 132 ++++++++++++++++++ .../ui/message/ConversationMessageRows.kt | 41 ++++++ 3 files changed, 296 insertions(+) create mode 100644 app/src/androidTest/kotlin/com/android/messaging/ui/conversation/messages/ui/message/rendering/ConversationMessageSenderA11yTest.kt create mode 100644 app/src/test/kotlin/com/android/messaging/ui/conversation/messages/ui/message/rendering/ConversationMessageSenderSemanticsTest.kt diff --git a/app/src/androidTest/kotlin/com/android/messaging/ui/conversation/messages/ui/message/rendering/ConversationMessageSenderA11yTest.kt b/app/src/androidTest/kotlin/com/android/messaging/ui/conversation/messages/ui/message/rendering/ConversationMessageSenderA11yTest.kt new file mode 100644 index 000000000..2c67f7e93 --- /dev/null +++ b/app/src/androidTest/kotlin/com/android/messaging/ui/conversation/messages/ui/message/rendering/ConversationMessageSenderA11yTest.kt @@ -0,0 +1,123 @@ +package com.android.messaging.ui.conversation.messages.ui.message.rendering + +import android.view.accessibility.AccessibilityNodeInfo +import androidx.test.ext.junit.runners.AndroidJUnit4 +import androidx.test.platform.app.InstrumentationRegistry +import com.android.messaging.R +import com.android.messaging.ui.conversation.messages.model.message.ConversationMessageUiModel +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +internal class ConversationMessageSenderA11yTest : BaseConversationMessageRenderingTest() { + + @Test + fun incomingMessageIsAnnouncedWithItsSender() { + setConversationMessageContent( + message = message( + text = INCOMING_TEXT, + status = ConversationMessageUiModel.Status.Incoming.Complete, + isIncoming = true, + senderDisplayName = SENDER_DISPLAY_NAME, + ), + showIncomingParticipantIdentity = false, + ) + + assertAnnouncedWithTheBody( + body = INCOMING_TEXT, + announcement = string( + resourceId = R.string.incoming_sender_content_description, + SENDER_DISPLAY_NAME, + ), + ) + } + + @Test + fun outgoingMessageIsAnnouncedAsSentByTheUser() { + setConversationMessageContent(message = message(text = OUTGOING_TEXT)) + + assertAnnouncedWithTheBody( + body = OUTGOING_TEXT, + announcement = string(resourceId = R.string.outgoing_sender_content_description), + ) + } + + private fun assertAnnouncedWithTheBody(body: String, announcement: String) { + composeTestRule.waitForIdle() + + val bubbleNodes = awaitFocusableNodesSpeaking(text = body) + + assertTrue( + "No screen reader focusable node speaks the message body \"$body\" at all. Tree: " + + dumpTree(), + bubbleNodes.isNotEmpty(), + ) + + assertTrue( + "TalkBack cannot tell who sent this message: the focusable node speaking \"$body\" " + + "never says \"$announcement\". Nodes: " + + bubbleNodes.joinToString { it.dumpSubtree() }, + bubbleNodes.any { node -> node.subtreeText().contains(announcement) }, + ) + } + + /** The bubbles TalkBack would stop on and read the body out from. */ + private fun awaitFocusableNodesSpeaking(text: String): List { + val uiAutomation = InstrumentationRegistry.getInstrumentation().uiAutomation + repeat(A11Y_TREE_POLL_ATTEMPTS) { + val matches = uiAutomation.rootInActiveWindow?.collectNodes().orEmpty().filter { node -> + node.isScreenReaderFocusable && node.subtreeText().contains(text) + } + if (matches.isNotEmpty()) { + return matches + } + Thread.sleep(A11Y_TREE_POLL_INTERVAL_MILLIS) + } + return emptyList() + } + + private fun dumpTree(): String { + val uiAutomation = InstrumentationRegistry.getInstrumentation().uiAutomation + return uiAutomation.rootInActiveWindow?.dumpSubtree() ?: "" + } + + private fun AccessibilityNodeInfo.subtreeText(): String { + return collectNodes().joinToString(separator = " ") { node -> + listOfNotNull(node.text, node.contentDescription).joinToString(separator = " ") + } + } + + private fun AccessibilityNodeInfo.dumpSubtree(): String { + return collectNodes().joinToString(separator = " / ") { it.describe() } + } + + private fun AccessibilityNodeInfo.collectNodes(): List { + return buildList { + add(this@collectNodes) + repeat(childCount) { index -> + getChild(index)?.let { addAll(it.collectNodes()) } + } + } + } + + private fun AccessibilityNodeInfo.describe(): String { + return "[text=$text, contentDescription=$contentDescription, " + + "screenReaderFocusable=$isScreenReaderFocusable]" + } + + private fun string(resourceId: Int, vararg formatArgs: Any): String { + return InstrumentationRegistry + .getInstrumentation() + .targetContext + .getString(resourceId, *formatArgs) + } + + private companion object { + private const val INCOMING_TEXT = "Can you review this before tonight?" + private const val OUTGOING_TEXT = "I am on my way." + private const val SENDER_DISPLAY_NAME = "Ada Lovelace" + private const val A11Y_TREE_POLL_ATTEMPTS = 20 + private const val A11Y_TREE_POLL_INTERVAL_MILLIS = 250L + } +} diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/ui/message/rendering/ConversationMessageSenderSemanticsTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/ui/message/rendering/ConversationMessageSenderSemanticsTest.kt new file mode 100644 index 000000000..c53657421 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/ui/conversation/messages/ui/message/rendering/ConversationMessageSenderSemanticsTest.kt @@ -0,0 +1,132 @@ +package com.android.messaging.ui.conversation.messages.ui.message.rendering + +import androidx.compose.ui.semantics.SemanticsProperties +import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.text.AnnotatedString +import com.android.common.test.helpers.targetContext +import com.android.messaging.R +import com.android.messaging.data.conversation.model.MessageId +import com.android.messaging.ui.conversation.conversationMessageBubbleTestTag +import com.android.messaging.ui.conversation.messages.model.message.ConversationMessageUiModel +import org.junit.Assert.assertEquals +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +internal class ConversationMessageSenderSemanticsTest : BaseConversationMessageRenderingTest() { + + @Test + fun incomingMessageWithoutAVisibleSenderIsStillAnnouncedWithOne() { + setConversationMessageContent( + message = incomingMessage(), + showIncomingParticipantIdentity = false, + ) + + assertEquals( + listOf( + AnnotatedString(text = incomingAnnouncement(sender = SENDER_DISPLAY_NAME)), + AnnotatedString(text = DEFAULT_BODY_TEXT), + ), + bubbleText(), + ) + } + + @Test + fun incomingMessageWithoutASenderNameFallsBackToTheUnknownSender() { + setConversationMessageContent( + message = incomingMessage(senderDisplayName = null), + showIncomingParticipantIdentity = false, + ) + + assertEquals( + listOf( + AnnotatedString( + text = incomingAnnouncement( + sender = targetContext.getString(R.string.unknown_sender), + ), + ), + AnnotatedString(text = DEFAULT_BODY_TEXT), + ), + bubbleText(), + ) + } + + @Test + fun clusteredIncomingMessageIsAnnouncedWithTheSenderItNoLongerShows() { + setConversationMessageContent( + message = incomingMessage(canClusterWithPrevious = true), + showIncomingParticipantIdentity = true, + ) + + assertEquals( + listOf( + AnnotatedString(text = incomingAnnouncement(sender = SENDER_DISPLAY_NAME)), + AnnotatedString(text = DEFAULT_BODY_TEXT), + ), + bubbleText(), + ) + } + + @Test + fun visibleSenderNameIsNotAnnouncedTwice() { + setConversationMessageContent( + message = incomingMessage(), + showIncomingParticipantIdentity = true, + ) + + assertEquals( + listOf( + AnnotatedString(text = SENDER_DISPLAY_NAME), + AnnotatedString(text = DEFAULT_BODY_TEXT), + ), + bubbleText(), + ) + } + + @Test + fun outgoingMessageIsAnnouncedAsSentByTheUser() { + setConversationMessageContent(message = message(isIncoming = false)) + + assertEquals( + listOf( + AnnotatedString( + text = targetContext.getString(R.string.outgoing_sender_content_description), + ), + AnnotatedString(text = DEFAULT_BODY_TEXT), + ), + bubbleText(), + ) + } + + private fun incomingMessage( + senderDisplayName: String? = SENDER_DISPLAY_NAME, + canClusterWithPrevious: Boolean = false, + ): ConversationMessageUiModel { + return message( + status = ConversationMessageUiModel.Status.Incoming.Complete, + isIncoming = true, + senderDisplayName = senderDisplayName, + canClusterWithPrevious = canClusterWithPrevious, + ) + } + + private fun incomingAnnouncement(sender: String): String { + return targetContext.getString(R.string.incoming_sender_content_description, sender) + } + + private fun bubbleText(): List { + return composeTestRule + .onNodeWithTag( + testTag = conversationMessageBubbleTestTag( + messageId = MessageId(DEFAULT_MESSAGE_ID), + ), + ) + .fetchSemanticsNode() + .config[SemanticsProperties.Text] + } + + private companion object { + private const val SENDER_DISPLAY_NAME = "Ada Lovelace" + } +} diff --git a/src/com/android/messaging/ui/conversation/messages/ui/message/ConversationMessageRows.kt b/src/com/android/messaging/ui/conversation/messages/ui/message/ConversationMessageRows.kt index 08b80b9d9..020dcdbfd 100644 --- a/src/com/android/messaging/ui/conversation/messages/ui/message/ConversationMessageRows.kt +++ b/src/com/android/messaging/ui/conversation/messages/ui/message/ConversationMessageRows.kt @@ -18,13 +18,17 @@ import androidx.compose.ui.draw.clip import androidx.compose.ui.hapticfeedback.HapticFeedbackType import androidx.compose.ui.platform.LocalHapticFeedback import androidx.compose.ui.platform.testTag +import androidx.compose.ui.res.stringResource import androidx.compose.ui.semantics.Role import androidx.compose.ui.semantics.role import androidx.compose.ui.semantics.selected import androidx.compose.ui.semantics.semantics +import androidx.compose.ui.semantics.text +import androidx.compose.ui.text.AnnotatedString import androidx.compose.ui.tooling.preview.PreviewLightDark import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp +import com.android.messaging.R import com.android.messaging.data.conversation.model.MessageId import com.android.messaging.ui.conversation.conversationMessageBubbleTestTag import com.android.messaging.ui.conversation.conversationMessageSelectionRowTestTag @@ -245,12 +249,17 @@ private fun Modifier.conversationMessageBubbleInteractionModifier( onMessageResendClick: () -> Unit, ): Modifier { val hapticFeedback = LocalHapticFeedback.current + val senderAnnouncement = conversationMessageSenderAnnouncement( + message = message, + isSenderLabelVisible = layout.showSender, + ) val bubbleModifier = this .testTag( tag = conversationMessageBubbleTestTag( messageId = message.messageId, ), ) + .conversationMessageSenderSemantics(announcement = senderAnnouncement) .clip(shape = layout.bubbleShape) return when { @@ -274,6 +283,38 @@ private fun Modifier.conversationMessageBubbleInteractionModifier( } } +@Composable +private fun conversationMessageSenderAnnouncement( + message: ConversationMessageUiModel, + isSenderLabelVisible: Boolean, +): String? { + return when { + !message.isIncoming -> stringResource(id = R.string.outgoing_sender_content_description) + isSenderLabelVisible -> null + + else -> { + stringResource( + id = R.string.incoming_sender_content_description, + message + .senderDisplayName + ?.takeIf(String::isNotBlank) + ?: stringResource(id = R.string.unknown_sender), + ) + } + } +} + +private fun Modifier.conversationMessageSenderSemantics(announcement: String?): Modifier { + return when { + announcement == null -> this + else -> { + semantics { + text = AnnotatedString(text = announcement) + } + } + } +} + @Composable internal fun ConversationMessageMetadataRow( message: ConversationMessageUiModel, From a6e0cbe142cdb4f9e2b075dc3ca4a447c5e9fa32 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Tue, 15 Sep 2026 00:25:11 +0300 Subject: [PATCH 17/21] Drop the doubled bottom margin under the New group button --- .../ui/conversation/entry/NewGroupButton.kt | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/com/android/messaging/ui/conversation/entry/NewGroupButton.kt b/src/com/android/messaging/ui/conversation/entry/NewGroupButton.kt index 83e7f26dd..6d8d5b8fd 100644 --- a/src/com/android/messaging/ui/conversation/entry/NewGroupButton.kt +++ b/src/com/android/messaging/ui/conversation/entry/NewGroupButton.kt @@ -13,11 +13,9 @@ import androidx.compose.animation.fadeIn import androidx.compose.animation.fadeOut import androidx.compose.animation.shrinkVertically import androidx.compose.animation.slideInVertically -import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.fillMaxWidth -import androidx.compose.foundation.layout.height +import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size import androidx.compose.material.icons.Icons import androidx.compose.material.icons.rounded.Group @@ -49,15 +47,12 @@ internal fun NewChatCreateGroupHeader( enter = newGroupButtonEnterTransition(), exit = newGroupButtonExitTransition(), ) { - Column( - verticalArrangement = Arrangement.spacedBy(space = 12.dp), - ) { - NewGroupButton( - modifier = Modifier.fillMaxWidth(), - onClick = onCreateGroupClick, - ) - Spacer(modifier = Modifier.height(12.dp)) - } + NewGroupButton( + modifier = Modifier + .fillMaxWidth() + .padding(bottom = 10.dp), + onClick = onCreateGroupClick, + ) } } From 0c657e74ea9d7084feda2e2c569ec2b594c7a445 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Tue, 15 Sep 2026 00:25:11 +0300 Subject: [PATCH 18/21] Stop the recipient card losing its bottom padding when chips appear --- .../component/RecipientSelectionQueryCard.kt | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionQueryCard.kt b/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionQueryCard.kt index ad6e1d21a..38b7043bf 100644 --- a/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionQueryCard.kt +++ b/src/com/android/messaging/ui/conversation/recipientpicker/component/RecipientSelectionQueryCard.kt @@ -42,6 +42,10 @@ import com.android.messaging.ui.recipientselection.preview.previewRecipientSelec import kotlinx.collections.immutable.ImmutableList private val recipientSelectionInputRowMinHeight = 32.dp +private val recipientSelectionInputRowPadding = PaddingValues( + horizontal = 16.dp, + vertical = 12.dp, +) internal fun recipientSelectionQueryCardUiState( uiState: RecipientSelectionContentUiState, @@ -232,7 +236,6 @@ private fun RecipientSelectionInputRow( currentKeyboardController.value?.show() } } - val outerPadding = recipientSelectionInputRowPadding(hasChips = recipients.isNotEmpty()) BoxWithConstraints( modifier = Modifier @@ -241,7 +244,7 @@ private fun RecipientSelectionInputRow( enabled = queryFieldUiState.enabled, onTap = onInputAreaTap, ) - .padding(paddingValues = outerPadding), + .padding(paddingValues = recipientSelectionInputRowPadding), ) { RecipientSelectionSelectedRecipientChips( recipients = recipients, @@ -276,17 +279,6 @@ private fun RecipientSelectionInputRow( } } -private fun recipientSelectionInputRowPadding(hasChips: Boolean): PaddingValues { - return when { - hasChips -> { - PaddingValues(start = 16.dp, end = 16.dp, top = 12.dp, bottom = 4.dp) - } - else -> { - PaddingValues(horizontal = 16.dp, vertical = 12.dp) - } - } -} - private fun Modifier.recipientSelectionFocusQueryOnUnhandledTap( enabled: Boolean, onTap: () -> Unit, From b978057097e5df26767082125972fc72e8703daf Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Thu, 17 Sep 2026 00:45:04 +0300 Subject: [PATCH 19/21] Stop both screens drawing at once when back interrupts an opening animation --- .../messaging/ui/navigation/AppNavDisplay.kt | 6 +++ .../ui/navigation/EnteringBackDecorator.kt | 43 +++++++++++++++++++ 2 files changed, 49 insertions(+) create mode 100644 src/com/android/messaging/ui/navigation/EnteringBackDecorator.kt diff --git a/src/com/android/messaging/ui/navigation/AppNavDisplay.kt b/src/com/android/messaging/ui/navigation/AppNavDisplay.kt index d05c9ff02..b5690f546 100644 --- a/src/com/android/messaging/ui/navigation/AppNavDisplay.kt +++ b/src/com/android/messaging/ui/navigation/AppNavDisplay.kt @@ -29,12 +29,17 @@ internal fun AppNavDisplay( val listDetailPaneDecorator = rememberListDetailPaneNavEntryDecorator(showsTwoPanes) val displayCornerDecorator = rememberDisplayCornerNavEntryDecorator() val paneTitleDecorator = rememberPaneTitleNavEntryDecorator() + val enteringBackDecorator = rememberEnteringBackNavEntryDecorator( + canPop = backStack.size > 1, + onBack = onBack, + ) val entryDecorators = remember( saveableStateHolderDecorator, viewModelStoreDecorator, listDetailPaneDecorator, displayCornerDecorator, paneTitleDecorator, + enteringBackDecorator, ) { listOf( saveableStateHolderDecorator, @@ -42,6 +47,7 @@ internal fun AppNavDisplay( listDetailPaneDecorator, displayCornerDecorator, paneTitleDecorator, + enteringBackDecorator, ) } diff --git a/src/com/android/messaging/ui/navigation/EnteringBackDecorator.kt b/src/com/android/messaging/ui/navigation/EnteringBackDecorator.kt new file mode 100644 index 000000000..7fd7f6773 --- /dev/null +++ b/src/com/android/messaging/ui/navigation/EnteringBackDecorator.kt @@ -0,0 +1,43 @@ +package com.android.messaging.ui.navigation + +import androidx.activity.compose.BackHandler +import androidx.compose.animation.EnterExitState +import androidx.compose.runtime.Composable +import androidx.compose.runtime.getValue +import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberUpdatedState +import androidx.navigation3.runtime.NavEntryDecorator +import androidx.navigation3.runtime.NavKey +import androidx.navigation3.ui.LocalNavAnimatedContentScope + +/** + * Falls back to ordinary back while an entry is becoming visible. + * + * Predictive back can reuse a cached forward enter transition when it interrupts navigation, + * producing mixed animations. Intercepting it until the entry settles avoids that artifact. + * Root back remains handled by the system. + */ +@Composable +internal fun rememberEnteringBackNavEntryDecorator( + canPop: Boolean, + onBack: () -> Unit, +): NavEntryDecorator { + val currentCanPop by rememberUpdatedState(canPop) + val currentOnBack by rememberUpdatedState(onBack) + + return remember { + NavEntryDecorator { entry -> + val transition = LocalNavAnimatedContentScope.current.transition + + BackHandler( + enabled = currentCanPop && + transition.targetState == EnterExitState.Visible && + transition.currentState != EnterExitState.Visible, + ) { + currentOnBack() + } + + entry.Content() + } + } +} From 2b2fa0c60c993ec0676de21edd1004c273957313 Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Thu, 17 Sep 2026 21:55:51 +0300 Subject: [PATCH 20/21] Map conversation list rows off the main thread --- .../chats/ConversationListViewModelTest.kt | 1 + .../ArchivedConversationListViewModel.kt | 25 ++++++++++++------- .../chats/ConversationListViewModel.kt | 21 ++++++++++------ 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/app/src/test/kotlin/com/android/messaging/ui/conversationlist/chats/ConversationListViewModelTest.kt b/app/src/test/kotlin/com/android/messaging/ui/conversationlist/chats/ConversationListViewModelTest.kt index 6c1642ce7..1e4f0e9f4 100644 --- a/app/src/test/kotlin/com/android/messaging/ui/conversationlist/chats/ConversationListViewModelTest.kt +++ b/app/src/test/kotlin/com/android/messaging/ui/conversationlist/chats/ConversationListViewModelTest.kt @@ -261,6 +261,7 @@ class ConversationListViewModelTest { optimisticSnapshotDelegate = optimisticSnapshotDelegate, debugFeaturesProvider = debugFeaturesProvider, resolveContactAction = resolveContactAction, + defaultDispatcher = mainDispatcherRule.testDispatcher, ) } } diff --git a/src/com/android/messaging/ui/conversationlist/archived/ArchivedConversationListViewModel.kt b/src/com/android/messaging/ui/conversationlist/archived/ArchivedConversationListViewModel.kt index 066355ded..ba5630db7 100644 --- a/src/com/android/messaging/ui/conversationlist/archived/ArchivedConversationListViewModel.kt +++ b/src/com/android/messaging/ui/conversationlist/archived/ArchivedConversationListViewModel.kt @@ -7,6 +7,7 @@ import com.android.messaging.data.conversationlist.model.ConversationListItem import com.android.messaging.data.conversationlist.model.ConversationListMode import com.android.messaging.data.conversationlist.model.ConversationListSnapshot import com.android.messaging.data.debug.DebugFeaturesProvider +import com.android.messaging.di.core.DefaultDispatcher import com.android.messaging.domain.conversation.usecase.participant.ResolveContactAction import com.android.messaging.domain.conversation.usecase.participant.model.ResolveContactActionResult import com.android.messaging.ui.contact.model.AddContactRequest @@ -22,6 +23,7 @@ import com.android.messaging.ui.conversationlist.model.ConversationListAvatarUiM import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.collections.immutable.toImmutableList +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted @@ -31,6 +33,7 @@ import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.drop import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn @@ -52,6 +55,8 @@ internal class ArchivedConversationListViewModel @Inject constructor( uiStateMapper: ArchivedConversationListUiStateMapper, private val resolveContactAction: ResolveContactAction, debugFeaturesProvider: DebugFeaturesProvider, + @param:DefaultDispatcher + private val defaultDispatcher: CoroutineDispatcher, ) : ViewModel(), ArchivedConversationListScreenModel { @@ -74,22 +79,24 @@ internal class ArchivedConversationListViewModel @Inject constructor( selectedConversationIds = selectedIds, isDebugEnabled = isDebugEnabled, ) - }.stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed( - stopTimeoutMillis = STATEFLOW_STOP_TIMEOUT_MILLIS, - ), - initialValue = State(), - ) + } + .flowOn(defaultDispatcher) + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed( + stopTimeoutMillis = STATEFLOW_STOP_TIMEOUT_MILLIS, + ), + initialValue = State(), + ) init { optimisticSnapshotDelegate.bind( scope = viewModelScope, - mode = ConversationListMode.Archived + mode = ConversationListMode.Archived, ) selectionDelegate.bind( scope = viewModelScope, - snapshot = snapshot + snapshot = snapshot, ) viewModelScope.launch { diff --git a/src/com/android/messaging/ui/conversationlist/chats/ConversationListViewModel.kt b/src/com/android/messaging/ui/conversationlist/chats/ConversationListViewModel.kt index f0430b441..08d461354 100644 --- a/src/com/android/messaging/ui/conversationlist/chats/ConversationListViewModel.kt +++ b/src/com/android/messaging/ui/conversationlist/chats/ConversationListViewModel.kt @@ -9,6 +9,7 @@ import com.android.messaging.data.conversationlist.model.ConversationListSnapsho import com.android.messaging.data.conversationlist.repository.ConversationListRepository import com.android.messaging.data.conversationsettings.model.SnoozeOption import com.android.messaging.data.debug.DebugFeaturesProvider +import com.android.messaging.di.core.DefaultDispatcher import com.android.messaging.domain.conversation.usecase.participant.ResolveContactAction import com.android.messaging.domain.conversation.usecase.participant.model.ResolveContactActionResult import com.android.messaging.ui.contact.model.AddContactRequest @@ -24,6 +25,7 @@ import com.android.messaging.ui.conversationlist.model.ConversationListAvatarUiM import dagger.hilt.android.lifecycle.HiltViewModel import javax.inject.Inject import kotlinx.collections.immutable.toImmutableList +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow @@ -31,6 +33,7 @@ import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.filterNotNull +import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.receiveAsFlow import kotlinx.coroutines.flow.stateIn import kotlinx.coroutines.launch @@ -52,6 +55,8 @@ internal class ConversationListViewModel @Inject constructor( private val optimisticSnapshotDelegate: ConversationListOptimisticSnapshotDelegate, private val resolveContactAction: ResolveContactAction, private val debugFeaturesProvider: DebugFeaturesProvider, + @param:DefaultDispatcher + private val defaultDispatcher: CoroutineDispatcher, ) : ViewModel(), ConversationListScreenModel { @@ -81,13 +86,15 @@ internal class ConversationListViewModel @Inject constructor( isScrollToTopVisible = isScrollToTopVisible, isDebugEnabled = isDebugEnabled, ) - }.stateIn( - scope = viewModelScope, - started = SharingStarted.WhileSubscribed( - stopTimeoutMillis = STATEFLOW_STOP_TIMEOUT_MILLIS, - ), - initialValue = State(), - ) + } + .flowOn(defaultDispatcher) + .stateIn( + scope = viewModelScope, + started = SharingStarted.WhileSubscribed( + stopTimeoutMillis = STATEFLOW_STOP_TIMEOUT_MILLIS, + ), + initialValue = State(), + ) init { optimisticSnapshotDelegate.bind( From ea49e30e17bbf9997d40ba2171cb8fad8358789c Mon Sep 17 00:00:00 2001 From: Artem Smirnov Date: Fri, 18 Sep 2026 16:59:02 +0300 Subject: [PATCH 21/21] Fix dark theme for Debug menu --- src/com/android/messaging/util/DebugUtils.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/com/android/messaging/util/DebugUtils.java b/src/com/android/messaging/util/DebugUtils.java index 373060989..2f083d773 100644 --- a/src/com/android/messaging/util/DebugUtils.java +++ b/src/com/android/messaging/util/DebugUtils.java @@ -31,7 +31,7 @@ import androidx.appcompat.app.AlertDialog; -import static androidx.appcompat.R.style.Theme_AppCompat_Light_Dialog_Alert; +import static androidx.appcompat.R.style.Theme_AppCompat_DayNight_Dialog_Alert; import com.android.messaging.Factory; import com.android.messaging.R; @@ -69,7 +69,7 @@ public class DebugUtils { public static final int DEBUG_SOUND_SERVER_REQUEST = 0; public static final int DEBUG_SOUND_DB_OP = 1; - private static final int DIALOG_THEME = Theme_AppCompat_Light_Dialog_Alert; + private static final int DIALOG_THEME = Theme_AppCompat_DayNight_Dialog_Alert; public static void maybePlayDebugNoise(final Context context, final int sound) { if (sDebugNoise) {