From c5c1eb4d50c39700b1ccdd1de4837b524ada5cf7 Mon Sep 17 00:00:00 2001 From: Roy Chou Date: Mon, 17 Apr 2023 14:08:30 +0000 Subject: [PATCH] chore(#MagnificationThumbnail): add feature flag file and fix size not correct sometimes We add MagnificationThumbnailFeatureFlag to encapsulates the feature flag from directly access in FullScreenMagnificationController. MagnificationThumbnailFeatureFlag is extended from MagnificationFeatureFlagBase so we can clear binder identity before access the flag. Besides, FullScreenMagnificationController can observe feature flag flips and create/destroy the thumbnail. Originally the thumbnail size would be huge sometimes. It's because the thumbnail layout size is not updated properly based on the magnification bounds before it shows on screen. Therefore, we call refreshThumbnail to update the size when the thumbnail is just created in FullScreenMagnificationController. Bug: 276979625 Bug: 276314641 Test: manually test with adb to flip the flag atest FullScreenMagnificationControllerTest Change-Id: I1480b73454ad32f2a294bace56a285ae462119d8 --- .../FullScreenMagnificationController.java | 98 +++++++++++++------ .../MagnificationThumbnailFeatureFlag.java | 46 +++++++++ ...FullScreenMagnificationControllerTest.java | 5 +- 3 files changed, 118 insertions(+), 31 deletions(-) create mode 100644 services/accessibility/java/com/android/server/accessibility/magnification/MagnificationThumbnailFeatureFlag.java diff --git a/services/accessibility/java/com/android/server/accessibility/magnification/FullScreenMagnificationController.java b/services/accessibility/java/com/android/server/accessibility/magnification/FullScreenMagnificationController.java index ed8a35f45176e..e78a9fd7ad0cd 100644 --- a/services/accessibility/java/com/android/server/accessibility/magnification/FullScreenMagnificationController.java +++ b/services/accessibility/java/com/android/server/accessibility/magnification/FullScreenMagnificationController.java @@ -38,7 +38,6 @@ import android.graphics.Region; import android.hardware.display.DisplayManagerInternal; import android.os.Handler; import android.os.Message; -import android.provider.DeviceConfig; import android.text.TextUtils; import android.util.DisplayMetrics; import android.util.MathUtils; @@ -57,6 +56,7 @@ import com.android.internal.R; import com.android.internal.accessibility.common.MagnificationConstants; import com.android.internal.annotations.GuardedBy; import com.android.internal.annotations.VisibleForTesting; +import com.android.internal.util.ConcurrentUtils; import com.android.internal.util.function.pooled.PooledLambda; import com.android.server.LocalServices; import com.android.server.accessibility.AccessibilityManagerService; @@ -110,6 +110,7 @@ public class FullScreenMagnificationController implements private boolean mAlwaysOnMagnificationEnabled = false; private final DisplayManagerInternal mDisplayManagerInternal; + private final MagnificationThumbnailFeatureFlag mMagnificationThumbnailFeatureFlag; @NonNull private final Supplier mThumbnailSupplier; /** @@ -177,9 +178,7 @@ public class FullScreenMagnificationController implements mDisplayId, mMagnificationRegion); mMagnificationRegion.getBounds(mMagnificationBounds); - if (mMagnificationThumbnail == null) { - mMagnificationThumbnail = mThumbnailSupplier.get(); - } + createThumbnailIfSupported(); return true; } @@ -207,7 +206,7 @@ public class FullScreenMagnificationController implements mRegistered = false; unregisterCallbackLocked(mDisplayId, delete); - destroyThumbNail(); + destroyThumbnail(); } mUnregisterPending = false; } @@ -345,7 +344,7 @@ public class FullScreenMagnificationController implements mMagnificationRegion.set(magnified); mMagnificationRegion.getBounds(mMagnificationBounds); - refreshThumbNail(getScale(), getCenterX(), getCenterY()); + refreshThumbnail(getScale(), getCenterX(), getCenterY()); // It's possible that our magnification spec is invalid with the new bounds. // Adjust the current spec's offsets if necessary. @@ -405,9 +404,9 @@ public class FullScreenMagnificationController implements } if (isActivated()) { - updateThumbNail(scale, centerX, centerY); + updateThumbnail(scale, centerX, centerY); } else { - hideThumbNail(); + hideThumbnail(); } } @@ -538,7 +537,7 @@ public class FullScreenMagnificationController implements mIdOfLastServiceToMagnify = INVALID_SERVICE_ID; sendSpecToAnimation(spec, animationCallback); - hideThumbNail(); + hideThumbnail(); return changed; } @@ -596,14 +595,14 @@ public class FullScreenMagnificationController implements } @GuardedBy("mLock") - void updateThumbNail(float scale, float centerX, float centerY) { + void updateThumbnail(float scale, float centerX, float centerY) { if (mMagnificationThumbnail != null) { mMagnificationThumbnail.updateThumbNail(scale, centerX, centerY); } } @GuardedBy("mLock") - void refreshThumbNail(float scale, float centerX, float centerY) { + void refreshThumbnail(float scale, float centerX, float centerY) { if (mMagnificationThumbnail != null) { mMagnificationThumbnail.setThumbNailBounds( mMagnificationBounds, @@ -615,20 +614,38 @@ public class FullScreenMagnificationController implements } @GuardedBy("mLock") - void hideThumbNail() { + void hideThumbnail() { if (mMagnificationThumbnail != null) { mMagnificationThumbnail.hideThumbNail(); } } @GuardedBy("mLock") - void destroyThumbNail() { + void createThumbnailIfSupported() { + if (mMagnificationThumbnail == null) { + mMagnificationThumbnail = mThumbnailSupplier.get(); + // We call refreshThumbnail when the thumbnail is just created to set current + // magnification bounds to thumbnail. It to prevent the thumbnail size has not yet + // updated properly and thus shows with huge size. (b/276314641) + refreshThumbnail(getScale(), getCenterX(), getCenterY()); + } + } + + @GuardedBy("mLock") + void destroyThumbnail() { if (mMagnificationThumbnail != null) { - hideThumbNail(); + hideThumbnail(); mMagnificationThumbnail = null; } } + void onThumbnailFeatureFlagChanged() { + synchronized (mLock) { + destroyThumbnail(); + createThumbnailIfSupported(); + } + } + /** * Updates the current magnification spec. * @@ -768,20 +785,7 @@ public class FullScreenMagnificationController implements lock, magnificationInfoChangedCallback, scaleProvider, - () -> { - if (DeviceConfig.getBoolean( - DeviceConfig.NAMESPACE_ACCESSIBILITY, - "enable_magnifier_thumbnail", - /* defaultValue= */ false)) { - return new MagnificationThumbnail( - context, - context.getSystemService(WindowManager.class), - new Handler(context.getMainLooper()) - ); - } - - return null; - }); + /* thumbnailSupplier= */ null); } /** Constructor for tests */ @@ -791,7 +795,7 @@ public class FullScreenMagnificationController implements @NonNull Object lock, @NonNull MagnificationInfoChangedCallback magnificationInfoChangedCallback, @NonNull MagnificationScaleProvider scaleProvider, - @NonNull Supplier thumbnailSupplier) { + Supplier thumbnailSupplier) { mControllerCtx = ctx; mLock = lock; mMainThreadId = mControllerCtx.getContext().getMainLooper().getThread().getId(); @@ -799,7 +803,41 @@ public class FullScreenMagnificationController implements addInfoChangedCallback(magnificationInfoChangedCallback); mScaleProvider = scaleProvider; mDisplayManagerInternal = LocalServices.getService(DisplayManagerInternal.class); - mThumbnailSupplier = thumbnailSupplier; + mMagnificationThumbnailFeatureFlag = new MagnificationThumbnailFeatureFlag(); + mMagnificationThumbnailFeatureFlag.addOnChangedListener( + ConcurrentUtils.DIRECT_EXECUTOR, this::onMagnificationThumbnailFeatureFlagChanged); + if (thumbnailSupplier != null) { + mThumbnailSupplier = thumbnailSupplier; + } else { + mThumbnailSupplier = () -> { + if (mMagnificationThumbnailFeatureFlag.isFeatureFlagEnabled()) { + return new MagnificationThumbnail( + ctx.getContext(), + ctx.getContext().getSystemService(WindowManager.class), + new Handler(ctx.getContext().getMainLooper()) + ); + } + return null; + }; + } + } + + private void onMagnificationThumbnailFeatureFlagChanged() { + synchronized (mLock) { + for (int i = 0; i < mDisplays.size(); i++) { + onMagnificationThumbnailFeatureFlagChanged(mDisplays.keyAt(i)); + } + } + } + + private void onMagnificationThumbnailFeatureFlagChanged(int displayId) { + synchronized (mLock) { + final DisplayMagnification display = mDisplays.get(displayId); + if (display == null) { + return; + } + display.onThumbnailFeatureFlagChanged(); + } } /** diff --git a/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationThumbnailFeatureFlag.java b/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationThumbnailFeatureFlag.java new file mode 100644 index 0000000000000..519f31b86f78a --- /dev/null +++ b/services/accessibility/java/com/android/server/accessibility/magnification/MagnificationThumbnailFeatureFlag.java @@ -0,0 +1,46 @@ +/* + * Copyright (C) 2023 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.server.accessibility.magnification; + +import android.provider.DeviceConfig; + +/** + * Encapsulates the feature flags for magnification thumbnail. {@see DeviceConfig} + * + * @hide + */ +public class MagnificationThumbnailFeatureFlag extends MagnificationFeatureFlagBase { + + private static final String NAMESPACE = DeviceConfig.NAMESPACE_ACCESSIBILITY; + private static final String FEATURE_NAME_ENABLE_MAGNIFIER_THUMBNAIL = + "enable_magnifier_thumbnail"; + + @Override + String getNamespace() { + return NAMESPACE; + } + + @Override + String getFeatureName() { + return FEATURE_NAME_ENABLE_MAGNIFIER_THUMBNAIL; + } + + @Override + boolean getDefaultValue() { + return false; + } +} diff --git a/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationControllerTest.java b/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationControllerTest.java index f1ad577fde885..35fc40cb60a69 100644 --- a/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/accessibility/magnification/FullScreenMagnificationControllerTest.java @@ -538,7 +538,10 @@ public class FullScreenMagnificationControllerTest { mConfigCaptor.capture()); assertConfigEquals(config, mConfigCaptor.getValue()); - verify(mMockThumbnail).setThumbNailBounds(any(), anyFloat(), anyFloat(), anyFloat()); + // The first time is triggered when the thumbnail is just created. + // The second time is triggered when the magnification region changed. + verify(mMockThumbnail, times(2)).setThumbNailBounds( + any(), anyFloat(), anyFloat(), anyFloat()); } @Test