Ensure settings provider joins the rescue party
The settings provider has logic to incrementally reset state
in an effort to recover from pushing a bad value putting the
system in a bad state. The settings provider was not force
persisting its state after a reset so after the reboot we
lost the reset changes. Also while resetting we were also
resetting the package name leading to a promotion of the reset
value to being set by the system. Updated the tests and while at
this added logic to synchronously persist critical settings
such as device provisioned.
Test: All tests pass. Manually tested end-to-end rescue party
bug:34677175
Change-Id: Ib240072df2fa549dae39c301008adf48cdf1573a
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
index 25e1f16..7a9ba20 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java
@@ -182,6 +182,18 @@
private static final Bundle NULL_SETTING_BUNDLE = Bundle.forPair(
Settings.NameValueTable.VALUE, null);
+ // Changes to these global settings are synchronously persisted
+ private static final Set<String> CRITICAL_GLOBAL_SETTINGS = new ArraySet<>();
+ static {
+ CRITICAL_GLOBAL_SETTINGS.add(Settings.Global.DEVICE_PROVISIONED);
+ }
+
+ // Changes to these secure settings are synchronously persisted
+ private static final Set<String> CRITICAL_SECURE_SETTINGS = new ArraySet<>();
+ static {
+ CRITICAL_SECURE_SETTINGS.add(Settings.Secure.USER_SETUP_COMPLETE);
+ }
+
// Per user secure settings that moved to the for all users global settings.
static final Set<String> sSecureMovedToGlobalSettings = new ArraySet<>();
static {
@@ -949,18 +961,18 @@
case MUTATION_OPERATION_INSERT: {
return mSettingsRegistry.insertSettingLocked(SETTINGS_TYPE_GLOBAL,
UserHandle.USER_SYSTEM, name, value, tag, makeDefault,
- getCallingPackage(), forceNotify);
+ getCallingPackage(), forceNotify, CRITICAL_GLOBAL_SETTINGS);
}
case MUTATION_OPERATION_DELETE: {
return mSettingsRegistry.deleteSettingLocked(SETTINGS_TYPE_GLOBAL,
- UserHandle.USER_SYSTEM, name, forceNotify);
+ UserHandle.USER_SYSTEM, name, forceNotify, CRITICAL_GLOBAL_SETTINGS);
}
case MUTATION_OPERATION_UPDATE: {
return mSettingsRegistry.updateSettingLocked(SETTINGS_TYPE_GLOBAL,
UserHandle.USER_SYSTEM, name, value, tag, makeDefault,
- getCallingPackage(), forceNotify);
+ getCallingPackage(), forceNotify, CRITICAL_GLOBAL_SETTINGS);
}
case MUTATION_OPERATION_RESET: {
@@ -1156,18 +1168,18 @@
case MUTATION_OPERATION_INSERT: {
return mSettingsRegistry.insertSettingLocked(SETTINGS_TYPE_SECURE,
owningUserId, name, value, tag, makeDefault,
- getCallingPackage(), forceNotify);
+ getCallingPackage(), forceNotify, CRITICAL_SECURE_SETTINGS);
}
case MUTATION_OPERATION_DELETE: {
return mSettingsRegistry.deleteSettingLocked(SETTINGS_TYPE_SECURE,
- owningUserId, name, forceNotify);
+ owningUserId, name, forceNotify, CRITICAL_SECURE_SETTINGS);
}
case MUTATION_OPERATION_UPDATE: {
return mSettingsRegistry.updateSettingLocked(SETTINGS_TYPE_SECURE,
owningUserId, name, value, tag, makeDefault,
- getCallingPackage(), forceNotify);
+ getCallingPackage(), forceNotify, CRITICAL_SECURE_SETTINGS);
}
case MUTATION_OPERATION_RESET: {
@@ -1304,18 +1316,20 @@
case MUTATION_OPERATION_INSERT: {
validateSystemSettingValue(name, value);
return mSettingsRegistry.insertSettingLocked(SETTINGS_TYPE_SYSTEM,
- owningUserId, name, value, null, false, getCallingPackage(), false);
+ owningUserId, name, value, null, false, getCallingPackage(),
+ false, null);
}
case MUTATION_OPERATION_DELETE: {
return mSettingsRegistry.deleteSettingLocked(SETTINGS_TYPE_SYSTEM,
- owningUserId, name, false);
+ owningUserId, name, false, null);
}
case MUTATION_OPERATION_UPDATE: {
validateSystemSettingValue(name, value);
return mSettingsRegistry.updateSettingLocked(SETTINGS_TYPE_SYSTEM,
- owningUserId, name, value, null, false, getCallingPackage(), false);
+ owningUserId, name, value, null, false, getCallingPackage(),
+ false, null);
}
}
@@ -1689,7 +1703,7 @@
return mSettingsRegistry.insertSettingLocked(SETTINGS_TYPE_SECURE,
owningUserId, Settings.Secure.LOCATION_PROVIDERS_ALLOWED, newProviders,
- tag, makeDefault, getCallingPackage(), forceNotify);
+ tag, makeDefault, getCallingPackage(), forceNotify, CRITICAL_SECURE_SETTINGS);
}
private static void warnOrThrowForUndesiredSecureSettingsMutationForTargetSdk(
@@ -2234,7 +2248,8 @@
}
public boolean insertSettingLocked(int type, int userId, String name, String value,
- String tag, boolean makeDefault, String packageName, boolean forceNotify) {
+ String tag, boolean makeDefault, String packageName, boolean forceNotify,
+ Set<String> criticalSettings) {
final int key = makeKey(type, userId);
boolean success = false;
@@ -2244,13 +2259,18 @@
tag, makeDefault, packageName);
}
+ if (success && criticalSettings != null && criticalSettings.contains(name)) {
+ settingsState.persistSyncLocked();
+ }
+
if (forceNotify || success) {
notifyForSettingsChange(key, name);
}
return success;
}
- public boolean deleteSettingLocked(int type, int userId, String name, boolean forceNotify) {
+ public boolean deleteSettingLocked(int type, int userId, String name, boolean forceNotify,
+ Set<String> criticalSettings) {
final int key = makeKey(type, userId);
boolean success = false;
@@ -2259,12 +2279,39 @@
success = settingsState.deleteSettingLocked(name);
}
+ if (success && criticalSettings != null && criticalSettings.contains(name)) {
+ settingsState.persistSyncLocked();
+ }
+
if (forceNotify || success) {
notifyForSettingsChange(key, name);
}
return success;
}
+ public boolean updateSettingLocked(int type, int userId, String name, String value,
+ String tag, boolean makeDefault, String packageName, boolean forceNotify,
+ Set<String> criticalSettings) {
+ final int key = makeKey(type, userId);
+
+ boolean success = false;
+ SettingsState settingsState = peekSettingsStateLocked(key);
+ if (settingsState != null) {
+ success = settingsState.updateSettingLocked(name, value, tag,
+ makeDefault, packageName);
+ }
+
+ if (success && criticalSettings != null && criticalSettings.contains(name)) {
+ settingsState.persistSyncLocked();
+ }
+
+ if (forceNotify || success) {
+ notifyForSettingsChange(key, name);
+ }
+
+ return success;
+ }
+
public Setting getSettingLocked(int type, int userId, String name) {
final int key = makeKey(type, userId);
@@ -2277,24 +2324,6 @@
return settingsState.getSettingLocked(name);
}
- public boolean updateSettingLocked(int type, int userId, String name, String value,
- String tag, boolean makeDefault, String packageName, boolean forceNotify) {
- final int key = makeKey(type, userId);
-
- boolean success = false;
- SettingsState settingsState = peekSettingsStateLocked(key);
- if (settingsState != null) {
- success = settingsState.updateSettingLocked(name, value, tag,
- makeDefault, packageName);
- }
-
- if (forceNotify || success) {
- notifyForSettingsChange(key, name);
- }
-
- return success;
- }
-
public void resetSettingsLocked(int type, int userId, String packageName, int mode,
String tag) {
final int key = makeKey(type, userId);
@@ -2306,56 +2335,78 @@
switch (mode) {
case Settings.RESET_MODE_PACKAGE_DEFAULTS: {
for (String name : settingsState.getSettingNamesLocked()) {
+ boolean someSettingChanged = false;
Setting setting = settingsState.getSettingLocked(name);
if (packageName.equals(setting.getPackageName())) {
if (tag != null && !tag.equals(setting.getTag())) {
continue;
}
- if (settingsState.resetSettingLocked(name, packageName)) {
+ if (settingsState.resetSettingLocked(name)) {
+ someSettingChanged = true;
notifyForSettingsChange(key, name);
}
}
+ if (someSettingChanged) {
+ settingsState.persistSyncLocked();
+ }
}
} break;
case Settings.RESET_MODE_UNTRUSTED_DEFAULTS: {
for (String name : settingsState.getSettingNamesLocked()) {
+ boolean someSettingChanged = false;
Setting setting = settingsState.getSettingLocked(name);
if (!SettingsState.isSystemPackage(getContext(),
setting.getPackageName())) {
- if (settingsState.resetSettingLocked(name, packageName)) {
+ if (settingsState.resetSettingLocked(name)) {
+ someSettingChanged = true;
notifyForSettingsChange(key, name);
}
}
+ if (someSettingChanged) {
+ settingsState.persistSyncLocked();
+ }
}
} break;
case Settings.RESET_MODE_UNTRUSTED_CHANGES: {
for (String name : settingsState.getSettingNamesLocked()) {
+ boolean someSettingChanged = false;
Setting setting = settingsState.getSettingLocked(name);
if (!SettingsState.isSystemPackage(getContext(),
setting.getPackageName())) {
if (setting.isDefaultFromSystem()) {
- if (settingsState.resetSettingLocked(name, packageName)) {
+ if (settingsState.resetSettingLocked(name)) {
+ someSettingChanged = true;
notifyForSettingsChange(key, name);
}
} else if (settingsState.deleteSettingLocked(name)) {
+ someSettingChanged = true;
notifyForSettingsChange(key, name);
}
}
+ if (someSettingChanged) {
+ settingsState.persistSyncLocked();
+ }
}
} break;
case Settings.RESET_MODE_TRUSTED_DEFAULTS: {
for (String name : settingsState.getSettingNamesLocked()) {
Setting setting = settingsState.getSettingLocked(name);
+ boolean someSettingChanged = false;
if (setting.isDefaultFromSystem()) {
- if (settingsState.resetSettingLocked(name, packageName)) {
+ if (settingsState.resetSettingLocked(name)) {
+ someSettingChanged = true;
notifyForSettingsChange(key, name);
}
} else if (settingsState.deleteSettingLocked(name)) {
+ someSettingChanged = true;
notifyForSettingsChange(key, name);
}
+ if (someSettingChanged) {
+ settingsState.persistSyncLocked();
+ }
}
} break;
}
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsService.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsService.java
index 2d59324..a6fadf9 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsService.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsService.java
@@ -113,7 +113,7 @@
String mKey = null;
String mValue = null;
String mPackageName = null;
- String mToken = null;
+ String mTag = null;
int mResetMode = -1;
boolean mMakeDefault;
@@ -185,7 +185,7 @@
if (peekNextArg() == null) {
valid = true;
} else {
- mToken = getNextArg();
+ mTag = getNextArg();
if (peekNextArg() == null) {
valid = true;
} else {
@@ -218,10 +218,10 @@
// what we have so far is a valid command
valid = true;
// keep going; there may be another PUT arg
- } else if (mToken == null) {
- mToken = arg;
- if ("default".equalsIgnoreCase(mToken)) {
- mToken = null;
+ } else if (mTag == null) {
+ mTag = arg;
+ if ("default".equalsIgnoreCase(mTag)) {
+ mTag = null;
mMakeDefault = true;
if (peekNextArg() == null) {
valid = true;
@@ -282,7 +282,7 @@
pout.println(getForUser(iprovider, mUser, mTable, mKey));
break;
case PUT:
- putForUser(iprovider, mUser, mTable, mKey, mValue, mToken, mMakeDefault);
+ putForUser(iprovider, mUser, mTable, mKey, mValue, mTag, mMakeDefault);
break;
case DELETE:
pout.println("Deleted "
@@ -294,7 +294,7 @@
}
break;
case RESET:
- resetForUser(iprovider, mUser, mTable, mToken);
+ resetForUser(iprovider, mUser, mTable, mTag);
break;
default:
perr.println("Unspecified command");
@@ -358,7 +358,7 @@
}
void putForUser(IContentProvider provider, int userHandle, final String table,
- final String key, final String value, String token, boolean makeDefault) {
+ final String key, final String value, String tag, boolean makeDefault) {
final String callPutCommand;
if ("system".equals(table)) {
callPutCommand = Settings.CALL_METHOD_PUT_SYSTEM;
@@ -378,7 +378,9 @@
Bundle arg = new Bundle();
arg.putString(Settings.NameValueTable.VALUE, value);
arg.putInt(Settings.CALL_METHOD_USER_KEY, userHandle);
- arg.putString(Settings.CALL_METHOD_TAG_KEY, token);
+ if (tag != null) {
+ arg.putString(Settings.CALL_METHOD_TAG_KEY, tag);
+ }
if (makeDefault) {
arg.putBoolean(Settings.CALL_METHOD_MAKE_DEFAULT_KEY, true);
}
@@ -409,7 +411,7 @@
}
void resetForUser(IContentProvider provider, int userHandle,
- String table, String token) {
+ String table, String tag) {
final String callResetCommand;
if ("secure".equals(table)) callResetCommand = Settings.CALL_METHOD_RESET_SECURE;
else if ("global".equals(table)) callResetCommand = Settings.CALL_METHOD_RESET_GLOBAL;
@@ -422,7 +424,9 @@
Bundle arg = new Bundle();
arg.putInt(Settings.CALL_METHOD_USER_KEY, userHandle);
arg.putInt(Settings.CALL_METHOD_RESET_MODE_KEY, mResetMode);
- arg.putString(Settings.CALL_METHOD_TAG_KEY, token);
+ if (tag != null) {
+ arg.putString(Settings.CALL_METHOD_TAG_KEY, tag);
+ }
String packageName = mPackageName != null ? mPackageName : resolveCallingPackage();
arg.putInt(Settings.CALL_METHOD_USER_KEY, userHandle);
provider.call(packageName, callResetCommand, null, arg);
@@ -465,9 +469,9 @@
pw.println(" Print this help text.");
pw.println(" get [--user <USER_ID> | current] NAMESPACE KEY");
pw.println(" Retrieve the current value of KEY.");
- pw.println(" put [--user <USER_ID> | current] NAMESPACE KEY VALUE [TOKEN] [default]");
+ pw.println(" put [--user <USER_ID> | current] NAMESPACE KEY VALUE [TAG] [default]");
pw.println(" Change the contents of KEY to VALUE.");
- pw.println(" TOKEN to associate with the setting.");
+ pw.println(" TAG to associate with the setting.");
pw.println(" {default} to set as the default, case-insensitive only for global/secure namespace");
pw.println(" delete NAMESPACE KEY");
pw.println(" Delete the entry for KEY.");
diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
index a74be35..56ae618 100644
--- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
+++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java
@@ -64,6 +64,7 @@
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.List;
+import java.util.Set;
/**
* This class contains the state for one type of settings. It is responsible
@@ -129,7 +130,7 @@
private static final String HISTORICAL_OPERATION_INITIALIZE = "initialize";
private static final String HISTORICAL_OPERATION_RESET = "reset";
- private static final String SHELL_PACKAGE_NAME = "shell";
+ private static final String SHELL_PACKAGE_NAME = "com.android.shell";
private static final String ROOT_PACKAGE_NAME = "root";
private static final String NULL_VALUE = "null";
@@ -307,7 +308,7 @@
Setting newState;
if (oldState != null) {
- if (!oldState.update(value, makeDefault, packageName, tag)) {
+ if (!oldState.update(value, makeDefault, packageName, tag, false)) {
return false;
}
newState = oldState;
@@ -351,7 +352,7 @@
}
// The settings provider must hold its lock when calling here.
- public boolean resetSettingLocked(String name, String packageName) {
+ public boolean resetSettingLocked(String name) {
if (TextUtils.isEmpty(name) || !hasSettingLocked(name)) {
return false;
}
@@ -362,7 +363,7 @@
String oldValue = setting.getValue();
String oldDefaultValue = setting.getDefaultValue();
- if (!setting.reset(packageName)) {
+ if (!setting.reset()) {
return false;
}
@@ -817,7 +818,7 @@
public Setting(String name, String value, boolean makeDefault, String packageName,
String tag) {
this.name = name;
- update(value, makeDefault, packageName, tag);
+ update(value, makeDefault, packageName, tag, false);
}
public Setting(String name, String value, String defaultValue,
@@ -877,16 +878,18 @@
}
/** @return whether the value changed */
- public boolean reset(String packageName) {
- return update(this.defaultValue, false, packageName, null);
+ public boolean reset() {
+ return update(this.defaultValue, false, packageName, null, true);
}
- public boolean update(String value, boolean setDefault, String packageName, String tag) {
+ public boolean update(String value, boolean setDefault, String packageName, String tag,
+ boolean forceNonSystemPackage) {
if (NULL_VALUE.equals(value)) {
value = null;
}
- final boolean callerSystem = !isNull() && isSystemPackage(mContext, packageName);
+ final boolean callerSystem = !forceNonSystemPackage &&
+ !isNull() && isSystemPackage(mContext, packageName);
// Settings set by the system are always defaults.
if (callerSystem) {
setDefault = true;
diff --git a/packages/SettingsProvider/test/src/com/android/providers/settings/BaseSettingsProviderTest.java b/packages/SettingsProvider/test/src/com/android/providers/settings/BaseSettingsProviderTest.java
index 0454b51..ab23af3 100644
--- a/packages/SettingsProvider/test/src/com/android/providers/settings/BaseSettingsProviderTest.java
+++ b/packages/SettingsProvider/test/src/com/android/providers/settings/BaseSettingsProviderTest.java
@@ -206,19 +206,22 @@
resetToDefaultsViaShell(type, packageName, null);
}
- protected static void resetToDefaultsViaShell(int type, String packageName, String token)
+ protected static void resetToDefaultsViaShell(int type, String packageName, String tag)
throws IOException {
switch (type) {
case SETTING_TYPE_GLOBAL: {
- executeShellCommand("settings reset global " + packageName + " " + token);
+ executeShellCommand("settings reset global " + packageName + " "
+ + (tag != null ? tag : ""));
} break;
case SETTING_TYPE_SECURE: {
- executeShellCommand("settings reset secure " + packageName + " " + token);
+ executeShellCommand("settings reset secure " + packageName + " "
+ + (tag != null ? tag : ""));
} break;
case SETTING_TYPE_SYSTEM: {
- executeShellCommand("settings reset system " + packageName + " " + token);
+ executeShellCommand("settings reset system " + packageName + " "
+ + (tag != null ? tag : ""));
} break;
default: {