summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Songchun Fan <schfan@google.com> 2020-10-29 11:13:10 -0700
committer Songchun Fan <schfan@google.com> 2020-11-04 15:36:27 +0000
commit2ea086d3f305184fd12d732c80e129c0d67bb89b (patch)
tree8466598103d553c8257c9b30a2f72901748f5433
parent68ef6737fecfa65dc175051045f5aa99a7f265f1 (diff)
[pm/incremental] disable unstartable state and hide related APIs
Per discussions in go/incremental-when-to-freeze, we will not mark an app unstartable, regarless of streaming health metrics. This CL is a temporary disablement. The startable state will remain true, and there will be no broadcasts about startability. The event tracking and health monitoring is kept as is. If we get feedback and need to put it back, it'll be a small change. Otherwise, we will clean up the related code. + also fix storage health monitoring params BUG: 171920377 Test: builds Change-Id: Idf7d32d0ca3a6e08c9673914f27f22b531b91ac2
-rw-r--r--api/current.txt5
-rw-r--r--core/api/current.txt5
-rw-r--r--core/java/android/content/Intent.java2
-rw-r--r--core/java/android/content/pm/PackageManager.java3
-rw-r--r--services/core/java/com/android/server/pm/IncrementalStates.java83
-rw-r--r--services/core/java/com/android/server/pm/PackageManagerService.java4
-rw-r--r--services/tests/servicestests/src/com/android/server/pm/IncrementalStatesTest.java103
7 files changed, 71 insertions, 134 deletions
diff --git a/api/current.txt b/api/current.txt
index 970cf6d14303..93eb32caf697 100644
--- a/api/current.txt
+++ b/api/current.txt
@@ -10683,8 +10683,6 @@ package android.content {
field public static final String ACTION_PACKAGE_REMOVED = "android.intent.action.PACKAGE_REMOVED";
field public static final String ACTION_PACKAGE_REPLACED = "android.intent.action.PACKAGE_REPLACED";
field public static final String ACTION_PACKAGE_RESTARTED = "android.intent.action.PACKAGE_RESTARTED";
- field public static final String ACTION_PACKAGE_STARTABLE = "android.intent.action.PACKAGE_STARTABLE";
- field public static final String ACTION_PACKAGE_UNSTARTABLE = "android.intent.action.PACKAGE_UNSTARTABLE";
field public static final String ACTION_PACKAGE_VERIFIED = "android.intent.action.PACKAGE_VERIFIED";
field public static final String ACTION_PASTE = "android.intent.action.PASTE";
field public static final String ACTION_PICK = "android.intent.action.PICK";
@@ -12312,9 +12310,6 @@ package android.content.pm {
field public static final int SYNCHRONOUS = 2; // 0x2
field @Nullable public static final java.util.List<java.security.cert.Certificate> TRUST_ALL;
field @NonNull public static final java.util.List<java.security.cert.Certificate> TRUST_NONE;
- field public static final int UNSTARTABLE_REASON_CONNECTION_ERROR = 1; // 0x1
- field public static final int UNSTARTABLE_REASON_INSUFFICIENT_STORAGE = 2; // 0x2
- field public static final int UNSTARTABLE_REASON_UNKNOWN = 0; // 0x0
field public static final int VERIFICATION_ALLOW = 1; // 0x1
field public static final int VERIFICATION_REJECT = -1; // 0xffffffff
field public static final int VERSION_CODE_HIGHEST = -1; // 0xffffffff
diff --git a/core/api/current.txt b/core/api/current.txt
index 5215d58a3532..9329e2ae47fc 100644
--- a/core/api/current.txt
+++ b/core/api/current.txt
@@ -10683,8 +10683,6 @@ package android.content {
field public static final String ACTION_PACKAGE_REMOVED = "android.intent.action.PACKAGE_REMOVED";
field public static final String ACTION_PACKAGE_REPLACED = "android.intent.action.PACKAGE_REPLACED";
field public static final String ACTION_PACKAGE_RESTARTED = "android.intent.action.PACKAGE_RESTARTED";
- field public static final String ACTION_PACKAGE_STARTABLE = "android.intent.action.PACKAGE_STARTABLE";
- field public static final String ACTION_PACKAGE_UNSTARTABLE = "android.intent.action.PACKAGE_UNSTARTABLE";
field public static final String ACTION_PACKAGE_VERIFIED = "android.intent.action.PACKAGE_VERIFIED";
field public static final String ACTION_PASTE = "android.intent.action.PASTE";
field public static final String ACTION_PICK = "android.intent.action.PICK";
@@ -12312,9 +12310,6 @@ package android.content.pm {
field public static final int SYNCHRONOUS = 2; // 0x2
field @Nullable public static final java.util.List<java.security.cert.Certificate> TRUST_ALL;
field @NonNull public static final java.util.List<java.security.cert.Certificate> TRUST_NONE;
- field public static final int UNSTARTABLE_REASON_CONNECTION_ERROR = 1; // 0x1
- field public static final int UNSTARTABLE_REASON_INSUFFICIENT_STORAGE = 2; // 0x2
- field public static final int UNSTARTABLE_REASON_UNKNOWN = 0; // 0x0
field public static final int VERIFICATION_ALLOW = 1; // 0x1
field public static final int VERIFICATION_REJECT = -1; // 0xffffffff
field public static final int VERSION_CODE_HIGHEST = -1; // 0xffffffff
diff --git a/core/java/android/content/Intent.java b/core/java/android/content/Intent.java
index 782f0cd04dfc..a03bdf2da2be 100644
--- a/core/java/android/content/Intent.java
+++ b/core/java/android/content/Intent.java
@@ -2740,6 +2740,7 @@ public class Intent implements Parcelable, Cloneable {
* </ul>
*
* <p class="note">This is a protected intent that can only be sent by the system.
+ * @hide
*/
@SdkConstant(SdkConstantType.BROADCAST_INTENT_ACTION)
public static final String ACTION_PACKAGE_STARTABLE = "android.intent.action.PACKAGE_STARTABLE";
@@ -2757,6 +2758,7 @@ public class Intent implements Parcelable, Cloneable {
* </ul>
*
* <p class="note">This is a protected intent that can only be sent by the system.
+ * @hide
*/
@SdkConstant(SdkConstantType.BROADCAST_INTENT_ACTION)
public static final String ACTION_PACKAGE_UNSTARTABLE =
diff --git a/core/java/android/content/pm/PackageManager.java b/core/java/android/content/pm/PackageManager.java
index 946c634e3b41..a77d8b17e1f7 100644
--- a/core/java/android/content/pm/PackageManager.java
+++ b/core/java/android/content/pm/PackageManager.java
@@ -3824,18 +3824,21 @@ public abstract class PackageManager {
* Unstartable state with no root cause specified. E.g., data loader seeing missing pages but
* unclear about the cause. This corresponds to a generic alert window shown to the user when
* the user attempts to launch the app.
+ * @hide
*/
public static final int UNSTARTABLE_REASON_UNKNOWN = 0;
/**
* Unstartable state due to connection issues that interrupt package loading.
* This corresponds to an alert window shown to the user indicating connection errors.
+ * @hide
*/
public static final int UNSTARTABLE_REASON_CONNECTION_ERROR = 1;
/**
* Unstartable state after encountering storage limitations.
* This corresponds to an alert window indicating limited storage.
+ * @hide
*/
public static final int UNSTARTABLE_REASON_INSUFFICIENT_STORAGE = 2;
diff --git a/services/core/java/com/android/server/pm/IncrementalStates.java b/services/core/java/com/android/server/pm/IncrementalStates.java
index 72803ac35a3f..780c522d2ae7 100644
--- a/services/core/java/com/android/server/pm/IncrementalStates.java
+++ b/services/core/java/com/android/server/pm/IncrementalStates.java
@@ -101,21 +101,18 @@ public final class IncrementalStates {
if (DEBUG) {
Slog.i(TAG, "received package commit event");
}
+ final boolean startableStateChanged;
synchronized (mLock) {
- if (!mStartableState.isStartable()) {
- mStartableState.adoptNewStartableStateLocked(true);
- }
+ startableStateChanged = mStartableState.adoptNewStartableStateLocked(true);
if (!isIncremental) {
updateProgressLocked(1);
}
}
- mHandler.post(PooledLambda.obtainRunnable(
- IncrementalStates::reportStartableState,
- IncrementalStates.this).recycleOnUse());
+ if (startableStateChanged) {
+ onStartableStateChanged();
+ }
if (!isIncremental) {
- mHandler.post(PooledLambda.obtainRunnable(
- IncrementalStates::reportFullyLoaded,
- IncrementalStates.this).recycleOnUse());
+ onLoadingStateChanged();
}
}
@@ -131,8 +128,7 @@ public final class IncrementalStates {
synchronized (mLock) {
if (mStartableState.isStartable() && mLoadingState.isLoading()) {
// Changing from startable -> unstartable only if app is still loading.
- mStartableState.adoptNewStartableStateLocked(false);
- startableStateChanged = true;
+ startableStateChanged = mStartableState.adoptNewStartableStateLocked(false);
} else {
// If the app is fully loaded, the crash or ANR is caused by the app itself, so
// we do not change the startable state.
@@ -140,12 +136,15 @@ public final class IncrementalStates {
}
}
if (startableStateChanged) {
- mHandler.post(PooledLambda.obtainRunnable(
- IncrementalStates::reportStartableState,
- IncrementalStates.this).recycleOnUse());
+ onStartableStateChanged();
}
}
+ private void onStartableStateChanged() {
+ // Disable startable state broadcasts
+ // TODO(b/171920377): completely remove unstartable state.
+ }
+
private void reportStartableState() {
final Callback callback;
final boolean startable;
@@ -165,6 +164,12 @@ public final class IncrementalStates {
}
}
+ private void onLoadingStateChanged() {
+ mHandler.post(PooledLambda.obtainRunnable(
+ IncrementalStates::reportFullyLoaded,
+ IncrementalStates.this).recycleOnUse());
+ }
+
private void reportFullyLoaded() {
final Callback callback;
synchronized (mLock) {
@@ -178,23 +183,20 @@ public final class IncrementalStates {
private class StatusConsumer implements Consumer<Integer> {
@Override
public void accept(Integer storageStatus) {
- final boolean oldState, newState;
+ final boolean startableStateChanged;
synchronized (mLock) {
if (!mLoadingState.isLoading()) {
// Do nothing if the package is already fully loaded
return;
}
- oldState = mStartableState.isStartable();
mStorageHealthStatus = storageStatus;
- updateStartableStateLocked();
- newState = mStartableState.isStartable();
+ startableStateChanged = updateStartableStateLocked();
}
- if (oldState != newState) {
- mHandler.post(PooledLambda.obtainRunnable(IncrementalStates::reportStartableState,
- IncrementalStates.this).recycleOnUse());
+ if (startableStateChanged) {
+ onStartableStateChanged();
}
}
- };
+ }
/**
* By calling this method, the caller indicates that there issues with the Incremental
@@ -239,14 +241,10 @@ public final class IncrementalStates {
newStartableState = mStartableState.isStartable();
}
if (!newLoadingState) {
- mHandler.post(PooledLambda.obtainRunnable(
- IncrementalStates::reportFullyLoaded,
- IncrementalStates.this).recycleOnUse());
+ onLoadingStateChanged();
}
if (newStartableState != oldStartableState) {
- mHandler.post(PooledLambda.obtainRunnable(
- IncrementalStates::reportStartableState,
- IncrementalStates.this).recycleOnUse());
+ onStartableStateChanged();
}
}
@@ -284,8 +282,9 @@ public final class IncrementalStates {
* health
* status. If the next state is different from the current state, proceed with state
* change.
+ * @return True if the new startable state is different from the old one.
*/
- private void updateStartableStateLocked() {
+ private boolean updateStartableStateLocked() {
final boolean currentState = mStartableState.isStartable();
boolean nextState = currentState;
if (!currentState) {
@@ -302,9 +301,9 @@ public final class IncrementalStates {
}
}
if (nextState == currentState) {
- return;
+ return false;
}
- mStartableState.adoptNewStartableStateLocked(nextState);
+ return mStartableState.adoptNewStartableStateLocked(nextState);
}
private void updateProgressLocked(float progress) {
@@ -343,12 +342,30 @@ public final class IncrementalStates {
return mUnstartableReason;
}
- public void adoptNewStartableStateLocked(boolean nextState) {
+ /**
+ * Adopt new startable state if it is different from the current state.
+ * @param nextState True if startable, false if unstartable.
+ * @return True if the state has changed, false otherwise.
+ */
+ public boolean adoptNewStartableStateLocked(boolean nextState) {
+ if (mIsStartable == nextState) {
+ return false;
+ }
+ if (!nextState) {
+ // Do nothing if the next state is "unstartable"; keep package always startable.
+ // TODO(b/171920377): completely remove unstartable state.
+ if (DEBUG) {
+ Slog.i(TAG, "Attempting to set startable state to false. Abort.");
+ }
+ return false;
+ }
if (DEBUG) {
- Slog.i(TAG, "startable state changed from " + mIsStartable + " to " + nextState);
+ Slog.i(TAG,
+ "startable state changed from " + mIsStartable + " to " + nextState);
}
mIsStartable = nextState;
mUnstartableReason = getUnstartableReasonLocked();
+ return true;
}
private int getUnstartableReasonLocked() {
diff --git a/services/core/java/com/android/server/pm/PackageManagerService.java b/services/core/java/com/android/server/pm/PackageManagerService.java
index ab6026009541..203321ea8edf 100644
--- a/services/core/java/com/android/server/pm/PackageManagerService.java
+++ b/services/core/java/com/android/server/pm/PackageManagerService.java
@@ -16478,8 +16478,8 @@ public class PackageManagerService extends IPackageManager.Stub
healthCheckParams.unhealthyTimeoutMs = INCREMENTAL_STORAGE_UNHEALTHY_TIMEOUT_MS;
healthCheckParams.unhealthyMonitoringMs =
INCREMENTAL_STORAGE_UNHEALTHY_MONITORING_MS;
- mIncrementalManager.registerHealthListener(codePath,
- new StorageHealthCheckParams(), incrementalHealthListener);
+ mIncrementalManager.registerHealthListener(codePath, healthCheckParams,
+ incrementalHealthListener);
}
// Ensure that the uninstall reason is UNKNOWN for users with the package installed.
diff --git a/services/tests/servicestests/src/com/android/server/pm/IncrementalStatesTest.java b/services/tests/servicestests/src/com/android/server/pm/IncrementalStatesTest.java
index 40d959d9793d..9ba096766be2 100644
--- a/services/tests/servicestests/src/com/android/server/pm/IncrementalStatesTest.java
+++ b/services/tests/servicestests/src/com/android/server/pm/IncrementalStatesTest.java
@@ -44,7 +44,6 @@ import java.util.concurrent.atomic.AtomicInteger;
public class IncrementalStatesTest {
private IncrementalStates mIncrementalStates;
private ConditionVariable mUnstartableCalled = new ConditionVariable();
- private ConditionVariable mStartableCalled = new ConditionVariable();
private ConditionVariable mFullyLoadedCalled = new ConditionVariable();
private AtomicInteger mUnstartableReason = new AtomicInteger(0);
private static final int WAIT_TIMEOUT_MILLIS = 1000; /* 1 second */
@@ -57,7 +56,6 @@ public class IncrementalStatesTest {
@Override
public void onPackageStartable() {
- mStartableCalled.open();
}
@Override
@@ -77,24 +75,22 @@ public class IncrementalStatesTest {
mIncrementalStates.setCallback(mCallback);
mIncrementalStates.onCommit(true);
// Test that package is now startable and loading
- assertTrue(mStartableCalled.block(WAIT_TIMEOUT_MILLIS));
assertTrue(mIncrementalStates.isStartable());
assertTrue(mIncrementalStates.isLoading());
- mStartableCalled.close();
mUnstartableCalled.close();
mFullyLoadedCalled.close();
}
/**
- * Test that startable state changes to false when Incremental Storage is unhealthy.
+ * Test that the package is still startable when Incremental Storage is unhealthy.
*/
@Test
public void testStartableTransition_IncrementalStorageUnhealthy() {
mIncrementalStates.onStorageHealthStatusChanged(
IStorageHealthListener.HEALTH_STATUS_UNHEALTHY);
- // Test that package is now unstartable
- assertTrue(mUnstartableCalled.block(WAIT_TIMEOUT_MILLIS));
- assertFalse(mIncrementalStates.isStartable());
+ // Test that package is still startable
+ assertFalse(mUnstartableCalled.block(WAIT_TIMEOUT_MILLIS));
+ assertTrue(mIncrementalStates.isStartable());
assertEquals(PackageManager.UNSTARTABLE_REASON_UNKNOWN, mUnstartableReason.get());
}
@@ -112,70 +108,31 @@ public class IncrementalStatesTest {
}
/**
- * Test that the package becomes unstartable when health status indicate storage issues.
+ * Test that the package is still startable when health status indicate storage issues.
*/
@Test
public void testStartableTransition_IncrementalStorageBlocked() {
mIncrementalStates.onStorageHealthStatusChanged(
IStorageHealthListener.HEALTH_STATUS_UNHEALTHY_STORAGE);
- // Test that package is now unstartable
- assertTrue(mUnstartableCalled.block(WAIT_TIMEOUT_MILLIS));
- assertFalse(mIncrementalStates.isStartable());
- assertEquals(PackageManager.UNSTARTABLE_REASON_INSUFFICIENT_STORAGE,
+ // Test that package is still startable
+ assertFalse(mUnstartableCalled.block(WAIT_TIMEOUT_MILLIS));
+ assertTrue(mIncrementalStates.isStartable());
+ assertEquals(PackageManager.UNSTARTABLE_REASON_UNKNOWN,
mUnstartableReason.get());
}
/**
- * Test that the package becomes unstartable when health status indicates transport issues.
+ * Test that the package is still startable when health status indicates transport issues.
*/
@Test
public void testStartableTransition_DataLoaderIntegrityError() {
mIncrementalStates.onStorageHealthStatusChanged(
IStorageHealthListener.HEALTH_STATUS_UNHEALTHY_TRANSPORT);
- // Test that package is now unstartable
- assertTrue(mUnstartableCalled.block(WAIT_TIMEOUT_MILLIS));
- assertFalse(mIncrementalStates.isStartable());
- assertEquals(PackageManager.UNSTARTABLE_REASON_CONNECTION_ERROR,
- mUnstartableReason.get());
- }
-
- /**
- * Test that the package becomes unstartable when Incremental Storage is unhealthy, and it
- * becomes startable again when Incremental Storage is healthy again.
- */
- @Test
- public void testStartableTransition_IncrementalStorageUnhealthyBackToHealthy()
- throws InterruptedException {
- mIncrementalStates.onStorageHealthStatusChanged(
- IStorageHealthListener.HEALTH_STATUS_UNHEALTHY);
- // Test that package is unstartable
- assertTrue(mUnstartableCalled.block(WAIT_TIMEOUT_MILLIS));
- assertFalse(mIncrementalStates.isStartable());
-
- mIncrementalStates.onStorageHealthStatusChanged(
- IStorageHealthListener.HEALTH_STATUS_OK);
- // Test that package is now startable
- assertTrue(mStartableCalled.block(WAIT_TIMEOUT_MILLIS));
- assertTrue(mIncrementalStates.isStartable());
- }
-
- /**
- * Test that the package becomes unstartable when health status indicates transportation issue,
- * and it becomes startable again when health status is ok again.
- */
- @Test
- public void testStartableTransition_DataLoaderUnhealthyBackToHealthy()
- throws InterruptedException {
- mIncrementalStates.onStorageHealthStatusChanged(
- IStorageHealthListener.HEALTH_STATUS_UNHEALTHY_TRANSPORT);
- // Test that package is unstartable
- assertTrue(mUnstartableCalled.block(WAIT_TIMEOUT_MILLIS));
- assertFalse(mIncrementalStates.isStartable());
-
- mIncrementalStates.onStorageHealthStatusChanged(IStorageHealthListener.HEALTH_STATUS_OK);
- // Test that package is now startable
- assertTrue(mStartableCalled.block(WAIT_TIMEOUT_MILLIS));
+ // Test that package is still startable
+ assertFalse(mUnstartableCalled.block(WAIT_TIMEOUT_MILLIS));
assertTrue(mIncrementalStates.isStartable());
+ assertEquals(PackageManager.UNSTARTABLE_REASON_UNKNOWN,
+ mUnstartableReason.get());
}
/**
@@ -197,43 +154,11 @@ public class IncrementalStatesTest {
}
/**
- * Test that when loading progress is 1, the package becomes fully loaded, and if the package
- * was unstartable, it becomes startable.
- */
- @Test
- public void testLoadingTransition_FullyLoadedWhenUnstartable() throws InterruptedException {
- mIncrementalStates.onStorageHealthStatusChanged(
- IStorageHealthListener.HEALTH_STATUS_UNHEALTHY);
- // Test that package is unstartable
- assertTrue(mUnstartableCalled.block(WAIT_TIMEOUT_MILLIS));
- assertFalse(mIncrementalStates.isStartable());
- // Test that package is still loading
- assertTrue(mIncrementalStates.isLoading());
-
- mIncrementalStates.setProgress(0.5f);
- // Test that package is still unstartable
- assertFalse(mStartableCalled.block(WAIT_TIMEOUT_MILLIS));
- assertFalse(mIncrementalStates.isStartable());
- mIncrementalStates.setProgress(1.0f);
- // Test that package is now startable
- assertTrue(mStartableCalled.block(WAIT_TIMEOUT_MILLIS));
- assertTrue(mIncrementalStates.isStartable());
- // Test that package is now fully loaded
- assertTrue(mFullyLoadedCalled.block(WAIT_TIMEOUT_MILLIS));
- assertFalse(mIncrementalStates.isLoading());
- }
-
- /**
* Test startability transitions if app crashes or anrs
*/
@Test
public void testStartableTransition_AppCrashOrAnr() {
mIncrementalStates.onCrashOrAnr();
- assertFalse(mIncrementalStates.isStartable());
- mIncrementalStates.setProgress(1.0f);
- assertTrue(mIncrementalStates.isStartable());
- mIncrementalStates.onCrashOrAnr();
- // Test that if fully loaded, app remains startable even if it has crashed
assertTrue(mIncrementalStates.isStartable());
}
}