From 264a377960308f94c506be2b9d319eba38a675fd Mon Sep 17 00:00:00 2001 From: Winson Chung Date: Fri, 21 May 2021 16:00:09 -0700 Subject: [PATCH] Tweak lifecycle of nav bar/view - Don't wait for the nav bar view to be attached to register the bar with the controller. This might (repro is rare) address a crash where the view could be created twice (with the same nav bar layer which is rejected by WM), because the initial bar has been created, but wasn't cleaned up because it wasn't attached/registered yet before the second request to recreate - As as result of the above, it's possible that other code will reference the bar before it's attached, so the bar needs to set the view reference when it is created - Also remove some deferred callbacks when detaching the view which could cause the callbacks to reference a null view Bug: 188728214 Bug: 183947954 Test: atest SystemUITests with all nav modes Change-Id: I9c05b273251c9f95cb5d693402ae53b35cc94ab5 --- .../systemui/navigationbar/NavigationBar.java | 159 ++++++------------ .../NavigationBarController.java | 10 +- .../buttons/ButtonDispatcher.java | 2 - 3 files changed, 56 insertions(+), 115 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java index 04f519958d567..f1d1ad1dff8e3 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBar.java @@ -31,6 +31,7 @@ import static android.view.WindowInsetsController.APPEARANCE_LOW_PROFILE_BARS; import static android.view.WindowInsetsController.APPEARANCE_OPAQUE_NAVIGATION_BARS; import static android.view.WindowInsetsController.APPEARANCE_SEMI_TRANSPARENT_NAVIGATION_BARS; import static android.view.WindowInsetsController.BEHAVIOR_SHOW_TRANSIENT_BARS_BY_SWIPE; +import static android.view.WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_ALWAYS; import static android.view.WindowManager.LayoutParams.PRIVATE_FLAG_NO_MOVE_ANIMATION; import static android.view.WindowManagerPolicyConstants.NAV_BAR_MODE_3BUTTON; import static android.view.WindowManagerPolicyConstants.NAV_BAR_MODE_GESTURAL; @@ -200,7 +201,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, private final UiEventLogger mUiEventLogger; private Bundle mSavedState; - private NavigationBarView mNavigationBarView = null; + private NavigationBarView mNavigationBarView; private @WindowVisibleState int mNavigationBarWindowState = WINDOW_STATE_SHOWING; @@ -392,7 +393,8 @@ public class NavigationBar implements View.OnAttachStateChangeListener, }; private final Runnable mAutoDim = () -> getBarTransitions().setAutoDim(true); - + private final Runnable mEnableLayoutTransitions = () -> + mNavigationBarView.setLayoutTransitionsEnabled(true); private final Runnable mOnVariableDurationHomeLongClick = () -> { if (onHomeLongClick(mNavigationBarView.getHomeButton().getCurrentView())) { mNavigationBarView.getHomeButton().getCurrentView().performHapticFeedback( @@ -488,7 +490,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, mA11yBtnMode = mAccessibilityButtonModeObserver.getCurrentAccessibilityButtonMode(); } - public View getView() { + public NavigationBarView getView() { return mNavigationBarView; } @@ -504,16 +506,19 @@ public class NavigationBar implements View.OnAttachStateChangeListener, | WindowManager.LayoutParams.FLAG_SLIPPERY, PixelFormat.TRANSLUCENT); lp.token = new Binder(); - lp.setTitle("NavigationBar" + mContext.getDisplayId()); lp.accessibilityTitle = mContext.getString(R.string.nav_bar); - lp.windowAnimations = 0; lp.privateFlags |= WindowManager.LayoutParams.PRIVATE_FLAG_COLOR_SPACE_AGNOSTIC; + lp.layoutInDisplayCutoutMode = LAYOUT_IN_DISPLAY_CUTOUT_MODE_ALWAYS; + lp.windowAnimations = 0; + lp.setTitle("NavigationBar" + mContext.getDisplayId()); + lp.setFitInsetsTypes(0 /* types */); NavigationBarFrame frame = (NavigationBarFrame) LayoutInflater.from(mContext).inflate( R.layout.navigation_bar_window, null); View barView = LayoutInflater.from(frame.getContext()).inflate( R.layout.navigation_bar, frame); barView.addOnAttachStateChangeListener(this); + mNavigationBarView = barView.findViewById(R.id.navigation_bar_view); if (DEBUG) Log.v(TAG, "addNavigationBar: about to add " + barView); mContext.getSystemService(WindowManager.class).addView(frame, lp); @@ -533,6 +538,19 @@ public class NavigationBar implements View.OnAttachStateChangeListener, mContentResolver.registerContentObserver( Settings.Secure.getUriFor(Settings.Secure.ASSIST_TOUCH_GESTURE_ENABLED), false, mAssistContentObserver, UserHandle.USER_ALL); + mAllowForceNavBarHandleOpaque = mContext.getResources().getBoolean( + R.bool.allow_force_nav_bar_handle_opaque); + mForceNavBarHandleOpaque = DeviceConfig.getBoolean( + DeviceConfig.NAMESPACE_SYSTEMUI, + NAV_BAR_HANDLE_FORCE_OPAQUE, + /* defaultValue = */ true); + mHomeButtonLongPressDurationMs = Optional.of(DeviceConfig.getLong( + DeviceConfig.NAMESPACE_SYSTEMUI, + HOME_BUTTON_LONG_PRESS_DURATION_MS, + /* defaultValue = */ 0 + )).filter(duration -> duration != 0); + DeviceConfig.addOnPropertiesChangedListener( + DeviceConfig.NAMESPACE_SYSTEMUI, mHandler::post, mOnPropertiesChangedListener); updateAssistantEntrypoints(); if (savedState != null) { @@ -543,25 +561,10 @@ public class NavigationBar implements View.OnAttachStateChangeListener, mTransientShown = savedState.getBoolean(EXTRA_TRANSIENT_STATE, false); } mSavedState = savedState; - mAccessibilityManagerWrapper.addCallback(mAccessibilityListener); // Respect the latest disabled-flags. mCommandQueue.recomputeDisableFlags(mDisplayId, false); - mAllowForceNavBarHandleOpaque = mContext.getResources().getBoolean( - R.bool.allow_force_nav_bar_handle_opaque); - mForceNavBarHandleOpaque = DeviceConfig.getBoolean( - DeviceConfig.NAMESPACE_SYSTEMUI, - NAV_BAR_HANDLE_FORCE_OPAQUE, - /* defaultValue = */ true); - mHomeButtonLongPressDurationMs = Optional.of(DeviceConfig.getLong( - DeviceConfig.NAMESPACE_SYSTEMUI, - HOME_BUTTON_LONG_PRESS_DURATION_MS, - /* defaultValue = */ 0 - )).filter(duration -> duration != 0); - DeviceConfig.addOnPropertiesChangedListener( - DeviceConfig.NAMESPACE_SYSTEMUI, mHandler::post, mOnPropertiesChangedListener); - mIsCurrentUserSetup = mDeviceProvisionedController.isCurrentUserSetup(); mDeviceProvisionedController.addCallback(mUserSetupListener); @@ -587,7 +590,6 @@ public class NavigationBar implements View.OnAttachStateChangeListener, @Override public void onViewAttachedToWindow(View v) { final Display display = v.getDisplay(); - mNavigationBarView = v.findViewById(R.id.navigation_bar_view); mNavigationBarView.setComponents(mStatusBarLazy.get().getPanelController()); mNavigationBarView.setDisabledFlags(mDisabledFlags1); mNavigationBarView.setOnVerticalChangedListener(this::onVerticalChanged); @@ -598,6 +600,9 @@ public class NavigationBar implements View.OnAttachStateChangeListener, mNavigationBarView.setNavigationIconHints(mNavigationIconHints); mNavigationBarView.setWindowVisible(isNavBarWindowVisible()); mNavigationBarView.setBehavior(mBehavior); + + mAccessibilityManagerWrapper.addCallback(mAccessibilityListener); + mSplitScreenOptional.ifPresent(mNavigationBarView::registerDockedListener); mPipOptional.ifPresent(mNavigationBarView::registerPipExclusionBoundsChangeListener); @@ -657,10 +662,8 @@ public class NavigationBar implements View.OnAttachStateChangeListener, @Override public void onViewDetachedFromWindow(View v) { - if (mNavigationBarView != null) { - mNavigationBarView.getBarTransitions().destroy(); - mNavigationBarView.getLightTransitionsController().destroy(mContext); - } + mNavigationBarView.getBarTransitions().destroy(); + mNavigationBarView.getLightTransitionsController().destroy(mContext); mOverviewProxyService.removeCallback(mOverviewProxyListener); mBroadcastDispatcher.unregisterReceiver(mBroadcastReceiver); if (mOrientationHandle != null) { @@ -671,6 +674,8 @@ public class NavigationBar implements View.OnAttachStateChangeListener, mOrientationHandleGlobalLayoutListener); } mHandler.removeCallbacks(mAutoDim); + mHandler.removeCallbacks(mOnVariableDurationHomeLongClick); + mHandler.removeCallbacks(mEnableLayoutTransitions); mNavigationBarView = null; mOrientationHandle = null; } @@ -682,9 +687,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, outState.putInt(EXTRA_APPEARANCE, mAppearance); outState.putInt(EXTRA_BEHAVIOR, mBehavior); outState.putBoolean(EXTRA_TRANSIENT_STATE, mTransientShown); - if (mNavigationBarView != null) { - mNavigationBarView.getLightTransitionsController().saveState(outState); - } + mNavigationBarView.getLightTransitionsController().saveState(outState); } /** @@ -809,15 +812,12 @@ public class NavigationBar implements View.OnAttachStateChangeListener, // mOrientedHandle is initialized lazily mOrientationHandle.setVisibility(View.GONE); } - if (mNavigationBarView != null) { - mNavigationBarView.setVisibility(View.VISIBLE); - mNavigationBarView.setOrientedHandleSamplingRegion(null); - } + mNavigationBarView.setVisibility(View.VISIBLE); + mNavigationBarView.setOrientedHandleSamplingRegion(null); } private void reconfigureHomeLongClick() { - if (mNavigationBarView == null - || mNavigationBarView.getHomeButton().getCurrentView() == null) { + if (mNavigationBarView.getHomeButton().getCurrentView() == null) { return; } if (mHomeButtonLongPressDurationMs.isPresent() || !mLongPressHomeEnabled) { @@ -844,17 +844,12 @@ public class NavigationBar implements View.OnAttachStateChangeListener, pw.println(" mHomeButtonLongPressDurationMs=" + mHomeButtonLongPressDurationMs); pw.println(" mLongPressHomeEnabled=" + mLongPressHomeEnabled); pw.println(" mAssistantTouchGestureEnabled=" + mAssistantTouchGestureEnabled); - - if (mNavigationBarView != null) { - pw.println(" mNavigationBarWindowState=" - + windowStateToString(mNavigationBarWindowState)); - pw.println(" mNavigationBarMode=" - + BarTransitions.modeToString(mNavigationBarMode)); - dumpBarTransitions(pw, "mNavigationBarView", mNavigationBarView.getBarTransitions()); - mNavigationBarView.dump(pw); - } else { - pw.print(" mNavigationBarView=null"); - } + pw.println(" mNavigationBarWindowState=" + + windowStateToString(mNavigationBarWindowState)); + pw.println(" mNavigationBarMode=" + + BarTransitions.modeToString(mNavigationBarMode)); + dumpBarTransitions(pw, "mNavigationBarView", mNavigationBarView.getBarTransitions()); + mNavigationBarView.dump(pw); } // ----- CommandQueue Callbacks ----- @@ -889,10 +884,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, if (hints == mNavigationIconHints) return; mNavigationIconHints = hints; - - if (mNavigationBarView != null) { - mNavigationBarView.setNavigationIconHints(hints); - } + mNavigationBarView.setNavigationIconHints(hints); checkBarModes(); updateSystemUiStateFlags(-1); } @@ -911,23 +903,12 @@ public class NavigationBar implements View.OnAttachStateChangeListener, orientSecondaryHomeHandle(); } if (DEBUG_WINDOW_STATE) Log.d(TAG, "Navigation bar " + windowStateToString(state)); - - if (mNavigationBarView != null) { - mNavigationBarView.setWindowVisible(isNavBarWindowVisible()); - } + mNavigationBarView.setWindowVisible(isNavBarWindowVisible()); } } @Override public void onRotationProposal(final int rotation, boolean isValid) { - if (mNavigationBarView == null) { - if (RotationContextButton.DEBUG_ROTATION) { - Log.v(TAG, "onRotationProposal proposedRotation=" + - Surface.rotationToString(rotation) + ", mNavigationBarView is null"); - } - return; - } - final int winRotation = mNavigationBarView.getDisplay().getRotation(); final boolean rotateSuggestionsDisabled = RotationButtonController .hasDisable2RotateSuggestionFlag(mDisabledFlags2); @@ -941,8 +922,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, + ", isValid=" + isValid + ", mNavBarWindowState=" + StatusBarManager.windowStateToString(mNavigationBarWindowState) + ", rotateSuggestionsDisabled=" + rotateSuggestionsDisabled - + ", isRotateButtonVisible=" + (mNavigationBarView == null ? "null" - : rotationButton.isVisible())); + + ", isRotateButtonVisible=" + rotationButton.isVisible()); } // Respect the disabled flag, no need for action as flag change callback will handle hiding @@ -983,9 +963,6 @@ public class NavigationBar implements View.OnAttachStateChangeListener, boolean nbModeChanged = false; if (mAppearance != appearance) { mAppearance = appearance; - if (getView() == null) { - return; - } nbModeChanged = updateBarMode(barMode(mTransientShown, appearance)); } if (mLightBarController != null) { @@ -994,9 +971,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, } if (mBehavior != behavior) { mBehavior = behavior; - if (mNavigationBarView != null) { - mNavigationBarView.setBehavior(behavior); - } + mNavigationBarView.setBehavior(behavior); updateSystemUiStateFlags(-1); } } @@ -1034,12 +1009,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, } private void handleTransientChanged() { - if (getView() == null) { - return; - } - if (mNavigationBarView != null) { - mNavigationBarView.onTransientStateChanged(mTransientShown); - } + mNavigationBarView.onTransientStateChanged(mTransientShown); final int barMode = barMode(mTransientShown, mAppearance); if (updateBarMode(barMode) && mLightBarController != null) { mLightBarController.onNavigationBarModeChanged(barMode); @@ -1092,9 +1062,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, | StatusBarManager.DISABLE_SEARCH); if (masked != mDisabledFlags1) { mDisabledFlags1 = masked; - if (mNavigationBarView != null) { - mNavigationBarView.setDisabledFlags(state1); - } + mNavigationBarView.setDisabledFlags(state1); updateScreenPinningGestures(); } @@ -1110,17 +1078,13 @@ public class NavigationBar implements View.OnAttachStateChangeListener, private void setDisabled2Flags(int state2) { // Method only called on change of disable2 flags - if (mNavigationBarView != null) { - mNavigationBarView.getRotationButtonController().onDisable2FlagChanged(state2); - } + mNavigationBarView.getRotationButtonController().onDisable2FlagChanged(state2); } // ----- Internal stuff ----- private void refreshLayout(int layoutDirection) { - if (mNavigationBarView != null) { - mNavigationBarView.setLayoutDirection(layoutDirection); - } + mNavigationBarView.setLayoutDirection(layoutDirection); } private boolean shouldDisableNavbarGestures() { @@ -1129,7 +1093,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, } private void repositionNavigationBar() { - if (mNavigationBarView == null || !mNavigationBarView.isAttachedToWindow()) return; + if (!mNavigationBarView.isAttachedToWindow()) return; prepareNavigationBarView(); @@ -1138,10 +1102,6 @@ public class NavigationBar implements View.OnAttachStateChangeListener, } private void updateScreenPinningGestures() { - if (mNavigationBarView == null) { - return; - } - // Change the cancel pin gesture to home and back if recents button is invisible boolean pinningActive = ActivityManagerWrapper.getInstance().isScreenPinningActive(); ButtonDispatcher backButton = mNavigationBarView.getBackButton(); @@ -1252,10 +1212,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, AssistManager.INVOCATION_TYPE_HOME_BUTTON_LONG_PRESS); mAssistManagerLazy.get().startAssist(args); mStatusBarLazy.get().awakenDreams(); - - if (mNavigationBarView != null) { - mNavigationBarView.abortCurrentGesture(); - } + mNavigationBarView.abortCurrentGesture(); return true; } @@ -1405,9 +1362,6 @@ public class NavigationBar implements View.OnAttachStateChangeListener, } void updateAccessibilityServicesState(AccessibilityManager accessibilityManager) { - if (mNavigationBarView == null) { - return; - } boolean[] feedbackEnabled = new boolean[1]; int a11yFlags = getA11yButtonState(feedbackEnabled); @@ -1593,7 +1547,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, public void disableAnimationsDuringHide(long delay) { mNavigationBarView.setLayoutTransitionsEnabled(false); - mNavigationBarView.postDelayed(() -> mNavigationBarView.setLayoutTransitionsEnabled(true), + mHandler.postDelayed(mEnableLayoutTransitions, delay + StackStateAnimator.ANIMATION_DURATION_GO_TO_FULL_SHADE); } @@ -1608,10 +1562,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, } public NavigationBarTransitions getBarTransitions() { - if (mNavigationBarView != null) { - return mNavigationBarView.getBarTransitions(); - } - return null; + return mNavigationBarView.getBarTransitions(); } public void finishBarAnimations() { @@ -1626,8 +1577,7 @@ public class NavigationBar implements View.OnAttachStateChangeListener, } private final Consumer mRotationWatcher = rotation -> { - if (mNavigationBarView != null - && mNavigationBarView.needsReorient(rotation)) { + if (mNavigationBarView.needsReorient(rotation)) { repositionNavigationBar(); } }; @@ -1635,9 +1585,6 @@ public class NavigationBar implements View.OnAttachStateChangeListener, private final BroadcastReceiver mBroadcastReceiver = new BroadcastReceiver() { @Override public void onReceive(Context context, Intent intent) { - if (mNavigationBarView == null) { - return; - } String action = intent.getAction(); if (Intent.ACTION_SCREEN_OFF.equals(action) || Intent.ACTION_SCREEN_ON.equals(action)) { diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java index ae9b5987bac51..f5b7c38802ddb 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java @@ -225,10 +225,7 @@ public class NavigationBarController implements Callbacks, if (navBar == null) { continue; } - NavigationBarView view = (NavigationBarView) navBar.getView(); - if (view != null) { - view.updateStates(); - } + navBar.getView().updateStates(); } }); } @@ -366,13 +363,12 @@ public class NavigationBarController implements Callbacks, mHandler, mNavBarOverlayController, mUiEventLogger); + mNavigationBars.put(displayId, navBar); View navigationBarView = navBar.createView(savedState); navigationBarView.addOnAttachStateChangeListener(new View.OnAttachStateChangeListener() { @Override public void onViewAttachedToWindow(View v) { - mNavigationBars.put(displayId, navBar); - if (result != null) { navBar.setImeWindowStatus(display.getDisplayId(), result.mImeToken, result.mImeWindowVis, result.mImeBackDisposition, @@ -448,7 +444,7 @@ public class NavigationBarController implements Callbacks, */ public @Nullable NavigationBarView getNavigationBarView(int displayId) { NavigationBar navBar = mNavigationBars.get(displayId); - return (navBar == null) ? null : (NavigationBarView) navBar.getView(); + return (navBar == null) ? null : navBar.getView(); } /** @return {@link NavigationBar} on the default display. */ diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/buttons/ButtonDispatcher.java b/packages/SystemUI/src/com/android/systemui/navigationbar/buttons/ButtonDispatcher.java index 4d250797f9447..02180160fc11d 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/buttons/ButtonDispatcher.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/buttons/ButtonDispatcher.java @@ -40,7 +40,6 @@ public class ButtonDispatcher { private final ArrayList mViews = new ArrayList<>(); private final int mId; - private final AssistManager mAssistManager; private View.OnClickListener mClickListener; private View.OnTouchListener mTouchListener; @@ -73,7 +72,6 @@ public class ButtonDispatcher { public ButtonDispatcher(int id) { mId = id; - mAssistManager = Dependency.get(AssistManager.class); } public void clear() {