From 4dc4b842bdd10e54898bd9f267309baac3bca4e4 Mon Sep 17 00:00:00 2001 From: Tyler Freeman Date: Tue, 23 May 2023 10:11:51 -0700 Subject: [PATCH 1/3] chore(magnification settings): add logWithPosition() to log things like which magnification size was tapped. Also from which magnifier the log panel opens (fullscreen or window). Bug: 281241589 Test: atest frameworks/base/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java && aster-previz Change-Id: I3d55c2c10507c5d6996d5744dc4339fabc54e29c --- .../accessibility/AccessibilityLogger.kt | 27 ++++++++++++++++--- .../accessibility/WindowMagnification.java | 27 ++++++++++++++----- .../WindowMagnificationTest.java | 14 +++++++--- 3 files changed, 54 insertions(+), 14 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt b/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt index 7b915961c046b..067f308ba8c4d 100644 --- a/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt @@ -18,6 +18,7 @@ package com.android.systemui.accessibility import com.android.internal.logging.UiEvent import com.android.internal.logging.UiEventLogger +import com.android.internal.logging.UiEventLogger.UiEventEnum import javax.inject.Inject /** @@ -27,14 +28,28 @@ import javax.inject.Inject */ class AccessibilityLogger @Inject constructor(private val uiEventLogger: UiEventLogger) { /** Logs the given event */ - fun log(event: UiEventLogger.UiEventEnum) { + fun log(event: UiEventEnum) { uiEventLogger.log(event) } + /** + * Logs the given event with an integer rank/position value. + * + * @param event the event to log + * @param position the rank or position value that the user interacted with in the UI + */ + fun logWithPosition(event: UiEventEnum, position: Int) { + uiEventLogger.logWithPosition(event, /* uid= */ 0, /* packageName= */ null, position) + } + /** Events regarding interaction with the magnifier settings panel */ enum class MagnificationSettingsEvent constructor(private val id: Int) : - UiEventLogger.UiEventEnum { - @UiEvent(doc = "Magnification settings panel opened.") + UiEventEnum { + @UiEvent( + doc = + "Magnification settings panel opened. The selection rank is from which " + + "magnifier mode it was opened (fullscreen or window)" + ) MAGNIFICATION_SETTINGS_PANEL_OPENED(1381), @UiEvent(doc = "Magnification settings panel closed") @@ -46,7 +61,11 @@ class AccessibilityLogger @Inject constructor(private val uiEventLogger: UiEvent @UiEvent(doc = "Magnification settings panel edit size save button clicked") MAGNIFICATION_SETTINGS_SIZE_EDITING_DEACTIVATED(1384), - @UiEvent(doc = "Magnification settings panel window size selected") + @UiEvent( + doc = + "Magnification settings panel window size selected. The selection rank is " + + "which size was selected." + ) MAGNIFICATION_SETTINGS_WINDOW_SIZE_SELECTED(1386); override fun getId(): Int = this.id diff --git a/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnification.java b/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnification.java index 2a14dc894d436..dfe21b5372570 100644 --- a/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnification.java +++ b/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnification.java @@ -16,6 +16,7 @@ package com.android.systemui.accessibility; +import static android.provider.Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN; import static android.provider.Settings.Secure.ACCESSIBILITY_MAGNIFICATION_MODE_WINDOW; import static android.view.WindowManager.LayoutParams.TYPE_ACCESSIBILITY_MAGNIFICATION_OVERLAY; @@ -346,7 +347,10 @@ public class WindowMagnification implements CoreStartable, CommandQueue.Callback @Override public void onSetMagnifierSize(int displayId, int index) { mHandler.post(() -> onSetMagnifierSizeInternal(displayId, index)); - mA11yLogger.log(MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_WINDOW_SIZE_SELECTED); + mA11yLogger.logWithPosition( + MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_WINDOW_SIZE_SELECTED, + index + ); } @Override @@ -377,9 +381,6 @@ public class WindowMagnification implements CoreStartable, CommandQueue.Callback @Override public void onSettingsPanelVisibilityChanged(int displayId, boolean shown) { mHandler.post(() -> onSettingsPanelVisibilityChangedInternal(displayId, shown)); - mA11yLogger.log(shown - ? MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_PANEL_OPENED - : MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_PANEL_CLOSED); } }; @@ -433,8 +434,22 @@ public class WindowMagnification implements CoreStartable, CommandQueue.Callback private void onSettingsPanelVisibilityChangedInternal(int displayId, boolean shown) { final WindowMagnificationController windowMagnificationController = mMagnificationControllerSupplier.get(displayId); - if (windowMagnificationController != null && windowMagnificationController.isActivated()) { - windowMagnificationController.updateDragHandleResourcesIfNeeded(shown); + if (windowMagnificationController != null) { + boolean isWindowMagnifierActivated = windowMagnificationController.isActivated(); + if (isWindowMagnifierActivated) { + windowMagnificationController.updateDragHandleResourcesIfNeeded(shown); + } + + if (shown) { + mA11yLogger.logWithPosition( + MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_PANEL_OPENED, + isWindowMagnifierActivated + ? ACCESSIBILITY_MAGNIFICATION_MODE_WINDOW + : ACCESSIBILITY_MAGNIFICATION_MODE_FULLSCREEN + ); + } else { + mA11yLogger.log(MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_PANEL_CLOSED); + } } } diff --git a/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java b/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java index db580742a68f6..808add6fe744c 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java @@ -117,6 +117,8 @@ public class WindowMagnificationTest extends SysuiTestCase { return null; }).when(mMagnificationSettingsController).closeMagnificationSettings(); + when(mWindowMagnificationController.isActivated()).thenReturn(true); + mCommandQueue = new CommandQueue(getContext(), mDisplayTracker); mWindowMagnification = new WindowMagnification(getContext(), getContext().getMainThreadHandler(), mCommandQueue, mModeSwitchesController, @@ -199,8 +201,10 @@ public class WindowMagnificationTest extends SysuiTestCase { waitForIdleSync(); verify(mMagnificationSettingsController).toggleSettingsPanelVisibility(); - verify(mA11yLogger).log( - eq(MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_PANEL_OPENED)); + verify(mA11yLogger).logWithPosition( + eq(MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_PANEL_OPENED), + eq(ACCESSIBILITY_MAGNIFICATION_MODE_WINDOW) + ); } @Test @@ -211,8 +215,10 @@ public class WindowMagnificationTest extends SysuiTestCase { waitForIdleSync(); verify(mWindowMagnificationController).changeMagnificationSize(eq(index)); - verify(mA11yLogger).log( - eq(MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_WINDOW_SIZE_SELECTED)); + verify(mA11yLogger).logWithPosition( + eq(MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_WINDOW_SIZE_SELECTED), + eq(index) + ); } @Test From b37885c8b075fc5f222db1c146470f5f7cecc094 Mon Sep 17 00:00:00 2001 From: Tyler Freeman Date: Fri, 19 May 2023 15:57:58 -0700 Subject: [PATCH 2/3] feat(magnification settings): add logThrottled() so we only log one scale event when they start moving the slider Bug: 281241589 Test: atest frameworks/base/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java && atest frameworks/base/packages/SystemUI/tests/src/com/android/systemui/accessibility/AccessibilityLoggerTest.java Change-Id: If38609f5317325945e1637903c82f78024a36cdd --- .../accessibility/AccessibilityLogger.kt | 34 +++++++- .../accessibility/AccessibilityLoggerTest.kt | 80 +++++++++++++++++++ 2 files changed, 113 insertions(+), 1 deletion(-) create mode 100644 packages/SystemUI/tests/src/com/android/systemui/accessibility/AccessibilityLoggerTest.kt diff --git a/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt b/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt index 067f308ba8c4d..8eaee0ead937c 100644 --- a/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt @@ -16,9 +16,11 @@ package com.android.systemui.accessibility +import com.android.internal.annotations.GuardedBy import com.android.internal.logging.UiEvent import com.android.internal.logging.UiEventLogger import com.android.internal.logging.UiEventLogger.UiEventEnum +import com.android.systemui.util.time.SystemClock import javax.inject.Inject /** @@ -26,7 +28,37 @@ import javax.inject.Inject * * See go/uievent */ -class AccessibilityLogger @Inject constructor(private val uiEventLogger: UiEventLogger) { +class AccessibilityLogger +@Inject +constructor(private val uiEventLogger: UiEventLogger, private val clock: SystemClock) { + + @GuardedBy("clock") private var lastTimeThrottledMs: Long = 0 + @GuardedBy("clock") private var lastEventThrottled: UiEventEnum? = null + + /** + * Logs the event, but any additional calls within the given delay window are ignored. The + * window resets every time a new event is received. i.e. it will only log one time until you + * wait at least [delayBeforeLoggingMs] before sending the next event. + * + *

Additionally, if a different type of event is passed in, the delay window for the previous + * one is forgotten. e.g. if you send two types of events interlaced all within the delay + * window, e.g. A->B->A within 1000ms, all three will be logged. + */ + @JvmOverloads fun logThrottled(event: UiEventEnum, delayBeforeLoggingMs: Int = 2000) { + synchronized(clock) { + val currentTimeMs = clock.elapsedRealtime() + val shouldThrottle = + event == lastEventThrottled && + currentTimeMs - lastTimeThrottledMs < delayBeforeLoggingMs + lastEventThrottled = event + lastTimeThrottledMs = currentTimeMs + if (shouldThrottle) { + return + } + } + log(event) + } + /** Logs the given event */ fun log(event: UiEventEnum) { uiEventLogger.log(event) diff --git a/packages/SystemUI/tests/src/com/android/systemui/accessibility/AccessibilityLoggerTest.kt b/packages/SystemUI/tests/src/com/android/systemui/accessibility/AccessibilityLoggerTest.kt new file mode 100644 index 0000000000000..deacac39b5875 --- /dev/null +++ b/packages/SystemUI/tests/src/com/android/systemui/accessibility/AccessibilityLoggerTest.kt @@ -0,0 +1,80 @@ +/* + * 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.systemui.accessibility + +import android.testing.AndroidTestingRunner +import androidx.test.filters.SmallTest +import com.android.internal.logging.UiEventLogger +import com.android.systemui.SysuiTestCase +import com.android.systemui.accessibility.AccessibilityLogger.MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_PANEL_CLOSED +import com.android.systemui.accessibility.AccessibilityLogger.MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_PANEL_OPENED +import com.android.systemui.util.time.FakeSystemClock +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.ArgumentMatchers.eq +import org.mockito.Mock +import org.mockito.Mockito.times +import org.mockito.Mockito.verify +import org.mockito.junit.MockitoJUnit + +@SmallTest +@RunWith(AndroidTestingRunner::class) +class AccessibilityLoggerTest : SysuiTestCase() { + @JvmField @Rule val mockito = MockitoJUnit.rule() + + private val fakeClock = FakeSystemClock() + @Mock private lateinit var fakeLogger: UiEventLogger + + private lateinit var a11yLogger: AccessibilityLogger + + @Before + fun setup() { + a11yLogger = AccessibilityLogger(fakeLogger, fakeClock) + } + + @Test + fun logThrottled_onceWithinWindow() { + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_OPENED, 1000) + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_OPENED, 1000) + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_OPENED, 1000) + fakeClock.advanceTime(100L) + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_OPENED, 1000) + fakeClock.advanceTime(900L) + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_OPENED, 1000) + fakeClock.advanceTime(1100L) + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_OPENED, 1000) + + verify(fakeLogger, times(2)).log(eq(MAGNIFICATION_SETTINGS_PANEL_OPENED)) + } + + @Test + fun logThrottled_interlacedLogsAllWithinWindow() { + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_OPENED, 1000) + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_CLOSED, 1000) + fakeClock.advanceTime(100L) + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_CLOSED, 1000) + fakeClock.advanceTime(200L) + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_OPENED, 1000) + fakeClock.advanceTime(1100L) + a11yLogger.logThrottled(MAGNIFICATION_SETTINGS_PANEL_OPENED, 1000) + + verify(fakeLogger, times(3)).log(eq(MAGNIFICATION_SETTINGS_PANEL_OPENED)) + verify(fakeLogger).log(eq(MAGNIFICATION_SETTINGS_PANEL_CLOSED)) + } +} From 365612d95969f33fde39a45ad2800b763719e765 Mon Sep 17 00:00:00 2001 From: Tyler Freeman Date: Fri, 12 May 2023 15:53:29 -0700 Subject: [PATCH 3/3] chore(magnification settings): add logging for zoom slider in settings panel Bug: 281241589 Test: atest frameworks/base/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java && aster-previz Change-Id: Ieeb523ce7dab1712b2d8c1272c7c92be5a223e13 --- .../com/android/systemui/accessibility/AccessibilityLogger.kt | 3 +++ .../android/systemui/accessibility/WindowMagnification.java | 3 +++ .../systemui/accessibility/WindowMagnificationTest.java | 2 ++ 3 files changed, 8 insertions(+) diff --git a/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt b/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt index 8eaee0ead937c..9e3a77802d40c 100644 --- a/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt +++ b/packages/SystemUI/src/com/android/systemui/accessibility/AccessibilityLogger.kt @@ -93,6 +93,9 @@ constructor(private val uiEventLogger: UiEventLogger, private val clock: SystemC @UiEvent(doc = "Magnification settings panel edit size save button clicked") MAGNIFICATION_SETTINGS_SIZE_EDITING_DEACTIVATED(1384), + @UiEvent(doc = "Magnification settings panel zoom slider changed") + MAGNIFICATION_SETTINGS_ZOOM_SLIDER_CHANGED(1385), + @UiEvent( doc = "Magnification settings panel window size selected. The selection rank is " + diff --git a/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnification.java b/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnification.java index dfe21b5372570..8f50af987ab30 100644 --- a/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnification.java +++ b/packages/SystemUI/src/com/android/systemui/accessibility/WindowMagnification.java @@ -371,6 +371,9 @@ public class WindowMagnification implements CoreStartable, CommandQueue.Callback if (mWindowMagnificationConnectionImpl != null) { mWindowMagnificationConnectionImpl.onPerformScaleAction(displayId, scale); } + mA11yLogger.logThrottled( + MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_ZOOM_SLIDER_CHANGED + ); } @Override diff --git a/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java b/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java index 808add6fe744c..104ca6986f18e 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/accessibility/WindowMagnificationTest.java @@ -259,6 +259,8 @@ public class WindowMagnificationTest extends SysuiTestCase { TEST_DISPLAY, scale); verify(mConnectionCallback).onPerformScaleAction(eq(TEST_DISPLAY), eq(scale)); + verify(mA11yLogger).logThrottled( + eq(MagnificationSettingsEvent.MAGNIFICATION_SETTINGS_ZOOM_SLIDER_CHANGED)); } @Test