Merge "[SettingsProvider] do not write under mLock" into main (56d8ba6b) · Commits · e / os / android_frameworks_base

packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java

+13 −13

Original line number	Original line	Diff line number	Diff line
	@@ -3243,7 +3243,7 @@ public class SettingsProvider extends ContentProvider {
	}		}

	if (success && criticalSettings != null && criticalSettings.contains(name)) {		if (success && criticalSettings != null && criticalSettings.contains(name)) {
	settingsState.persistSyncLocked();		settingsState.persistSettingsLocked();
	}		}

	if (forceNotify \|\| success) {		if (forceNotify \|\| success) {
	@@ -3294,7 +3294,7 @@ public class SettingsProvider extends ContentProvider {
	}		}

	if (success && criticalSettings != null && criticalSettings.contains(name)) {		if (success && criticalSettings != null && criticalSettings.contains(name)) {
	settingsState.persistSyncLocked();		settingsState.persistSettingsLocked();
	}		}

	if (forceNotify \|\| success) {		if (forceNotify \|\| success) {
	@@ -3319,7 +3319,7 @@ public class SettingsProvider extends ContentProvider {
	}		}

	if (success && criticalSettings != null && criticalSettings.contains(name)) {		if (success && criticalSettings != null && criticalSettings.contains(name)) {
	settingsState.persistSyncLocked();		settingsState.persistSettingsLocked();
	}		}

	if (forceNotify \|\| success) {		if (forceNotify \|\| success) {
	@@ -3385,7 +3385,7 @@ public class SettingsProvider extends ContentProvider {
	}		}
	}		}
	if (someSettingChanged) {		if (someSettingChanged) {
	settingsState.persistSyncLocked();		settingsState.persistSettingsLocked();
	success = true;		success = true;
	}		}
	}		}
	@@ -3407,7 +3407,7 @@ public class SettingsProvider extends ContentProvider {
	}		}
	}		}
	if (someSettingChanged) {		if (someSettingChanged) {
	settingsState.persistSyncLocked();		settingsState.persistSettingsLocked();
	success = true;		success = true;
	}		}
	}		}
	@@ -3435,7 +3435,7 @@ public class SettingsProvider extends ContentProvider {
	}		}
	}		}
	if (someSettingChanged) {		if (someSettingChanged) {
	settingsState.persistSyncLocked();		settingsState.persistSettingsLocked();
	success = true;		success = true;
	}		}
	}		}
	@@ -3460,7 +3460,7 @@ public class SettingsProvider extends ContentProvider {
	logSettingChanged(userId, name, type, CHANGE_TYPE_DELETE);		logSettingChanged(userId, name, type, CHANGE_TYPE_DELETE);
	}		}
	if (someSettingChanged) {		if (someSettingChanged) {
	settingsState.persistSyncLocked();		settingsState.persistSettingsLocked();
	success = true;		success = true;
	}		}
	}		}
	@@ -3559,7 +3559,7 @@ public class SettingsProvider extends ContentProvider {
	ensureSettingsStateLocked(systemKey);		ensureSettingsStateLocked(systemKey);
	SettingsState systemSettings = mSettingsStates.get(systemKey);		SettingsState systemSettings = mSettingsStates.get(systemKey);
	migrateLegacySettingsLocked(systemSettings, database, TABLE_SYSTEM);		migrateLegacySettingsLocked(systemSettings, database, TABLE_SYSTEM);
	systemSettings.persistSyncLocked();		systemSettings.persistSettingsLocked();

	// Move over the secure settings.		// Move over the secure settings.
	// Do this after System settings, since this is the first thing we check when deciding		// Do this after System settings, since this is the first thing we check when deciding
	@@ -3569,7 +3569,7 @@ public class SettingsProvider extends ContentProvider {
	SettingsState secureSettings = mSettingsStates.get(secureKey);		SettingsState secureSettings = mSettingsStates.get(secureKey);
	migrateLegacySettingsLocked(secureSettings, database, TABLE_SECURE);		migrateLegacySettingsLocked(secureSettings, database, TABLE_SECURE);
	ensureSecureSettingAndroidIdSetLocked(secureSettings);		ensureSecureSettingAndroidIdSetLocked(secureSettings);
	secureSettings.persistSyncLocked();		secureSettings.persistSettingsLocked();

	// Move over the global settings if owner.		// Move over the global settings if owner.
	// Do this last, since this is the first thing we check when deciding		// Do this last, since this is the first thing we check when deciding
	@@ -3585,7 +3585,7 @@ public class SettingsProvider extends ContentProvider {
	mSettingsCreationBuildId, null, true,		mSettingsCreationBuildId, null, true,
	SettingsState.SYSTEM_PACKAGE_NAME);		SettingsState.SYSTEM_PACKAGE_NAME);
	}		}
	globalSettings.persistSyncLocked();		globalSettings.persistSettingsLocked();
	}		}

	// Drop the database as now all is moved and persisted.		// Drop the database as now all is moved and persisted.
	@@ -4404,16 +4404,16 @@ public class SettingsProvider extends ContentProvider {
	if (userId == UserHandle.USER_SYSTEM) {		if (userId == UserHandle.USER_SYSTEM) {
	SettingsState globalSettings = getGlobalSettingsLocked();		SettingsState globalSettings = getGlobalSettingsLocked();
	ensureLegacyDefaultValueAndSystemSetUpdatedLocked(globalSettings, userId);		ensureLegacyDefaultValueAndSystemSetUpdatedLocked(globalSettings, userId);
	globalSettings.persistSyncLocked();		globalSettings.persistSettingsLocked();
	}		}

	SettingsState secureSettings = getSecureSettingsLocked(mUserId);		SettingsState secureSettings = getSecureSettingsLocked(mUserId);
	ensureLegacyDefaultValueAndSystemSetUpdatedLocked(secureSettings, userId);		ensureLegacyDefaultValueAndSystemSetUpdatedLocked(secureSettings, userId);
	secureSettings.persistSyncLocked();		secureSettings.persistSettingsLocked();

	SettingsState systemSettings = getSystemSettingsLocked(mUserId);		SettingsState systemSettings = getSystemSettingsLocked(mUserId);
	ensureLegacyDefaultValueAndSystemSetUpdatedLocked(systemSettings, userId);		ensureLegacyDefaultValueAndSystemSetUpdatedLocked(systemSettings, userId);
	systemSettings.persistSyncLocked();		systemSettings.persistSettingsLocked();

	currentVersion = 146;		currentVersion = 146;
	}		}

