From 138b83347b8da29166ee2eb09fa8126686bda3c7 Mon Sep 17 00:00:00 2001 From: Adrian Roos Date: Tue, 11 Nov 2014 13:51:07 +0100 Subject: [PATCH] Obliterate LockPatternUtilsCache It is disabled dead code already and not useful anymore with the new caching in LockSettingsService. Bug: 18163444 Change-Id: Icc184e923e0fbeab31ed128336c01f835b24c6f2 --- Android.mk | 1 - CleanSpec.mk | 1 + .../com/android/internal/widget/ILockSettings.aidl | 4 - .../internal/widget/ILockSettingsObserver.aidl | 31 --- .../android/internal/widget/LockPatternUtils.java | 13 +- .../internal/widget/LockPatternUtilsCache.java | 262 --------------------- .../com/android/server/LockSettingsService.java | 67 ------ 7 files changed, 2 insertions(+), 377 deletions(-) delete mode 100644 core/java/com/android/internal/widget/ILockSettingsObserver.aidl delete mode 100644 core/java/com/android/internal/widget/LockPatternUtilsCache.java diff --git a/Android.mk b/Android.mk index 9630e3457b91..ed85df0ffef7 100644 --- a/Android.mk +++ b/Android.mk @@ -288,7 +288,6 @@ LOCAL_SRC_FILES += \ core/java/com/android/internal/view/IInputMethodSession.aidl \ core/java/com/android/internal/view/IInputSessionCallback.aidl \ core/java/com/android/internal/widget/ILockSettings.aidl \ - core/java/com/android/internal/widget/ILockSettingsObserver.aidl \ core/java/com/android/internal/widget/IRemoteViewsFactory.aidl \ core/java/com/android/internal/widget/IRemoteViewsAdapterConnection.aidl \ keystore/java/android/security/IKeyChainAliasCallback.aidl \ diff --git a/CleanSpec.mk b/CleanSpec.mk index 28c2172294ba..d66022428a85 100644 --- a/CleanSpec.mk +++ b/CleanSpec.mk @@ -224,6 +224,7 @@ $(call add-clean-step, rm -rf $(OUT_DIR)/target/common/obj/JAVA_LIBRARIES/servic $(call add-clean-step, rm -rf $(PRODUCT_OUT)/system/bin/inputflinger $(PRODUCT_OUT)/symbols/system/bin/inputflinger) $(call add-clean-step, rm -rf $(OUT_DIR)/target/common/obj/APPS/RsFountainFbo_intermediates) $(call add-clean-step, rm -rf $(OUT_DIR)/target/common/obj/JAVA_LIBRARIES/framework-base_intermediates/src/telecomm/java/com/android/internal/telecomm) +$(call add-clean-step, rm -rf $(OUT_DIR)/target/common/obj/JAVA_LIBRARIES/framework_intermediates/src/core/java/com/android/internal/widget/ILockSettingsObserver.java) # ****************************************************************** # NEWER CLEAN STEPS MUST BE AT THE END OF THE LIST ABOVE THIS BANNER diff --git a/core/java/com/android/internal/widget/ILockSettings.aidl b/core/java/com/android/internal/widget/ILockSettings.aidl index c70841b04931..9501f92fc75f 100644 --- a/core/java/com/android/internal/widget/ILockSettings.aidl +++ b/core/java/com/android/internal/widget/ILockSettings.aidl @@ -16,8 +16,6 @@ package com.android.internal.widget; -import com.android.internal.widget.ILockSettingsObserver; - /** {@hide} */ interface ILockSettings { void setBoolean(in String key, in boolean value, in int userId); @@ -34,6 +32,4 @@ interface ILockSettings { boolean havePattern(int userId); boolean havePassword(int userId); void removeUser(int userId); - void registerObserver(in ILockSettingsObserver observer); - void unregisterObserver(in ILockSettingsObserver observer); } diff --git a/core/java/com/android/internal/widget/ILockSettingsObserver.aidl b/core/java/com/android/internal/widget/ILockSettingsObserver.aidl deleted file mode 100644 index edf8f0e27d52..000000000000 --- a/core/java/com/android/internal/widget/ILockSettingsObserver.aidl +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright (C) 2014 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.internal.widget; - -/** {@hide} */ -oneway interface ILockSettingsObserver { - /** - * Called when a lock setting has changed. - * - * Note: Impementations of this should do as little work as possible, because this may be - * called synchronously while writing a setting. - * - * @param key the key of the setting that has changed or {@code null} if any may have changed. - * @param userId the user whose setting has changed. - */ - void onLockSettingChanged(in String key, in int userId); -} diff --git a/core/java/com/android/internal/widget/LockPatternUtils.java b/core/java/com/android/internal/widget/LockPatternUtils.java index a4b8380d730a..3ccced50e93c 100644 --- a/core/java/com/android/internal/widget/LockPatternUtils.java +++ b/core/java/com/android/internal/widget/LockPatternUtils.java @@ -65,13 +65,6 @@ public class LockPatternUtils { private static final boolean DEBUG = false; /** - * If true, LockPatternUtils will cache its values in-process. While this leads to faster reads, - * it can cause problems because writes to to the settings are no longer synchronous - * across all processes. - */ - private static final boolean ENABLE_CLIENT_CACHE = false; - - /** * The maximum number of incorrect attempts before the user is prevented * from trying again for {@link #FAILED_ATTEMPT_TIMEOUT_MS}. */ @@ -216,11 +209,7 @@ public class LockPatternUtils { if (mLockSettingsService == null) { ILockSettings service = ILockSettings.Stub.asInterface( ServiceManager.getService("lock_settings")); - if (ENABLE_CLIENT_CACHE) { - mLockSettingsService = LockPatternUtilsCache.getInstance(service); - } else { - mLockSettingsService = service; - } + mLockSettingsService = service; } return mLockSettingsService; } diff --git a/core/java/com/android/internal/widget/LockPatternUtilsCache.java b/core/java/com/android/internal/widget/LockPatternUtilsCache.java deleted file mode 100644 index a9524fffff87..000000000000 --- a/core/java/com/android/internal/widget/LockPatternUtilsCache.java +++ /dev/null @@ -1,262 +0,0 @@ -/* - * Copyright (C) 2014 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.internal.widget; - -import android.os.IBinder; -import android.os.RemoteException; -import android.os.UserHandle; -import android.util.ArrayMap; -import android.util.Log; - -/** - * A decorator for {@link ILockSettings} that caches the key-value responses in memory. - * - * Specifically, the return values of {@link #getString(String, String, int)}, - * {@link #getLong(String, long, int)} and {@link #getBoolean(String, boolean, int)} are cached. - */ -public class LockPatternUtilsCache implements ILockSettings { - - private static final String TAG = "LockPatternUtilsCache"; - - public static final String HAS_LOCK_PATTERN_CACHE_KEY - = "LockPatternUtils.Cache.HasLockPatternCacheKey"; - public static final String HAS_LOCK_PASSWORD_CACHE_KEY - = "LockPatternUtils.Cache.HasLockPasswordCacheKey"; - - private static LockPatternUtilsCache sInstance; - - private final ILockSettings mService; - - /** Only access when holding {@code mCache} lock. */ - private final ArrayMap mCache = new ArrayMap<>(); - - /** Only access when holding {@link #mCache} lock. */ - private final CacheKey mCacheKey = new CacheKey(); - - - public static synchronized LockPatternUtilsCache getInstance(ILockSettings service) { - if (sInstance == null) { - sInstance = new LockPatternUtilsCache(service); - } - return sInstance; - } - - // ILockSettings - - public LockPatternUtilsCache(ILockSettings service) { - mService = service; - try { - service.registerObserver(mObserver); - } catch (RemoteException e) { - // Not safe to do caching without the observer. System process has probably died - // anyway, so crashing here is fine. - throw new RuntimeException(e); - } - } - - public void setBoolean(String key, boolean value, int userId) throws RemoteException { - invalidateCache(key, userId); - mService.setBoolean(key, value, userId); - putCache(key, userId, value); - } - - public void setLong(String key, long value, int userId) throws RemoteException { - invalidateCache(key, userId); - mService.setLong(key, value, userId); - putCache(key, userId, value); - } - - public void setString(String key, String value, int userId) throws RemoteException { - invalidateCache(key, userId); - mService.setString(key, value, userId); - putCache(key, userId, value); - } - - public long getLong(String key, long defaultValue, int userId) throws RemoteException { - Object value = peekCache(key, userId); - if (value instanceof Long) { - return (long) value; - } - long result = mService.getLong(key, defaultValue, userId); - putCache(key, userId, result); - return result; - } - - public String getString(String key, String defaultValue, int userId) throws RemoteException { - Object value = peekCache(key, userId); - if (value instanceof String) { - return (String) value; - } - String result = mService.getString(key, defaultValue, userId); - putCache(key, userId, result); - return result; - } - - public boolean getBoolean(String key, boolean defaultValue, int userId) throws RemoteException { - Object value = peekCache(key, userId); - if (value instanceof Boolean) { - return (boolean) value; - } - boolean result = mService.getBoolean(key, defaultValue, userId); - putCache(key, userId, result); - return result; - } - - @Override - public void setLockPattern(String pattern, int userId) throws RemoteException { - invalidateCache(HAS_LOCK_PATTERN_CACHE_KEY, userId); - mService.setLockPattern(pattern, userId); - putCache(HAS_LOCK_PATTERN_CACHE_KEY, userId, pattern != null); - } - - @Override - public boolean checkPattern(String pattern, int userId) throws RemoteException { - return mService.checkPattern(pattern, userId); - } - - @Override - public void setLockPassword(String password, int userId) throws RemoteException { - invalidateCache(HAS_LOCK_PASSWORD_CACHE_KEY, userId); - mService.setLockPassword(password, userId); - putCache(HAS_LOCK_PASSWORD_CACHE_KEY, userId, password != null); - } - - @Override - public boolean checkPassword(String password, int userId) throws RemoteException { - return mService.checkPassword(password, userId); - } - - @Override - public boolean checkVoldPassword(int userId) throws RemoteException { - return mService.checkVoldPassword(userId); - } - - @Override - public boolean havePattern(int userId) throws RemoteException { - Object value = peekCache(HAS_LOCK_PATTERN_CACHE_KEY, userId); - if (value instanceof Boolean) { - return (boolean) value; - } - boolean result = mService.havePattern(userId); - putCache(HAS_LOCK_PATTERN_CACHE_KEY, userId, result); - return result; - } - - @Override - public boolean havePassword(int userId) throws RemoteException { - Object value = peekCache(HAS_LOCK_PASSWORD_CACHE_KEY, userId); - if (value instanceof Boolean) { - return (boolean) value; - } - boolean result = mService.havePassword(userId); - putCache(HAS_LOCK_PASSWORD_CACHE_KEY, userId, result); - return result; - } - - @Override - public void removeUser(int userId) throws RemoteException { - mService.removeUser(userId); - } - - @Override - public void registerObserver(ILockSettingsObserver observer) throws RemoteException { - mService.registerObserver(observer); - } - - @Override - public void unregisterObserver(ILockSettingsObserver observer) throws RemoteException { - mService.unregisterObserver(observer); - } - - @Override - public IBinder asBinder() { - return mService.asBinder(); - } - - // Caching - - private Object peekCache(String key, int userId) { - if (!validateUserId(userId)) return null; - synchronized (mCache) { - // Safe to reuse mCacheKey, because it is not stored in the map. - return mCache.get(mCacheKey.set(key, userId)); - } - } - - private void putCache(String key, int userId, Object value) { - if (!validateUserId(userId)) return; - synchronized (mCache) { - // Create a new key, because this will be stored in the map. - mCache.put(new CacheKey().set(key, userId), value); - } - } - - private void invalidateCache(String key, int userId) { - if (!validateUserId(userId)) return; - synchronized (mCache) { - if (key != null) { - // Safe to reuse mCacheKey, because it is not stored in the map. - mCache.remove(mCacheKey.set(key, userId)); - } else { - mCache.clear(); - } - } - } - - private final ILockSettingsObserver mObserver = new ILockSettingsObserver.Stub() { - @Override - public void onLockSettingChanged(String key, int userId) throws RemoteException { - invalidateCache(key, userId); - } - }; - - private final boolean validateUserId(int userId) { - if (userId < UserHandle.USER_OWNER) { - Log.e(TAG, "User " + userId + " not supported: Must be a concrete user."); - return false; - } - return true; - } - - private static final class CacheKey { - String key; - int userId; - - public CacheKey set(String key, int userId) { - this.key = key; - this.userId = userId; - return this; - } - - public CacheKey copy() { - return new CacheKey().set(key, userId); - } - - @Override - public boolean equals(Object obj) { - if (!(obj instanceof CacheKey)) - return false; - CacheKey o = (CacheKey) obj; - return userId == o.userId && key.equals(o.key); - } - - @Override - public int hashCode() { - return key.hashCode() ^ userId; - } - } -} diff --git a/services/core/java/com/android/server/LockSettingsService.java b/services/core/java/com/android/server/LockSettingsService.java index ae84846e1744..77662cc4969a 100644 --- a/services/core/java/com/android/server/LockSettingsService.java +++ b/services/core/java/com/android/server/LockSettingsService.java @@ -43,13 +43,10 @@ import android.provider.Settings.Secure; import android.provider.Settings.SettingNotFoundException; import android.security.KeyStore; import android.text.TextUtils; -import android.util.Log; import android.util.Slog; import com.android.internal.widget.ILockSettings; -import com.android.internal.widget.ILockSettingsObserver; import com.android.internal.widget.LockPatternUtils; -import com.android.internal.widget.LockPatternUtilsCache; import java.util.ArrayList; import java.util.Arrays; @@ -65,9 +62,6 @@ public class LockSettingsService extends ILockSettings.Stub { private static final String PERMISSION = ACCESS_KEYGUARD_SECURE_STORAGE; - private static final String SYSTEM_DEBUGGABLE = "ro.debuggable"; - - private static final String TAG = "LockSettingsService"; private final Context mContext; @@ -77,8 +71,6 @@ public class LockSettingsService extends ILockSettings.Stub { private LockPatternUtils mLockPatternUtils; private boolean mFirstCallToVold; - private final ArrayList mObservers = new ArrayList<>(); - public LockSettingsService(Context context) { mContext = context; // Open the database @@ -233,7 +225,6 @@ public class LockSettingsService extends ILockSettings.Stub { private void setStringUnchecked(String key, int userId, String value) { mStorage.writeKeyValue(key, value, userId); - notifyObservers(key, userId); } @Override @@ -261,52 +252,6 @@ public class LockSettingsService extends ILockSettings.Stub { } @Override - public void registerObserver(ILockSettingsObserver remote) throws RemoteException { - synchronized (mObservers) { - for (int i = 0; i < mObservers.size(); i++) { - if (mObservers.get(i).remote.asBinder() == remote.asBinder()) { - boolean isDebuggable = "1".equals(SystemProperties.get(SYSTEM_DEBUGGABLE, "0")); - if (isDebuggable) { - throw new IllegalStateException("Observer was already registered."); - } else { - Log.e(TAG, "Observer was already registered."); - return; - } - } - } - LockSettingsObserver o = new LockSettingsObserver(); - o.remote = remote; - o.remote.asBinder().linkToDeath(o, 0); - mObservers.add(o); - } - } - - @Override - public void unregisterObserver(ILockSettingsObserver remote) throws RemoteException { - synchronized (mObservers) { - for (int i = 0; i < mObservers.size(); i++) { - if (mObservers.get(i).remote.asBinder() == remote.asBinder()) { - mObservers.remove(i); - return; - } - } - } - } - - public void notifyObservers(String key, int userId) { - synchronized (mObservers) { - for (int i = 0; i < mObservers.size(); i++) { - try { - mObservers.get(i).remote.onLockSettingChanged(key, userId); - } catch (RemoteException e) { - // The stack trace is not really helpful here. - Log.e(TAG, "Failed to notify ILockSettingsObserver: " + e); - } - } - } - } - - @Override public boolean havePassword(int userId) throws RemoteException { // Do we need a permissions check here? @@ -354,7 +299,6 @@ public class LockSettingsService extends ILockSettings.Stub { final byte[] hash = LockPatternUtils.patternToHash( LockPatternUtils.stringToPattern(pattern)); mStorage.writePatternHash(hash, userId); - notifyObservers(LockPatternUtilsCache.HAS_LOCK_PATTERN_CACHE_KEY, userId); } @Override @@ -364,7 +308,6 @@ public class LockSettingsService extends ILockSettings.Stub { maybeUpdateKeystore(password, userId); mStorage.writePasswordHash(mLockPatternUtils.passwordToHash(password, userId), userId); - notifyObservers(LockPatternUtilsCache.HAS_LOCK_PASSWORD_CACHE_KEY, userId); } @Override @@ -452,7 +395,6 @@ public class LockSettingsService extends ILockSettings.Stub { checkWritePermission(userId); mStorage.removeUser(userId); - notifyObservers(null /* key */, userId); final KeyStore ks = KeyStore.getInstance(); final int userUid = UserHandle.getUid(userId, Process.SYSTEM_UID); @@ -491,13 +433,4 @@ public class LockSettingsService extends ILockSettings.Stub { } return null; } - - private class LockSettingsObserver implements DeathRecipient { - ILockSettingsObserver remote; - - @Override - public void binderDied() { - mObservers.remove(this); - } - } } -- 2.11.0