From f400d32813b21c384668b8955be7d06880602d9d Mon Sep 17 00:00:00 2001
From: Hawkwood Glazier <jglazier@google.com>
Date: Wed, 7 Dec 2022 20:19:11 +0000
Subject: [PATCH] Fix Positioning of large custom clocks within frame

This reverts Iaeddce2d99a2b8db5afb6ac8ca508eeab3ae223e without
reintroducing b/259758161. It does this by compensating for the
change in margin in KeyguardClockSwitchController.getClockBottom().
This maintains existing functionality for the aod behaviour in
KeyguardClockPositionAlgorithm. As part of validation I confirmed
that the arguments to KeyguardClockPositionAlgorithm.setup did not
change with this patch.

Bug: 259758161
Fixes: 261754025
Test: Manually checked KeyguardClockPositionAlgorithm
Change-Id: I119abb49a018f6fb505100ea985d3b4121ac59f7
---
 .../shared/clocks/DefaultClockController.kt   | 11 ++-
 .../layout/keyguard_clock_switch.xml          |  1 -
 .../KeyguardClockSwitchController.java        | 19 ++++-
 .../KeyguardClockSwitchControllerTest.java    | 78 +++++++++++++++----
 4 files changed, 89 insertions(+), 20 deletions(-)

diff --git a/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/DefaultClockController.kt b/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/DefaultClockController.kt
index e1f21742bf93d..869884474ffe3 100644
--- a/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/DefaultClockController.kt
+++ b/packages/SystemUI/customization/src/com/android/systemui/shared/clocks/DefaultClockController.kt
@@ -20,6 +20,7 @@ import android.graphics.Rect
 import android.icu.text.NumberFormat
 import android.util.TypedValue
 import android.view.LayoutInflater
+import android.view.View
 import android.widget.FrameLayout
 import androidx.annotation.VisibleForTesting
 import com.android.systemui.customization.R
@@ -151,9 +152,15 @@ class DefaultClockController(
         view: AnimatableClockView,
     ) : DefaultClockFaceController(view) {
         override fun recomputePadding(targetRegion: Rect?) {
-            // Ignore Target Region until top padding fixed in aod
+            // We center the view within the targetRegion instead of within the parent
+            // view by computing the difference and adding that to the padding.
+            val parent = view.parent
+            val yDiff =
+                if (targetRegion != null && parent is View && parent.isLaidOut())
+                    targetRegion.centerY() - parent.height / 2f
+                else 0f
             val lp = view.getLayoutParams() as FrameLayout.LayoutParams
-            lp.topMargin = (-0.5f * view.bottom).toInt()
+            lp.topMargin = (-0.5f * view.bottom + yDiff).toInt()
             view.setLayoutParams(lp)
         }
 
diff --git a/packages/SystemUI/res-keyguard/layout/keyguard_clock_switch.xml b/packages/SystemUI/res-keyguard/layout/keyguard_clock_switch.xml
index 218c5cc9b7fec..b49afeef09f3b 100644
--- a/packages/SystemUI/res-keyguard/layout/keyguard_clock_switch.xml
+++ b/packages/SystemUI/res-keyguard/layout/keyguard_clock_switch.xml
@@ -35,7 +35,6 @@
         android:visibility="invisible" />
     <FrameLayout
         android:id="@+id/lockscreen_clock_view_large"
-        android:layout_marginTop="@dimen/keyguard_large_clock_top_margin"
         android:layout_width="match_parent"
         android:layout_height="match_parent"
         android:clipChildren="false"
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java
index 789f6218caf9d..788f1200d6030 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java
@@ -78,6 +78,7 @@ public class KeyguardClockSwitchController extends ViewController<KeyguardClockS
     private int mCurrentClockSize = SMALL;
 
     private int mKeyguardSmallClockTopMargin = 0;
+    private int mKeyguardLargeClockTopMargin = 0;
     private final ClockRegistry.ClockChangeListener mClockChangedListener;
 
     private ViewGroup mStatusArea;
@@ -164,6 +165,8 @@ public class KeyguardClockSwitchController extends ViewController<KeyguardClockS
         mClockEventController.registerListeners(mView);
         mKeyguardSmallClockTopMargin =
                 mView.getResources().getDimensionPixelSize(R.dimen.keyguard_clock_top_margin);
+        mKeyguardLargeClockTopMargin =
+                mView.getResources().getDimensionPixelSize(R.dimen.keyguard_large_clock_top_margin);
 
         if (mOnlyClock) {
             View ksv = mView.findViewById(R.id.keyguard_slice_view);
@@ -246,6 +249,8 @@ public class KeyguardClockSwitchController extends ViewController<KeyguardClockS
         mView.onDensityOrFontScaleChanged();
         mKeyguardSmallClockTopMargin =
                 mView.getResources().getDimensionPixelSize(R.dimen.keyguard_clock_top_margin);
+        mKeyguardLargeClockTopMargin =
+                mView.getResources().getDimensionPixelSize(R.dimen.keyguard_large_clock_top_margin);
         mView.updateClockTargetRegions();
     }
 
@@ -324,10 +329,18 @@ public class KeyguardClockSwitchController extends ViewController<KeyguardClockS
         }
 
         if (mLargeClockFrame.getVisibility() == View.VISIBLE) {
+            // This gets the expected clock bottom if mLargeClockFrame had a top margin, but it's
+            // top margin only contributed to height and didn't move the top of the view (as this
+            // was the computation previously). As we no longer have a margin, we add this back
+            // into the computation manually.
             int frameHeight = mLargeClockFrame.getHeight();
             int clockHeight = clock.getLargeClock().getView().getHeight();
-            return frameHeight / 2 + clockHeight / 2;
+            return frameHeight / 2 + clockHeight / 2 + mKeyguardLargeClockTopMargin / -2;
         } else {
+            // This is only called if we've never shown the large clock as the frame is inflated
+            // with 'gone', but then the visibility is never set when it is animated away by
+            // KeyguardClockSwitch, instead it is removed from the view hierarchy.
+            // TODO(b/261755021): Cleanup Large Frame Visibility
             int clockHeight = clock.getSmallClock().getView().getHeight();
             return clockHeight + statusBarHeaderHeight + mKeyguardSmallClockTopMargin;
         }
