From 0abf06dcfb70f34f12fb3ca3d0dc432a770a5e7a Mon Sep 17 00:00:00 2001 From: Josh Tsuji Date: Thu, 3 Nov 2022 14:54:28 -0400 Subject: [PATCH] Keep status bar icons hidden during camera launch. Before: https://drive.google.com/file/d/1FmJFP8ZZ0J6r0DWnfD9HR5JeWVEizHI_/view?usp=share_link After: https://drive.google.com/file/d/1U743mR-0ZzzJxK-rDOl2rZiFdEio9EDg/view?usp=share_link This has been a long standing issue with camera launch that looks pretty bad. When we start occluding, the status bar icons are shown, only be to be hidden shortly after the animation ends when we receive a callback from WM that the top app hides the status bar. This was an unexpectedly difficult fix as there's a brief period between the launch animation ending and System UI receiving a call that the top app wants to hide the status bar, where there is no existing state that can tell us that we should be hiding the icons. A similar case is currently handled in StatusBarHideIconsForBouncerManager by posting a callback delayed to not hide/show icons within 500ms, but that's not particularly robust. Fixes: 257292822 Test: manually launch camera Test: atest CollapsedStatusBarFragmentTest Change-Id: Ib54917b4e65529a2fd809eb67159d57d64e8097f --- .../phone/dagger/StatusBarViewModule.java | 10 ++- .../fragment/CollapsedStatusBarFragment.java | 65 +++++++++++++++- .../window/StatusBarWindowStateController.kt | 4 + .../CollapsedStatusBarFragmentTest.java | 76 ++++++++++++++++++- 4 files changed, 151 insertions(+), 4 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/StatusBarViewModule.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/StatusBarViewModule.java index dc902666874d0..0b856f66cdcc2 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/StatusBarViewModule.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/dagger/StatusBarViewModule.java @@ -23,6 +23,7 @@ import android.view.LayoutInflater; import android.view.View; import android.view.ViewStub; +import com.android.keyguard.KeyguardUpdateMonitor; import com.android.keyguard.LockIconView; import com.android.systemui.R; import com.android.systemui.battery.BatteryMeterView; @@ -68,6 +69,7 @@ import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallController; import com.android.systemui.statusbar.policy.BatteryController; import com.android.systemui.statusbar.policy.ConfigurationController; import com.android.systemui.statusbar.policy.KeyguardStateController; +import com.android.systemui.statusbar.window.StatusBarWindowStateController; import com.android.systemui.tuner.TunerService; import com.android.systemui.util.CarrierConfigTracker; import com.android.systemui.util.settings.SecureSettings; @@ -299,7 +301,9 @@ public abstract class StatusBarViewModule { OperatorNameViewController.Factory operatorNameViewControllerFactory, SecureSettings secureSettings, @Main Executor mainExecutor, - DumpManager dumpManager + DumpManager dumpManager, + StatusBarWindowStateController statusBarWindowStateController, + KeyguardUpdateMonitor keyguardUpdateMonitor ) { return new CollapsedStatusBarFragment(statusBarFragmentComponentFactory, ongoingCallController, @@ -321,7 +325,9 @@ public abstract class StatusBarViewModule { operatorNameViewControllerFactory, secureSettings, mainExecutor, - dumpManager); + dumpManager, + statusBarWindowStateController, + keyguardUpdateMonitor); } /** diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragment.java b/packages/SystemUI/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragment.java index 9f3fd727be24b..c495207c7a179 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragment.java +++ b/packages/SystemUI/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragment.java @@ -45,6 +45,7 @@ import android.widget.LinearLayout; import androidx.annotation.VisibleForTesting; +import com.android.keyguard.KeyguardUpdateMonitor; import com.android.systemui.Dumpable; import com.android.systemui.R; import com.android.systemui.animation.Interpolators; @@ -77,6 +78,8 @@ import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallController; import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallListener; import com.android.systemui.statusbar.policy.EncryptionHelper; import com.android.systemui.statusbar.policy.KeyguardStateController; +import com.android.systemui.statusbar.window.StatusBarWindowStateController; +import com.android.systemui.statusbar.window.StatusBarWindowStateListener; import com.android.systemui.util.CarrierConfigTracker; import com.android.systemui.util.CarrierConfigTracker.CarrierConfigChangedListener; import com.android.systemui.util.CarrierConfigTracker.DefaultDataSubscriptionChangedListener; @@ -135,6 +138,8 @@ public class CollapsedStatusBarFragment extends Fragment implements CommandQueue private final SecureSettings mSecureSettings; private final Executor mMainExecutor; private final DumpManager mDumpManager; + private final StatusBarWindowStateController mStatusBarWindowStateController; + private final KeyguardUpdateMonitor mKeyguardUpdateMonitor; private List mBlockedIcons = new ArrayList<>(); private Map mStartableStates = new ArrayMap<>(); @@ -177,6 +182,22 @@ public class CollapsedStatusBarFragment extends Fragment implements CommandQueue } }; + /** + * Whether we've launched the secure camera over the lockscreen, but haven't yet received a + * status bar window state change afterward. + * + * We wait for this state change (which will tell us whether to show/hide the status bar icons) + * so that there is no flickering/jump cutting during the camera launch. + */ + private boolean mWaitingForWindowStateChangeAfterCameraLaunch = false; + + /** + * Listener that updates {@link #mWaitingForWindowStateChangeAfterCameraLaunch} when it receives + * a new status bar window state. + */ + private final StatusBarWindowStateListener mStatusBarWindowStateListener = state -> + mWaitingForWindowStateChangeAfterCameraLaunch = false; + @SuppressLint("ValidFragment") public CollapsedStatusBarFragment( StatusBarFragmentComponent.Factory statusBarFragmentComponentFactory, @@ -199,7 +220,9 @@ public class CollapsedStatusBarFragment extends Fragment implements CommandQueue OperatorNameViewController.Factory operatorNameViewControllerFactory, SecureSettings secureSettings, @Main Executor mainExecutor, - DumpManager dumpManager + DumpManager dumpManager, + StatusBarWindowStateController statusBarWindowStateController, + KeyguardUpdateMonitor keyguardUpdateMonitor ) { mStatusBarFragmentComponentFactory = statusBarFragmentComponentFactory; mOngoingCallController = ongoingCallController; @@ -222,6 +245,20 @@ public class CollapsedStatusBarFragment extends Fragment implements CommandQueue mSecureSettings = secureSettings; mMainExecutor = mainExecutor; mDumpManager = dumpManager; + mStatusBarWindowStateController = statusBarWindowStateController; + mKeyguardUpdateMonitor = keyguardUpdateMonitor; + } + + @Override + public void onCreate(Bundle savedInstanceState) { + super.onCreate(savedInstanceState); + mStatusBarWindowStateController.addListener(mStatusBarWindowStateListener); + } + + @Override + public void onDestroy() { + super.onDestroy(); + mStatusBarWindowStateController.removeListener(mStatusBarWindowStateListener); } @Override @@ -270,6 +307,11 @@ public class CollapsedStatusBarFragment extends Fragment implements CommandQueue mCarrierConfigTracker.addDefaultDataSubscriptionChangedListener(mDefaultDataListener); } + @Override + public void onCameraLaunchGestureDetected(int source) { + mWaitingForWindowStateChangeAfterCameraLaunch = true; + } + @VisibleForTesting void updateBlockedIcons() { mBlockedIcons.clear(); @@ -494,6 +536,27 @@ public class CollapsedStatusBarFragment extends Fragment implements CommandQueue && mNotificationPanelViewController.hideStatusBarIconsWhenExpanded()) { return true; } + + // When launching the camera over the lockscreen, the icons become visible momentarily + // before animating out, since we're not yet aware that the launching camera activity is + // fullscreen. Even once the activity finishes launching, it takes a short time before WM + // decides that the top app wants to hide the icons and tells us to hide them. To ensure + // that this high-visibility animation is smooth, keep the icons hidden during a camera + // launch until we receive a window state change which indicates that the activity is done + // launching and WM has decided to show/hide the icons. For extra safety (to ensure the + // icons don't remain hidden somehow) we double check that the camera is still showing, the + // status bar window isn't hidden, and we're still occluded as well, though these checks + // are typically unnecessary. + final boolean hideIconsForSecureCamera = + (mWaitingForWindowStateChangeAfterCameraLaunch || + !mStatusBarWindowStateController.windowIsShowing()) && + mKeyguardUpdateMonitor.isSecureCameraLaunchedOverKeyguard() && + mKeyguardStateController.isOccluded(); + + if (hideIconsForSecureCamera) { + return true; + } + return mStatusBarHideIconsForBouncerManager.getShouldHideStatusBarIconsForBouncer(); } diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/window/StatusBarWindowStateController.kt b/packages/SystemUI/src/com/android/systemui/statusbar/window/StatusBarWindowStateController.kt index 60f6df66cb5a0..8f424b2e251e9 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/window/StatusBarWindowStateController.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/window/StatusBarWindowStateController.kt @@ -61,6 +61,10 @@ class StatusBarWindowStateController @Inject constructor( listeners.add(listener) } + fun removeListener(listener: StatusBarWindowStateListener) { + listeners.remove(listener) + } + /** Returns true if the window is currently showing. */ fun windowIsShowing() = windowState == WINDOW_STATE_SHOWING diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragmentTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragmentTest.java index 438271c489e68..1a87eeb9d3254 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragmentTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/phone/fragment/CollapsedStatusBarFragmentTest.java @@ -23,10 +23,12 @@ import static com.android.systemui.statusbar.events.SystemStatusAnimationSchedul import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.atLeast; +import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -45,6 +47,7 @@ import android.widget.FrameLayout; import androidx.test.filters.SmallTest; +import com.android.keyguard.KeyguardUpdateMonitor; import com.android.systemui.R; import com.android.systemui.SysuiBaseFragmentTest; import com.android.systemui.dump.DumpManager; @@ -68,6 +71,8 @@ import com.android.systemui.statusbar.phone.StatusBarLocationPublisher; import com.android.systemui.statusbar.phone.fragment.dagger.StatusBarFragmentComponent; import com.android.systemui.statusbar.phone.ongoingcall.OngoingCallController; import com.android.systemui.statusbar.policy.KeyguardStateController; +import com.android.systemui.statusbar.window.StatusBarWindowStateController; +import com.android.systemui.statusbar.window.StatusBarWindowStateListener; import com.android.systemui.util.CarrierConfigTracker; import com.android.systemui.util.concurrency.FakeExecutor; import com.android.systemui.util.settings.SecureSettings; @@ -80,6 +85,9 @@ import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import java.util.ArrayList; +import java.util.List; + @RunWith(AndroidTestingRunner.class) @RunWithLooper(setAsMainLooper = true) @SmallTest @@ -120,6 +128,12 @@ public class CollapsedStatusBarFragmentTest extends SysuiBaseFragmentTest { private StatusBarHideIconsForBouncerManager mStatusBarHideIconsForBouncerManager; @Mock private DumpManager mDumpManager; + @Mock + private StatusBarWindowStateController mStatusBarWindowStateController; + @Mock + private KeyguardUpdateMonitor mKeyguardUpdateMonitor; + + private List mStatusBarWindowStateListeners = new ArrayList<>(); public CollapsedStatusBarFragmentTest() { super(CollapsedStatusBarFragment.class); @@ -129,6 +143,14 @@ public class CollapsedStatusBarFragmentTest extends SysuiBaseFragmentTest { public void setup() { injectLeakCheckedDependencies(ALL_SUPPORTED_CLASSES); mDependency.injectMockDependency(DarkIconDispatcher.class); + + // Keep the window state listeners so we can dispatch to them to test the status bar + // fragment's response. + doAnswer(invocation -> { + mStatusBarWindowStateListeners.add(invocation.getArgument(0)); + return null; + }).when(mStatusBarWindowStateController).addListener( + any(StatusBarWindowStateListener.class)); } @Test @@ -416,6 +438,27 @@ public class CollapsedStatusBarFragmentTest extends SysuiBaseFragmentTest { assertFalse(contains); } + @Test + public void testStatusBarIcons_hiddenThroughoutCameraLaunch() { + final CollapsedStatusBarFragment fragment = resumeAndGetFragment(); + + mockSecureCameraLaunch(fragment, true /* launched */); + + // Status icons should be invisible or gone, but certainly not VISIBLE. + assertNotEquals(View.VISIBLE, getEndSideContentView().getVisibility()); + assertNotEquals(View.VISIBLE, getClockView().getVisibility()); + + mockSecureCameraLaunchFinished(); + + assertNotEquals(View.VISIBLE, getEndSideContentView().getVisibility()); + assertNotEquals(View.VISIBLE, getClockView().getVisibility()); + + mockSecureCameraLaunch(fragment, false /* launched */); + + assertEquals(View.VISIBLE, getEndSideContentView().getVisibility()); + assertEquals(View.VISIBLE, getClockView().getVisibility()); + } + @Override protected Fragment instantiate(Context context, String className, Bundle arguments) { MockitoAnnotations.initMocks(this); @@ -459,7 +502,9 @@ public class CollapsedStatusBarFragmentTest extends SysuiBaseFragmentTest { mOperatorNameViewControllerFactory, mSecureSettings, mExecutor, - mDumpManager); + mDumpManager, + mStatusBarWindowStateController, + mKeyguardUpdateMonitor); } private void setUpDaggerComponent() { @@ -482,6 +527,35 @@ public class CollapsedStatusBarFragmentTest extends SysuiBaseFragmentTest { mNotificationAreaInner); } + /** + * Configure mocks to return values consistent with the secure camera animating itself launched + * over the keyguard. + */ + private void mockSecureCameraLaunch(CollapsedStatusBarFragment fragment, boolean launched) { + when(mKeyguardUpdateMonitor.isSecureCameraLaunchedOverKeyguard()).thenReturn(launched); + when(mKeyguardStateController.isOccluded()).thenReturn(launched); + + if (launched) { + fragment.onCameraLaunchGestureDetected(0 /* source */); + } else { + for (StatusBarWindowStateListener listener : mStatusBarWindowStateListeners) { + listener.onStatusBarWindowStateChanged(StatusBarManager.WINDOW_STATE_SHOWING); + } + } + + fragment.disable(DEFAULT_DISPLAY, 0, 0, false); + } + + /** + * Configure mocks to return values consistent with the secure camera showing over the keyguard + * with its launch animation finished. + */ + private void mockSecureCameraLaunchFinished() { + for (StatusBarWindowStateListener listener : mStatusBarWindowStateListeners) { + listener.onStatusBarWindowStateChanged(StatusBarManager.WINDOW_STATE_HIDDEN); + } + } + private CollapsedStatusBarFragment resumeAndGetFragment() { mFragments.dispatchResume(); processAllMessages();