Skip to content

Commit

Permalink
[Canary branch 6063] [Auto-disable Accessibility] Fix a11y recreate i…
Browse files Browse the repository at this point in the history
…ssue with ToolbarManager

This CL adds a temporary solution to the infinite recreate issue that is
blocking the M118 release.

With this CL we add new methods for checking the AccessibilityState and
whether or not any gesture performing or touch exploration accessibility
services are enabled. These methods will ignore any caching and fetch a
fresh value. This will have a negative performance impact, so we minimize
this to the ToolbarManager related code by added duplicate methods where
required to create a separate code path. No other clients of the
ReturnToChromeUtil should be affected by this change.

(cherry picked from commit 320569b)

Change-Id: I34dd89ddeee2c327ba2367c920907b0a0537434e
Bug: 1491862
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4936730
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Commit-Queue: Mark Schillaci <mschillaci@google.com>
Reviewed-by: Theresa Sullivan <twellington@chromium.org>
Reviewed-by: Xi Han <hanxi@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1209053}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4937236
Reviewed-by: Krishna Govind <govind@chromium.org>
Owners-Override: Krishna Govind <govind@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/6063@{#6}
Cr-Branched-From: cc84fa1-refs/heads/main@{#1208799}
  • Loading branch information
mschillaci authored and Krishna Govind committed Oct 12, 2023
1 parent 2b9e1be commit 369471a
Show file tree
Hide file tree
Showing 4 changed files with 70 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -541,6 +541,20 @@ public static boolean isStartSurfaceEnabled(Context context) {
&& !DeviceFormFactor.isNonMultiDisplayContextOnTablet(context);
}

// TODO(mschillaci): Remove this was recreate issue is resolved. See crbug.com/1491862.
/** See {@link isStartSurfaceEnabled} */
@Deprecated
public static boolean isStartSurfaceEnabled_ForToolbarManager(Context context) {
// When creating initial tab, i.e. cold start without restored tabs, we should only show
// StartSurface as the HomePage if Single Pane is enabled, HomePage is not customized, not
// on tablet, accessibility is not enabled or the tab group continuation feature is enabled.
return (!DseNewTabUrlManager.isNewTabSearchEngineUrlAndroidEnabled()
|| DseNewTabUrlManager.isDefaultSearchEngineGoogle())
&& StartSurfaceConfiguration.isStartSurfaceFlagEnabled()
&& !shouldHideStartSurfaceWithAccessibilityOn_ForToolbarManager(context)
&& !DeviceFormFactor.isNonMultiDisplayContextOnTablet(context);
}

/**
* @return Whether start surface should be hidden when accessibility is enabled. If it's true,
* NTP is shown as homepage. Also, when time threshold is reached, grid tab switcher or
Expand All @@ -552,6 +566,16 @@ public static boolean shouldHideStartSurfaceWithAccessibilityOn(Context context)
&& !(ChromeFeatureList.sStartSurfaceWithAccessibility.isEnabled());
}

// TODO(mschillaci): Remove this when recreate issue is resolved. See crbug.com/1491862.
/** See {@link shouldHideStartSurfaceWithAccessibilityOn} */
@Deprecated
public static boolean shouldHideStartSurfaceWithAccessibilityOn_ForToolbarManager(
Context context) {
// TODO(crbug.com/1127732): Move this method back to StartSurfaceConfiguration.
return ChromeAccessibilityUtil.get().isAccessibilityEnabledHeavy()
&& !(ChromeFeatureList.sStartSurfaceWithAccessibility.isEnabled());
}

/**
* @param tabModelSelector The tab model selector.
* @return the total tab count, and works before native initialization.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -598,7 +598,8 @@ public void onLowMemory() {}
mCustomTabThemeColorProvider = new SettableThemeColorProvider(/* context= */ mActivity);

mActivityTabProvider = tabProvider;
mIsStartSurfaceEnabled = ReturnToChromeUtil.isStartSurfaceEnabled(mActivity);
mIsStartSurfaceEnabled =
ReturnToChromeUtil.isStartSurfaceEnabled_ForToolbarManager(mActivity);
mIsStartSurfaceRefactorEnabled =
ReturnToChromeUtil.isStartSurfaceRefactorEnabled(mActivity);

Expand Down Expand Up @@ -663,7 +664,7 @@ public void onLowMemory() {}
// logo click events are processed in NewTabPageLayout. This callback passed
// into TopToolbarCoordinator will only be used for StartSurfaceToolbar, so add
// an assertion here.
assert ReturnToChromeUtil.isStartSurfaceEnabled(mActivity);
assert ReturnToChromeUtil.isStartSurfaceEnabled_ForToolbarManager(mActivity);
ReturnToChromeUtil.handleLoadUrlFromStartSurface(urlParams,
/*isBackground=*/false,
/*incognito=*/false, startSurfaceParentTabSupplier.get());
Expand Down Expand Up @@ -1112,7 +1113,7 @@ public void onOverlayPanelHidden() {
mLocationBarModel.setStartSurfaceState(mStartSurfaceState);
if (!mIsStartSurfaceRefactorEnabled) {
mStartSurfaceStateObserver = (newState, shouldShowToolbar) -> {
assert ReturnToChromeUtil.isStartSurfaceEnabled(mActivity);
assert ReturnToChromeUtil.isStartSurfaceEnabled_ForToolbarManager(mActivity);
mStartSurfaceState = newState;
mLocationBarModel.setStartSurfaceState(mStartSurfaceState);
mToolbar.updateStartSurfaceToolbarState(newState, shouldShowToolbar, null);
Expand Down Expand Up @@ -1299,7 +1300,7 @@ public boolean isLocationBarShown() {
&& mLayoutStateProvider.getActiveLayoutType() == LayoutType.START_SURFACE) {
return false;
} else if (mStartSurfaceState == StartSurfaceState.SHOWN_HOMEPAGE
&& ReturnToChromeUtil.isStartSurfaceEnabled(mActivity)) {
&& mIsStartSurfaceEnabled) {
return false;
}

Expand Down Expand Up @@ -1749,10 +1750,14 @@ private void onOrientationChange(int newOrientation) {

@Override
public void onAccessibilityModeChanged(boolean enabled) {
if (mIsStartSurfaceEnabled != ReturnToChromeUtil.isStartSurfaceEnabled(mActivity)
&& !sSkipRecreateForTesting && !mIsCustomTab) {
if (mIsStartSurfaceEnabled
!= ReturnToChromeUtil.isStartSurfaceEnabled_ForToolbarManager(mActivity)
&& !sSkipRecreateForTesting
&& !mIsCustomTab) {
// If Start surface is disabled or re-enabled due to the accessibility change, restarts
// the activity to create the correct Toolbar from scratch.
// Note: Do not use the |enabled| value passes here, since it may disagree with the
// value from ReturnToChromeUtil.
recreateActivityWithTabReparenting();
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ public boolean isAccessibilityEnabled() {
return AccessibilityState.isAccessibilityEnabled();
}

@Deprecated
public boolean isAccessibilityEnabledHeavy() {
return AccessibilityState.isAccessibilityEnabledHeavy();
}

@Override
public void setAccessibilityEnabledForTesting(@Nullable Boolean isEnabled) {
AccessibilityState.setIsPerformGesturesEnabledForTesting(Boolean.TRUE.equals(isEnabled));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,36 +260,50 @@ public static boolean isScreenReaderEnabled() {
*/
public static boolean isTouchExplorationEnabled() {
if (!sInitialized) {
fetchAccessibilityManager();
return sAccessibilityManager.isTouchExplorationEnabled();
return isTouchExplorationEnabledHeavy();
}
return sState.isTouchExplorationEnabled;
}

@Deprecated
public static boolean isTouchExplorationEnabledHeavy() {
fetchAccessibilityManager();
return sAccessibilityManager.isTouchExplorationEnabled();
}

public static boolean isPerformGesturesEnabled() {
if (!sInitialized) {
if (sPreInitCachedValuePerformGesturesEnabled != null) {
return sPreInitCachedValuePerformGesturesEnabled;
}

fetchAccessibilityManager();
if (sAccessibilityManager.isEnabled()) {
for (AccessibilityServiceInfo service :
sAccessibilityManager.getEnabledAccessibilityServiceList(
AccessibilityServiceInfo.FEEDBACK_ALL_MASK)) {
if ((service.getCapabilities()
& AccessibilityServiceInfo.CAPABILITY_CAN_PERFORM_GESTURES)
!= 0) {
sPreInitCachedValuePerformGesturesEnabled = true;
return true;
}
sPreInitCachedValuePerformGesturesEnabled = isPerformGesturesEnabledHeavy();
return sPreInitCachedValuePerformGesturesEnabled;
}

return sState.isPerformGesturesEnabled;
}

@Deprecated
public static boolean isPerformGesturesEnabledHeavy() {
// TODO(mschillaci): Remove when recreate issue is resolved. See crbug.com/1491862.
fetchAccessibilityManager();
if (sAccessibilityManager.isEnabled()) {
for (AccessibilityServiceInfo service :
sAccessibilityManager.getEnabledAccessibilityServiceList(
AccessibilityServiceInfo.FEEDBACK_ALL_MASK)) {
if (0 != (service.getCapabilities() & CAPABILITY_CAN_PERFORM_GESTURES)) {
return true;
}
}
sPreInitCachedValuePerformGesturesEnabled = false;
return false;
}
return false;
}

return sState.isPerformGesturesEnabled;
@Deprecated
public static boolean isAccessibilityEnabledHeavy() {
return AccessibilityState.isTouchExplorationEnabledHeavy()
|| AccessibilityState.isPerformGesturesEnabledHeavy();
}

/**
Expand Down

0 comments on commit 369471a

Please sign in to comment.