From 9502131e4daf52ceafd58e9579d5e899082e2565 Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Tue, 17 Aug 2021 22:58:23 -0700 Subject: [PATCH 1/6] Suppress identical communal show requests. This changelist prevents multiple show requests from being invoked on the CommunalHostViewController, preventing unnecessary show cycles. Bug: 195115696 Test: atest CommunalHostViewControllerTest#testMultipleShowRequestSuppression Change-Id: Iec4dd0a04b93ccfdf88bbbc9576323cce076a280 --- .../communal/CommunalHostViewController.java | 60 ++++++++++++++++--- .../NotificationPanelViewController.java | 2 - .../CommunalHostViewControllerTest.java | 16 +++++ .../phone/NotificationPanelViewTest.java | 2 - 4 files changed, 67 insertions(+), 13 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java b/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java index ba6373c5bd20c..b84d9ce147d62 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java +++ b/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java @@ -38,6 +38,8 @@ import com.google.common.util.concurrent.ListenableFuture; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.ref.WeakReference; +import java.util.Objects; +import java.util.Optional; import java.util.concurrent.Executor; import javax.inject.Inject; @@ -55,7 +57,8 @@ public class CommunalHostViewController extends ViewController private final KeyguardUpdateMonitor mKeyguardUpdateMonitor; private final KeyguardStateController mKeyguardStateController; private final StatusBarStateController mStatusBarStateController; - private WeakReference mLastSource; + private WeakReference mCurrentSource; + private Optional mLastRequest = Optional.empty(); private int mState; private float mQsExpansion; private float mShadeExpansion; @@ -78,6 +81,37 @@ public class CommunalHostViewController extends ViewController private ViewController mCommunalViewController; + private static class ShowRequest { + private boolean mShouldShow; + private WeakReference mSource; + + ShowRequest(boolean shouldShow, WeakReference source) { + mShouldShow = shouldShow; + mSource = source; + } + + CommunalSource getSource() { + return mSource != null ? mSource.get() : null; + } + + boolean shouldShow() { + return mShouldShow; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof ShowRequest)) return false; + ShowRequest that = (ShowRequest) o; + return mShouldShow == that.mShouldShow && Objects.equals(getSource(), that.getSource()); + } + + @Override + public int hashCode() { + return Objects.hash(mShouldShow, mSource); + } + } + private KeyguardUpdateMonitorCallback mKeyguardUpdateCallback = new KeyguardUpdateMonitorCallback() { @Override @@ -253,18 +287,26 @@ public class CommunalHostViewController extends ViewController } private void showSource() { + final ShowRequest request = new ShowRequest( + (mState & SHOW_COMMUNAL_VIEW_REQUIRED_STATES) == SHOW_COMMUNAL_VIEW_REQUIRED_STATES + && (mState & SHOW_COMMUNAL_VIEW_INVALID_STATES) == 0 + && mCurrentSource != null, + mCurrentSource); + + if (mLastRequest.isPresent() && Objects.equals(mLastRequest.get(), request)) { + return; + } + + mLastRequest = Optional.of(request); + // Make sure all necessary states are present for showing communal and all invalid states // are absent mMainExecutor.execute(() -> { - final CommunalSource currentSource = mLastSource != null ? mLastSource.get() : null; - if (DEBUG) { - Log.d(TAG, "showSource. currentSource:" + currentSource); + Log.d(TAG, "showSource. currentSource:" + request.getSource()); } - if ((mState & SHOW_COMMUNAL_VIEW_REQUIRED_STATES) == SHOW_COMMUNAL_VIEW_REQUIRED_STATES - && (mState & SHOW_COMMUNAL_VIEW_INVALID_STATES) == 0 - && currentSource != null) { + if (request.shouldShow()) { mView.removeAllViews(); // Make view visible. @@ -273,7 +315,7 @@ public class CommunalHostViewController extends ViewController final Context context = mView.getContext(); final ListenableFuture listenableFuture = - currentSource.requestCommunalView(context); + request.getSource().requestCommunalView(context); if (listenableFuture == null) { Log.e(TAG, "could not request communal view"); @@ -308,7 +350,7 @@ public class CommunalHostViewController extends ViewController * @param source The new {@link CommunalSource}, {@code null} if not set. */ public void show(WeakReference source) { - mLastSource = source; + mCurrentSource = source; showSource(); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java index 09eb5c4bea6f6..4ab6e92ca1be3 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java @@ -4583,8 +4583,6 @@ public class NotificationPanelViewController extends PanelViewController { mStatusBarStateController.removeCallback(mStatusBarStateListener); mConfigurationController.removeCallback(mConfigurationListener); mCommunalSourceMonitor.removeCallback(mCommunalSourceMonitorCallback); - // Clear source when detached. - setCommunalSource(null /*source*/); mFalsingManager.removeTapListener(mFalsingTapListener); mCommunalStateController.removeCallback(mCommunalStateCallback); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java index 3b750e7c1c2d4..ea00c6f7fb104 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java @@ -16,7 +16,9 @@ package com.android.systemui.communal; +import static org.mockito.Mockito.any; import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -226,4 +228,18 @@ public class CommunalHostViewControllerTest extends SysuiTestCase { verify(mChildView).setAlpha(alpha); verify(mCommunalView).setAlpha(alpha); } + + @Test + public void testMultipleShowRequestSuppression() { + // Ensure first request invokes source. + mController.show(new WeakReference<>(mCommunalSource)); + mFakeExecutor.runAllReady(); + verify(mCommunalSource).requestCommunalView(any()); + clearInvocations(mCommunalSource); + + // Ensure subsequent identical request is suppressed + mController.show(new WeakReference<>(mCommunalSource)); + mFakeExecutor.runAllReady(); + verify(mCommunalSource, never()).requestCommunalView(any()); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java index 38341b83446ab..f6e06cc6a59b9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java @@ -874,8 +874,6 @@ public class NotificationPanelViewTest extends SysuiTestCase { clearInvocations(mCommunalHostViewController); givenViewDetached(); verify(mCommunalSourceMonitor).removeCallback(any()); - verify(mCommunalHostViewController).show(sourceCapture.capture()); - assertThat(sourceCapture.getValue()).isEqualTo(null); } @Test From 510970be3a0d70f8fe016d59645cd0a0040564bc Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Wed, 18 Aug 2021 00:07:26 -0700 Subject: [PATCH 2/6] Adjust the top of the communal view with keyguard. This change moves the communal view with the keyguard when swiped away to the bouncer. Test: CommunalHostViewPositionAlgorithmTest Test: NotificationPanelViewTest#testCommunalPositionUpdate Bug: 195601027 Change-Id: I53b6caf89f2842d54885a12ec892c0967f94061d --- .../communal/CommunalHostViewController.java | 14 ++++ .../CommunalHostViewPositionAlgorithm.java | 74 +++++++++++++++++++ .../NotificationPanelViewController.java | 28 +++++++ ...CommunalHostViewPositionAlgorithmTest.java | 44 +++++++++++ .../phone/NotificationPanelViewTest.java | 14 ++++ 5 files changed, 174 insertions(+) create mode 100644 packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewPositionAlgorithm.java create mode 100644 packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewPositionAlgorithmTest.java diff --git a/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java b/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java index b84d9ce147d62..b06f4430d0e9e 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java +++ b/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java @@ -28,6 +28,10 @@ import com.android.keyguard.KeyguardVisibilityHelper; import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.plugins.statusbar.StatusBarStateController; import com.android.systemui.statusbar.StatusBarState; +import com.android.systemui.statusbar.notification.AnimatableProperty; +import com.android.systemui.statusbar.notification.PropertyAnimator; +import com.android.systemui.statusbar.notification.stack.AnimationProperties; +import com.android.systemui.statusbar.notification.stack.StackStateAnimator; import com.android.systemui.statusbar.phone.DozeParameters; import com.android.systemui.statusbar.phone.UnlockedScreenOffAnimationController; import com.android.systemui.statusbar.policy.KeyguardStateController; @@ -51,6 +55,8 @@ public class CommunalHostViewController extends ViewController private static final String TAG = "CommunalController"; private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); private static final String STATE_LIST_FORMAT = "[%s]"; + private static final AnimationProperties COMMUNAL_ANIMATION_PROPERTIES = + new AnimationProperties().setDuration(StackStateAnimator.ANIMATION_DURATION_STANDARD); private final Executor mMainExecutor; private final CommunalStateController mCommunalStateController; @@ -354,6 +360,14 @@ public class CommunalHostViewController extends ViewController showSource(); } + /** + * Update position of the view with an optional animation + */ + public void updatePosition(int y, boolean animate) { + PropertyAnimator.setProperty(mView, AnimatableProperty.Y, y, COMMUNAL_ANIMATION_PROPERTIES, + animate); + } + /** * Invoked when the quick settings is expanded. * @param expansionFraction the percentage the QS shade has been expanded. diff --git a/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewPositionAlgorithm.java b/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewPositionAlgorithm.java new file mode 100644 index 0000000000000..424da0b111c5a --- /dev/null +++ b/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewPositionAlgorithm.java @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2021 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.communal; + +import android.util.Log; + +import com.android.systemui.statusbar.phone.NotificationPanelViewController; + +/** + * {@link CommunalHostViewPositionAlgorithm} calculates the position of the communal view given + * input such as the notification panel position. + */ +public class CommunalHostViewPositionAlgorithm { + private static final String TAG = "CommunalPositionAlg"; + private static final boolean DEBUG = Log.isLoggable(TAG, Log.DEBUG); + + /** + * @see NotificationPanelViewController#getExpandedFraction() + */ + private float mPanelExpansion; + + /** + * Height of {@link CommunalHostView}. + */ + private int mCommunalHeight; + + /** + * A data container for the result of the position algorithm. + */ + public static class Result { + /** + * The y translation of the clock. + */ + public int communalY; + } + + /** + * Sets the conditions under which the result should be calculated from. + * @param panelExpansion The percentage the keyguard panel has been moved upwards. + * @param communalHeight The height of the communal panel. + */ + public void setup(float panelExpansion, int communalHeight) { + if (DEBUG) { + Log.d(TAG, "setup. panelExpansion:" + panelExpansion); + } + mPanelExpansion = panelExpansion; + mCommunalHeight = communalHeight; + } + + /** + * Calculates the position based on factors input through {link {@link #setup(float, int)}}. + * @param result The resulting calculations. + */ + public void run(Result result) { + // The panel expansion relates to the keyguard expansion. At full expansion, the communal + // view should be aligned at the top (0). Otherwise, it should be shifted offscreen by the + // unexpanded amount. + result.communalY = (int) ((1 - mPanelExpansion) * -mCommunalHeight); + } +} diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java index 4ab6e92ca1be3..d0c9962ba5b4f 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java @@ -108,6 +108,7 @@ import com.android.systemui.classifier.Classifier; import com.android.systemui.classifier.FalsingCollector; import com.android.systemui.communal.CommunalHostView; import com.android.systemui.communal.CommunalHostViewController; +import com.android.systemui.communal.CommunalHostViewPositionAlgorithm; import com.android.systemui.communal.CommunalSource; import com.android.systemui.communal.CommunalSourceMonitor; import com.android.systemui.communal.CommunalStateController; @@ -381,6 +382,12 @@ public class NotificationPanelViewController extends PanelViewController { private final KeyguardClockPositionAlgorithm.Result mClockPositionResult = new KeyguardClockPositionAlgorithm.Result(); + private final CommunalHostViewPositionAlgorithm + mCommunalPositionAlgorithm = + new CommunalHostViewPositionAlgorithm(); + private final CommunalHostViewPositionAlgorithm.Result + mCommunalPositionResult = + new CommunalHostViewPositionAlgorithm.Result(); private boolean mIsExpanding; private boolean mBlockTouches; @@ -1307,6 +1314,11 @@ public class NotificationPanelViewController extends PanelViewController { boolean animate = mNotificationStackScrollLayoutController.isAddOrRemoveAnimationPending(); int stackScrollerPadding; boolean onKeyguard = isOnKeyguard(); + + if (onKeyguard) { + updateCommunalViewAppearance(); + } + if (onKeyguard || forceClockUpdate) { updateClockAppearance(); } @@ -1332,6 +1344,22 @@ public class NotificationPanelViewController extends PanelViewController { mAnimateNextPositionUpdate = false; } + private void updateCommunalViewAppearance() { + if (mCommunalViewController == null) { + return; + } + + float expandedFraction = + mUnlockedScreenOffAnimationController.isScreenOffAnimationPlaying() + ? 1.0f : getExpandedFraction(); + mCommunalPositionAlgorithm.setup(expandedFraction, mCommunalView.getHeight()); + mCommunalPositionAlgorithm.run(mCommunalPositionResult); + boolean animate = + mNotificationStackScrollLayoutController.isAddOrRemoveAnimationPending() + || mAnimateNextPositionUpdate; + mCommunalViewController.updatePosition(mCommunalPositionResult.communalY, animate); + } + private void updateClockAppearance() { int userSwitcherPreferredY = mStatusBarHeaderHeightKeyguard; boolean bypassEnabled = mKeyguardBypassController.getBypassEnabled(); diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewPositionAlgorithmTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewPositionAlgorithmTest.java new file mode 100644 index 0000000000000..0a0266bc19554 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewPositionAlgorithmTest.java @@ -0,0 +1,44 @@ +/* + * Copyright (C) 2021 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.communal; + +import static com.google.common.truth.Truth.assertThat; + +import android.test.suitebuilder.annotation.SmallTest; + +import com.android.systemui.SysuiTestCase; +import com.android.systemui.communal.CommunalHostViewPositionAlgorithm.Result; + +import org.junit.Test; + + +@SmallTest +public class CommunalHostViewPositionAlgorithmTest extends SysuiTestCase { + @Test + public void testOutput() { + final float expansion = 0.25f; + final int height = 120; + + final CommunalHostViewPositionAlgorithm algorithm = new CommunalHostViewPositionAlgorithm(); + algorithm.setup(expansion, height); + final Result result = new Result(); + algorithm.run(result); + + // Verify the communal view is shifted offscreen vertically by the correct amount. + assertThat((1 - expansion) * -height).isEqualTo(result.communalY); + } +} diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java index f6e06cc6a59b9..99f6354b27cee 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java @@ -908,6 +908,20 @@ public class NotificationPanelViewTest extends SysuiTestCase { verify(mCommunalHostViewController).setAlpha(anyFloat()); } + @Test + public void testCommunalPositionUpdate() { + // Verify that the communal position is updated on interaction with the + // NotificationPanelViewController. Note that there a number of paths where the position + // might be updated and therefore the check isn't strictly on a single invocation. + clearInvocations(mCommunalHostViewController); + final View.OnLayoutChangeListener layoutChangeListener = + mNotificationPanelViewController.createLayoutChangeListener(); + mNotificationPanelViewController.mStatusBarStateController.setState(KEYGUARD); + layoutChangeListener.onLayoutChange(mView, 0, 0, 200, 200, 0, 0, 200, 200); + verify(mCommunalHostViewController, atLeast(1)) + .updatePosition(anyInt(), anyBoolean()); + } + private void triggerPositionClockAndNotifications() { mNotificationPanelViewController.closeQs(); } From 1a99bc681816725f144f8e5d4d95491e130b0847 Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Wed, 18 Aug 2021 00:10:01 -0700 Subject: [PATCH 3/6] Address communal UI glitches. This changelist addresses two sources of UI glitches: - The CommunalHostViewController is now initialized once, preventing multiple, potentially stale, signals to the communal code. - Bouncer presence has been removed from the invalid states as the communal view can be shown as the same time as the bouncer during transition. Bug: 195601027 Test: atest CommunalHostViewControllerTest#testNoShowInvocationOnBouncer Change-Id: I64893d9e57cba8dbbdb6085e7a6e2b677455f273 --- .../communal/CommunalHostViewController.java | 6 +-- .../NotificationPanelViewController.java | 24 ++++++--- .../CommunalHostViewControllerTest.java | 51 +++++++++---------- .../phone/NotificationPanelViewTest.java | 3 +- 4 files changed, 44 insertions(+), 40 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java b/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java index b06f4430d0e9e..73e5afe1a2dc1 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java +++ b/packages/SystemUI/src/com/android/systemui/communal/CommunalHostViewController.java @@ -81,7 +81,7 @@ public class CommunalHostViewController extends ViewController // Only show communal view when keyguard is showing and not dozing. private static final int SHOW_COMMUNAL_VIEW_REQUIRED_STATES = STATE_KEYGUARD_SHOWING; private static final int SHOW_COMMUNAL_VIEW_INVALID_STATES = - STATE_DOZING | STATE_BOUNCER_SHOWING | STATE_KEYGUARD_OCCLUDED; + STATE_DOZING | STATE_KEYGUARD_OCCLUDED; private final KeyguardVisibilityHelper mKeyguardVisibilityHelper; @@ -216,9 +216,7 @@ public class CommunalHostViewController extends ViewController } } @Override - public void init() { - super.init(); - + public void onInit() { setState(STATE_KEYGUARD_SHOWING, mKeyguardStateController.isShowing()); setState(STATE_DOZING, mStatusBarStateController.isDozing()); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java index d0c9962ba5b4f..f0a35d4cc6bc6 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/NotificationPanelViewController.java @@ -119,6 +119,7 @@ import com.android.systemui.dagger.qualifiers.Main; import com.android.systemui.doze.DozeLog; import com.android.systemui.fragments.FragmentHostManager.FragmentListener; import com.android.systemui.fragments.FragmentService; +import com.android.systemui.idle.IdleHostView; import com.android.systemui.idle.IdleHostViewController; import com.android.systemui.idle.dagger.IdleViewComponent; import com.android.systemui.media.KeyguardMediaController; @@ -902,10 +903,20 @@ public class NotificationPanelViewController extends PanelViewController { mIdleHostViewController = idleViewComponent.getIdleHostViewController(); mIdleHostViewController.init(); + if (mCommunalView != null) { + CommunalViewComponent communalViewComponent = + mCommunalViewComponentFactory.build(mCommunalView); + mCommunalViewController = + communalViewComponent.getCommunalHostViewController(); + mCommunalViewController.init(); + } + + updateViewControllers( mView.findViewById(R.id.keyguard_status_view), userAvatarView, keyguardUserSwitcherView, + mView.findViewById(R.id.idle_host_view), mCommunalView); mNotificationContainerParent = mView.findViewById(R.id.notification_container_parent); NotificationStackScrollLayout stackScrollLayout = mView.findViewById( @@ -996,6 +1007,7 @@ public class NotificationPanelViewController extends PanelViewController { private void updateViewControllers(KeyguardStatusView keyguardStatusView, UserAvatarView userAvatarView, KeyguardUserSwitcherView keyguardUserSwitcherView, + IdleHostView idleHostView, CommunalHostView communalView) { // Re-associate the KeyguardStatusViewController KeyguardStatusViewComponent statusViewComponent = @@ -1003,13 +1015,9 @@ public class NotificationPanelViewController extends PanelViewController { mKeyguardStatusViewController = statusViewComponent.getKeyguardStatusViewController(); mKeyguardStatusViewController.init(); - if (communalView != null) { - CommunalViewComponent communalViewComponent = - mCommunalViewComponentFactory.build(communalView); - mCommunalViewController = - communalViewComponent.getCommunalHostViewController(); - mCommunalViewController.init(); - } + IdleViewComponent idleViewComponent = mIdleViewComponentFactory.build(idleHostView); + mIdleHostViewController = idleViewComponent.getIdleHostViewController(); + mIdleHostViewController.init(); if (mKeyguardUserSwitcherController != null) { // Try to close the switcher so that callbacks are triggered if necessary. @@ -1174,7 +1182,7 @@ public class NotificationPanelViewController extends PanelViewController { mBigClockContainer.removeAllViews(); updateViewControllers(mView.findViewById(R.id.keyguard_status_view), userAvatarView, - keyguardUserSwitcherView, mCommunalView); + keyguardUserSwitcherView, mView.findViewById(R.id.idle_host_view), mCommunalView); // Update keyguard bottom area int index = mView.indexOfChild(mKeyguardBottomArea); diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java index ea00c6f7fb104..6cfa40af32663 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/CommunalHostViewControllerTest.java @@ -16,6 +16,7 @@ package com.android.systemui.communal; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.never; @@ -120,33 +121,6 @@ public class CommunalHostViewControllerTest extends SysuiTestCase { verify(mCommunalView).setVisibility(View.INVISIBLE); } - @Test - public void testHideOnBouncer() { - ArgumentCaptor callbackCapture = - ArgumentCaptor.forClass(KeyguardUpdateMonitorCallback.class); - - // Capture callback value for later use. - verify(mKeyguardUpdateMonitor).registerCallback(callbackCapture.capture()); - - // Establish a visible communal view. - mController.show(new WeakReference<>(mCommunalSource)); - mFakeExecutor.runAllReady(); - verify(mCommunalView).setVisibility(View.VISIBLE); - Mockito.clearInvocations(mCommunalView); - - // Trigger bouncer. - Mockito.clearInvocations(mCommunalView); - callbackCapture.getValue().onKeyguardBouncerChanged(true); - mFakeExecutor.runAllReady(); - verify(mCommunalView).setVisibility(View.INVISIBLE); - - // Hide bouncer - Mockito.clearInvocations(mCommunalView); - callbackCapture.getValue().onKeyguardBouncerChanged(false); - mFakeExecutor.runAllReady(); - verify(mCommunalView).setVisibility(View.VISIBLE); - } - @Test public void testHideOnOcclude() { ArgumentCaptor callbackCapture = @@ -242,4 +216,27 @@ public class CommunalHostViewControllerTest extends SysuiTestCase { mFakeExecutor.runAllReady(); verify(mCommunalSource, never()).requestCommunalView(any()); } + + @Test + public void testNoShowInvocationOnBouncer() { + ArgumentCaptor callbackCapture = + ArgumentCaptor.forClass(KeyguardUpdateMonitorCallback.class); + + // Capture callback value for later use. + verify(mKeyguardUpdateMonitor).registerCallback(callbackCapture.capture()); + + // Set source so it will be cleared if in invalid state. + mController.show(new WeakReference<>(mCommunalSource)); + mFakeExecutor.runAllReady(); + clearInvocations(mCommunalStateController, mCommunalView); + + // Change bouncer to showing. + callbackCapture.getValue().onKeyguardBouncerChanged(true); + mFakeExecutor.runAllReady(); + + // Verify that there were no requests to remove all child views or set the communal + // state to not showing. + verify(mCommunalStateController, never()).setCommunalViewShowing(eq(false)); + verify(mCommunalView, never()).removeAllViews(); + } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java index 99f6354b27cee..a5c952725f62f 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/NotificationPanelViewTest.java @@ -851,9 +851,10 @@ public class NotificationPanelViewTest extends SysuiTestCase { @Test public void testCommunalhostViewControllerInit() { + verify(mCommunalHostViewController, times(1)).init(); clearInvocations(mCommunalHostViewController); givenViewAttached(); - verify(mCommunalHostViewController).init(); + verify(mCommunalHostViewController, never()).init(); } @Test From 04bed117340242aac54c6b8dcf629bbbd1cf3789 Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Fri, 20 Aug 2021 08:28:37 -0700 Subject: [PATCH 4/6] Allow for transparent communal views. This changelist modifies the pixel format of the communal SurfaceView to allow for transparent pixels. Bug: 197322620 Test: atest CommunalSurfaceViewControllerTest Change-Id: I4f237307dcc6457053ef9ca08d28a122de3ab9da --- .../communal/service/CommunalSurfaceViewController.java | 2 ++ .../communal/service/CommunalSurfaceViewControllerTest.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java b/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java index 4927e0908c5a2..d554b5e26d4f5 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java +++ b/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java @@ -18,6 +18,7 @@ package com.android.systemui.communal.service; import android.annotation.IntDef; import android.content.res.Resources; +import android.graphics.PixelFormat; import android.graphics.Region; import android.util.Log; import android.view.IWindow; @@ -125,6 +126,7 @@ public class CommunalSurfaceViewController extends ViewController { @Override protected void onInit() { + mView.getHolder().setFormat(PixelFormat.TRANSPARENT); mView.getHolder().addCallback(mSurfaceHolderCallback); mView.addOnLayoutChangeListener(mOnLayoutChangeListener); } diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java index 6ca1357bef20c..c2d6b9aff96d9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import android.content.res.Resources; +import android.graphics.PixelFormat; import android.graphics.Region; import android.os.IBinder; import android.view.Display; @@ -125,6 +126,7 @@ public class CommunalSurfaceViewControllerTest extends SysuiTestCase { final ArgumentCaptor callbackCapture = ArgumentCaptor.forClass(SurfaceHolder.Callback.class); verify(mSurfaceHolder).addCallback(callbackCapture.capture()); + verify(mSurfaceHolder).setFormat(PixelFormat.TRANSPARENT); mCallback = callbackCapture.getValue(); final ArgumentCaptor listenerCapture = From 8379b15dc569f851c807146a19dda7186d8065df Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Mon, 23 Aug 2021 16:24:01 -0700 Subject: [PATCH 5/6] Always reparent SurfacePackage in setChildSurfacePackage. Currently, SurfaceView#setChildSurfacePackage only reparents a provided SurfacePackage when there is no SurfaceControl already present. This can lead to the SurfacePackage potentially never being reparented. This changelist addresses the issue by always reparenting the SurfacePackage, following cleanup of the existing SurfaceControl. Bug: 197568243 Test: atest SurfaceControlViewHostTests#testCanReplaceSurfacePackage Change-Id: Ib0bcfee5cb9801e1b1c4f1f300af9862141b12f3 --- core/java/android/view/SurfaceView.java | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java index a4e7a43cc9347..643c1bc68d2cf 100644 --- a/core/java/android/view/SurfaceView.java +++ b/core/java/android/view/SurfaceView.java @@ -1889,10 +1889,12 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall public void setChildSurfacePackage(@NonNull SurfaceControlViewHost.SurfacePackage p) { final SurfaceControl lastSc = mSurfacePackage != null ? mSurfacePackage.getSurfaceControl() : null; - if (mSurfaceControl != null && lastSc != null) { - mTmpTransaction.reparent(lastSc, null).apply(); - mSurfacePackage.release(); - } else if (mSurfaceControl != null) { + if (mSurfaceControl != null) { + if (lastSc != null) { + mTmpTransaction.reparent(lastSc, null); + mSurfacePackage.release(); + } + reparentSurfacePackage(mTmpTransaction, p); mTmpTransaction.apply(); } From 49af78d4b93e4a01f13f2e9d1dc4ce12098148f0 Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Mon, 23 Aug 2021 17:46:44 -0700 Subject: [PATCH 6/6] Request new communal surface on layout changes. This changelist causes a request to be made on every layout change to the communal SurfaceView. This fixes the issues where the SurfaceView is not resized properly. This change also ensures that subsequent identical show requests are ignored. Bug: 197574964 Test: CommunalSurfaceViewControllerTest#testLayoutChange Change-Id: I480b6ab5b9728f173cf4fd3e6042aa70ba60db89 --- .../communal/service/CommunalSourceImpl.java | 51 ++++++++++++++++--- .../CommunalSurfaceViewController.java | 27 +++++++--- .../CommunalSurfaceViewControllerTest.java | 45 +++++++++++++--- 3 files changed, 102 insertions(+), 21 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSourceImpl.java b/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSourceImpl.java index d8bf2dc73c3dd..b8070abd90b83 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSourceImpl.java +++ b/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSourceImpl.java @@ -38,6 +38,7 @@ import com.google.common.util.concurrent.ListenableFuture; import java.lang.ref.WeakReference; import java.util.ArrayList; +import java.util.Objects; import java.util.concurrent.Executor; import javax.inject.Inject; @@ -78,6 +79,45 @@ public class CommunalSourceImpl implements CommunalSource { } } + static class Request { + private final int mWidth; + private final int mHeight; + private final int mDisplayId; + private final IBinder mHostToken; + + Request(int width, int height, int displayId, IBinder hostToken) { + mWidth = width; + mHeight = height; + mDisplayId = displayId; + mHostToken = hostToken; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (!(o instanceof Request)) return false; + Request request = (Request) o; + return mWidth == request.mWidth && mHeight == request.mHeight + && mDisplayId == request.mDisplayId && Objects.equals(mHostToken, + request.mHostToken); + } + + @Override + public int hashCode() { + return Objects.hash(mWidth, mHeight, mDisplayId, mHostToken); + } + + @Override + public String toString() { + return "Request{" + + "mWidth=" + mWidth + + ", mHeight=" + mHeight + + ", mDisplayId=" + mDisplayId + + ", mHostToken=" + mHostToken + + '}'; + } + } + // mConnected is initialized to true as it is presumed instances are constructed with valid // proxies. The source can never be reconnected once the proxy has died. Once this value // becomes false, the source will always report disconnected to registering callbacks. @@ -148,18 +188,15 @@ public class CommunalSourceImpl implements CommunalSource { * Called internally to request a new {@link android.view.SurfaceControlViewHost.SurfacePackage} * for showing communal content. * - * @param hostToken The HostToken necessary to generate a {@link SurfaceControlViewHost}. - * @param displayId The id of the display the surface will be shown on. - * @param width The width of the surface. - * @param height The height of the surface. + * @param request A request with the parameters for the new communal surface. * @return A future that returns the resulting * {@link android.view.SurfaceControlViewHost.SurfacePackage}. */ protected ListenableFuture requestCommunalSurface( - IBinder hostToken, int displayId, int width, int height) { + Request request) { return CallbackToFutureAdapter.getFuture(completer -> { - mSourceProxy.getCommunalSurface(hostToken, width, height, displayId, - new ICommunalSurfaceCallback.Stub() { + mSourceProxy.getCommunalSurface(request.mHostToken, request.mWidth, request.mHeight, + request.mDisplayId, new ICommunalSurfaceCallback.Stub() { @Override public void onSurface( SurfaceControlViewHost.SurfacePackage surfacePackage) { diff --git a/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java b/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java index d554b5e26d4f5..0de50297d04f4 100644 --- a/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java +++ b/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java @@ -37,6 +37,7 @@ import com.android.systemui.util.ViewController; import com.google.common.util.concurrent.ListenableFuture; +import java.util.Optional; import java.util.concurrent.Executor; /** @@ -64,6 +65,8 @@ public class CommunalSurfaceViewController extends ViewController { private int mCurrentState; + private Optional mLastRequest = Optional.empty(); + // The current in-flight request for a surface package. private ListenableFuture mCurrentSurfaceFuture; @@ -100,6 +103,9 @@ public class CommunalSurfaceViewController extends ViewController { mSurfaceViewTouchableRegion.set(left, top + topMargin, right, bottom - bottomMargin); updateTouchExclusion(); + + // Trigger showing (or hiding) surface based on new dimensions. + showSurface(); } }; @@ -149,7 +155,7 @@ public class CommunalSurfaceViewController extends ViewController { mCurrentState = newState; - showSurface(newState == STATE_CAN_SHOW_SURFACE); + showSurface(); updateTouchExclusion(); } @@ -167,25 +173,34 @@ public class CommunalSurfaceViewController extends ViewController { } } - private void showSurface(boolean show) { + private void showSurface() { mView.setWillNotDraw(false); - if (!show) { + if (mCurrentState != STATE_CAN_SHOW_SURFACE) { // If the surface is no longer showing, cancel any in-flight requests. if (mCurrentSurfaceFuture != null) { mCurrentSurfaceFuture.cancel(true); mCurrentSurfaceFuture = null; } + mLastRequest = Optional.empty(); mView.setWillNotDraw(true); return; } + final CommunalSourceImpl.Request request = new CommunalSourceImpl.Request( + mView.getMeasuredWidth(), mView.getMeasuredHeight(), + mView.getDisplay().getDisplayId(), mView.getHostToken()); + + if (mLastRequest.isPresent() && mLastRequest.get().equals(request)) { + return; + } + + mLastRequest = Optional.of(request); + // Since this method is only called when the state has changed, mCurrentSurfaceFuture should // be null here. - mCurrentSurfaceFuture = mSource.requestCommunalSurface(mView.getHostToken(), - mView.getDisplay().getDisplayId(), mView.getMeasuredWidth(), - mView.getMeasuredHeight()); + mCurrentSurfaceFuture = mSource.requestCommunalSurface(request); mCurrentSurfaceFuture.addListener(new Runnable() { @Override diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java index c2d6b9aff96d9..cf2e029fabfcd 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java @@ -18,9 +18,9 @@ package com.android.systemui.communal.service; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.clearInvocations; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -136,7 +136,7 @@ public class CommunalSurfaceViewControllerTest extends SysuiTestCase { mPackageFuture = SettableFuture.create(); - when(mCommunalSource.requestCommunalSurface(any(), anyInt(), anyInt(), anyInt())) + when(mCommunalSource.requestCommunalSurface(any())) .thenReturn(mPackageFuture); } @@ -144,19 +144,20 @@ public class CommunalSurfaceViewControllerTest extends SysuiTestCase { public void testSetSurfacePackage() { // There should be no requests without the proper state. verify(mCommunalSource, times(0)) - .requestCommunalSurface(any(), anyInt(), anyInt(), anyInt()); + .requestCommunalSurface(any()); // The full state must be present to make a request. mController.onViewAttached(); verify(mCommunalSource, times(0)) - .requestCommunalSurface(any(), anyInt(), anyInt(), anyInt()); + .requestCommunalSurface(any()); clearInvocations(mSurfaceView); // Request surface view once all conditions are met. mCallback.surfaceCreated(mSurfaceHolder); - verify(mCommunalSource) - .requestCommunalSurface(mHostToken, DISPLAY_ID, MEASURED_WIDTH, MEASURED_HEIGHT); + final CommunalSourceImpl.Request expectedRequest = new CommunalSourceImpl.Request( + MEASURED_WIDTH, MEASURED_HEIGHT, DISPLAY_ID, mHostToken); + verify(mCommunalSource).requestCommunalSurface(eq(expectedRequest)); when(mSurfaceView.isAttachedToWindow()).thenReturn(true); @@ -216,8 +217,9 @@ public class CommunalSurfaceViewControllerTest extends SysuiTestCase { mFakeExecutor.runAllReady(); clearInvocations(mSurfaceView); - verify(mCommunalSource, times(1)) - .requestCommunalSurface(mHostToken, DISPLAY_ID, MEASURED_WIDTH, MEASURED_HEIGHT); + final CommunalSourceImpl.Request expectedRequest = new CommunalSourceImpl.Request( + MEASURED_WIDTH, MEASURED_HEIGHT, DISPLAY_ID, mHostToken); + verify(mCommunalSource, times(1)).requestCommunalSurface(eq(expectedRequest)); mController.onViewDetached(); assertTrue(mPackageFuture.isCancelled()); @@ -270,4 +272,31 @@ public class CommunalSurfaceViewControllerTest extends SysuiTestCase { verify(mNotificationShadeWindowController) .setTouchExclusionRegion(eq(new Region())); } + + @Test + public void testLayoutChange() { + final int left = 0; + final int top = 0; + final int right = 200; + final int bottom = 100; + + givenSurfacePresent(); + + // Layout change should trigger a request to get new communal surface. + mLayoutChangeListener.onLayoutChange(mSurfaceView, left, top, right, bottom, 0, 0, 0, + 0); + // Note that the measured are preset and different than the layout input. + final CommunalSourceImpl.Request expectedRequest = + new CommunalSourceImpl.Request(MEASURED_WIDTH, MEASURED_HEIGHT, DISPLAY_ID, + mHostToken); + verify(mCommunalSource) + .requestCommunalSurface(eq(expectedRequest)); + + clearInvocations(mCommunalSource); + + // Subsequent matching layout change should not trigger any request. + mLayoutChangeListener.onLayoutChange(mSurfaceView, left, top, right, bottom, 0, 0, 0, + 0); + verify(mCommunalSource, never()).requestCommunalSurface(any()); + } }