Merge "Promote FeatureFlag best practices in notifications code & tests" into udc-qpr-dev (febba3b6) · Commits · e / os / android_frameworks_base

packages/SystemUI/src/com/android/systemui/flags/ViewRefactorFlag.kt

0 → 100644

+99 −0

Original line number	Diff line number	Diff line
		/*
		* Copyright (C) 2023 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.systemui.flags

		import android.util.Log
		import com.android.systemui.Dependency

		/**
		* This class promotes best practices for flag guarding System UI view refactors.
		* * [isEnabled] allows changing an implementation.
		* * [assertDisabled] allows authors to flag code as being "dead" when the flag gets enabled and
		* ensure that it is not being invoked accidentally in the post-flag refactor.
		* * [expectEnabled] allows authors to guard new code with a "safe" alternative when invoked on
		* flag-disabled builds, but with a check that should crash eng builds or tests when the
		* expectation is violated.
		*
		* The constructors prefer that you provide a [FeatureFlags] instance, but does not require it,
		* falling back to [Dependency.get]. This fallback should ONLY be used to flag-guard code changes
		* inside views where injecting flag values after initialization can be error-prone.
		*/
		class ViewRefactorFlag
		private constructor(
		private val injectedFlags: FeatureFlags?,
		private val flag: BooleanFlag,
		private val readFlagValue: (FeatureFlags) -> Boolean
		) {
		@JvmOverloads
		constructor(
		flags: FeatureFlags? = null,
		flag: UnreleasedFlag
		) : this(flags, flag, { it.isEnabled(flag) })

		@JvmOverloads
		constructor(
		flags: FeatureFlags? = null,
		flag: ReleasedFlag
		) : this(flags, flag, { it.isEnabled(flag) })

		/** Whether the flag is enabled. Called to switch between an old behavior and a new behavior. */
		val isEnabled by lazy {
		@Suppress("DEPRECATION")
		val featureFlags = injectedFlags ?: Dependency.get(FeatureFlags::class.java)
		readFlagValue(featureFlags)
		}

		/**
		* Called to ensure code is only run when the flag is disabled. This will throw an exception if
		* the flag is enabled to ensure that the refactor author catches issues in testing.
		*
		* Example usage:
		* ```
		* public void setController(NotificationShelfController notificationShelfController) {
		* mShelfRefactor.assertDisabled();
		* mController = notificationShelfController;
		* }
		* ````
		*/
		fun assertDisabled() = check(!isEnabled) { "Code path not supported when $flag is enabled." }

		/**
		* Called to ensure code is only run when the flag is enabled. This protects users from the
		* unintended behaviors caused by accidentally running new logic, while also crashing on an eng
		* build to ensure that the refactor author catches issues in testing.
		*
		* Example usage:
		* ```
		* public void setShelfIcons(NotificationIconContainer icons) {
		* if (mShelfRefactor.expectEnabled()) {
		* mShelfIcons = icons;
		* }
		* }
		* ```
		*/
		fun expectEnabled(): Boolean {
		if (!isEnabled) {
		val message = "Code path not supported when $flag is disabled."
		Log.wtf(TAG, message, Exception(message))
		}
		return isEnabled
		}

		private companion object {
		private const val TAG = "ViewRefactorFlag"
		}
		}

packages/SystemUI/src/com/android/systemui/statusbar/LegacyNotificationShelfControllerImpl.java

+0 −2