@@ -345,11 +358,15 @@ public class KeyguardClockSwitchController extends ViewController<KeyguardClockS
         if (mLargeClockFrame.getVisibility() == View.VISIBLE) {
             return clock.getLargeClock().getView().getHeight();
         } else {
+            // Is not called except in certain edge cases, see comment in getClockBottom
+            // TODO(b/261755021): Cleanup Large Frame Visibility
             return clock.getSmallClock().getView().getHeight();
         }
     }
 
     boolean isClockTopAligned() {
+        // Returns false except certain edge cases, see comment in getClockBottom
+        // TODO(b/261755021): Cleanup Large Frame Visibility
         return mLargeClockFrame.getVisibility() != View.VISIBLE;
     }
 
diff --git a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardClockSwitchControllerTest.java b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardClockSwitchControllerTest.java
index 61c7bb500e6ab..c8e753844c64f 100644
--- a/packages/SystemUI/tests/src/com/android/keyguard/KeyguardClockSwitchControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/keyguard/KeyguardClockSwitchControllerTest.java
@@ -29,7 +29,6 @@ import static org.mockito.Mockito.when;
 
 import android.content.res.Resources;
 import android.database.ContentObserver;
-import android.graphics.Rect;
 import android.net.Uri;
 import android.os.UserHandle;
 import android.provider.Settings;
@@ -47,6 +46,8 @@ import com.android.systemui.dump.DumpManager;
 import com.android.systemui.keyguard.KeyguardUnlockAnimationController;
 import com.android.systemui.plugins.ClockAnimations;
 import com.android.systemui.plugins.ClockController;
+import com.android.systemui.plugins.ClockEvents;
+import com.android.systemui.plugins.ClockFaceController;
 import com.android.systemui.plugins.statusbar.StatusBarStateController;
 import com.android.systemui.shared.clocks.AnimatableClockView;
 import com.android.systemui.shared.clocks.ClockRegistry;