packages/SettingsProvider/src/com/android/providers/settings/SettingsState.java

+20 −2

Original line number	Original line	Diff line number	Diff line
	@@ -72,6 +72,7 @@ import java.util.List;
	import java.util.Map;		import java.util.Map;
	import java.util.Objects;		import java.util.Objects;
	import java.util.Set;		import java.util.Set;
			import java.util.concurrent.CountDownLatch;

	/**		/**
	* This class contains the state for one type of settings. It is responsible		* This class contains the state for one type of settings. It is responsible
	@@ -589,9 +590,10 @@ final class SettingsState {
	}		}

	// The settings provider must hold its lock when calling here.		// The settings provider must hold its lock when calling here.
	public void persistSyncLocked() {		public void persistSettingsLocked() {
	mHandler.removeMessages(MyHandler.MSG_PERSIST_SETTINGS);		mHandler.removeMessages(MyHandler.MSG_PERSIST_SETTINGS);
	doWriteState();		// schedule a write operation right away
			mHandler.obtainMessage(MyHandler.MSG_PERSIST_SETTINGS).sendToTarget();
	}		}

	// The settings provider must hold its lock when calling here.		// The settings provider must hold its lock when calling here.
	@@ -1725,4 +1727,20 @@ final class SettingsState {
	return mPackageToMemoryUsage.getOrDefault(packageName, 0);		return mPackageToMemoryUsage.getOrDefault(packageName, 0);
	}		}
	}		}

			/**
			* Allow tests to wait for the handler to finish handling all the remaining messages
			*/
			@VisibleForTesting
			public void waitForHandler() {
			final CountDownLatch latch = new CountDownLatch(1);
			synchronized (mLock) {
			mHandler.post(latch::countDown);
			}
			try {
			latch.await();
			} catch (InterruptedException e) {
			// ignored
			}
			}
	}		}

packages/SettingsProvider/test/src/com/android/providers/settings/SettingsStateTest.java

+33 −16

