From 3aafe19994584c293db4a764f2bda6594d2d9d89 Mon Sep 17 00:00:00 2001 From: Bill Lin Date: Fri, 2 Jul 2021 22:13:19 +0800 Subject: [PATCH] Improve SystemUIBootTiming performance in OneHandedController Fix race condition in SystemUIBootTiming issue. 1. Avoid updateOneHandedEnabled() called twice during init a. [O] come from setOneHandedEnabled() b. [X] come from setSwipeToNotificationEnabled() 2. Passively waiting shortcut enabled changed callback, and then query setting ACCESSIBILITY_BUTTON_TARGETS status. 3. Add a new flag isShortcutEnabled to cache shortcut enabled state. 4. By new OHM Settings UX design setSwipeToNotificationEnabled() should NOT setEnabledGesturalOverlay(). 5. Remove redundant auto enabled Toast and print in log instead. (In general case, user is not able to see the Toast) Test: atest google/perf/boottime/boottime-test Test: atest SystemUITests Test: atest WMShellTests Test: manual factory reboot, OHM should disabled by default Bug: 192612924 Change-Id: I8b24c99bc8bd5bfb17c73a6bb8494109d9a8170d --- .../shell/onehanded/OneHandedController.java | 44 +++++++++---------- .../onehanded/OneHandedSettingsUtil.java | 3 +- .../onehanded/OneHandedControllerTest.java | 8 ++-- 3 files changed, 28 insertions(+), 27 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedController.java index 1cc7ed3afcf74..54b81ad782564 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedController.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedController.java @@ -43,7 +43,6 @@ import android.util.Slog; import android.view.Surface; import android.view.WindowManager; import android.view.accessibility.AccessibilityManager; -import android.widget.Toast; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -79,6 +78,7 @@ public class OneHandedController implements RemoteCallable private volatile boolean mIsOneHandedEnabled; private volatile boolean mIsSwipeToNotificationEnabled; + private boolean mIsShortcutEnabled; private boolean mTaskChangeToExit; private boolean mLockedDisabled; private boolean mKeyguardShowing; @@ -143,6 +143,7 @@ public class OneHandedController implements RemoteCallable private final ContentObserver mTimeoutObserver; private final ContentObserver mTaskChangeExitObserver; private final ContentObserver mSwipeToNotificationEnabledObserver; + private final ContentObserver mShortcutEnabledObserver; private AccessibilityManager.AccessibilityStateChangeListener mAccessibilityStateChangeListener = @@ -174,13 +175,13 @@ public class OneHandedController implements RemoteCallable @Override public void onStartFinished(Rect bounds) { mState.setState(STATE_ACTIVE); - notifyShortcutState(STATE_ACTIVE); + notifyShortcutStateChanged(STATE_ACTIVE); } @Override public void onStopFinished(Rect bounds) { mState.setState(STATE_NONE); - notifyShortcutState(STATE_NONE); + notifyShortcutStateChanged(STATE_NONE); } }; @@ -295,6 +296,7 @@ public class OneHandedController implements RemoteCallable mTaskChangeExitObserver = getObserver(this::onTaskChangeExitSettingChanged); mSwipeToNotificationEnabledObserver = getObserver(this::onSwipeToNotificationEnabledChanged); + mShortcutEnabledObserver = getObserver(this::onShortcutEnabledChanged); mDisplayController.addDisplayChangingController(mRotationController); setupCallback(); @@ -349,11 +351,13 @@ public class OneHandedController implements RemoteCallable */ void setSwipeToNotificationEnabled(boolean enabled) { mIsSwipeToNotificationEnabled = enabled; - updateOneHandedEnabled(); } @VisibleForTesting - void notifyShortcutState(@OneHandedState.State int state) { + void notifyShortcutStateChanged(@OneHandedState.State int state) { + if (!isShortcutEnabled()) { + return; + } mOneHandedSettingsUtil.setOneHandedModeActivated( mContext.getContentResolver(), state == STATE_ACTIVE ? 1 : 0, mUserId); } @@ -443,6 +447,9 @@ public class OneHandedController implements RemoteCallable mOneHandedSettingsUtil.registerSettingsKeyObserver( Settings.Secure.SWIPE_BOTTOM_TO_NOTIFICATION_ENABLED, mContext.getContentResolver(), mSwipeToNotificationEnabledObserver, newUserId); + mOneHandedSettingsUtil.registerSettingsKeyObserver( + Settings.Secure.ACCESSIBILITY_BUTTON_TARGETS, + mContext.getContentResolver(), mShortcutEnabledObserver, newUserId); } private void unregisterSettingObservers() { @@ -454,6 +461,8 @@ public class OneHandedController implements RemoteCallable mTaskChangeExitObserver); mOneHandedSettingsUtil.unregisterSettingsKeyObserver(mContext.getContentResolver(), mSwipeToNotificationEnabledObserver); + mOneHandedSettingsUtil.unregisterSettingsKeyObserver(mContext.getContentResolver(), + mShortcutEnabledObserver); } private void updateSettings() { @@ -489,15 +498,6 @@ public class OneHandedController implements RemoteCallable } } - @VisibleForTesting - void notifyUserConfigChanged(boolean success) { - if (!success) { - return; - } - // TODO Check UX if popup Toast to notify user when auto-enabled one-handed is good option. - Toast.makeText(mContext, R.string.one_handed_tutorial_title, Toast.LENGTH_LONG).show(); - } - @VisibleForTesting void onActivatedActionChanged() { if (!isShortcutEnabled()) { @@ -508,7 +508,7 @@ public class OneHandedController implements RemoteCallable if (!isOneHandedEnabled()) { final boolean success = mOneHandedSettingsUtil.setOneHandedModeEnabled( mContext.getContentResolver(), 1 /* Enabled for shortcut */, mUserId); - notifyUserConfigChanged(success); + Slog.d(TAG, "Auto enabled One-handed mode by shortcut trigger, success=" + success); } if (isSwipeToNotificationEnabled()) { @@ -596,11 +596,11 @@ public class OneHandedController implements RemoteCallable mOneHandedUiEventLogger.writeEvent(enabled ? OneHandedUiEventLogger.EVENT_ONE_HANDED_SETTINGS_SHOW_NOTIFICATION_ENABLED_ON : OneHandedUiEventLogger.EVENT_ONE_HANDED_SETTINGS_SHOW_NOTIFICATION_ENABLED_OFF); + } - // Also checks one handed mode settings since they all need gesture overlay. - setEnabledGesturalOverlay( - enabled || mOneHandedSettingsUtil.getSettingsOneHandedModeEnabled( - mContext.getContentResolver(), mUserId), true /* DelayExecute */); + void onShortcutEnabledChanged() { + mIsShortcutEnabled = mOneHandedSettingsUtil.getShortcutEnabled( + mContext.getContentResolver(), mUserId); } private void setupTimeoutListener() { @@ -620,7 +620,7 @@ public class OneHandedController implements RemoteCallable @VisibleForTesting boolean isShortcutEnabled() { - return mOneHandedSettingsUtil.getShortcutEnabled(mContext.getContentResolver(), mUserId); + return mIsShortcutEnabled; } @VisibleForTesting @@ -634,9 +634,9 @@ public class OneHandedController implements RemoteCallable } // If setting is pull screen, notify shortcut one_handed_mode_activated to reset - // and align status with current mState when function enabled. + // and align status with current mState when one-handed gesture enabled. if (isOneHandedEnabled() && !isSwipeToNotificationEnabled()) { - notifyShortcutState(mState.getState()); + notifyShortcutStateChanged(mState.getState()); } mTouchHandler.onOneHandedEnabled(mIsOneHandedEnabled); diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedSettingsUtil.java b/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedSettingsUtil.java index 3baa69f0033a7..5911f8d66b324 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedSettingsUtil.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/onehanded/OneHandedSettingsUtil.java @@ -23,6 +23,7 @@ import android.content.ContentResolver; import android.database.ContentObserver; import android.net.Uri; import android.provider.Settings; +import android.text.TextUtils; import androidx.annotation.Nullable; @@ -170,7 +171,7 @@ public final class OneHandedSettingsUtil { public boolean getShortcutEnabled(ContentResolver resolver, int userId) { final String targets = Settings.Secure.getStringForUser(resolver, Settings.Secure.ACCESSIBILITY_BUTTON_TARGETS, userId); - return targets != null ? targets.contains(ONE_HANDED_MODE_TARGET_NAME) : false; + return TextUtils.isEmpty(targets) ? false : targets.contains(ONE_HANDED_MODE_TARGET_NAME); } /** diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedControllerTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedControllerTest.java index be786fb55b30e..9ec7d304d520d 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedControllerTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/onehanded/OneHandedControllerTest.java @@ -26,6 +26,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.never; import static org.mockito.Mockito.spy; @@ -406,7 +407,7 @@ public class OneHandedControllerTest extends OneHandedTestCase { false); mSpiedOneHandedController.onActivatedActionChanged(); - verify(mSpiedOneHandedController).notifyUserConfigChanged(anyBoolean()); + verify(mMockSettingsUitl).setOneHandedModeEnabled(any(), eq(1), anyInt()); } @Test @@ -441,7 +442,7 @@ public class OneHandedControllerTest extends OneHandedTestCase { mSpiedOneHandedController.registerEventCallback(mMockEventCallback); mSpiedOneHandedController.setOneHandedEnabled(true); - verify(mSpiedOneHandedController).notifyShortcutState(anyInt()); + verify(mSpiedOneHandedController).notifyShortcutStateChanged(anyInt()); } @Test @@ -468,7 +469,7 @@ public class OneHandedControllerTest extends OneHandedTestCase { false /* To avoid test runner create Toast */); mSpiedOneHandedController.onActivatedActionChanged(); - verify(mSpiedOneHandedController).notifyUserConfigChanged(anyBoolean()); + verify(mMockSettingsUitl).setOneHandedModeEnabled(any(), eq(1), anyInt()); } @Test @@ -481,6 +482,5 @@ public class OneHandedControllerTest extends OneHandedTestCase { mSpiedOneHandedController.onActivatedActionChanged(); verify(mMockSettingsUitl, never()).setOneHandedModeEnabled(any(), anyInt(), anyInt()); - verify(mSpiedOneHandedController, never()).notifyUserConfigChanged(anyBoolean()); } }