@@ -88,7 +89,15 @@ public class KeyguardClockSwitchControllerTest extends SysuiTestCase {
     @Mock
     KeyguardUnlockAnimationController mKeyguardUnlockAnimationController;
     @Mock
-    private ClockController mClock;
+    private ClockController mClockController;
+    @Mock
+    private ClockFaceController mLargeClockController;
+    @Mock
+    private ClockFaceController mSmallClockController;
+    @Mock
+    private ClockAnimations mClockAnimations;
+    @Mock
+    private ClockEvents mClockEvents;
     @Mock
     DumpManager mDumpManager;
     @Mock
@@ -97,10 +106,12 @@ public class KeyguardClockSwitchControllerTest extends SysuiTestCase {
     @Mock
     private NotificationIconContainer mNotificationIcons;
     @Mock
-    private AnimatableClockView mClockView;
+    private AnimatableClockView mSmallClockView;
     @Mock
     private AnimatableClockView mLargeClockView;
     @Mock
+    private FrameLayout mSmallClockFrame;
+    @Mock
     private FrameLayout mLargeClockFrame;
     @Mock
     private SecureSettings mSecureSettings;
@@ -121,9 +132,14 @@ public class KeyguardClockSwitchControllerTest extends SysuiTestCase {
                 mock(RelativeLayout.LayoutParams.class));
         when(mView.getContext()).thenReturn(getContext());
         when(mView.getResources()).thenReturn(mResources);
+        when(mResources.getDimensionPixelSize(R.dimen.keyguard_clock_top_margin))
+                .thenReturn(100);
+        when(mResources.getDimensionPixelSize(R.dimen.keyguard_large_clock_top_margin))
+                .thenReturn(-200);
 
         when(mView.findViewById(R.id.lockscreen_clock_view_large)).thenReturn(mLargeClockFrame);
-        when(mClockView.getContext()).thenReturn(getContext());
+        when(mView.findViewById(R.id.lockscreen_clock_view)).thenReturn(mSmallClockFrame);
+        when(mSmallClockView.getContext()).thenReturn(getContext());
         when(mLargeClockView.getContext()).thenReturn(getContext());
 
         when(mView.isAttachedToWindow()).thenReturn(true);
@@ -144,7 +160,14 @@ public class KeyguardClockSwitchControllerTest extends SysuiTestCase {
         );
 
         when(mStatusBarStateController.getState()).thenReturn(StatusBarState.SHADE);
-        when(mClockRegistry.createCurrentClock()).thenReturn(mClock);
+        when(mLargeClockController.getView()).thenReturn(mLargeClockView);
+        when(mSmallClockController.getView()).thenReturn(mSmallClockView);
+        when(mClockController.getLargeClock()).thenReturn(mLargeClockController);
+        when(mClockController.getSmallClock()).thenReturn(mSmallClockController);
+        when(mClockController.getEvents()).thenReturn(mClockEvents);
+        when(mClockController.getAnimations()).thenReturn(mClockAnimations);
+        when(mClockRegistry.createCurrentClock()).thenReturn(mClockController);
+        when(mClockEventController.getClock()).thenReturn(mClockController);
 
         mSliceView = new View(getContext());
         when(mView.findViewById(R.id.keyguard_slice_view)).thenReturn(mSliceView);
@@ -203,8 +226,8 @@ public class KeyguardClockSwitchControllerTest extends SysuiTestCase {
         verify(mClockRegistry).registerClockChangeListener(listenerArgumentCaptor.capture());
 
         listenerArgumentCaptor.getValue().onClockChanged();
-        verify(mView, times(2)).setClock(mClock, StatusBarState.SHADE);
-        verify(mClockEventController, times(2)).setClock(mClock);
+        verify(mView, times(2)).setClock(mClockController, StatusBarState.SHADE);
+        verify(mClockEventController, times(2)).setClock(mClockController);
     }
 
     @Test
@@ -262,17 +285,40 @@ public class KeyguardClockSwitchControllerTest extends SysuiTestCase {
 
     @Test
     public void testGetClockAnimationsForwardsToClock() {
-        ClockController mockClockController = mock(ClockController.class);
-        ClockAnimations mockClockAnimations = mock(ClockAnimations.class);
-        when(mClockEventController.getClock()).thenReturn(mockClockController);
-        when(mockClockController.getAnimations()).thenReturn(mockClockAnimations);
-
-        Rect r1 = new Rect(1, 2, 3, 4);
-        Rect r2 = new Rect(5, 6, 7, 8);
-        mController.getClockAnimations().onPositionUpdated(r1, r2, 0.2f);
-        verify(mockClockAnimations).onPositionUpdated(r1, r2, 0.2f);
+        assertEquals(mClockAnimations, mController.getClockAnimations());
     }
 
+    @Test
+    public void testGetLargeClockBottom_returnsExpectedValue() {
+        when(mLargeClockFrame.getVisibility()).thenReturn(View.VISIBLE);
+        when(mLargeClockFrame.getHeight()).thenReturn(100);
+        when(mSmallClockFrame.getHeight()).thenReturn(50);
+        when(mLargeClockView.getHeight()).thenReturn(40);
+        when(mSmallClockView.getHeight()).thenReturn(20);
+        mController.init();
+
+        assertEquals(170, mController.getClockBottom(1000));
+    }
+
+    @Test
+    public void testGetSmallLargeClockBottom_returnsExpectedValue() {
+        when(mLargeClockFrame.getVisibility()).thenReturn(View.GONE);
+        when(mLargeClockFrame.getHeight()).thenReturn(100);
+        when(mSmallClockFrame.getHeight()).thenReturn(50);
+        when(mLargeClockView.getHeight()).thenReturn(40);
+        when(mSmallClockView.getHeight()).thenReturn(20);
+        mController.init();
+
+        assertEquals(1120, mController.getClockBottom(1000));
+    }
+
+    @Test
+    public void testGetClockBottom_nullClock_returnsZero() {
+        when(mClockEventController.getClock()).thenReturn(null);
+        assertEquals(0, mController.getClockBottom(10));
+    }
+
+
     private void verifyAttachment(VerificationMode times) {
         verify(mClockRegistry, times).registerClockChangeListener(
                 any(ClockRegistry.ClockChangeListener.class));