Original line number	Original line	Diff line number	Diff line
	@@ -76,6 +76,14 @@ public class SettingsStateTest extends AndroidTestCase {
	mSettingsFile.delete();		mSettingsFile.delete();
	}		}

			@Override
			protected void tearDown() throws Exception {
			if (mSettingsFile != null) {
			mSettingsFile.delete();
			}
			super.tearDown();
			}

	public void testIsBinary() {		public void testIsBinary() {
	assertFalse(SettingsState.isBinary(" abc 日本語"));		assertFalse(SettingsState.isBinary(" abc 日本語"));

	@@ -149,11 +157,10 @@ public class SettingsStateTest extends AndroidTestCase {
	* Make sure settings can be written to a file and also can be read.		* Make sure settings can be written to a file and also can be read.
	*/		*/
	public void testReadWrite() {		public void testReadWrite() {
	final File file = new File(getContext().getCacheDir(), "setting.xml");
	file.delete();
	final Object lock = new Object();		final Object lock = new Object();

	final SettingsState ssWriter = new SettingsState(getContext(), lock, file, 1,		assertFalse(mSettingsFile.exists());
			final SettingsState ssWriter = new SettingsState(getContext(), lock, mSettingsFile, 1,
	SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper());		SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper());
	ssWriter.setVersionLocked(SettingsState.SETTINGS_VERSION_NEW_ENCODING);		ssWriter.setVersionLocked(SettingsState.SETTINGS_VERSION_NEW_ENCODING);

	@@ -162,11 +169,13 @@ public class SettingsStateTest extends AndroidTestCase {
	ssWriter.insertSettingLocked("k3", null, null, false, "p2");		ssWriter.insertSettingLocked("k3", null, null, false, "p2");
	ssWriter.insertSettingLocked("k4", CRAZY_STRING, null, false, "p3");		ssWriter.insertSettingLocked("k4", CRAZY_STRING, null, false, "p3");
	synchronized (lock) {		synchronized (lock) {
	ssWriter.persistSyncLocked();		ssWriter.persistSettingsLocked();
	}		}
			ssWriter.waitForHandler();
	final SettingsState ssReader = new SettingsState(getContext(), lock, file, 1,		assertTrue(mSettingsFile.exists());
			final SettingsState ssReader = new SettingsState(getContext(), lock, mSettingsFile, 1,
	SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper());		SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper());

	synchronized (lock) {		synchronized (lock) {
	assertEquals("\u0000", ssReader.getSettingLocked("k1").getValue());		assertEquals("\u0000", ssReader.getSettingLocked("k1").getValue());
	assertEquals("abc", ssReader.getSettingLocked("k2").getValue());		assertEquals("abc", ssReader.getSettingLocked("k2").getValue());
	@@ -179,10 +188,8 @@ public class SettingsStateTest extends AndroidTestCase {
	* In version 120, value "null" meant {code NULL}.		* In version 120, value "null" meant {code NULL}.
	*/		*/
	public void testUpgrade() throws Exception {		public void testUpgrade() throws Exception {
	final File file = new File(getContext().getCacheDir(), "setting.xml");
	file.delete();
	final Object lock = new Object();		final Object lock = new Object();
	final PrintStream os = new PrintStream(new FileOutputStream(file));		final PrintStream os = new PrintStream(new FileOutputStream(mSettingsFile));
	os.print(		os.print(
	"<?xml version='1.0' encoding='UTF-8' standalone='yes' ?>" +		"<?xml version='1.0' encoding='UTF-8' standalone='yes' ?>" +
	"<settings version=\"120\">" +		"<settings version=\"120\">" +
	@@ -192,7 +199,7 @@ public class SettingsStateTest extends AndroidTestCase {
	"</settings>");		"</settings>");
	os.close();		os.close();

	final SettingsState ss = new SettingsState(getContext(), lock, file, 1,		final SettingsState ss = new SettingsState(getContext(), lock, mSettingsFile, 1,
	SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper());		SettingsState.MAX_BYTES_PER_APP_PACKAGE_UNLIMITED, Looper.getMainLooper());
	synchronized (lock) {		synchronized (lock) {
	SettingsState.Setting s;		SettingsState.Setting s;
	@@ -213,7 +220,8 @@ public class SettingsStateTest extends AndroidTestCase {
	public void testInitializeSetting_preserveFlagNotSet() {		public void testInitializeSetting_preserveFlagNotSet() {
	SettingsState settingsWriter = getSettingStateObject();		SettingsState settingsWriter = getSettingStateObject();
	settingsWriter.insertSettingLocked(SETTING_NAME, "1", null, false, TEST_PACKAGE);		settingsWriter.insertSettingLocked(SETTING_NAME, "1", null, false, TEST_PACKAGE);
	settingsWriter.persistSyncLocked();		settingsWriter.persistSettingsLocked();
			settingsWriter.waitForHandler();

	SettingsState settingsReader = getSettingStateObject();		SettingsState settingsReader = getSettingStateObject();
	assertFalse(settingsReader.getSettingLocked(SETTING_NAME).isValuePreservedInRestore());		assertFalse(settingsReader.getSettingLocked(SETTING_NAME).isValuePreservedInRestore());
	@@ -223,7 +231,8 @@ public class SettingsStateTest extends AndroidTestCase {
	SettingsState settingsWriter = getSettingStateObject();		SettingsState settingsWriter = getSettingStateObject();
	settingsWriter.insertSettingLocked(SETTING_NAME, "1", null, false, TEST_PACKAGE);		settingsWriter.insertSettingLocked(SETTING_NAME, "1", null, false, TEST_PACKAGE);
	settingsWriter.insertSettingLocked(SETTING_NAME, "2", null, false, TEST_PACKAGE);		settingsWriter.insertSettingLocked(SETTING_NAME, "2", null, false, TEST_PACKAGE);
	settingsWriter.persistSyncLocked();		settingsWriter.persistSettingsLocked();
			settingsWriter.waitForHandler();

	SettingsState settingsReader = getSettingStateObject();		SettingsState settingsReader = getSettingStateObject();
	assertTrue(settingsReader.getSettingLocked(SETTING_NAME).isValuePreservedInRestore());		assertTrue(settingsReader.getSettingLocked(SETTING_NAME).isValuePreservedInRestore());
	@@ -234,7 +243,8 @@ public class SettingsStateTest extends AndroidTestCase {
	settingsWriter.insertSettingLocked(SETTING_NAME, "1", null, false, TEST_PACKAGE);		settingsWriter.insertSettingLocked(SETTING_NAME, "1", null, false, TEST_PACKAGE);
	settingsWriter.insertSettingLocked(SETTING_NAME, "2", null, false, false, TEST_PACKAGE,		settingsWriter.insertSettingLocked(SETTING_NAME, "2", null, false, false, TEST_PACKAGE,
	/* overrideableByRestore */ true);		/* overrideableByRestore */ true);
	settingsWriter.persistSyncLocked();		settingsWriter.persistSettingsLocked();
			settingsWriter.waitForHandler();

	SettingsState settingsReader = getSettingStateObject();		SettingsState settingsReader = getSettingStateObject();
	assertFalse(settingsReader.getSettingLocked(SETTING_NAME).isValuePreservedInRestore());		assertFalse(settingsReader.getSettingLocked(SETTING_NAME).isValuePreservedInRestore());
	@@ -250,7 +260,8 @@ public class SettingsStateTest extends AndroidTestCase {
	// already been set to true.		// already been set to true.
	settingsWriter.insertSettingLocked(SETTING_NAME, "2", null, false, false, TEST_PACKAGE,		settingsWriter.insertSettingLocked(SETTING_NAME, "2", null, false, false, TEST_PACKAGE,
	/* overrideableByRestore */ true);		/* overrideableByRestore */ true);
	settingsWriter.persistSyncLocked();		settingsWriter.persistSettingsLocked();
			settingsWriter.waitForHandler();

	SettingsState settingsReader = getSettingStateObject();		SettingsState settingsReader = getSettingStateObject();
	assertTrue(settingsReader.getSettingLocked(SETTING_NAME).isValuePreservedInRestore());		assertTrue(settingsReader.getSettingLocked(SETTING_NAME).isValuePreservedInRestore());
	@@ -481,8 +492,11 @@ public class SettingsStateTest extends AndroidTestCase {
	settingsState.insertSettingLocked(		settingsState.insertSettingLocked(
	FLAG_NAME_1_STAGED, VALUE1, null, false, TEST_PACKAGE);		FLAG_NAME_1_STAGED, VALUE1, null, false, TEST_PACKAGE);
	settingsState.insertSettingLocked(FLAG_NAME_2, VALUE2, null, false, TEST_PACKAGE);		settingsState.insertSettingLocked(FLAG_NAME_2, VALUE2, null, false, TEST_PACKAGE);
	settingsState.persistSyncLocked();		settingsState.persistSettingsLocked();
			}
			settingsState.waitForHandler();

			synchronized (lock) {
	assertEquals(VALUE1, settingsState.getSettingLocked(FLAG_NAME_1_STAGED).getValue());		assertEquals(VALUE1, settingsState.getSettingLocked(FLAG_NAME_1_STAGED).getValue());
	assertEquals(VALUE2, settingsState.getSettingLocked(FLAG_NAME_2).getValue());		assertEquals(VALUE2, settingsState.getSettingLocked(FLAG_NAME_2).getValue());
	}		}
	@@ -522,7 +536,10 @@ public class SettingsStateTest extends AndroidTestCase {
	synchronized (lock) {		synchronized (lock) {
	settingsState.insertSettingLocked(INVALID_STAGED_FLAG_1,		settingsState.insertSettingLocked(INVALID_STAGED_FLAG_1,
	VALUE2, null, false, TEST_PACKAGE);		VALUE2, null, false, TEST_PACKAGE);
	settingsState.persistSyncLocked();		settingsState.persistSettingsLocked();
			}
			settingsState.waitForHandler();
			synchronized (lock) {
	assertEquals(VALUE2, settingsState.getSettingLocked(INVALID_STAGED_FLAG_1).getValue());		assertEquals(VALUE2, settingsState.getSettingLocked(INVALID_STAGED_FLAG_1).getValue());
	}		}