diff options
17 files changed, 1058 insertions, 103 deletions
diff --git a/packages/SettingsLib/DataStore/Android.bp b/packages/SettingsLib/DataStore/Android.bp index 868a4a50577e..9fafcabad81b 100644 --- a/packages/SettingsLib/DataStore/Android.bp +++ b/packages/SettingsLib/DataStore/Android.bp @@ -11,6 +11,8 @@ android_library { static_libs: [ "androidx.annotation_annotation", "androidx.collection_collection-ktx", + "androidx.core_core-ktx", "guava", ], + kotlincflags: ["-Xjvm-default=all"], } diff --git a/packages/SettingsLib/DataStore/README.md b/packages/SettingsLib/DataStore/README.md new file mode 100644 index 000000000000..30cb9932f104 --- /dev/null +++ b/packages/SettingsLib/DataStore/README.md @@ -0,0 +1,164 @@ +# Datastore library + +This library aims to manage datastore in a consistent way. + +## Overview + +A datastore is required to extend the `BackupRestoreStorage` class and implement +either `Observable` or `KeyedObservable` interface, which enforces: + +- Backup and restore: Datastore should support + [data backup](https://developer.android.com/guide/topics/data/backup) to + preserve user experiences on a new device. +- Observer pattern: The + [observer pattern](https://en.wikipedia.org/wiki/Observer_pattern) allows to + monitor data change in the datastore and + - trigger + [BackupManager.dataChanged](https://developer.android.com/reference/android/app/backup/BackupManager#dataChanged\(\)) + automatically. + - track data change event to log metrics. + - update internal state and take action. + +### Backup and restore + +The Android backup framework provides +[BackupAgentHelper](https://developer.android.com/reference/android/app/backup/BackupAgentHelper) +and +[BackupHelper](https://developer.android.com/reference/android/app/backup/BackupHelper) +to back up a datastore. However, there are several caveats when implement +`BackupHelper`: + +- performBackup: The data is updated incrementally but it is not well + documented. The `ParcelFileDescriptor` state parameters are normally ignored + and data is updated even there is no change. +- restoreEntity: The implementation must take care not to seek or close the + underlying data source, nor read more than size() bytes from the stream when + restore (see + [BackupDataInputStream](https://developer.android.com/reference/android/app/backup/BackupDataInputStream)). + It is possible a `BackupHelper` prevents other `BackupHelper`s from + restoring data. +- writeNewStateDescription: Existing implementations rarely notice that this + callback is invoked after all entities are restored, and check if necessary + data are all restored in `restoreEntity` (e.g. + [BatteryBackupHelper](https://cs.android.com/android/platform/superproject/main/+/main:packages/apps/Settings/src/com/android/settings/fuelgauge/BatteryBackupHelper.java;l=144;drc=cca804e1ed504e2d477be1e3db00fb881ca32736)), + which is not robust sometimes. + +This library provides more clear API and offers some improvements: + +- The implementation only needs to focus on the `BackupRestoreEntity` + interface. The `InputStream` of restore will ensure bounded data are read, + and close the stream will be no-op. +- The library computes checksum of the backup data automatically, so that + unchanged data will not be sent to Android backup system. +- Data compression is supported: + - ZIP best compression is enabled by default, no extra effort needs to be + taken. + - It is safe to switch between compression and no compression in future, + the backup data will add 1 byte header to recognize the codec. + - To support other compression algorithms, simply wrap over the + `InputStream` and `OutputStream`. Actually, the checksum is computed in + this way by + [CheckedInputStream](https://developer.android.com/reference/java/util/zip/CheckedInputStream) + and + [CheckedOutputStream](https://developer.android.com/reference/java/util/zip/CheckedOutputStream), + see `BackupRestoreStorage` implementation for more details. +- Enhanced forward compatibility for file is enabled: If a backup includes + data that didn't exist in earlier versions of the app, the data can still be + successfully restored in those older versions. This is achieved by extending + the `BackupRestoreFileStorage` class, and `BackupRestoreFileArchiver` will + treat each file as an entity and do the backup / restore. +- Manual `BackupManager.dataChanged` call is unnecessary now, the library will + do the invocation (see next section). + +### Observer pattern + +Manual `BackupManager.dataChanged` call is required by current backup framework. +In practice, it is found that `SharedPreferences` usages foget to invoke the +API. Besides, there are common use cases to log metrics when data is changed. +Consequently, observer pattern is employed to resolve the issues. + +If the datastore is key-value based (e.g. `SharedPreferences`), implements the +`KeyedObservable` interface to offer fine-grained observer. Otherwise, +implements `Observable`. The library provides thread-safe implementations +(`KeyedDataObservable` / `DataObservable`), and Kotlin delegation will be +helpful. + +Keep in mind that the implementation should call `KeyedObservable.notifyChange` +/ `Observable.notifyChange` whenever internal data is changed, so that the +registered observer will be notified properly. + +## Usage and example + +For `SharedPreferences` use case, leverage the `SharedPreferencesStorage`. To +back up other file based storage, extend the `BackupRestoreFileStorage` class. + +Here is an example of customized datastore, which has a string to back up: + +```kotlin +class MyDataStore : ObservableBackupRestoreStorage() { + // Another option is make it a StringEntity type and maintain a String field inside StringEntity + @Volatile // backup/restore happens on Binder thread + var data: String? = null + private set + + fun setData(data: String?) { + this.data = data + notifyChange(ChangeReason.UPDATE) + } + + override val name: String + get() = "MyData" + + override fun createBackupRestoreEntities(): List<BackupRestoreEntity> = + listOf(StringEntity("data")) + + private inner class StringEntity(override val key: String) : BackupRestoreEntity { + override fun backup( + backupContext: BackupContext, + outputStream: OutputStream, + ) = + if (data != null) { + outputStream.write(data!!.toByteArray(UTF_8)) + EntityBackupResult.UPDATE + } else { + EntityBackupResult.DELETE + } + + override fun restore(restoreContext: RestoreContext, inputStream: InputStream) { + data = String(inputStream.readAllBytes(), UTF_8) + // NOTE: The library will call notifyChange(ChangeReason.RESTORE) for you + } + } + + override fun onRestoreFinished() { + // TODO: Update state with the restored data. Use this callback instead "restore()" in case + // the restore action involves several entities. + // NOTE: The library will call notifyChange(ChangeReason.RESTORE) for you + } +} +``` + +In the application class: + +```kotlin +class MyApplication : Application() { + override fun onCreate() { + super.onCreate(); + BackupRestoreStorageManager.getInstance(this).add(MyDataStore()); + } +} +``` + +In the custom `BackupAgentHelper` class: + +```kotlin +class MyBackupAgentHelper : BackupAgentHelper() { + override fun onCreate() { + BackupRestoreStorageManager.getInstance(this).addBackupAgentHelpers(this); + } + + override fun onRestoreFinished() { + BackupRestoreStorageManager.getInstance(this).onRestoreFinished(); + } +} +``` diff --git a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupCodec.kt b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupCodec.kt new file mode 100644 index 000000000000..550645fbebef --- /dev/null +++ b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupCodec.kt @@ -0,0 +1,98 @@ +/* + * Copyright (C) 2024 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.settingslib.datastore + +import androidx.annotation.IntDef +import java.io.InputStream +import java.io.OutputStream +import java.util.zip.Deflater +import java.util.zip.DeflaterOutputStream +import java.util.zip.InflaterInputStream + +/** Unique id of the codec. */ +@Target(AnnotationTarget.TYPE) +@IntDef( + BackupCodecId.NO_OP.toInt(), + BackupCodecId.ZIP.toInt(), +) +@Retention(AnnotationRetention.SOURCE) +annotation class BackupCodecId { + companion object { + /** Unknown reason of the change. */ + const val NO_OP: Byte = 0 + /** Data is updated. */ + const val ZIP: Byte = 1 + } +} + +/** How to encode/decode the backup data. */ +interface BackupCodec { + /** Unique id of the codec. */ + val id: @BackupCodecId Byte + + /** Name of the codec. */ + val name: String + + /** Encodes the backup data. */ + fun encode(outputStream: OutputStream): OutputStream + + /** Decodes the backup data. */ + fun decode(inputStream: InputStream): InputStream + + companion object { + @JvmStatic + fun fromId(id: @BackupCodecId Byte): BackupCodec = + when (id) { + BackupCodecId.NO_OP -> BackupNoOpCodec() + BackupCodecId.ZIP -> BackupZipCodec.BEST_COMPRESSION + else -> throw IllegalArgumentException("Unknown codec id $id") + } + } +} + +/** Codec without any additional encoding/decoding. */ +class BackupNoOpCodec : BackupCodec { + override val id + get() = BackupCodecId.NO_OP + + override val name + get() = "N/A" + + override fun encode(outputStream: OutputStream) = outputStream + + override fun decode(inputStream: InputStream) = inputStream +} + +/** Codec with ZIP compression. */ +class BackupZipCodec( + private val compressionLevel: Int, + override val name: String, +) : BackupCodec { + override val id + get() = BackupCodecId.ZIP + + override fun encode(outputStream: OutputStream) = + DeflaterOutputStream(outputStream, Deflater(compressionLevel)) + + override fun decode(inputStream: InputStream) = InflaterInputStream(inputStream) + + companion object { + val DEFAULT_COMPRESSION = BackupZipCodec(Deflater.DEFAULT_COMPRESSION, "ZipDefault") + val BEST_COMPRESSION = BackupZipCodec(Deflater.BEST_COMPRESSION, "ZipBestCompression") + val BEST_SPEED = BackupZipCodec(Deflater.BEST_SPEED, "ZipBestSpeed") + } +} diff --git a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreContext.kt b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreContext.kt index c6d6f772c5df..8fe618dfcc82 100644 --- a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreContext.kt +++ b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreContext.kt @@ -20,7 +20,6 @@ import android.app.backup.BackupAgent import android.app.backup.BackupDataOutput import android.app.backup.BackupHelper import android.os.Build -import android.os.ParcelFileDescriptor import androidx.annotation.RequiresApi /** @@ -31,23 +30,8 @@ import androidx.annotation.RequiresApi */ class BackupContext internal constructor( - /** - * An open, read-only file descriptor pointing to the last backup state provided by the - * application. May be null, in which case no prior state is being provided and the application - * should perform a full backup. - * - * TODO: the state should support marshall/unmarshall for incremental back up. - */ - val oldState: ParcelFileDescriptor?, - /** An open, read/write BackupDataOutput pointing to the backup data destination. */ private val data: BackupDataOutput, - - /** - * An open, read/write file descriptor pointing to an empty file. The application should record - * the final backup. - */ - val newState: ParcelFileDescriptor, ) { /** * The quota in bytes for the application's current backup operation. @@ -68,5 +52,9 @@ internal constructor( @RequiresApi(Build.VERSION_CODES.P) get() = data.transportFlags } -/** Context for restore. */ -class RestoreContext(val key: String) +/** + * Context for restore. + * + * @param key Entity key + */ +class RestoreContext internal constructor(val key: String) diff --git a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreEntity.kt b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreEntity.kt index 6a7ef5a35ac8..817ee4c56b19 100644 --- a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreEntity.kt +++ b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreEntity.kt @@ -36,6 +36,13 @@ interface BackupRestoreEntity { val key: String /** + * Codec used to encode/decode the backup data. + * + * When it is null, the [BackupRestoreStorage.defaultCodec] will be used. + */ + fun codec(): BackupCodec? = null + + /** * Backs up the entity. * * @param backupContext context for backup diff --git a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreFileArchiver.kt b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreFileArchiver.kt new file mode 100644 index 000000000000..9d3fb66c7ce0 --- /dev/null +++ b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreFileArchiver.kt @@ -0,0 +1,123 @@ +/* + * Copyright (C) 2024 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.settingslib.datastore + +import android.app.backup.BackupDataInputStream +import android.content.Context +import android.util.Log +import java.io.File +import java.io.InputStream +import java.io.OutputStream +import java.util.zip.CheckedInputStream + +/** + * File archiver to handle backup and restore for all the [BackupRestoreFileStorage] subclasses. + * + * Compared with [android.app.backup.FileBackupHelper], this class supports forward-compatibility + * like the [com.google.android.libraries.backup.PersistentBackupAgentHelper]: the app does not need + * to know the list of files in advance at restore time. + */ +internal class BackupRestoreFileArchiver( + private val context: Context, + private val fileStorages: List<BackupRestoreFileStorage>, +) : BackupRestoreStorage() { + override val name: String + get() = "file_archiver" + + override fun createBackupRestoreEntities(): List<BackupRestoreEntity> = + fileStorages.map { it.toBackupRestoreEntity() } + + override fun wrapBackupOutputStream(codec: BackupCodec, outputStream: OutputStream) = + outputStream + + override fun wrapRestoreInputStream(codec: BackupCodec, inputStream: InputStream) = inputStream + + override fun restoreEntity(data: BackupDataInputStream) { + val key = data.key + val fileStorage = fileStorages.firstOrNull { it.storageFilePath == key } + val file = + if (fileStorage != null) { + if (!fileStorage.enableRestore()) { + Log.i(LOG_TAG, "[$name] $key restore disabled") + return + } + fileStorage.restoreFile + } else { // forward-compatibility + Log.i(LOG_TAG, "Restore unknown file $key") + File(context.dataDirCompat, key) + } + Log.i(LOG_TAG, "[$name] Restore ${data.size()} bytes for $key to $file") + val inputStream = LimitedNoCloseInputStream(data) + val checksum = createChecksum() + val checkedInputStream = CheckedInputStream(inputStream, checksum) + try { + val codec = BackupCodec.fromId(checkedInputStream.read().toByte()) + if (fileStorage != null && fileStorage.defaultCodec().id != codec.id) { + Log.i( + LOG_TAG, + "[$name] $key different codec: ${codec.id}, ${fileStorage.defaultCodec().id}" + ) + } + file.parentFile?.mkdirs() // ensure parent folders are created + val wrappedInputStream = codec.decode(checkedInputStream) + val bytesCopied = file.outputStream().use { wrappedInputStream.copyTo(it) } + Log.i(LOG_TAG, "[$name] $key restore $bytesCopied bytes with ${codec.name}") + fileStorage?.onRestoreFinished(file) + entityStates[key] = checksum.value + } catch (e: Exception) { + Log.e(LOG_TAG, "[$name] Fail to restore $key", e) + } + } + + override fun onRestoreFinished() { + fileStorages.forEach { it.onRestoreFinished() } + } +} + +private fun BackupRestoreFileStorage.toBackupRestoreEntity() = + object : BackupRestoreEntity { + override val key: String + get() = storageFilePath + + override fun backup( + backupContext: BackupContext, + outputStream: OutputStream, + ): EntityBackupResult { + if (!enableBackup(backupContext)) { + Log.i(LOG_TAG, "[$name] $key backup disabled") + return EntityBackupResult.INTACT + } + val file = backupFile + prepareBackup(file) + if (!file.exists()) { + Log.i(LOG_TAG, "[$name] $key not exist") + return EntityBackupResult.DELETE + } + val codec = codec() ?: defaultCodec() + // MUST close to flush the data + wrapBackupOutputStream(codec, outputStream).use { stream -> + val bytesCopied = file.inputStream().use { it.copyTo(stream) } + Log.i(LOG_TAG, "[$name] $key backup $bytesCopied bytes with ${codec.name}") + } + onBackupFinished(file) + return EntityBackupResult.UPDATE + } + + override fun restore(restoreContext: RestoreContext, inputStream: InputStream) { + // no-op, BackupRestoreFileArchiver#restoreEntity will restore files + } + } diff --git a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreFileStorage.kt b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreFileStorage.kt new file mode 100644 index 000000000000..b531bd141e03 --- /dev/null +++ b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreFileStorage.kt @@ -0,0 +1,83 @@ +/* + * Copyright (C) 2024 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.settingslib.datastore + +import android.content.Context +import androidx.core.content.ContextCompat +import java.io.File + +/** + * A file-based storage with backup and restore support. + * + * [BackupRestoreFileArchiver] will handle the backup and restore based on file path for all + * subclasses. + * + * @param context Context to retrieve data dir + * @param storageFilePath Storage file path, which MUST be relative to the [Context.getDataDir] + * folder. This is used as the entity name for backup and restore. + */ +abstract class BackupRestoreFileStorage( + val context: Context, + val storageFilePath: String, +) : BackupRestoreStorage() { + + /** The absolute path of the file to backup. */ + open val backupFile: File + get() = File(context.dataDirCompat, storageFilePath) + + /** The absolute path of the file to restore. */ + open val restoreFile: File + get() = backupFile + + fun checkFilePaths() { + if (storageFilePath.isEmpty() || storageFilePath[0] == File.separatorChar) { + throw IllegalArgumentException("$storageFilePath is not valid path") + } + if (!backupFile.isAbsolute) { + throw IllegalArgumentException("backupFile is not absolute") + } + if (!restoreFile.isAbsolute) { + throw IllegalArgumentException("restoreFile is not absolute") + } + } + + /** + * Callback before [backupFile] is backed up. + * + * @param file equals to [backupFile] + */ + open fun prepareBackup(file: File) {} + + /** + * Callback when [backupFile] is restored. + * + * @param file equals to [backupFile] + */ + open fun onBackupFinished(file: File) {} + + /** + * Callback when [restoreFile] is restored. + * + * @param file equals to [restoreFile] + */ + open fun onRestoreFinished(file: File) {} + + final override fun createBackupRestoreEntities(): List<BackupRestoreEntity> = listOf() +} + +internal val Context.dataDirCompat: File + get() = ContextCompat.getDataDir(this)!! diff --git a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreStorage.kt b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreStorage.kt index 88d9dd6400ee..c4c00cbb8191 100644 --- a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreStorage.kt +++ b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreStorage.kt @@ -22,11 +22,21 @@ import android.app.backup.BackupDataOutput import android.app.backup.BackupHelper import android.os.ParcelFileDescriptor import android.util.Log +import androidx.collection.MutableScatterMap import com.google.common.io.ByteStreams import java.io.ByteArrayOutputStream +import java.io.DataInputStream +import java.io.DataOutputStream +import java.io.EOFException +import java.io.FileInputStream +import java.io.FileOutputStream import java.io.FilterInputStream import java.io.InputStream import java.io.OutputStream +import java.util.zip.CRC32 +import java.util.zip.CheckedInputStream +import java.util.zip.CheckedOutputStream +import java.util.zip.Checksum internal const val LOG_TAG = "BackupRestoreStorage" @@ -45,87 +55,197 @@ abstract class BackupRestoreStorage : BackupHelper { */ abstract val name: String - private val entities: List<BackupRestoreEntity> by lazy { createBackupRestoreEntities() } + /** + * Entity states represented by checksum. + * + * Map key is the entity key, map value is the checksum of backup data. + */ + protected val entityStates = MutableScatterMap<String, Long>() + + /** Entities created by [createBackupRestoreEntities]. This field is for restore only. */ + private var entities: List<BackupRestoreEntity>? = null /** Entities to back up and restore. */ abstract fun createBackupRestoreEntities(): List<BackupRestoreEntity> - override fun performBackup( + /** Default codec used to encode/decode the entity data. */ + open fun defaultCodec(): BackupCodec = BackupZipCodec.BEST_COMPRESSION + + final override fun performBackup( oldState: ParcelFileDescriptor?, data: BackupDataOutput, newState: ParcelFileDescriptor, ) { - val backupContext = BackupContext(oldState, data, newState) + oldState.readEntityStates(entityStates) + val backupContext = BackupContext(data) if (!enableBackup(backupContext)) { Log.i(LOG_TAG, "[$name] Backup disabled") return } Log.i(LOG_TAG, "[$name] Backup start") + val checksum = createChecksum() + // recreate entities for backup to avoid stale states + val entities = createBackupRestoreEntities() for (entity in entities) { val key = entity.key val outputStream = ByteArrayOutputStream() + checksum.reset() + val checkedOutputStream = CheckedOutputStream(outputStream, checksum) + val codec = entity.codec() ?: defaultCodec() val result = try { - entity.backup(backupContext, wrapBackupOutputStream(outputStream)) + entity.backup(backupContext, wrapBackupOutputStream(codec, checkedOutputStream)) } catch (exception: Exception) { Log.e(LOG_TAG, "[$name] Fail to backup entity $key", exception) continue } when (result) { EntityBackupResult.UPDATE -> { - val payload = outputStream.toByteArray() - val size = payload.size - data.writeEntityHeader(key, size) - data.writeEntityData(payload, size) - Log.i(LOG_TAG, "[$name] Backup entity $key: $size bytes") + val value = checksum.value + if (entityStates.put(key, value) != value) { + val payload = outputStream.toByteArray() + val size = payload.size + data.writeEntityHeader(key, size) + data.writeEntityData(payload, size) + Log.i(LOG_TAG, "[$name] Backup entity $key: $size bytes") + } else { + Log.i( + LOG_TAG, + "[$name] Backup entity $key unchanged: ${outputStream.size()} bytes" + ) + } } EntityBackupResult.INTACT -> { Log.i(LOG_TAG, "[$name] Backup entity $key intact") } EntityBackupResult.DELETE -> { + entityStates.remove(key) data.writeEntityHeader(key, -1) Log.i(LOG_TAG, "[$name] Backup entity $key deleted") } } } + newState.writeAndClearEntityStates() Log.i(LOG_TAG, "[$name] Backup end") } /** Returns if backup is enabled. */ open fun enableBackup(backupContext: BackupContext): Boolean = true - fun wrapBackupOutputStream(outputStream: OutputStream): OutputStream { - return outputStream + open fun wrapBackupOutputStream(codec: BackupCodec, outputStream: OutputStream): OutputStream { + // write a codec id header for safe restore + outputStream.write(codec.id.toInt()) + return codec.encode(outputStream) } + /** This callback is invoked for every backed up entity. */ override fun restoreEntity(data: BackupDataInputStream) { val key = data.key if (!enableRestore()) { Log.i(LOG_TAG, "[$name] Restore disabled, ignore entity $key") return } - val entity = entities.firstOrNull { it.key == key } + val entity = ensureEntities().firstOrNull { it.key == key } if (entity == null) { Log.w(LOG_TAG, "[$name] Cannot find handler for entity $key") return } Log.i(LOG_TAG, "[$name] Restore $key: ${data.size()} bytes") val restoreContext = RestoreContext(key) + val codec = entity.codec() ?: defaultCodec() + val inputStream = LimitedNoCloseInputStream(data) + val checksum = createChecksum() + val checkedInputStream = CheckedInputStream(inputStream, checksum) try { - entity.restore(restoreContext, wrapRestoreInputStream(data)) + entity.restore(restoreContext, wrapRestoreInputStream(codec, checkedInputStream)) + entityStates[key] = checksum.value } catch (exception: Exception) { Log.e(LOG_TAG, "[$name] Fail to restore entity $key", exception) } } + private fun ensureEntities(): List<BackupRestoreEntity> = + entities ?: createBackupRestoreEntities().also { entities = it } + /** Returns if restore is enabled. */ open fun enableRestore(): Boolean = true - fun wrapRestoreInputStream(inputStream: BackupDataInputStream): InputStream { - return LimitedNoCloseInputStream(inputStream) + open fun wrapRestoreInputStream( + codec: BackupCodec, + inputStream: InputStream, + ): InputStream { + // read the codec id first to check if it is expected codec + val id = inputStream.read() + val expectedId = codec.id.toInt() + if (id == expectedId) return codec.decode(inputStream) + Log.i(LOG_TAG, "Expect codec id $expectedId but got $id") + return BackupCodec.fromId(id.toByte()).decode(inputStream) } - override fun writeNewStateDescription(newState: ParcelFileDescriptor) {} + final override fun writeNewStateDescription(newState: ParcelFileDescriptor) { + entities = null // clear to reduce memory footprint + newState.writeAndClearEntityStates() + onRestoreFinished() + } + + /** Callbacks when restore finished. */ + open fun onRestoreFinished() {} + + private fun ParcelFileDescriptor?.readEntityStates(state: MutableScatterMap<String, Long>) { + state.clear() + if (this == null) return + // do not close the streams + val fileInputStream = FileInputStream(fileDescriptor) + val dataInputStream = DataInputStream(fileInputStream) + try { + val version = dataInputStream.readByte() + if (version != STATE_VERSION) { + Log.w( + LOG_TAG, + "[$name] Unexpected state version, read:$version, expected:$STATE_VERSION" + ) + return + } + var count = dataInputStream.readInt() + while (count-- > 0) { + val key = dataInputStream.readUTF() + val checksum = dataInputStream.readLong() + state[key] = checksum + } + } catch (exception: Exception) { + if (exception is EOFException) { + Log.d(LOG_TAG, "[$name] Hit EOF when read state file") + } else { + Log.e(LOG_TAG, "[$name] Fail to read state file", exception) + } + state.clear() + } + } + + private fun ParcelFileDescriptor.writeAndClearEntityStates() { + // do not close the streams + val fileOutputStream = FileOutputStream(fileDescriptor) + val dataOutputStream = DataOutputStream(fileOutputStream) + try { + dataOutputStream.writeByte(STATE_VERSION.toInt()) + dataOutputStream.writeInt(entityStates.size) + entityStates.forEach { key, value -> + dataOutputStream.writeUTF(key) + dataOutputStream.writeLong(value) + } + } catch (exception: Exception) { + Log.e(LOG_TAG, "[$name] Fail to write state file", exception) + } + entityStates.clear() + entityStates.trim() // trim to reduce memory footprint + } + + companion object { + private const val STATE_VERSION: Byte = 0 + + /** Checksum for entity backup data. */ + fun createChecksum(): Checksum = CRC32() + } } /** diff --git a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreStorageManager.kt b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreStorageManager.kt index 221e2e89c33d..cfdcaff4d34c 100644 --- a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreStorageManager.kt +++ b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/BackupRestoreStorageManager.kt @@ -26,32 +26,30 @@ import java.util.concurrent.ConcurrentHashMap /** Manager of [BackupRestoreStorage]. */ class BackupRestoreStorageManager private constructor(private val application: Application) { - private val storages = ConcurrentHashMap<String, BackupRestoreStorage>() + private val storageWrappers = ConcurrentHashMap<String, StorageWrapper>() private val executor = MoreExecutors.directExecutor() - private val observer = Observer { reason -> notifyBackupManager(null, reason) } - - private val keyedObserver = - KeyedObserver<Any?> { key, reason -> notifyBackupManager(key, reason) } - - private fun notifyBackupManager(key: Any?, reason: Int) { - // prefer not triggering backup immediately after restore - if (reason == ChangeReason.RESTORE) return - // TODO: log storage name - Log.d(LOG_TAG, "Notify BackupManager data changed for change: key=$key") - BackupManager.dataChanged(application.packageName) - } - /** * Adds all the registered [BackupRestoreStorage] as the helpers of given [BackupAgentHelper]. * + * All [BackupRestoreFileStorage]s will be wrapped as a single [BackupRestoreFileArchiver]. + * * @see BackupAgentHelper.addHelper */ fun addBackupAgentHelpers(backupAgentHelper: BackupAgentHelper) { - for ((keyPrefix, storage) in storages) { - backupAgentHelper.addHelper(keyPrefix, storage) + val fileStorages = mutableListOf<BackupRestoreFileStorage>() + for ((keyPrefix, storageWrapper) in storageWrappers) { + val storage = storageWrapper.storage + if (storage is BackupRestoreFileStorage) { + fileStorages.add(storage) + } else { + backupAgentHelper.addHelper(keyPrefix, storage) + } } + // Always add file archiver even fileStorages is empty to handle forward compatibility + val fileArchiver = BackupRestoreFileArchiver(application, fileStorages) + backupAgentHelper.addHelper(fileArchiver.name, fileArchiver) } /** @@ -60,15 +58,8 @@ class BackupRestoreStorageManager private constructor(private val application: A * The observers of the storages will be notified. */ fun onRestoreFinished() { - for (storage in storages.values) { - storage.notifyRestoreFinished() - } - } - - private fun BackupRestoreStorage.notifyRestoreFinished() { - when (this) { - is KeyedObservable<*> -> notifyChange(ChangeReason.RESTORE) - is Observable -> notifyChange(ChangeReason.RESTORE) + for (storageWrapper in storageWrappers.values) { + storageWrapper.notifyRestoreFinished() } } @@ -87,51 +78,84 @@ class BackupRestoreStorageManager private constructor(private val application: A * The storage MUST implement [KeyedObservable] or [Observable]. */ fun add(storage: BackupRestoreStorage) { + if (storage is BackupRestoreFileStorage) storage.checkFilePaths() val name = storage.name - val oldStorage = storages.put(name, storage) + val oldStorage = storageWrappers.put(name, StorageWrapper(storage))?.storage if (oldStorage != null) { throw IllegalStateException( "Storage name '$name' conflicts:\n\told: $oldStorage\n\tnew: $storage" ) } - storage.addObserver() - } - - private fun BackupRestoreStorage.addObserver() { - when (this) { - is KeyedObservable<*> -> addObserver(keyedObserver, executor) - is Observable -> addObserver(observer, executor) - else -> - throw IllegalArgumentException( - "$this does not implement either KeyedObservable or Observable" - ) - } } /** Removes all the storages. */ fun removeAll() { - for ((name, _) in storages) remove(name) + for ((name, _) in storageWrappers) remove(name) } /** Removes storage with given name. */ fun remove(name: String): BackupRestoreStorage? { - val storage = storages.remove(name) - storage?.removeObserver() - return storage - } - - private fun BackupRestoreStorage.removeObserver() { - when (this) { - is KeyedObservable<*> -> removeObserver(keyedObserver) - is Observable -> removeObserver(observer) - } + val storageWrapper = storageWrappers.remove(name) + storageWrapper?.removeObserver() + return storageWrapper?.storage } /** Returns storage with given name. */ - fun get(name: String): BackupRestoreStorage? = storages[name] + fun get(name: String): BackupRestoreStorage? = storageWrappers[name]?.storage /** Returns storage with given name, exception is raised if not found. */ - fun getOrThrow(name: String): BackupRestoreStorage = storages[name]!! + fun getOrThrow(name: String): BackupRestoreStorage = storageWrappers[name]!!.storage + + private inner class StorageWrapper(val storage: BackupRestoreStorage) : + Observer, KeyedObserver<Any?> { + init { + when (storage) { + is KeyedObservable<*> -> storage.addObserver(this, executor) + is Observable -> storage.addObserver(this, executor) + else -> + throw IllegalArgumentException( + "$this does not implement either KeyedObservable or Observable" + ) + } + } + + override fun onChanged(reason: Int) = onKeyChanged(null, reason) + + override fun onKeyChanged(key: Any?, reason: Int) { + notifyBackupManager(key, reason) + } + + private fun notifyBackupManager(key: Any?, reason: Int) { + val name = storage.name + // prefer not triggering backup immediately after restore + if (reason == ChangeReason.RESTORE) { + Log.d( + LOG_TAG, + "Notify BackupManager dataChanged ignored for restore: storage=$name key=$key" + ) + return + } + Log.d( + LOG_TAG, + "Notify BackupManager dataChanged: storage=$name key=$key reason=$reason" + ) + BackupManager.dataChanged(application.packageName) + } + + fun removeObserver() { + when (storage) { + is KeyedObservable<*> -> storage.removeObserver(this) + is Observable -> storage.removeObserver(this) + } + } + + fun notifyRestoreFinished() { + when (storage) { + is KeyedObservable<*> -> storage.notifyChange(ChangeReason.RESTORE) + is Observable -> storage.notifyChange(ChangeReason.RESTORE) + } + } + } companion object { @Volatile private var instance: BackupRestoreStorageManager? = null diff --git a/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/SharedPreferencesStorage.kt b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/SharedPreferencesStorage.kt new file mode 100644 index 000000000000..0c1b41799f09 --- /dev/null +++ b/packages/SettingsLib/DataStore/src/com/android/settingslib/datastore/SharedPreferencesStorage.kt @@ -0,0 +1,199 @@ +/* + * Copyright (C) 2024 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.settingslib.datastore + +import android.content.Context +import android.content.SharedPreferences +import android.os.Build +import android.util.Log +import androidx.core.content.ContextCompat +import java.io.File + +/** + * [SharedPreferences] based storage. + * + * The backup and restore is handled by [BackupRestoreFileArchiver] to achieve forward-compatibility + * just like `PersistentBackupAgentHelper`. + * + * Simple file based backup and restore is not safe, which incurs multi-thread file writes in + * SharedPreferences file. Additionally, SharedPreferences has in-memory state, so reload is needed. + * However, there is no public reload API on SharedPreferences and listeners are not notified in + * current private implementation. As such, an intermediate SharedPreferences file is introduced for + * backup and restore. + * + * Note that existing entries in the SharedPreferences will NOT be deleted before restore. + * + * @param context Context to get SharedPreferences + * @param name Name of the SharedPreferences + * @param mode Operating mode, see [Context.getSharedPreferences] + * @param verbose Verbose logging on key/value pairs during backup/restore. Enable for dev only! + * @param filter Filter of key/value pairs for backup and restore. + */ +class SharedPreferencesStorage +@JvmOverloads +constructor( + context: Context, + override val name: String, + mode: Int, + private val verbose: Boolean = (Build.TYPE == "eng"), + private val filter: (String, Any?) -> Boolean = { _, _ -> true }, +) : + BackupRestoreFileStorage(context, context.getSharedPreferencesFilePath(name)), + KeyedObservable<String> by KeyedDataObservable() { + + private val sharedPreferences = context.getSharedPreferences(name, mode) + + /** Name of the intermediate SharedPreferences. */ + private val intermediateName: String + get() = "_br_$name" + + private val intermediateSharedPreferences: SharedPreferences + get() { + // use MODE_MULTI_PROCESS to ensure a reload + return context.getSharedPreferences(intermediateName, Context.MODE_MULTI_PROCESS) + } + + private val sharedPreferencesListener = + SharedPreferences.OnSharedPreferenceChangeListener { _, key -> + if (key != null) { + notifyChange(key, ChangeReason.UPDATE) + } else { + // On Android >= R, SharedPreferences.Editor.clear() will trigger this case + notifyChange(ChangeReason.DELETE) + } + } + + init { + // listener is weakly referenced, so unregister is optional + sharedPreferences.registerOnSharedPreferenceChangeListener(sharedPreferencesListener) + } + + override val backupFile: File + // use a different file to avoid multi-thread file write + get() = context.getSharedPreferencesFile(intermediateName) + + override fun prepareBackup(file: File) { + val editor = intermediateSharedPreferences.merge(sharedPreferences.all, "Backup") + // commit to ensure data is write to disk synchronously + if (!editor.commit()) { + Log.w(LOG_TAG, "[$name] fail to commit") + } + } + + override fun onBackupFinished(file: File) { + intermediateSharedPreferences.delete(intermediateName) + } + + override fun onRestoreFinished(file: File) { + // Unregister listener to avoid notify observer during restore because there might be + // dependency between keys. BackupRestoreStorageManager.onRestoreFinished will notify + // observers consistently once restore finished. + sharedPreferences.unregisterOnSharedPreferenceChangeListener(sharedPreferencesListener) + val restored = intermediateSharedPreferences + val editor = sharedPreferences.merge(restored.all, "Restore") + editor.apply() // apply to avoid blocking + sharedPreferences.registerOnSharedPreferenceChangeListener(sharedPreferencesListener) + // clear the intermediate SharedPreferences + restored.delete(intermediateName) + } + + private fun SharedPreferences.delete(name: String) { + if (deleteSharedPreferences(name)) { + Log.i(LOG_TAG, "SharedPreferences $name deleted") + } else { + edit().clear().apply() + } + } + + private fun deleteSharedPreferences(name: String): Boolean = + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N) { + context.deleteSharedPreferences(name) + } else { + false + } + + private fun SharedPreferences.merge( + entries: Map<String, Any?>, + operation: String + ): SharedPreferences.Editor { + val editor = edit() + for ((key, value) in entries) { + if (!filter.invoke(key, value)) { + if (verbose) Log.v(LOG_TAG, "[$name] $operation skips $key=$value") + continue + } + when (value) { + is Boolean -> { + editor.putBoolean(key, value) + if (verbose) Log.v(LOG_TAG, "[$name] $operation Boolean $key=$value") + } + is Float -> { + editor.putFloat(key, value) + if (verbose) Log.v(LOG_TAG, "[$name] $operation Float $key=$value") + } + is Int -> { + editor.putInt(key, value) + if (verbose) Log.v(LOG_TAG, "[$name] $operation Int $key=$value") + } + is Long -> { + editor.putLong(key, value) + if (verbose) Log.v(LOG_TAG, "[$name] $operation Long $key=$value") + } + is String -> { + editor.putString(key, value) + if (verbose) Log.v(LOG_TAG, "[$name] $operation String $key=$value") + } + is Set<*> -> { + val nonString = value.firstOrNull { it !is String } + if (nonString != null) { + Log.e( + LOG_TAG, + "[$name] $operation StringSet $key=$value" + + " but non string found: $nonString (${nonString.javaClass})", + ) + } else { + @Suppress("UNCHECKED_CAST") editor.putStringSet(key, value as Set<String>) + if (verbose) Log.v(LOG_TAG, "[$name] $operation StringSet $key=$value") + } + } + else -> { + Log.e( + LOG_TAG, + "[$name] $operation $key=$value, unknown type: ${value?.javaClass}" + ) + } + } + } + return editor + } + + companion object { + private fun Context.getSharedPreferencesFilePath(name: String): String { + val file = getSharedPreferencesFile(name) + return file.relativeTo(ContextCompat.getDataDir(this)!!).toString() + } + + /** Returns the absolute path of shared preferences file. */ + @JvmStatic + fun Context.getSharedPreferencesFile(name: String): File { + // ContextImpl.getSharedPreferencesPath is private + return File(getSharedPreferencesDir(), "$name.xml") + } + + private fun Context.getSharedPreferencesDir() = File(dataDirCompat, "shared_prefs") + } +} diff --git a/packages/SettingsLib/DataStore/tests/Android.bp b/packages/SettingsLib/DataStore/tests/Android.bp new file mode 100644 index 000000000000..8770dfa013d0 --- /dev/null +++ b/packages/SettingsLib/DataStore/tests/Android.bp @@ -0,0 +1,24 @@ +package { + default_applicable_licenses: ["frameworks_base_license"], +} + +android_app { + name: "SettingsLibDataStoreShell", + platform_apis: true, +} + +android_robolectric_test { + name: "SettingsLibDataStoreTest", + srcs: ["src/**/*"], + static_libs: [ + "SettingsLibDataStore", + "androidx.test.ext.junit", + "guava", + "mockito-robolectric-prebuilt", // mockito deps order matters! + "mockito-kotlin2", + ], + java_resource_dirs: ["config"], + instrumentation_for: "SettingsLibDataStoreShell", + coverage_libs: ["SettingsLibDataStore"], + upstream: true, +} diff --git a/packages/SettingsLib/DataStore/tests/AndroidManifest.xml b/packages/SettingsLib/DataStore/tests/AndroidManifest.xml new file mode 100644 index 000000000000..ffc24e4330d1 --- /dev/null +++ b/packages/SettingsLib/DataStore/tests/AndroidManifest.xml @@ -0,0 +1,6 @@ +<?xml version="1.0" encoding="utf-8"?> +<manifest xmlns:android="http://schemas.android.com/apk/res/android" + package="com.android.settingslib.datastore.test"> + + <application android:debuggable="true" /> +</manifest> diff --git a/packages/SettingsLib/DataStore/tests/config/robolectric.properties b/packages/SettingsLib/DataStore/tests/config/robolectric.properties new file mode 100644 index 000000000000..fab7251d020b --- /dev/null +++ b/packages/SettingsLib/DataStore/tests/config/robolectric.properties @@ -0,0 +1 @@ +sdk=NEWEST_SDK diff --git a/packages/SettingsLib/DataStore/tests/src/com/android/settingslib/datastore/ObserverTest.kt b/packages/SettingsLib/DataStore/tests/src/com/android/settingslib/datastore/ObserverTest.kt new file mode 100644 index 000000000000..bb791dc9a23c --- /dev/null +++ b/packages/SettingsLib/DataStore/tests/src/com/android/settingslib/datastore/ObserverTest.kt @@ -0,0 +1,109 @@ +/* + * Copyright (C) 2024 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.settingslib.datastore + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat +import com.google.common.util.concurrent.MoreExecutors +import java.util.concurrent.Executor +import java.util.concurrent.atomic.AtomicInteger +import org.junit.Assert.assertThrows +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.junit.MockitoJUnit +import org.mockito.junit.MockitoRule +import org.mockito.kotlin.any +import org.mockito.kotlin.never +import org.mockito.kotlin.reset +import org.mockito.kotlin.verify + +@RunWith(AndroidJUnit4::class) +class ObserverTest { + @get:Rule val mockitoRule: MockitoRule = MockitoJUnit.rule() + + @Mock private lateinit var observer1: Observer + + @Mock private lateinit var observer2: Observer + + @Mock private lateinit var executor: Executor + + private val observable = DataObservable() + + @Test + fun addObserver_sameExecutor() { + observable.addObserver(observer1, executor) + observable.addObserver(observer1, executor) + } + + @Test + fun addObserver_differentExecutor() { + observable.addObserver(observer1, executor) + assertThrows(IllegalStateException::class.java) { + observable.addObserver(observer1, MoreExecutors.directExecutor()) + } + } + + @Test + fun addObserver_weaklyReferenced() { + val counter = AtomicInteger() + var observer: Observer? = Observer { counter.incrementAndGet() } + observable.addObserver(observer!!, MoreExecutors.directExecutor()) + + observable.notifyChange(ChangeReason.UPDATE) + assertThat(counter.get()).isEqualTo(1) + + // trigger GC, the observer callback should not be invoked + @Suppress("unused") + observer = null + System.gc() + System.runFinalization() + + observable.notifyChange(ChangeReason.UPDATE) + assertThat(counter.get()).isEqualTo(1) + } + + @Test + fun addObserver_notifyObservers_removeObserver() { + observable.addObserver(observer1, MoreExecutors.directExecutor()) + observable.addObserver(observer2, executor) + + observable.notifyChange(ChangeReason.DELETE) + + verify(observer1).onChanged(ChangeReason.DELETE) + verify(observer2, never()).onChanged(any()) + verify(executor).execute(any()) + + reset(observer1, executor) + observable.removeObserver(observer2) + + observable.notifyChange(ChangeReason.UPDATE) + verify(observer1).onChanged(ChangeReason.UPDATE) + verify(executor, never()).execute(any()) + } + + @Test + fun notifyChange_addObserverWithinCallback() { + // ConcurrentModificationException is raised if it is not implemented correctly + observable.addObserver( + { observable.addObserver(observer1, executor) }, + MoreExecutors.directExecutor() + ) + observable.notifyChange(ChangeReason.UPDATE) + } +} diff --git a/services/core/java/com/android/server/wm/DeferredDisplayUpdater.java b/services/core/java/com/android/server/wm/DeferredDisplayUpdater.java index 7052982d3203..123633d28f4b 100644 --- a/services/core/java/com/android/server/wm/DeferredDisplayUpdater.java +++ b/services/core/java/com/android/server/wm/DeferredDisplayUpdater.java @@ -171,18 +171,25 @@ public class DeferredDisplayUpdater implements DisplayUpdater { mDisplayContent.mInitialDisplayHeight); final int fromRotation = mDisplayContent.getRotation(); - onStartCollect.run(); - - ProtoLog.d(WM_DEBUG_WINDOW_TRANSITIONS, - "DeferredDisplayUpdater: applied DisplayInfo after deferring"); - - if (physicalDisplayUpdated) { - onDisplayUpdated(transition, fromRotation, startBounds); - } else { - final TransitionRequestInfo.DisplayChange displayChange = - getCurrentDisplayChange(fromRotation, startBounds); - mDisplayContent.mTransitionController.requestStartTransition(transition, - /* startTask= */ null, /* remoteTransition= */ null, displayChange); + mDisplayContent.mAtmService.deferWindowLayout(); + try { + onStartCollect.run(); + + ProtoLog.d(WM_DEBUG_WINDOW_TRANSITIONS, + "DeferredDisplayUpdater: applied DisplayInfo after deferring"); + + if (physicalDisplayUpdated) { + onDisplayUpdated(transition, fromRotation, startBounds); + } else { + final TransitionRequestInfo.DisplayChange displayChange = + getCurrentDisplayChange(fromRotation, startBounds); + mDisplayContent.mTransitionController.requestStartTransition(transition, + /* startTask= */ null, /* remoteTransition= */ null, displayChange); + } + } finally { + // Run surface placement after requestStartTransition, so shell side can receive + // the transition request before handling task info changes. + mDisplayContent.mAtmService.continueWindowLayout(); } }); } diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index d3acd716aed3..f25780c9469c 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -6175,7 +6175,12 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp * @param onDisplayChangeApplied callback that is called when the changes are applied */ void requestDisplayUpdate(@NonNull Runnable onDisplayChangeApplied) { - mDisplayUpdater.updateDisplayInfo(onDisplayChangeApplied); + mAtmService.deferWindowLayout(); + try { + mDisplayUpdater.updateDisplayInfo(onDisplayChangeApplied); + } finally { + mAtmService.continueWindowLayout(); + } } void onDisplayInfoUpdated(@NonNull DisplayInfo newDisplayInfo) { diff --git a/services/core/java/com/android/server/wm/SnapshotCache.java b/services/core/java/com/android/server/wm/SnapshotCache.java index 64d8c7555fa6..86804360f6f4 100644 --- a/services/core/java/com/android/server/wm/SnapshotCache.java +++ b/services/core/java/com/android/server/wm/SnapshotCache.java @@ -16,7 +16,6 @@ package com.android.server.wm; import android.annotation.Nullable; -import android.hardware.HardwareBuffer; import android.util.ArrayMap; import android.window.TaskSnapshot; @@ -93,10 +92,6 @@ abstract class SnapshotCache<TYPE extends WindowContainer> { if (entry != null) { mAppIdMap.remove(entry.topApp); mRunningCache.remove(id); - final HardwareBuffer buffer = entry.snapshot.getHardwareBuffer(); - if (buffer != null) { - buffer.close(); - } } } } |