Original line number	Diff line number	Diff line
		@@ -19,7 +19,6 @@ package com.android.systemui.statusbar;
		import android.view.View;

		import com.android.systemui.flags.FeatureFlags;
		import com.android.systemui.flags.Flags;
		import com.android.systemui.statusbar.notification.row.ActivatableNotificationViewController;
		import com.android.systemui.statusbar.notification.row.dagger.NotificationRowScope;
		import com.android.systemui.statusbar.notification.stack.AmbientState;
		@@ -52,7 +51,6 @@ public class LegacyNotificationShelfControllerImpl implements NotificationShelfC
		mActivatableNotificationViewController = activatableNotificationViewController;
		mKeyguardBypassController = keyguardBypassController;
		mStatusBarStateController = statusBarStateController;
		mView.setSensitiveRevealAnimEnabled(featureFlags.isEnabled(Flags.SENSITIVE_REVEAL_ANIM));
		mOnAttachStateChangeListener = new View.OnAttachStateChangeListener() {
		@Override
		public void onViewAttachedToWindow(View v) {

packages/SystemUI/src/com/android/systemui/statusbar/NotificationShelf.java

+25 −48

Original line number	Diff line number	Diff line
		@@ -24,7 +24,6 @@ import android.content.res.Resources;
		import android.graphics.Rect;
		import android.util.AttributeSet;
		import android.util.IndentingPrintWriter;
		import android.util.Log;
		import android.util.MathUtils;
		import android.view.View;
		import android.view.ViewGroup;
		@@ -40,6 +39,8 @@ import com.android.internal.annotations.VisibleForTesting;
		import com.android.internal.policy.SystemBarUtils;
		import com.android.systemui.R;
		import com.android.systemui.animation.ShadeInterpolation;
		import com.android.systemui.flags.Flags;
		import com.android.systemui.flags.ViewRefactorFlag;
		import com.android.systemui.plugins.statusbar.StatusBarStateController.StateListener;
		import com.android.systemui.shade.transition.LargeScreenShadeInterpolator;
		import com.android.systemui.statusbar.notification.NotificationUtils;
		@@ -95,8 +96,10 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		private float mCornerAnimationDistance;
		private NotificationShelfController mController;
		private float mActualWidth = -1;
		private boolean mSensitiveRevealAnimEnabled;
		private boolean mShelfRefactorFlagEnabled;
		private final ViewRefactorFlag mSensitiveRevealAnim =
		new ViewRefactorFlag(Flags.SENSITIVE_REVEAL_ANIM);
		private final ViewRefactorFlag mShelfRefactor =
		new ViewRefactorFlag(Flags.NOTIFICATION_SHELF_REFACTOR);
		private boolean mCanModifyColorOfNotifications;
		private boolean mCanInteract;
		private NotificationStackScrollLayout mHostLayout;
		@@ -130,7 +133,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St

		public void bind(AmbientState ambientState,
		NotificationStackScrollLayoutController hostLayoutController) {
		assertRefactorFlagDisabled();
		mShelfRefactor.assertDisabled();
		mAmbientState = ambientState;
		mHostLayoutController = hostLayoutController;
		hostLayoutController.setOnNotificationRemovedListener((child, isTransferInProgress) -> {
		@@ -140,7 +143,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St

		public void bind(AmbientState ambientState, NotificationStackScrollLayout hostLayout,
		NotificationRoundnessManager roundnessManager) {
		if (!checkRefactorFlagEnabled()) return;
		if (!mShelfRefactor.expectEnabled()) return;
		mAmbientState = ambientState;
		mHostLayout = hostLayout;
		mRoundnessManager = roundnessManager;
		@@ -268,7 +271,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		}

		final float stackEnd = ambientState.getStackY() + ambientState.getStackHeight();
		if (mSensitiveRevealAnimEnabled && viewState.hidden) {
		if (mSensitiveRevealAnim.isEnabled() && viewState.hidden) {
		// if the shelf is hidden, position it at the end of the stack (plus the clip
		// padding), such that when it appears animated, it will smoothly move in from the
		// bottom, without jump cutting any notifications
		@@ -279,7 +282,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		}

		private int getSpeedBumpIndex() {
		if (mShelfRefactorFlagEnabled) {
		if (mShelfRefactor.isEnabled()) {
		return mHostLayout.getSpeedBumpIndex();
		} else {
		return mHostLayoutController.getSpeedBumpIndex();
		@@ -413,7 +416,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		expandingAnimated, isLastChild, shelfClipStart);

		// TODO(b/172289889) scale mPaddingBetweenElements with expansion amount
		if ((!mSensitiveRevealAnimEnabled && ((isLastChild && !child.isInShelf())
		if ((!mSensitiveRevealAnim.isEnabled() && ((isLastChild && !child.isInShelf())
		\|\| backgroundForceHidden)) \|\| aboveShelf) {
		notificationClipEnd = shelfStart + getIntrinsicHeight();
		} else {
		@@ -462,7 +465,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		// if the shelf is visible, but if the shelf is hidden, it causes incorrect curling.
		// notificationClipEnd handles the discrepancy between a visible and hidden shelf,
		// so we use that when on the keyguard (and while animating away) to reduce curling.
		final float keyguardSafeShelfStart = !mSensitiveRevealAnimEnabled
		final float keyguardSafeShelfStart = !mSensitiveRevealAnim.isEnabled()
		&& mAmbientState.isOnKeyguard() ? notificationClipEnd : shelfStart;
		updateCornerRoundnessOnScroll(anv, viewStart, keyguardSafeShelfStart);
		}
		@@ -504,7 +507,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		}

		private ExpandableView getHostLayoutChildAt(int index) {
		if (mShelfRefactorFlagEnabled) {
		if (mShelfRefactor.isEnabled()) {
		return (ExpandableView) mHostLayout.getChildAt(index);
		} else {
		return mHostLayoutController.getChildAt(index);
		@@ -512,7 +515,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		}

		private int getHostLayoutChildCount() {
		if (mShelfRefactorFlagEnabled) {
		if (mShelfRefactor.isEnabled()) {
		return mHostLayout.getChildCount();
		} else {
		return mHostLayoutController.getChildCount();
		@@ -520,7 +523,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		}

		private boolean canModifyColorOfNotifications() {
		if (mShelfRefactorFlagEnabled) {
		if (mShelfRefactor.isEnabled()) {
		return mCanModifyColorOfNotifications && mAmbientState.isShadeExpanded();
		} else {
		return mController.canModifyColorOfNotifications();
		@@ -583,7 +586,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		}

		private boolean isViewAffectedBySwipe(ExpandableView expandableView) {
		if (!mShelfRefactorFlagEnabled) {
		if (!mShelfRefactor.isEnabled()) {
		return mHostLayoutController.isViewAffectedBySwipe(expandableView);
		} else {
		return mRoundnessManager.isViewAffectedBySwipe(expandableView);
		@@ -607,7 +610,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		}

		private View getHostLayoutTransientView(int index) {
		if (mShelfRefactorFlagEnabled) {
		if (mShelfRefactor.isEnabled()) {
		return mHostLayout.getTransientView(index);
		} else {
		return mHostLayoutController.getTransientView(index);
		@@ -615,7 +618,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		}

		private int getHostLayoutTransientViewCount() {
		if (mShelfRefactorFlagEnabled) {
		if (mShelfRefactor.isEnabled()) {
		return mHostLayout.getTransientViewCount();
		} else {
		return mHostLayoutController.getTransientViewCount();
		@@ -961,7 +964,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St

		@Override
		public void onStateChanged(int newState) {
		assertRefactorFlagDisabled();
		mShelfRefactor.assertDisabled();
		mStatusBarState = newState;
		updateInteractiveness();
		}
		@@ -975,7 +978,7 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		}

		private boolean canInteract() {
		if (mShelfRefactorFlagEnabled) {
		if (mShelfRefactor.isEnabled()) {
		return mCanInteract;
		} else {
		return mStatusBarState == StatusBarState.KEYGUARD;
		@@ -1018,32 +1021,18 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		return false;
		}

		private void assertRefactorFlagDisabled() {
		if (mShelfRefactorFlagEnabled) {
		NotificationShelfController.throwIllegalFlagStateError(false);
		}
		}

		private boolean checkRefactorFlagEnabled() {
		if (!mShelfRefactorFlagEnabled) {
		Log.wtf(TAG,
		"Code path not supported when Flags.NOTIFICATION_SHELF_REFACTOR is disabled.");
		}
		return mShelfRefactorFlagEnabled;
		}

		public void setController(NotificationShelfController notificationShelfController) {
		assertRefactorFlagDisabled();
		mShelfRefactor.assertDisabled();
		mController = notificationShelfController;
		}

		public void setCanModifyColorOfNotifications(boolean canModifyColorOfNotifications) {
		if (!checkRefactorFlagEnabled()) return;
		if (!mShelfRefactor.expectEnabled()) return;
		mCanModifyColorOfNotifications = canModifyColorOfNotifications;
		}

		public void setCanInteract(boolean canInteract) {
		if (!checkRefactorFlagEnabled()) return;
		if (!mShelfRefactor.expectEnabled()) return;
		mCanInteract = canInteract;
		updateInteractiveness();
		}
		@@ -1053,27 +1042,15 @@ public class NotificationShelf extends ActivatableNotificationView implements St
		}

		private int getIndexOfViewInHostLayout(ExpandableView child) {
		if (mShelfRefactorFlagEnabled) {
		if (mShelfRefactor.isEnabled()) {
		return mHostLayout.indexOfChild(child);
		} else {
		return mHostLayoutController.indexOfChild(child);
		}
		}

		/**
		* Set whether the sensitive reveal animation feature flag is enabled
		* @param enabled true if enabled
		*/
		public void setSensitiveRevealAnimEnabled(boolean enabled) {
		mSensitiveRevealAnimEnabled = enabled;
		}

		public void setRefactorFlagEnabled(boolean enabled) {
		mShelfRefactorFlagEnabled = enabled;
		}

		public void requestRoundnessResetFor(ExpandableView child) {
		if (!checkRefactorFlagEnabled()) return;
		if (!mShelfRefactor.expectEnabled()) return;
		child.requestRoundnessReset(SHELF_SCROLL);
		}

packages/SystemUI/src/com/android/systemui/statusbar/NotificationShelfController.kt

+0 −28

Original line number	Diff line number	Diff line
		@@ -16,11 +16,8 @@

		package com.android.systemui.statusbar

		import android.util.Log
		import android.view.View
		import android.view.View.OnClickListener
		import com.android.systemui.flags.FeatureFlags
		import com.android.systemui.flags.Flags
		import com.android.systemui.statusbar.notification.row.ExpandableView
		import com.android.systemui.statusbar.notification.stack.AmbientState
		import com.android.systemui.statusbar.notification.stack.NotificationStackScrollLayout
		@@ -49,29 +46,4 @@ interface NotificationShelfController {

		/** @see View.setOnClickListener */
		fun setOnClickListener(listener: OnClickListener)

		companion object {
		@JvmStatic
		fun assertRefactorFlagDisabled(featureFlags: FeatureFlags) {
		if (featureFlags.isEnabled(Flags.NOTIFICATION_SHELF_REFACTOR)) {
		throwIllegalFlagStateError(expected = false)
		}
		}

		@JvmStatic
		fun checkRefactorFlagEnabled(featureFlags: FeatureFlags): Boolean =
		featureFlags.isEnabled(Flags.NOTIFICATION_SHELF_REFACTOR).also { enabled ->
		if (!enabled) {
		Log.wtf("NotifShelf", getErrorMessage(expected = true))
		}
		}

		@JvmStatic
		fun throwIllegalFlagStateError(expected: Boolean): Nothing =
		error(getErrorMessage(expected))

		private fun getErrorMessage(expected: Boolean): String =
		"Code path not supported when Flags.NOTIFICATION_SHELF_REFACTOR is " +
		if (expected) "disabled" else "enabled"
		}
		}

packages/SystemUI/src/com/android/systemui/statusbar/notification/Roundable.kt

+5 −6

Original line number	Diff line number	Diff line
		@@ -3,10 +3,10 @@ package com.android.systemui.statusbar.notification
		import android.util.FloatProperty
		import android.view.View
		import androidx.annotation.FloatRange
		import com.android.systemui.Dependency
		import com.android.systemui.R
		import com.android.systemui.flags.FeatureFlags
		import com.android.systemui.flags.Flags
		import com.android.systemui.flags.ViewRefactorFlag
		import com.android.systemui.statusbar.notification.stack.AnimationProperties
		import com.android.systemui.statusbar.notification.stack.StackStateAnimator
		import kotlin.math.abs
		@@ -46,14 +46,14 @@ interface Roundable {
		@JvmDefault
		val topCornerRadius: Float
		get() =
		if (roundableState.newHeadsUpAnimFlagEnabled) roundableState.topCornerRadius
		if (roundableState.newHeadsUpAnim.isEnabled) roundableState.topCornerRadius
		else topRoundness * maxRadius

		/** Current bottom corner in pixel, based on [bottomRoundness] and [maxRadius] */
		@JvmDefault
		val bottomCornerRadius: Float
		get() =
		if (roundableState.newHeadsUpAnimFlagEnabled) roundableState.bottomCornerRadius
		if (roundableState.newHeadsUpAnim.isEnabled) roundableState.bottomCornerRadius
		else bottomRoundness * maxRadius

		/** Get and update the current radii */
		@@ -335,13 +335,12 @@ constructor(
		internal val targetView: View,
		private val roundable: Roundable,
		maxRadius: Float,
		private val featureFlags: FeatureFlags = Dependency.get(FeatureFlags::class.java)
		featureFlags: FeatureFlags? = null
		) {
		internal var maxRadius = maxRadius
		private set

		internal val newHeadsUpAnimFlagEnabled
		get() = featureFlags.isEnabled(Flags.IMPROVED_HUN_ANIMATIONS)
		internal val newHeadsUpAnim = ViewRefactorFlag(featureFlags, Flags.IMPROVED_HUN_ANIMATIONS)

		/** Animatable for top roundness */
		private val topAnimatable = topAnimatable(roundable)