From 0cfacf2dfa08d46cbedc5b957df806564956eea8 Mon Sep 17 00:00:00 2001 From: Heemin Seog Date: Sat, 19 Oct 2019 16:18:38 -0700 Subject: [PATCH 01/10] DO NOT MERGE Remove use of Dependency in SliceProvider SliceProviders seem to be created before onCreate is called on the application which causes some issues with using Dependency in Car System UI. Bug: 142974942 Test: manual for car sys ui and atest KeyguardSliceProviderTest on sdk_gphone_x86 emulator Change-Id: I18d14d51c6b0b3219923b640d9f2145ed9b02b94 (cherry picked from commit e9c5f9625d988edd48703fb208a20ae2c0bfefbe) --- .../com/android/systemui/keyguard/KeyguardSliceProvider.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardSliceProvider.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardSliceProvider.java index 5795dcce861c0..e408745699edd 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardSliceProvider.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardSliceProvider.java @@ -49,7 +49,6 @@ import androidx.slice.builders.SliceAction; import com.android.internal.annotations.VisibleForTesting; import com.android.keyguard.KeyguardUpdateMonitor; import com.android.keyguard.KeyguardUpdateMonitorCallback; -import com.android.systemui.Dependency; import com.android.systemui.R; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.NotificationMediaManager; @@ -59,6 +58,7 @@ import com.android.systemui.statusbar.phone.KeyguardBypassController; import com.android.systemui.statusbar.policy.NextAlarmController; import com.android.systemui.statusbar.policy.NextAlarmControllerImpl; import com.android.systemui.statusbar.policy.ZenModeController; +import com.android.systemui.statusbar.policy.ZenModeControllerImpl; import com.android.systemui.util.wakelock.SettableWakeLock; import com.android.systemui.util.wakelock.WakeLock; @@ -316,7 +316,7 @@ public class KeyguardSliceProvider extends SliceProvider implements mContentResolver = getContext().getContentResolver(); mNextAlarmController = new NextAlarmControllerImpl(getContext()); mNextAlarmController.addCallback(this); - mZenModeController = Dependency.get(ZenModeController.class); + mZenModeController = new ZenModeControllerImpl(getContext(), mHandler); mZenModeController.addCallback(this); mDatePattern = getContext().getString(R.string.system_ui_aod_date_pattern); mPendingIntent = PendingIntent.getActivity(getContext(), 0, new Intent(), 0); From 83c8318a29776a78343decd59f70b2fda1073d37 Mon Sep 17 00:00:00 2001 From: Heemin Seog Date: Tue, 26 Nov 2019 13:33:37 -0800 Subject: [PATCH 02/10] DO NOT MERGE Using Dependency.get causes issues for CarSysUI Bug: 145218907 Test: manual Change-Id: I488123f79ea0040a4929f2b28d3719f12f1e6a03 (cherry picked from commit b5c4ac05bd985e4c5d06b48056f54244d8b3f6da) --- .../systemui/statusbar/KeyguardIndicationController.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/KeyguardIndicationController.java b/packages/SystemUI/src/com/android/systemui/statusbar/KeyguardIndicationController.java index dd5255afaf7fb..8ee7305d99c56 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/KeyguardIndicationController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/KeyguardIndicationController.java @@ -144,7 +144,7 @@ public class KeyguardIndicationController implements StateListener, Dependency.get(AccessibilityController.class), UnlockMethodCache.getInstance(context), Dependency.get(StatusBarStateController.class), - Dependency.get(KeyguardUpdateMonitor.class), + KeyguardUpdateMonitor.getInstance(context), Dependency.get(DockManager.class)); } From 8fb203e39eda748e96461136a4cb754980375cf2 Mon Sep 17 00:00:00 2001 From: Jian Jin Date: Wed, 11 Dec 2019 00:35:05 +0000 Subject: [PATCH 03/10] Revert "DO NOT MERGE - Add AUTOMOTIVE_USER_SETUP_IN_PROGRESS to Settings" This reverts commit ba4b561c3e292cca1e2b58276ff2671ef6d53ae7. Reason for revert: Move to CarSettings Change-Id: I91fd1fa7dee2400095a014c189a40706b853c471 (cherry picked from commit db70de36565616f2ff33ddb646e584fe0a460625) --- core/java/android/provider/Settings.java | 10 ---------- core/proto/android/providers/settings/secure.proto | 9 +-------- .../src/android/provider/SettingsBackupTest.java | 1 - .../providers/settings/SettingsProtoDumpUtil.java | 4 ---- .../providers/settings/SettingsProvider.java | 13 ------------- 5 files changed, 1 insertion(+), 36 deletions(-) diff --git a/core/java/android/provider/Settings.java b/core/java/android/provider/Settings.java index 9c416a0b5ad9a..b9f5348027e6e 100644 --- a/core/java/android/provider/Settings.java +++ b/core/java/android/provider/Settings.java @@ -5979,16 +5979,6 @@ public final class Settings { @TestApi public static final String USER_SETUP_COMPLETE = "user_setup_complete"; - /** - * Whether the Auto Embedded Setup Wizard (0 = false, 1 = true) is in progress. - * This differs from USER_SETUP_COMPLETE in that it can be reset back to 0 - * in case Setup Wizard has been re-enabled on Auto Embedded devices. - * - * @hide - */ - public static final String AUTOMOTIVE_USER_SETUP_IN_PROGRESS = - "automotive_user_setup_in_progress"; - /** * Indicates that the user has not started setup personalization. * One of the possible states for {@link #USER_SETUP_PERSONALIZATION_STATE}. diff --git a/core/proto/android/providers/settings/secure.proto b/core/proto/android/providers/settings/secure.proto index d0be86f7babd4..ef413b9b04cf2 100644 --- a/core/proto/android/providers/settings/secure.proto +++ b/core/proto/android/providers/settings/secure.proto @@ -139,13 +139,6 @@ message SecureSettingsProto { } optional AutomaticStorageManager automatic_storage_manager = 9; - message Automotive { - option (android.msg_privacy).dest = DEST_EXPLICIT; - - optional SettingProto automotive_user_setup_in_progress = 1 [ (android.privacy).dest = DEST_AUTOMATIC ]; - } - optional Automotive automotive = 77; - message Backup { option (android.msg_privacy).dest = DEST_EXPLICIT; @@ -573,5 +566,5 @@ message SecureSettingsProto { // Please insert fields in alphabetical order and group them into messages // if possible (to avoid reaching the method limit). - // Next tag = 78; + // Next tag = 77; } diff --git a/core/tests/coretests/src/android/provider/SettingsBackupTest.java b/core/tests/coretests/src/android/provider/SettingsBackupTest.java index 029a06ef3aa9b..b7b02a3d61f1a 100644 --- a/core/tests/coretests/src/android/provider/SettingsBackupTest.java +++ b/core/tests/coretests/src/android/provider/SettingsBackupTest.java @@ -703,7 +703,6 @@ public class SettingsBackupTest { Settings.Secure.UNSAFE_VOLUME_MUSIC_ACTIVE_MS, Settings.Secure.USB_AUDIO_AUTOMATIC_ROUTING_DISABLED, Settings.Secure.USER_SETUP_COMPLETE, - Settings.Secure.AUTOMOTIVE_USER_SETUP_IN_PROGRESS, Settings.Secure.USER_SETUP_PERSONALIZATION_STATE, Settings.Secure.VOICE_INTERACTION_SERVICE, Settings.Secure.VOICE_RECOGNITION_SERVICE, diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java index 514d3523f750b..894ba9cd2f061 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProtoDumpUtil.java @@ -2411,10 +2411,6 @@ class SettingsProtoDumpUtil { Settings.Secure.USER_SETUP_COMPLETE, SecureSettingsProto.USER_SETUP_COMPLETE); - dumpSetting(s, p, - Settings.Secure.AUTOMOTIVE_USER_SETUP_IN_PROGRESS, - SecureSettingsProto.Automotive.AUTOMOTIVE_USER_SETUP_IN_PROGRESS); - final long voiceToken = p.start(SecureSettingsProto.VOICE); dumpSetting(s, p, Settings.Secure.VOICE_INTERACTION_SERVICE, diff --git a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java index c900308fec946..fbfbe7fc1d9b9 100644 --- a/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java +++ b/packages/SettingsProvider/src/com/android/providers/settings/SettingsProvider.java @@ -4449,19 +4449,6 @@ public class SettingsProvider extends ContentProvider { currentVersion = 183; } - if (currentVersion == 183) { - // Version 169: by default, automotive setup wizard is in progress - final SettingsState secureSettings = getSecureSettingsLocked(userId); - Setting currentSetting = secureSettings.getSettingLocked( - Secure.AUTOMOTIVE_USER_SETUP_IN_PROGRESS); - if (currentSetting.isNull()) { - secureSettings.insertSettingLocked( - Secure.AUTOMOTIVE_USER_SETUP_IN_PROGRESS, "1", null, true, - SettingsState.SYSTEM_PACKAGE_NAME); - } - currentVersion = 184; - } - // vXXX: Add new settings above this point. if (currentVersion != newVersion) { From 40dd65db3996399530cf549b3afe0c3a5f884b78 Mon Sep 17 00:00:00 2001 From: Fabian Kozynski Date: Wed, 18 Dec 2019 09:54:09 -0500 Subject: [PATCH 04/10] Prevent sending early termination of appop use If a package starts a particular appop both as active and noted, once one of them is finished (usually noted after 5s), a message will be sent to callbacks indicating the end of the use. However, the app op may still be active. This could result in the removal of indicators prematurely from notifications. This change prevents that from happening by checking if the app op is still in use by that combination uid/package (either active or noted) and not notifying listeners if that's the case. Test: atest AppOpsControllerTest Test: use app from bug report. Observe that notification does not lose the microphone indicator Bug: 144092031 Merged-In: I180e7c257e6171e7686ba7eda9bf02249358ed0 Change-Id: I94473c3ccf1318dac29f067dade91e540e20285e (cherry picked from commit 1a459638398446938a20b32fa0fbc63ad4bd505f) --- .../systemui/appops/AppOpsControllerImpl.java | 51 ++++++- .../systemui/appops/AppOpsControllerTest.java | 138 +++++++++++++++++- 2 files changed, 179 insertions(+), 10 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java b/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java index 7a74dba27dce3..b2eedaf085cf6 100644 --- a/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java +++ b/packages/SystemUI/src/com/android/systemui/appops/AppOpsControllerImpl.java @@ -195,20 +195,32 @@ public class AppOpsControllerImpl implements AppOpsController, mNotedItems.remove(item); if (DEBUG) Log.w(TAG, "Removed item: " + item.toString()); } - notifySuscribers(code, uid, packageName, false); + boolean active; + // Check if the item is also active + synchronized (mActiveItems) { + active = getAppOpItem(mActiveItems, code, uid, packageName) != null; + } + if (!active) { + notifySuscribers(code, uid, packageName, false); + } } - private void addNoted(int code, int uid, String packageName) { + private boolean addNoted(int code, int uid, String packageName) { AppOpItem item; + boolean createdNew = false; synchronized (mNotedItems) { item = getAppOpItem(mNotedItems, code, uid, packageName); if (item == null) { item = new AppOpItem(code, uid, packageName, System.currentTimeMillis()); mNotedItems.add(item); if (DEBUG) Log.w(TAG, "Added item: " + item.toString()); + createdNew = true; } } + // We should keep this so we make sure it cannot time out. + mBGHandler.removeCallbacksAndMessages(item); mBGHandler.scheduleRemoval(item, NOTED_OP_TIME_DELAY_MS); + return createdNew; } /** @@ -255,23 +267,46 @@ public class AppOpsControllerImpl implements AppOpsController, @Override public void onOpActiveChanged(int code, int uid, String packageName, boolean active) { - if (updateActives(code, uid, packageName, active)) { - notifySuscribers(code, uid, packageName, active); + if (DEBUG) { + Log.w(TAG, String.format("onActiveChanged(%d,%d,%s,%s", code, uid, packageName, + Boolean.toString(active))); + } + boolean activeChanged = updateActives(code, uid, packageName, active); + if (!activeChanged) return; // early return + // Check if the item is also noted, in that case, there's no update. + boolean alsoNoted; + synchronized (mNotedItems) { + alsoNoted = getAppOpItem(mNotedItems, code, uid, packageName) != null; + } + // If active is true, we only send the update if the op is not actively noted (already true) + // If active is false, we only send the update if the op is not actively noted (prevent + // early removal) + if (!alsoNoted) { + mBGHandler.post(() -> notifySuscribers(code, uid, packageName, active)); } } @Override public void onOpNoted(int code, int uid, String packageName, int result) { if (DEBUG) { - Log.w(TAG, "Op: " + code + " with result " + AppOpsManager.MODE_NAMES[result]); + Log.w(TAG, "Noted op: " + code + " with result " + + AppOpsManager.MODE_NAMES[result] + " for package " + packageName); } if (result != AppOpsManager.MODE_ALLOWED) return; - addNoted(code, uid, packageName); - notifySuscribers(code, uid, packageName, true); + boolean notedAdded = addNoted(code, uid, packageName); + if (!notedAdded) return; // early return + boolean alsoActive; + synchronized (mActiveItems) { + alsoActive = getAppOpItem(mActiveItems, code, uid, packageName) != null; + } + if (!alsoActive) { + mBGHandler.post(() -> notifySuscribers(code, uid, packageName, true)); + } } private void notifySuscribers(int code, int uid, String packageName, boolean active) { if (mCallbacksByCode.containsKey(code)) { + if (DEBUG) Log.d(TAG, "Notifying of change in package " + packageName); for (Callback cb: mCallbacksByCode.get(code)) { cb.onActiveStateChanged(code, uid, packageName, active); } @@ -295,7 +330,7 @@ public class AppOpsControllerImpl implements AppOpsController, } - protected final class H extends Handler { + protected class H extends Handler { H(Looper looper) { super(looper); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java index 59d5c243af730..a1842f821fe0e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/appops/AppOpsControllerTest.java @@ -28,6 +28,8 @@ import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static java.lang.Thread.sleep; + import android.app.AppOpsManager; import android.content.pm.PackageManager; import android.os.UserHandle; @@ -36,7 +38,6 @@ import android.testing.TestableLooper; import androidx.test.filters.SmallTest; -import com.android.systemui.Dependency; import com.android.systemui.SysuiTestCase; import org.junit.Before; @@ -45,6 +46,8 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.List; + @SmallTest @RunWith(AndroidTestingRunner.class) @TestableLooper.RunWithLooper @@ -63,14 +66,16 @@ public class AppOpsControllerTest extends SysuiTestCase { private AppOpsControllerImpl.H mMockHandler; private AppOpsControllerImpl mController; + private TestableLooper mTestableLooper; @Before public void setUp() { MockitoAnnotations.initMocks(this); + mTestableLooper = TestableLooper.get(this); getContext().addMockSystemService(AppOpsManager.class, mAppOpsManager); - mController = new AppOpsControllerImpl(mContext, Dependency.get(Dependency.BG_LOOPER)); + mController = new AppOpsControllerImpl(mContext, mTestableLooper.getLooper()); } @Test @@ -94,6 +99,7 @@ public class AppOpsControllerTest extends SysuiTestCase { AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true); mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, AppOpsManager.MODE_ALLOWED); + mTestableLooper.processAllMessages(); verify(mCallback).onActiveStateChanged(AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true); } @@ -103,6 +109,7 @@ public class AppOpsControllerTest extends SysuiTestCase { mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback); mController.onOpActiveChanged( AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true); + mTestableLooper.processAllMessages(); verify(mCallback, never()).onActiveStateChanged( anyInt(), anyInt(), anyString(), anyBoolean()); } @@ -113,6 +120,7 @@ public class AppOpsControllerTest extends SysuiTestCase { mController.removeCallback(new int[]{AppOpsManager.OP_RECORD_AUDIO}, mCallback); mController.onOpActiveChanged( AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true); + mTestableLooper.processAllMessages(); verify(mCallback, never()).onActiveStateChanged( anyInt(), anyInt(), anyString(), anyBoolean()); } @@ -123,6 +131,7 @@ public class AppOpsControllerTest extends SysuiTestCase { mController.removeCallback(new int[]{AppOpsManager.OP_CAMERA}, mCallback); mController.onOpActiveChanged( AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true); + mTestableLooper.processAllMessages(); verify(mCallback).onActiveStateChanged(AppOpsManager.OP_RECORD_AUDIO, TEST_UID, TEST_PACKAGE_NAME, true); } @@ -185,4 +194,129 @@ public class AppOpsControllerTest extends SysuiTestCase { verify(mMockHandler).removeCallbacksAndMessages(null); assertTrue(mController.getActiveAppOps().isEmpty()); } + + @Test + public void noDoubleUpdateOnOpNoted() { + mController.setBGHandler(mMockHandler); + + mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, + AppOpsManager.MODE_ALLOWED); + mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, + AppOpsManager.MODE_ALLOWED); + + // Only one post to notify subscribers + verify(mMockHandler, times(1)).post(any()); + + List list = mController.getActiveAppOps(); + assertEquals(1, list.size()); + } + + @Test + public void onDoubleOPNoted_scheduleTwiceForRemoval() { + mController.setBGHandler(mMockHandler); + + mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, + AppOpsManager.MODE_ALLOWED); + mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, + AppOpsManager.MODE_ALLOWED); + + // Only one post to notify subscribers + verify(mMockHandler, times(2)).scheduleRemoval(any(), anyLong()); + } + + @Test + public void testActiveOpNotRemovedAfterNoted() throws InterruptedException { + // Replaces the timeout delay with 5 ms + AppOpsControllerImpl.H testHandler = mController.new H(mTestableLooper.getLooper()) { + @Override + public void scheduleRemoval(AppOpItem item, long timeToRemoval) { + super.scheduleRemoval(item, 5L); + } + }; + + mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback); + mController.setBGHandler(testHandler); + + mController.onOpActiveChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true); + + mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, + AppOpsManager.MODE_ALLOWED); + + mTestableLooper.processAllMessages(); + List list = mController.getActiveAppOps(); + verify(mCallback).onActiveStateChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true); + + // Duplicates are not removed between active and noted + assertEquals(2, list.size()); + + sleep(10L); + + mTestableLooper.processAllMessages(); + + verify(mCallback, never()).onActiveStateChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, false); + list = mController.getActiveAppOps(); + assertEquals(1, list.size()); + } + + @Test + public void testNotedNotRemovedAfterActive() { + mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback); + + mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, + AppOpsManager.MODE_ALLOWED); + + mController.onOpActiveChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true); + + mTestableLooper.processAllMessages(); + List list = mController.getActiveAppOps(); + verify(mCallback).onActiveStateChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true); + + // Duplicates are not removed between active and noted + assertEquals(2, list.size()); + + mController.onOpActiveChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, false); + + mTestableLooper.processAllMessages(); + + verify(mCallback, never()).onActiveStateChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, false); + list = mController.getActiveAppOps(); + assertEquals(1, list.size()); + } + + @Test + public void testNotedAndActiveOnlyOneCall() { + mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback); + + mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, + AppOpsManager.MODE_ALLOWED); + + mController.onOpActiveChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true); + + mTestableLooper.processAllMessages(); + verify(mCallback).onActiveStateChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true); + } + + @Test + public void testActiveAndNotedOnlyOneCall() { + mController.addCallback(new int[]{AppOpsManager.OP_FINE_LOCATION}, mCallback); + + mController.onOpActiveChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true); + + mController.onOpNoted(AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, + AppOpsManager.MODE_ALLOWED); + + mTestableLooper.processAllMessages(); + verify(mCallback).onActiveStateChanged( + AppOpsManager.OP_FINE_LOCATION, TEST_UID, TEST_PACKAGE_NAME, true); + } } From 5f6b5390aeb51297e4dcb995c8fbbdc46559b8b7 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Thu, 4 Jul 2019 16:10:08 +0800 Subject: [PATCH 05/10] Fix potential double destroy of AssetManager Assume there is a XmlBlock [X] created by a AssetManager [A] ([A] will have mNumRefs = 2). After [A].close is called (mNumRefs = 1) and then both [X] and [A] are going to be GCed, if [A].finalize is called first (nativeDestroy), the later [X].finalize will invoke [A].xmlBlockGone that triggers the second nativeDestroy of [A] and leads to crash. By clearing the mObject in AssetManager.finalize, the decRefsLocked from other paths won't call nativeDestroy again. Bug: 136721562 Bug: 144028297 Test: atest AssetManagerTest Test: Build and install CorePerfTests adb shell am instrument -w -r --no-hidden-api-checks -e class \ android.app.ResourcesPerfTest#getLayoutAndTravese,android.graphics.perftests.RenderNodePerfTest \ com.android.perftests.core/androidx.test.runner.AndroidJUnitRunner Change-Id: Ia938502d2443f5a6de6a3cabdb7ce1d41d3ff6d1 Merged-In: Ia938502d2443f5a6de6a3cabdb7ce1d41d3ff6d1 (cherry picked from commit 0a8a1e9d40a3cdff06150c43c623fa4c415226b6) --- core/java/android/content/res/AssetManager.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/core/java/android/content/res/AssetManager.java b/core/java/android/content/res/AssetManager.java index e5ef67b7d4bd8..2420a6109155a 100644 --- a/core/java/android/content/res/AssetManager.java +++ b/core/java/android/content/res/AssetManager.java @@ -1157,8 +1157,11 @@ public final class AssetManager implements AutoCloseable { } } - if (mObject != 0) { - nativeDestroy(mObject); + synchronized (this) { + if (mObject != 0) { + nativeDestroy(mObject); + mObject = 0; + } } } From 21b9dc78d6d441f51cc1635563d68cb15afd9e2b Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Tue, 17 Dec 2019 11:21:02 -0800 Subject: [PATCH 06/10] Revoke 'always' web handler status when not autoverifying If an app has previously used autoVerify to make claims about its status re handling web navigation intents, but is updated such that it no longer makes those claims, step down its "official handler" status as though it had never invoked autoVerify in the first place. Bug: 146204120 Test: manual: as described in bug; observe policy before/after via 'adb shell dumpsys package d' Test: atest CtsOsHostTestCases Change-Id: I58502d1b32d793aba9aa772fa2ad5ac38acca48a Merged-In: I58502d1b32d793aba9aa772fa2ad5ac38acca48a (cherry picked from commit d2a71cc4b8f11688f85f33507b75d00041c14852) --- .../server/pm/PackageManagerService.java | 44 ++++++++++++++----- .../java/com/android/server/pm/Settings.java | 1 + 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java index 04cebb3092166..9afaae11b39ae 100644 --- a/services/core/java/com/android/server/pm/PackageManagerService.java +++ b/services/core/java/com/android/server/pm/PackageManagerService.java @@ -18094,36 +18094,48 @@ public class PackageManagerService extends IPackageManager.Stub int count = 0; final String packageName = pkg.packageName; + boolean handlesWebUris = false; + final boolean alreadyVerified; synchronized (mPackages) { // If this is a new install and we see that we've already run verification for this // package, we have nothing to do: it means the state was restored from backup. - if (!replacing) { - IntentFilterVerificationInfo ivi = - mSettings.getIntentFilterVerificationLPr(packageName); - if (ivi != null) { - if (DEBUG_DOMAIN_VERIFICATION) { - Slog.i(TAG, "Package " + packageName+ " already verified: status=" - + ivi.getStatusString()); - } - return; + final IntentFilterVerificationInfo ivi = + mSettings.getIntentFilterVerificationLPr(packageName); + alreadyVerified = (ivi != null); + if (!replacing && alreadyVerified) { + if (DEBUG_DOMAIN_VERIFICATION) { + Slog.i(TAG, "Package " + packageName + " already verified: status=" + + ivi.getStatusString()); } + return; } - // If any filters need to be verified, then all need to be. + // If any filters need to be verified, then all need to be. In addition, we need to + // know whether an updating app has any web navigation intent filters, to re- + // examine handling policy even if not re-verifying. boolean needToVerify = false; for (PackageParser.Activity a : pkg.activities) { for (ActivityIntentInfo filter : a.intents) { + if (filter.handlesWebUris(true)) { + handlesWebUris = true; + } if (filter.needsVerification() && needsNetworkVerificationLPr(filter)) { if (DEBUG_DOMAIN_VERIFICATION) { Slog.d(TAG, "Intent filter needs verification, so processing all filters"); } needToVerify = true; + // It's safe to break out here because filter.needsVerification() + // can only be true if filter.handlesWebUris(true) returns true, so + // we've already noted that. break; } } } + // Note whether this app publishes any web navigation handling support at all, + // and whether there are any web-nav filters that fit the profile for running + // a verification pass now. if (needToVerify) { final int verificationId = mIntentFilterVerificationToken++; for (PackageParser.Activity a : pkg.activities) { @@ -18141,13 +18153,23 @@ public class PackageManagerService extends IPackageManager.Stub } if (count > 0) { + // count > 0 means that we're running a full verification pass if (DEBUG_DOMAIN_VERIFICATION) Slog.d(TAG, "Starting " + count + " IntentFilter verification" + (count > 1 ? "s" : "") + " for userId:" + userId); mIntentFilterVerifier.startVerifications(userId); + } else if (alreadyVerified && handlesWebUris) { + // App used autoVerify in the past, no longer does, but still handles web + // navigation starts. + if (DEBUG_DOMAIN_VERIFICATION) { + Slog.d(TAG, "App changed web filters but no longer verifying - resetting policy"); + } + synchronized (mPackages) { + clearIntentFilterVerificationsLPw(packageName, userId); + } } else { if (DEBUG_DOMAIN_VERIFICATION) { - Slog.d(TAG, "No filters or not all autoVerify for " + packageName); + Slog.d(TAG, "No web filters or no prior verify policy for " + packageName); } } } diff --git a/services/core/java/com/android/server/pm/Settings.java b/services/core/java/com/android/server/pm/Settings.java index d9e4db29de071..169f8cb91e801 100644 --- a/services/core/java/com/android/server/pm/Settings.java +++ b/services/core/java/com/android/server/pm/Settings.java @@ -1252,6 +1252,7 @@ public final class Settings { return false; } ps.clearDomainVerificationStatusForUser(userId); + ps.setIntentFilterVerificationInfo(null); return true; } From 3484a2dd5c380c9ef7a2a6e6113a1ef01f2f7aed Mon Sep 17 00:00:00 2001 From: Alexey Kuzmin Date: Tue, 7 Jan 2020 11:59:18 +0000 Subject: [PATCH 07/10] Fix serialization issue of ExternalVibration Remove excessive serialization of Audio Attributes Bug: 140417434 Test: atest ExternalVibrationTest#testSerialization Change-Id: Ib7ceaed875889126a53f874eec64fab4817e48d1 (cherry picked from commit b1a33a8b4fd4b1603c0465a904be29f0c4a07e64) --- core/java/android/os/ExternalVibration.java | 1 - .../src/android/os/ExternalVibrationTest.java | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 core/tests/coretests/src/android/os/ExternalVibrationTest.java diff --git a/core/java/android/os/ExternalVibration.java b/core/java/android/os/ExternalVibration.java index 37ca868598f56..041d21fabd6fa 100644 --- a/core/java/android/os/ExternalVibration.java +++ b/core/java/android/os/ExternalVibration.java @@ -157,7 +157,6 @@ public class ExternalVibration implements Parcelable { out.writeInt(mUid); out.writeString(mPkg); writeAudioAttributes(mAttrs, out, flags); - out.writeParcelable(mAttrs, flags); out.writeStrongBinder(mController.asBinder()); out.writeStrongBinder(mToken); } diff --git a/core/tests/coretests/src/android/os/ExternalVibrationTest.java b/core/tests/coretests/src/android/os/ExternalVibrationTest.java new file mode 100644 index 0000000000000..3b872d5a7ff17 --- /dev/null +++ b/core/tests/coretests/src/android/os/ExternalVibrationTest.java @@ -0,0 +1,47 @@ +/* + * Copyright (C) 2020 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 android.os; + +import static junit.framework.Assert.assertEquals; + +import static org.mockito.Mockito.mock; + +import android.media.AudioAttributes; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.junit.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +public class ExternalVibrationTest { + @Test + public void testSerialization() { + AudioAttributes audio = new AudioAttributes.Builder().build(); + IExternalVibrationController controller = mock(IExternalVibrationController.class); + ExternalVibration original = new ExternalVibration( + 123, // uid + "pkg", + audio, + controller); + Parcel p = Parcel.obtain(); + original.writeToParcel(p, 0); + p.setDataPosition(0); + ExternalVibration restored = ExternalVibration.CREATOR.createFromParcel(p); + assertEquals(original, restored); + } +} + From d3490a2530a512ba147d06ebb982f83fb1708e4e Mon Sep 17 00:00:00 2001 From: Yu-Han Yang Date: Wed, 26 Feb 2020 12:46:01 -0800 Subject: [PATCH 08/10] Allow settingIgnored for DBH request if inEmergency Bug: 150232136 Test: on device Change-Id: Ia987418a591d716b787d406d725338a8563a55dd (cherry picked from commit 6d19cef854bdb382507daefae2a5956400a255d5) --- .../android/server/location/GnssConfiguration.java | 9 ++++++--- .../server/location/GnssLocationProvider.java | 13 +++++++++---- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/services/core/java/com/android/server/location/GnssConfiguration.java b/services/core/java/com/android/server/location/GnssConfiguration.java index 86a84e3128993..18d9f69c9735c 100644 --- a/services/core/java/com/android/server/location/GnssConfiguration.java +++ b/services/core/java/com/android/server/location/GnssConfiguration.java @@ -79,7 +79,7 @@ class GnssConfiguration { // Represents an HAL interface version. Instances of this class are created in the JNI layer // and returned through native methods. - private static class HalInterfaceVersion { + static class HalInterfaceVersion { final int mMajor; final int mMinor; @@ -205,6 +205,10 @@ class GnssConfiguration { native_set_satellite_blacklist(constellations, svids); } + HalInterfaceVersion getHalInterfaceVersion() { + return native_get_gnss_configuration_version(); + } + interface SetCarrierProperty { boolean set(int value); } @@ -231,8 +235,7 @@ class GnssConfiguration { logConfigurations(); - final HalInterfaceVersion gnssConfigurationIfaceVersion = - native_get_gnss_configuration_version(); + final HalInterfaceVersion gnssConfigurationIfaceVersion = getHalInterfaceVersion(); if (gnssConfigurationIfaceVersion != null) { // Set to a range checked value. if (isConfigEsExtensionSecSupported(gnssConfigurationIfaceVersion) diff --git a/services/core/java/com/android/server/location/GnssLocationProvider.java b/services/core/java/com/android/server/location/GnssLocationProvider.java index 4b79677b475b5..342e9b8c3006b 100644 --- a/services/core/java/com/android/server/location/GnssLocationProvider.java +++ b/services/core/java/com/android/server/location/GnssLocationProvider.java @@ -772,10 +772,15 @@ public class GnssLocationProvider extends AbstractLocationProvider implements locationRequest.setProvider(provider); - // Ignore location settings if in emergency mode. - if (isUserEmergency && mNIHandler.getInEmergency()) { - locationRequest.setLocationSettingsIgnored(true); - durationMillis *= EMERGENCY_LOCATION_UPDATE_DURATION_MULTIPLIER; + // Ignore location settings if in emergency mode. This is only allowed for + // isUserEmergency request (introduced in HAL v2.0), or DBH request in HAL v1.1. + if (mNIHandler.getInEmergency()) { + GnssConfiguration.HalInterfaceVersion halVersion = + mGnssConfiguration.getHalInterfaceVersion(); + if (isUserEmergency || (halVersion.mMajor < 2 && !independentFromGnss)) { + locationRequest.setLocationSettingsIgnored(true); + durationMillis *= EMERGENCY_LOCATION_UPDATE_DURATION_MULTIPLIER; + } } Log.i(TAG, From a08d52535db71bcad173b8a70939f027f4e88fe5 Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Fri, 28 Feb 2020 23:04:58 +0000 Subject: [PATCH 09/10] DO NOT MERGE: Revert: Freeup lock when IME is set inactive and unbound Reason for revert: Caused an unexpected regression Bug 144174015 Bug: 139806621 Bug: 144103599 Fix: 144174015 Test: Manually verified Bug 144174015 disappeared as follows 1. Open Gmail then start composing an email 2. Swipe up the home button to recents then re-launch Gmail 3. Do the step 2 several times. 4. Make sure that you can still type something on Gmail. Change-Id: I04a77afea17f9d3eb05017fa00313fad4e48cd5c (cherry picked from commit 9494c9dbb73b0ce237cb2f64fba90434b1b09c09) --- .../view/inputmethod/InputMethodManager.java | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index d3618adca6c49..d75810ad0e372 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -655,14 +655,14 @@ public final class InputMethodManager { } catch (RemoteException e) { } } - } - // Check focus again in case that "onWindowFocus" is called before - // handling this message. - if (mServedView != null && canStartInput(mServedView)) { - if (checkFocusNoStartInput(mRestartOnNextWindowFocus)) { - final int reason = active ? StartInputReason.ACTIVATED_BY_IMMS - : StartInputReason.DEACTIVATED_BY_IMMS; - startInputInner(reason, null, 0, 0, 0); + // Check focus again in case that "onWindowFocus" is called before + // handling this message. + if (mServedView != null && canStartInput(mServedView)) { + if (checkFocusNoStartInput(mRestartOnNextWindowFocus)) { + final int reason = active ? StartInputReason.ACTIVATED_BY_IMMS + : StartInputReason.DEACTIVATED_BY_IMMS; + startInputInner(reason, null, 0, 0, 0); + } } } return; @@ -1225,10 +1225,6 @@ public final class InputMethodManager { */ void clearBindingLocked() { if (DEBUG) Log.v(TAG, "Clearing binding!"); - if (mWindowFocusGainFuture != null) { - mWindowFocusGainFuture.cancel(false /* mayInterruptIfRunning */); - mWindowFocusGainFuture = null; - } clearConnectionLocked(); setInputChannelLocked(null); mBindSequence = -1; From e823c54aca4a363b6588117676140aa87d0e87ec Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Fri, 28 Feb 2020 23:04:58 +0000 Subject: [PATCH 10/10] DO NOT MERGE: Revert Move startInput for WINDOW_FOCUS_GAIN to background thread Reason for revert: Caused an unexpected regression Bug 144174015 Bug: 139806621 Bug: 144103599 Fix: 144174015 Test: Manually verified Bug 144174015 disappeared as follows 1. Open Gmail then start composing an email 2. Swipe up the home button to recents then re-launch Gmail 3. Do the step 2 several times. 4. Make sure that you can still type something on Gmail. Change-Id: I9265f01ed2f6e4aca7728d278f06ceea5633dac5 (cherry picked from commit 344858dd9c5616a0de16fa0ac72e0d384e2406c4) --- .../view/inputmethod/InputMethodManager.java | 77 ++++++------------- 1 file changed, 22 insertions(+), 55 deletions(-) diff --git a/core/java/android/view/inputmethod/InputMethodManager.java b/core/java/android/view/inputmethod/InputMethodManager.java index d75810ad0e372..032af1c5c7b58 100644 --- a/core/java/android/view/inputmethod/InputMethodManager.java +++ b/core/java/android/view/inputmethod/InputMethodManager.java @@ -92,10 +92,7 @@ import java.util.Comparator; import java.util.List; import java.util.Map; import java.util.Objects; -import java.util.concurrent.CancellationException; -import java.util.concurrent.CompletableFuture; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; /** @@ -424,13 +421,6 @@ public final class InputMethodManager { int mCursorCandStart; int mCursorCandEnd; - /** - * Initial startInput with {@link StartInputReason.WINDOW_FOCUS_GAIN} is executed - * in a background thread. Later, if there is an actual startInput it will wait on - * main thread till the background thread completes. - */ - private CompletableFuture mWindowFocusGainFuture; - /** * The instance that has previously been sent to the input method. */ @@ -1608,18 +1598,6 @@ public final class InputMethodManager { boolean startInputInner(@StartInputReason int startInputReason, @Nullable IBinder windowGainingFocus, @StartInputFlags int startInputFlags, @SoftInputModeFlags int softInputMode, int windowFlags) { - if (startInputReason != StartInputReason.WINDOW_FOCUS_GAIN - && mWindowFocusGainFuture != null) { - try { - mWindowFocusGainFuture.get(); - } catch (ExecutionException | InterruptedException e) { - // do nothing - } catch (CancellationException e) { - // window no longer has focus. - return true; - } - } - final View view; synchronized (mH) { view = mServedView; @@ -1973,38 +1951,31 @@ public final class InputMethodManager { startInputFlags |= StartInputFlags.FIRST_WINDOW_FOCUS_GAIN; } - final boolean forceNewFocus1 = forceNewFocus; - final int startInputFlags1 = startInputFlags; - if (mWindowFocusGainFuture != null) { - mWindowFocusGainFuture.cancel(false/* mayInterruptIfRunning */); + if (checkFocusNoStartInput(forceNewFocus)) { + // We need to restart input on the current focus view. This + // should be done in conjunction with telling the system service + // about the window gaining focus, to help make the transition + // smooth. + if (startInputInner(StartInputReason.WINDOW_FOCUS_GAIN, rootView.getWindowToken(), + startInputFlags, softInputMode, windowFlags)) { + return; + } } - mWindowFocusGainFuture = CompletableFuture.runAsync(() -> { - if (checkFocusNoStartInput(forceNewFocus1)) { - // We need to restart input on the current focus view. This - // should be done in conjunction with telling the system service - // about the window gaining focus, to help make the transition - // smooth. - if (startInputInner(StartInputReason.WINDOW_FOCUS_GAIN, rootView.getWindowToken(), - startInputFlags1, softInputMode, windowFlags)) { - return; - } - } - // For some reason we didn't do a startInput + windowFocusGain, so - // we'll just do a window focus gain and call it a day. - synchronized (mH) { - try { - if (DEBUG) Log.v(TAG, "Reporting focus gain, without startInput"); - mService.startInputOrWindowGainedFocus( - StartInputReason.WINDOW_FOCUS_GAIN_REPORT_ONLY, mClient, - rootView.getWindowToken(), startInputFlags1, softInputMode, windowFlags, - null, null, 0 /* missingMethodFlags */, - rootView.getContext().getApplicationInfo().targetSdkVersion); - } catch (RemoteException e) { - throw e.rethrowFromSystemServer(); - } + // For some reason we didn't do a startInput + windowFocusGain, so + // we'll just do a window focus gain and call it a day. + synchronized (mH) { + try { + if (DEBUG) Log.v(TAG, "Reporting focus gain, without startInput"); + mService.startInputOrWindowGainedFocus( + StartInputReason.WINDOW_FOCUS_GAIN_REPORT_ONLY, mClient, + rootView.getWindowToken(), startInputFlags, softInputMode, windowFlags, + null, null, 0 /* missingMethodFlags */, + rootView.getContext().getApplicationInfo().targetSdkVersion); + } catch (RemoteException e) { + throw e.rethrowFromSystemServer(); } - }); + } } /** @hide */ @@ -2019,10 +1990,6 @@ public final class InputMethodManager { // If the mCurRootView is losing window focus, release the strong reference to it // so as not to prevent it from being garbage-collected. mCurRootView = null; - if (mWindowFocusGainFuture != null) { - mWindowFocusGainFuture.cancel(false /* mayInterruptIfRunning */); - mWindowFocusGainFuture = null; - } } else { if (DEBUG) { Log.v(TAG, "Ignoring onPreWindowFocus()."