From 4f5949f5baa2d2040e904b0a71b235468d7cf386 Mon Sep 17 00:00:00 2001 From: atrost Date: Mon, 9 Mar 2020 18:17:23 +0000 Subject: [PATCH] Allow overriding disabled changes in final release. Since disabled changes are disabled by default, our policy should allow opting in to them in the final release. Add toString to OverrideAllowedState for easier test debugging. Test: atest com.android.server.compat.OverrideValidatorImplTest Bug: 144552011 Change-Id: Iff01ee44d30d3e5703a980cddaf1b38435756c3c --- .../internal/compat/OverrideAllowedState.java | 26 ++++++++++++++++++- .../android/server/compat/CompatConfig.java | 13 ++++++++++ .../server/compat/OverrideValidatorImpl.java | 7 ++--- .../compat/OverrideValidatorImplTest.java | 14 +++++++--- 4 files changed, 52 insertions(+), 8 deletions(-) diff --git a/core/java/com/android/internal/compat/OverrideAllowedState.java b/core/java/com/android/internal/compat/OverrideAllowedState.java index bed41b37ac816..9a78ad2011cf4 100644 --- a/core/java/com/android/internal/compat/OverrideAllowedState.java +++ b/core/java/com/android/internal/compat/OverrideAllowedState.java @@ -50,7 +50,7 @@ public final class OverrideAllowedState implements Parcelable { public static final int DISABLED_NOT_DEBUGGABLE = 1; /** * Change cannot be overridden, due to the build being non-debuggable and the change being - * non-targetSdk. + * enabled regardless of targetSdk. */ public static final int DISABLED_NON_TARGET_SDK = 2; /** @@ -159,4 +159,28 @@ public final class OverrideAllowedState implements Parcelable { && appTargetSdk == otherState.appTargetSdk && changeIdTargetSdk == otherState.changeIdTargetSdk; } + + private String stateName() { + switch (state) { + case ALLOWED: + return "ALLOWED"; + case DISABLED_NOT_DEBUGGABLE: + return "DISABLED_NOT_DEBUGGABLE"; + case DISABLED_NON_TARGET_SDK: + return "DISABLED_NON_TARGET_SDK"; + case DISABLED_TARGET_SDK_TOO_HIGH: + return "DISABLED_TARGET_SDK_TOO_HIGH"; + case PACKAGE_DOES_NOT_EXIST: + return "PACKAGE_DOES_NOT_EXIST"; + case LOGGING_ONLY_CHANGE: + return "LOGGING_ONLY_CHANGE"; + } + return "UNKNOWN"; + } + + @Override + public String toString() { + return "OverrideAllowedState(state=" + stateName() + "; appTargetSdk=" + appTargetSdk + + "; changeIdTargetSdk=" + changeIdTargetSdk + ")"; + } } diff --git a/services/core/java/com/android/server/compat/CompatConfig.java b/services/core/java/com/android/server/compat/CompatConfig.java index bfc2f82d586d2..61bede987bae5 100644 --- a/services/core/java/com/android/server/compat/CompatConfig.java +++ b/services/core/java/com/android/server/compat/CompatConfig.java @@ -218,6 +218,19 @@ final class CompatConfig { } } + /** + * Returns whether the change is marked as disabled. + */ + boolean isDisabled(long changeId) { + synchronized (mChanges) { + CompatChange c = mChanges.get(changeId); + if (c == null) { + return false; + } + return c.getDisabled(); + } + } + /** * Removes an override previously added via {@link #addOverride(long, String, boolean)}. This * restores the default behaviour for the given change and app, once any app processes have been diff --git a/services/core/java/com/android/server/compat/OverrideValidatorImpl.java b/services/core/java/com/android/server/compat/OverrideValidatorImpl.java index 9e18c74265d7e..f5d6e5ac46e5d 100644 --- a/services/core/java/com/android/server/compat/OverrideValidatorImpl.java +++ b/services/core/java/com/android/server/compat/OverrideValidatorImpl.java @@ -59,6 +59,7 @@ public class OverrideValidatorImpl extends IOverrideValidator.Stub { boolean debuggableBuild = mAndroidBuildClassifier.isDebuggableBuild(); boolean finalBuild = mAndroidBuildClassifier.isFinalBuild(); int minTargetSdk = mCompatConfig.minTargetSdkForChangeId(changeId); + boolean disabled = mCompatConfig.isDisabled(changeId); // Allow any override for userdebug or eng builds. if (debuggableBuild) { @@ -83,12 +84,12 @@ public class OverrideValidatorImpl extends IOverrideValidator.Stub { if (!finalBuild) { return new OverrideAllowedState(ALLOWED, appTargetSdk, minTargetSdk); } - // Do not allow overriding non-target sdk gated changes on user builds - if (minTargetSdk == -1) { + // Do not allow overriding default enabled changes on user builds + if (minTargetSdk == -1 && !disabled) { return new OverrideAllowedState(DISABLED_NON_TARGET_SDK, appTargetSdk, minTargetSdk); } // Only allow to opt-in for a targetSdk gated change. - if (applicationInfo.targetSdkVersion < minTargetSdk) { + if (disabled || applicationInfo.targetSdkVersion < minTargetSdk) { return new OverrideAllowedState(ALLOWED, appTargetSdk, minTargetSdk); } return new OverrideAllowedState(DISABLED_TARGET_SDK_TOO_HIGH, appTargetSdk, minTargetSdk); diff --git a/services/tests/servicestests/src/com/android/server/compat/OverrideValidatorImplTest.java b/services/tests/servicestests/src/com/android/server/compat/OverrideValidatorImplTest.java index 16291b256c449..425c7247e50fd 100644 --- a/services/tests/servicestests/src/com/android/server/compat/OverrideValidatorImplTest.java +++ b/services/tests/servicestests/src/com/android/server/compat/OverrideValidatorImplTest.java @@ -176,7 +176,8 @@ public class OverrideValidatorImplTest { CompatConfig config = CompatConfigBuilder.create(betaBuild(), mContext) .addTargetSdkChangeWithId(TARGET_SDK_BEFORE, 1) .addTargetSdkChangeWithId(TARGET_SDK, 2) - .addTargetSdkChangeWithId(TARGET_SDK_AFTER, 3).build(); + .addTargetSdkChangeWithId(TARGET_SDK_AFTER, 3) + .addDisabledChangeWithId(4).build(); IOverrideValidator overrideValidator = config.getOverrideValidator(); when(mPackageManager.getApplicationInfo(eq(PACKAGE_NAME), anyInt())) .thenReturn(ApplicationInfoBuilder.create() @@ -190,6 +191,8 @@ public class OverrideValidatorImplTest { overrideValidator.getOverrideAllowedState(2, PACKAGE_NAME); OverrideAllowedState stateTargetSdkAfterChange = overrideValidator.getOverrideAllowedState(3, PACKAGE_NAME); + OverrideAllowedState stateDisabledChange = + overrideValidator.getOverrideAllowedState(4, PACKAGE_NAME); assertThat(stateTargetSdkLessChange) .isEqualTo(new OverrideAllowedState(ALLOWED, TARGET_SDK, TARGET_SDK_BEFORE)); @@ -197,6 +200,8 @@ public class OverrideValidatorImplTest { .isEqualTo(new OverrideAllowedState(ALLOWED, TARGET_SDK, TARGET_SDK)); assertThat(stateTargetSdkAfterChange) .isEqualTo(new OverrideAllowedState(ALLOWED, TARGET_SDK, TARGET_SDK_AFTER)); + assertThat(stateDisabledChange) + .isEqualTo(new OverrideAllowedState(ALLOWED, TARGET_SDK, -1)); } @Test @@ -343,21 +348,22 @@ public class OverrideValidatorImplTest { } @Test - public void getOverrideAllowedState_finalBuildDisabledChangeDebugApp_rejectOverride() + public void getOverrideAllowedState_finalBuildDisabledChangeDebugApp_allowOverride() throws Exception { CompatConfig config = CompatConfigBuilder.create(finalBuild(), mContext) - .addDisabledChangeWithId(1).build(); + .addDisabledChangeWithId(1).build(); IOverrideValidator overrideValidator = config.getOverrideValidator(); when(mPackageManager.getApplicationInfo(eq(PACKAGE_NAME), anyInt())) .thenReturn(ApplicationInfoBuilder.create() .withPackageName(PACKAGE_NAME) + .withTargetSdk(TARGET_SDK) .debuggable().build()); OverrideAllowedState allowedState = overrideValidator.getOverrideAllowedState(1, PACKAGE_NAME); assertThat(allowedState) - .isEqualTo(new OverrideAllowedState(DISABLED_NON_TARGET_SDK, -1, -1)); + .isEqualTo(new OverrideAllowedState(ALLOWED, TARGET_SDK, -1)); } @Test