From d069a888cf32f105bf6843a7083770f5b82af74e Mon Sep 17 00:00:00 2001 From: Annie Meng Date: Tue, 13 Mar 2018 15:31:40 +0000 Subject: [PATCH] DO NOT MERGE Create a setting for backup/restore agent timeouts Part of push to make backup and restore agent timeouts configurable. Creates a Global setting for the current static BackupManagerService timeouts so that they can be overriden with P/H. We keep the current default values, which will be updated once we investigate what more appropriate values are. Remame the constants to better reflect what they're used for. Next, we will update the framework to use these constants. This depends on the refactor of how we observe changes to key value backup settings (ag/3709997). Bug: 70276070 Test: m -j RunFrameworksServicesRoboTests ROBOTEST_FILTER=BackupAgentTimeoutParametersTest Change-Id: Id506314ce0c8bd5e4d1d8b4001b26cbad0056c99 --- core/java/android/provider/Settings.java | 22 +++ core/proto/android/providers/settings.proto | 3 +- .../android/provider/SettingsBackupTest.java | 4 +- .../SettingsProvider/res/values/defaults.xml | 3 + .../settings/SettingsProtoDumpUtil.java | 3 + .../providers/settings/SettingsProvider.java | 25 ++- .../backup/BackupAgentTimeoutParameters.java | 150 ++++++++++++++++++ .../BackupAgentTimeoutParametersTest.java | 140 ++++++++++++++++ 8 files changed, 343 insertions(+), 7 deletions(-) create mode 100644 services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java create mode 100644 services/robotests/src/com/android/server/backup/BackupAgentTimeoutParametersTest.java diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 2b5cece4bb35b..e46a5f05c48d7 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -12338,6 +12338,28 @@ public final class Settings { * @hide */ public static final String SHOW_ZEN_UPGRADE_NOTIFICATION = "show_zen_upgrade_notification"; + + /** + * Backup and restore agent timeout parameters. + * These parameters are represented by a comma-delimited key-value list. + * + * The following strings are supported as keys: + * 
+         *     kv_backup_agent_timeout_millis         (long)
+         *     full_backup_agent_timeout_millis       (long)
+         *     shared_backup_agent_timeout_millis     (long)
+         *     restore_agent_timeout_millis           (long)
+         *     restore_agent_finished_timeout_millis  (long)
+         *
+ * + * They map to milliseconds represented as longs. + * + * Ex: "kv_backup_agent_timeout_millis=30000,full_backup_agent_timeout_millis=300000" + * + * @hide + */ + public static final String BACKUP_AGENT_TIMEOUT_PARAMETERS = + "backup_agent_timeout_parameters"; } /** diff --git a/core/proto/android/providers/settings.proto b/core/proto/android/providers/settings.proto index 23c5661f05332..a818e2095ca31 100644 --- a/core/proto/android/providers/settings.proto +++ b/core/proto/android/providers/settings.proto @@ -502,9 +502,10 @@ message GlobalSettingsProto { optional SettingProto show_restart_in_crash_dialog = 383 [ (android.privacy).dest = DEST_AUTOMATIC ]; optional SettingProto show_mute_in_crash_dialog = 384 [ (android.privacy).dest = DEST_AUTOMATIC ]; optional SettingsProto show_zen_upgrade_notification = 385 [ (android.privacy).dest = DEST_AUTOMATIC ]; + optional SettingsProto backup_agent_timeout_parameters = 386; // Please insert fields in the same order as in // frameworks/base/core/java/android/provider/Settings.java. - // Next tag = 386; + // Next tag = 387; } message SecureSettingsProto { diff --git a/core/tests/coretests/src/android/provider/SettingsBackupTest.java b/core/tests/coretests/src/android/provider/SettingsBackupTest.java index 73fb7139cbd48..a08eae94b3f66 100644 --- a/core/tests/coretests/src/android/provider/SettingsBackupTest.java +++ b/core/tests/coretests/src/android/provider/SettingsBackupTest.java @@ -463,8 +463,8 @@ public class SettingsBackupTest { Settings.Global.ZRAM_ENABLED, Settings.Global.OVERRIDE_SETTINGS_PROVIDER_RESTORE_ANY_VERSION, Settings.Global.CHAINED_BATTERY_ATTRIBUTION_ENABLED, - Settings.Global.HIDDEN_API_BLACKLIST_EXEMPTIONS); - + Settings.Global.HIDDEN_API_BLACKLIST_EXEMPTIONS, + Settings.Global.BACKUP_AGENT_TIMEOUT_PARAMETERS); private static final Set BACKUP_BLACKLISTED_SECURE_SETTINGS = newHashSet( Settings.Secure.ACCESSIBILITY_SOFT_KEYBOARD_MODE, diff --git a/packages/SettingsProvider/res/values/defaults.xml b/packages/SettingsProvider/res/values/defaults.xml index 1cd02f418352e..ecda53a15f481 100644 --- a/packages/SettingsProvider/res/values/defaults.xml +++ b/packages/SettingsProvider/res/values/defaults.xml @@ -207,4 +207,7 @@ Else (if negative), turning on dnd manually will surface a dialog that prompts user to specify a duration.--> 0 + + + diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java index 53cff4e845493..9a438398127d6 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java @@ -1253,6 +1253,9 @@ class SettingsProtoDumpUtil { dumpSetting(s, p, Settings.Global.SHOW_ZEN_UPGRADE_NOTIFICATION, GlobalSettingsProto.SHOW_ZEN_UPGRADE_NOTIFICATION); + dumpSetting(s, p, + Settings.Global.BACKUP_AGENT_TIMEOUT_PARAMETERS, + GlobalSettingsProto.BACKUP_AGENT_TIMEOUT_PARAMETERS); // Please insert new settings using the same order as in Settings.Global. p.end(token); diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index 6398858479076..da62d94f9c637 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -2938,7 +2938,7 @@ public class SettingsProvider extends ContentProvider { } private final class UpgradeController { - private static final int SETTINGS_VERSION = 157; + private static final int SETTINGS_VERSION = 158; private final int mUserId; @@ -3584,7 +3584,7 @@ public class SettingsProvider extends ContentProvider { } if (currentVersion == 155) { - // Version 155: Set the default value for CHARGING_STARTED_SOUND. + // Version 156: Set the default value for CHARGING_STARTED_SOUND. final SettingsState globalSettings = getGlobalSettingsLocked(); final String oldValue = globalSettings.getSettingLocked( Global.CHARGING_STARTED_SOUND).getValue(); @@ -3605,7 +3605,7 @@ public class SettingsProvider extends ContentProvider { } if (currentVersion == 156) { - // Version 156: Set a default value for zen duration + // Version 157: Set a default value for zen duration final SettingsState globalSettings = getGlobalSettingsLocked(); final Setting currentSetting = globalSettings.getSettingLocked( Global.ZEN_DURATION); @@ -3616,9 +3616,26 @@ public class SettingsProvider extends ContentProvider { Global.ZEN_DURATION, defaultZenDuration, null, true, SettingsState.SYSTEM_PACKAGE_NAME); } - currentVersion = 157; } + + if (currentVersion == 157) { + // Version 158: Set default value for BACKUP_AGENT_TIMEOUT_PARAMETERS. + final SettingsState globalSettings = getGlobalSettingsLocked(); + final String oldValue = globalSettings.getSettingLocked( + Settings.Global.BACKUP_AGENT_TIMEOUT_PARAMETERS).getValue(); + if (TextUtils.equals(null, oldValue)) { + final String defaultValue = getContext().getResources().getString( + R.string.def_backup_agent_timeout_parameters); + if (!TextUtils.isEmpty(defaultValue)) { + globalSettings.insertSettingLocked( + Settings.Global.BACKUP_AGENT_TIMEOUT_PARAMETERS, defaultValue, + null, true, + SettingsState.SYSTEM_PACKAGE_NAME); + } + } + currentVersion = 158; + } // vXXX: Add new settings above this point. if (currentVersion != newVersion) { diff --git a/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java b/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java new file mode 100644 index 0000000000000..4de2c9b8e79f4 --- /dev/null +++ b/services/backup/java/com/android/server/backup/BackupAgentTimeoutParameters.java @@ -0,0 +1,150 @@ +/* + * Copyright (C) 2018 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.server.backup; + +import android.content.ContentResolver; +import android.os.Handler; +import android.provider.Settings; +import android.util.KeyValueListParser; +import android.util.KeyValueSettingObserver; +import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; + +/** + * Configure backup and restore agent timeouts. + * + *

These timeout parameters are stored in Settings.Global to be configurable flags with P/H. They + * are represented as a comma-delimited key value list. + */ +public class BackupAgentTimeoutParameters extends KeyValueSettingObserver { + @VisibleForTesting + public static final String SETTING = Settings.Global.BACKUP_AGENT_TIMEOUT_PARAMETERS; + + @VisibleForTesting + public static final String SETTING_KV_BACKUP_AGENT_TIMEOUT_MILLIS = + "kv_backup_agent_timeout_millis"; + + @VisibleForTesting + public static final String SETTING_FULL_BACKUP_AGENT_TIMEOUT_MILLIS = + "full_backup_agent_timeout_millis"; + + @VisibleForTesting + public static final String SETTING_SHARED_BACKUP_AGENT_TIMEOUT_MILLIS = + "shared_backup_agent_timeout_millis"; + + @VisibleForTesting + public static final String SETTING_RESTORE_AGENT_TIMEOUT_MILLIS = + "restore_agent_timeout_millis"; + + @VisibleForTesting + public static final String SETTING_RESTORE_AGENT_FINISHED_TIMEOUT_MILLIS = + "restore_agent_finished_timeout_millis"; + + // Default values + @VisibleForTesting public static final long DEFAULT_KV_BACKUP_AGENT_TIMEOUT_MILLIS = 30 * 1000; + + @VisibleForTesting + public static final long DEFAULT_FULL_BACKUP_AGENT_TIMEOUT_MILLIS = 5 * 60 * 1000; + + @VisibleForTesting + public static final long DEFAULT_SHARED_BACKUP_AGENT_TIMEOUT_MILLIS = 30 * 60 * 1000; + + @VisibleForTesting public static final long DEFAULT_RESTORE_AGENT_TIMEOUT_MILLIS = 60 * 1000; + + @VisibleForTesting + public static final long DEFAULT_RESTORE_AGENT_FINISHED_TIMEOUT_MILLIS = 30 * 1000; + + @GuardedBy("mLock") + private long mKvBackupAgentTimeoutMillis; + + @GuardedBy("mLock") + private long mFullBackupAgentTimeoutMillis; + + @GuardedBy("mLock") + private long mSharedBackupAgentTimeoutMillis; + + @GuardedBy("mLock") + private long mRestoreAgentTimeoutMillis; + + @GuardedBy("mLock") + private long mRestoreAgentFinishedTimeoutMillis; + + private final Object mLock = new Object(); + + public BackupAgentTimeoutParameters(Handler handler, ContentResolver resolver) { + super(handler, resolver, Settings.Global.getUriFor(SETTING)); + } + + public String getSettingValue(ContentResolver resolver) { + return Settings.Global.getString(resolver, SETTING); + } + + public void update(KeyValueListParser parser) { + synchronized (mLock) { + mKvBackupAgentTimeoutMillis = + parser.getLong( + SETTING_KV_BACKUP_AGENT_TIMEOUT_MILLIS, + DEFAULT_KV_BACKUP_AGENT_TIMEOUT_MILLIS); + mFullBackupAgentTimeoutMillis = + parser.getLong( + SETTING_FULL_BACKUP_AGENT_TIMEOUT_MILLIS, + DEFAULT_FULL_BACKUP_AGENT_TIMEOUT_MILLIS); + mSharedBackupAgentTimeoutMillis = + parser.getLong( + SETTING_SHARED_BACKUP_AGENT_TIMEOUT_MILLIS, + DEFAULT_SHARED_BACKUP_AGENT_TIMEOUT_MILLIS); + mRestoreAgentTimeoutMillis = + parser.getLong( + SETTING_RESTORE_AGENT_TIMEOUT_MILLIS, + DEFAULT_RESTORE_AGENT_TIMEOUT_MILLIS); + mRestoreAgentFinishedTimeoutMillis = + parser.getLong( + SETTING_RESTORE_AGENT_FINISHED_TIMEOUT_MILLIS, + DEFAULT_RESTORE_AGENT_FINISHED_TIMEOUT_MILLIS); + } + } + + public long getKvBackupAgentTimeoutMillis() { + synchronized (mLock) { + return mKvBackupAgentTimeoutMillis; + } + } + + public long getFullBackupAgentTimeoutMillis() { + synchronized (mLock) { + return mFullBackupAgentTimeoutMillis; + } + } + + public long getSharedBackupAgentTimeoutMillis() { + synchronized (mLock) { + return mSharedBackupAgentTimeoutMillis; + } + } + + public long getRestoreAgentTimeoutMillis() { + synchronized (mLock) { + return mRestoreAgentTimeoutMillis; + } + } + + public long getRestoreAgentFinishedTimeoutMillis() { + synchronized (mLock) { + return mRestoreAgentFinishedTimeoutMillis; + } + } +} diff --git a/services/robotests/src/com/android/server/backup/BackupAgentTimeoutParametersTest.java b/services/robotests/src/com/android/server/backup/BackupAgentTimeoutParametersTest.java new file mode 100644 index 0000000000000..801451ee549b9 --- /dev/null +++ b/services/robotests/src/com/android/server/backup/BackupAgentTimeoutParametersTest.java @@ -0,0 +1,140 @@ +/* + * Copyright (C) 2018 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.server.backup; + +import static org.junit.Assert.assertEquals; + +import android.content.ContentResolver; +import android.content.Context; +import android.os.Handler; +import android.platform.test.annotations.Presubmit; +import android.provider.Settings; +import android.util.KeyValueSettingObserver; +import com.android.server.testing.FrameworkRobolectricTestRunner; +import com.android.server.testing.SystemLoaderClasses; +import com.android.server.testing.SystemLoaderPackages; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.RuntimeEnvironment; +import org.robolectric.annotation.Config; + +/** Tests for {@link BackupAgentTimeoutParameters}. */ +@RunWith(FrameworkRobolectricTestRunner.class) +@Config(manifest = Config.NONE, sdk = 26) +@SystemLoaderPackages({"com.android.server.backup"}) +@SystemLoaderClasses({KeyValueSettingObserver.class}) +@Presubmit +public class BackupAgentTimeoutParametersTest { + private ContentResolver mContentResolver; + private BackupAgentTimeoutParameters mParameters; + + /** Initialize timeout parameters and start observing changes. */ + @Before + public void setUp() { + Context context = RuntimeEnvironment.application.getApplicationContext(); + + mContentResolver = context.getContentResolver(); + mParameters = new BackupAgentTimeoutParameters(new Handler(), mContentResolver); + mParameters.start(); + } + + /** Stop observing changes to the setting. */ + @After + public void tearDown() { + mParameters.stop(); + } + + /** Tests that timeout parameters are initialized with default values on creation. */ + @Test + public void testGetParameters_afterConstructorWithStart_returnsDefaultValues() { + long kvBackupAgentTimeoutMillis = mParameters.getKvBackupAgentTimeoutMillis(); + long fullBackupAgentTimeoutMillis = mParameters.getFullBackupAgentTimeoutMillis(); + long sharedBackupAgentTimeoutMillis = mParameters.getSharedBackupAgentTimeoutMillis(); + long restoreAgentTimeoutMillis = mParameters.getRestoreAgentTimeoutMillis(); + long restoreAgentFinishedTimeoutMillis = mParameters.getRestoreAgentFinishedTimeoutMillis(); + + assertEquals( + BackupAgentTimeoutParameters.DEFAULT_KV_BACKUP_AGENT_TIMEOUT_MILLIS, + kvBackupAgentTimeoutMillis); + assertEquals( + BackupAgentTimeoutParameters.DEFAULT_FULL_BACKUP_AGENT_TIMEOUT_MILLIS, + fullBackupAgentTimeoutMillis); + assertEquals( + BackupAgentTimeoutParameters.DEFAULT_SHARED_BACKUP_AGENT_TIMEOUT_MILLIS, + sharedBackupAgentTimeoutMillis); + assertEquals( + BackupAgentTimeoutParameters.DEFAULT_RESTORE_AGENT_TIMEOUT_MILLIS, + restoreAgentTimeoutMillis); + assertEquals( + BackupAgentTimeoutParameters.DEFAULT_RESTORE_AGENT_FINISHED_TIMEOUT_MILLIS, + restoreAgentFinishedTimeoutMillis); + } + + /** + * Tests that timeout parameters are updated when we call start, even when a setting change + * occurs while we are not observing. + */ + @Test + public void testGetParameters_withSettingChangeBeforeStart_updatesValues() { + mParameters.stop(); + long testTimeout = BackupAgentTimeoutParameters.DEFAULT_KV_BACKUP_AGENT_TIMEOUT_MILLIS * 2; + final String setting = + BackupAgentTimeoutParameters.SETTING_KV_BACKUP_AGENT_TIMEOUT_MILLIS + + "=" + + testTimeout; + putStringAndNotify(setting); + mParameters.start(); + + long kvBackupAgentTimeoutMillis = mParameters.getKvBackupAgentTimeoutMillis(); + + assertEquals(testTimeout, kvBackupAgentTimeoutMillis); + } + + /** + * Tests that timeout parameters are updated when a setting change occurs while we are observing + * changes. + */ + @Test + public void testGetParameters_withSettingChangeAfterStart_updatesValues() { + long testTimeout = BackupAgentTimeoutParameters.DEFAULT_KV_BACKUP_AGENT_TIMEOUT_MILLIS * 2; + final String setting = + BackupAgentTimeoutParameters.SETTING_KV_BACKUP_AGENT_TIMEOUT_MILLIS + + "=" + + testTimeout; + putStringAndNotify(setting); + + long kvBackupAgentTimeoutMillis = mParameters.getKvBackupAgentTimeoutMillis(); + + assertEquals(testTimeout, kvBackupAgentTimeoutMillis); + } + + /** + * Robolectric does not notify observers of changes to settings so we have to trigger it here. + * Currently, the mock of {@link Settings.Secure#putString(ContentResolver, String, String)} + * only stores the value. TODO: Implement properly in ShadowSettings. + */ + private void putStringAndNotify(String value) { + Settings.Global.putString(mContentResolver, BackupAgentTimeoutParameters.SETTING, value); + + // We pass null as the observer since notifyChange iterates over all available observers and + // we don't have access to the local observer. + mContentResolver.notifyChange( + Settings.Global.getUriFor(BackupAgentTimeoutParameters.SETTING), /*observer*/ null); + } +}