From 6a62f99b50598bc0c4d3383cd1ec0982489b73ba Mon Sep 17 00:00:00 2001
From: Hawkwood Glazier <jglazier@google.com>
Date: Tue, 2 Aug 2022 16:49:33 +0000
Subject: [PATCH] [DO NOT MERGE] Correct large clock position

This corrects the large clock positioning error that was introduced with
Ia053aaec6987e00de980388a6c024d8bcf5203a2. It does this by rearranging
the layout slightly to get back to the same position. Of note:
 - Previously the margin for the large clock was computed dynamically
   and applied to the large clock frame from a static resource and
   dynamic value based on the font. Now the static portion is applied
   directly from the resource to the clock frame in the layout resource.
   The dynamic portion is applied to the AnimatableClockView directly
   instead of it's parent as before.
 - ChildClipping has been disabled on the parent view as the dynamic
   portion of this margin moves the AnimatableClockView outside of
   it's parent's bounds.
 - The margin that is now applied to the AnimatableClockView is
   scaled by 0.5 to account produce the ccorrect placement after
   center-aligning the view in it's parent. Before the full value
   was applied to the parent, creating an equivalent effect.

This also fixes the clock translation issue when the splitshade appears
and disappears. They were both fundamentally layout issues with the
refactored clock code.

This change was manually tested by flashing each device and comparing
the clock position in every configuration against a refenece image from
a dogfood build. Additionally the motion of the clock was validated
when the splitshade appears and disappears on a tablet in landscape.

Test: Manual validated screenshots from several devices against reference
images from a known good build. Additionally validated with a screenshot
test, although golden images had to be updated.

Bug: 229771520
Fixes: 241083568
Fixes: 241084042
Change-Id: I273c2996ff67c7b5f77c6bafb292d53275c7bc47
---
 .../layout/keyguard_clock_switch.xml          | 10 +-
 packages/SystemUI/res/values/dimens.xml       |  2 +-
 .../shared/res/layout/clock_default_small.xml |  2 +-
 .../shared/clocks/DefaultClockProvider.kt     | 91 +++++++++++--------
 .../android/keyguard/KeyguardClockSwitch.java |  7 +-
 .../KeyguardClockSwitchController.java        |  5 +-
 .../shared/clocks/DefaultClockProviderTest.kt | 14 ++-
 7 files changed, 72 insertions(+), 59 deletions(-)

diff --git a/packages/SystemUI/res-keyguard/layout/keyguard_clock_switch.xml b/packages/SystemUI/res-keyguard/layout/keyguard_clock_switch.xml
index 8b8ebf00e190d..3ad7c8c4369ca 100644
--- a/packages/SystemUI/res-keyguard/layout/keyguard_clock_switch.xml
+++ b/packages/SystemUI/res-keyguard/layout/keyguard_clock_switch.xml
@@ -23,6 +23,7 @@
     android:id="@+id/keyguard_clock_container"
     android:layout_width="match_parent"
     android:layout_height="wrap_content"
+    android:clipChildren="false"
     android:layout_gravity="center_horizontal|top">
     <FrameLayout
         android:id="@+id/lockscreen_clock_view"
@@ -30,16 +31,13 @@
         android:layout_height="wrap_content"
         android:layout_alignParentStart="true"
         android:layout_alignParentTop="true"
-        android:paddingStart="@dimen/clock_padding_start">
-    </FrameLayout>
+        android:paddingStart="@dimen/clock_padding_start" />
     <FrameLayout
         android:id="@+id/lockscreen_clock_view_large"
         android:layout_width="match_parent"
         android:layout_height="match_parent"
-        android:layout_below="@id/keyguard_slice_view"
-        android:paddingTop="@dimen/keyguard_large_clock_top_padding"
-        android:visibility="gone">
-    </FrameLayout>
+        android:layout_marginTop="@dimen/keyguard_large_clock_top_margin"
+        android:visibility="gone" />
 
     <!-- Not quite optimal but needed to translate these items as a group. The
          NotificationIconContainer has its own logic for translation. -->
diff --git a/packages/SystemUI/res/values/dimens.xml b/packages/SystemUI/res/values/dimens.xml
index a77575418e5f2..e3be36503c873 100644
--- a/packages/SystemUI/res/values/dimens.xml
+++ b/packages/SystemUI/res/values/dimens.xml
@@ -669,7 +669,7 @@
     <!-- When large clock is showing, offset the smartspace by this amount -->
     <dimen name="keyguard_smartspace_top_offset">12dp</dimen>
     <!-- With the large clock, move up slightly from the center -->
-    <dimen name="keyguard_large_clock_top_padding">100dp</dimen>
+    <dimen name="keyguard_large_clock_top_margin">-60dp</dimen>
 
     <dimen name="notification_scrim_corner_radius">32dp</dimen>
 
diff --git a/packages/SystemUI/shared/res/layout/clock_default_small.xml b/packages/SystemUI/shared/res/layout/clock_default_small.xml
index 390ff5e3ff787..ff6d7f9e22403 100644
--- a/packages/SystemUI/shared/res/layout/clock_default_small.xml
+++ b/packages/SystemUI/shared/res/layout/clock_default_small.xml
@@ -18,7 +18,7 @@
 -->
 <com.android.systemui.shared.clocks.AnimatableClockView
     xmlns:android="http://schemas.android.com/apk/res/android"
-    android:layout_width="wrap_content"
+    android:layout_width="match_parent"
     android:layout_height="wrap_content"
     android:layout_gravity="start"
     android:gravity="start"
diff --git a/packages/SystemUI/shared/src/com/android/systemui/shared/clocks/DefaultClockProvider.kt b/packages/SystemUI/shared/src/com/android/systemui/shared/clocks/DefaultClockProvider.kt
index 2099f165b0792..5c20b691b9910 100644
--- a/packages/SystemUI/shared/src/com/android/systemui/shared/clocks/DefaultClockProvider.kt
+++ b/packages/SystemUI/shared/src/com/android/systemui/shared/clocks/DefaultClockProvider.kt
@@ -13,12 +13,14 @@
  */
 package com.android.systemui.shared.clocks
 
+import android.content.Context
 import android.content.res.Resources
 import android.graphics.Color
 import android.graphics.drawable.Drawable
 import android.icu.text.NumberFormat
 import android.util.TypedValue
 import android.view.LayoutInflater
+import android.widget.FrameLayout
 import com.android.internal.annotations.VisibleForTesting
 import com.android.systemui.dagger.qualifiers.Main
 import com.android.systemui.plugins.Clock
@@ -39,6 +41,7 @@ const val DEFAULT_CLOCK_ID = "DEFAULT"
 
 /** Provides the default system clock */
 class DefaultClockProvider @Inject constructor(
+    val ctx: Context,
     val layoutInflater: LayoutInflater,
     @Main val resources: Resources
 ) : ClockProvider {
@@ -49,7 +52,7 @@ class DefaultClockProvider @Inject constructor(
         if (id != DEFAULT_CLOCK_ID) {
             throw IllegalArgumentException("$id is unsupported by $TAG")
         }
-        return DefaultClock(layoutInflater, resources)
+        return DefaultClock(ctx, layoutInflater, resources)
     }
 
     override fun getClockThumbnail(id: ClockId): Drawable? {
@@ -69,14 +72,13 @@ class DefaultClockProvider @Inject constructor(
  * AnimatableClockView used by the existing lockscreen clock.
  */
 class DefaultClock(
+        ctx: Context,
         private val layoutInflater: LayoutInflater,
         private val resources: Resources
 ) : Clock {
-    override val smallClock =
-        layoutInflater.inflate(R.layout.clock_default_small, null) as AnimatableClockView
-    override val largeClock =
-        layoutInflater.inflate(R.layout.clock_default_large, null) as AnimatableClockView
-    private val clocks = listOf(smallClock, largeClock)
+    override val smallClock: AnimatableClockView
+    override val largeClock: AnimatableClockView
+    private val clocks get() = listOf(smallClock, largeClock)
 
     private val burmeseNf = NumberFormat.getInstance(Locale.forLanguageTag("my"))
     private val burmeseNumerals = burmeseNf.format(FORMAT_NUMBER.toLong())
@@ -84,17 +86,42 @@ class DefaultClock(
         resources.getFloat(R.dimen.keyguard_clock_line_spacing_scale_burmese)
     private val defaultLineSpacing = resources.getFloat(R.dimen.keyguard_clock_line_spacing_scale)
 
-    private fun updateClockColor(clock: AnimatableClockView, isRegionDark: Boolean) {
-        val color = if (isRegionDark) {
-            resources.getColor(android.R.color.system_accent1_100)
-        } else {
-            resources.getColor(android.R.color.system_accent2_600)
-        }
-        clock.setColors(DOZE_COLOR, color)
-        clock.animateAppearOnLockscreen()
+    override val events: ClockEvents
+    override lateinit var animations: ClockAnimations
+        private set
+
+    init {
+        val parent = FrameLayout(ctx)
+
+        smallClock = layoutInflater.inflate(
+            R.layout.clock_default_small,
+            parent,
+            false
+        ) as AnimatableClockView
+
+        largeClock = layoutInflater.inflate(
+            R.layout.clock_default_large,
+            parent,
+            false
+        ) as AnimatableClockView
+
+        events = DefaultClockEvents()
+        animations = DefaultClockAnimations(0f, 0f)
+
+        events.onLocaleChanged(Locale.getDefault())
+
+        // DOZE_COLOR is a placeholder, and will be assigned correctly in initialize
+        clocks.forEach { it.setColors(DOZE_COLOR, DOZE_COLOR) }
     }
 
-    override val events = object : ClockEvents {
+    override fun initialize(resources: Resources, dozeFraction: Float, foldFraction: Float) {
+        recomputePadding()
+        animations = DefaultClockAnimations(dozeFraction, foldFraction)
+        events.onColorPaletteChanged(resources, true, true)
+        events.onTimeTick()
+    }
+
+    inner class DefaultClockEvents() : ClockEvents {
         override fun onTimeTick() = clocks.forEach { it.refreshTime() }
 
         override fun onTimeFormatChanged(is24Hr: Boolean) =
@@ -136,9 +163,6 @@ class DefaultClock(
         }
     }
 
-    override var animations = DefaultClockAnimations(0f, 0f)
-        private set
-
     inner class DefaultClockAnimations(
         dozeFraction: Float,
         foldFraction: Float
@@ -194,29 +218,20 @@ class DefaultClock(
         }
     }
 
-    init {
-        events.onLocaleChanged(Locale.getDefault())
-        clocks.forEach { it.setColors(DOZE_COLOR, DOZE_COLOR) }
-    }
-
-    override fun initialize(
-            resources: Resources,
-            dozeFraction: Float,
-            foldFraction: Float
-    ) {
-        recomputePadding()
-        animations = DefaultClockAnimations(dozeFraction, foldFraction)
-        events.onColorPaletteChanged(
-                resources,
-                true,
-                true
-        )
-        events.onTimeTick()
+    private fun updateClockColor(clock: AnimatableClockView, isRegionDark: Boolean) {
+        val color = if (isRegionDark) {
+            resources.getColor(android.R.color.system_accent1_100)
+        } else {
+            resources.getColor(android.R.color.system_accent2_600)
+        }
+        clock.setColors(DOZE_COLOR, color)
+        clock.animateAppearOnLockscreen()
     }
 
     private fun recomputePadding() {
-        val topPadding = -1 * (largeClock.bottom.toInt() - 180)
-        largeClock.setPadding(0, topPadding, 0, 0)
+        val lp = largeClock.getLayoutParams() as FrameLayout.LayoutParams
+        lp.topMargin = (-0.5f * largeClock.bottom).toInt()
+        largeClock.setLayoutParams(lp)
     }
 
     override fun dump(pw: PrintWriter) = clocks.forEach { it.dump(pw) }
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitch.java b/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitch.java
index 10d6d9fda548c..d7cd1d0dbc6f0 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitch.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitch.java
@@ -8,7 +8,6 @@ import android.content.Context;
 import android.util.AttributeSet;
 import android.util.Log;
 import android.view.View;
-import android.view.ViewGroup;
 import android.widget.FrameLayout;
 import android.widget.RelativeLayout;
 
@@ -106,9 +105,7 @@ public class KeyguardClockSwitch extends RelativeLayout {
         }
 
         // Attach small and big clock views to hierarchy.
-        mSmallClockFrame.addView(clock.getSmallClock(), -1,
-                new ViewGroup.LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT,
-                        ViewGroup.LayoutParams.WRAP_CONTENT));
+        mSmallClockFrame.addView(clock.getSmallClock());
         mLargeClockFrame.addView(clock.getLargeClock());
     }
 
@@ -223,7 +220,7 @@ public class KeyguardClockSwitch extends RelativeLayout {
 
     public void dump(PrintWriter pw, String[] args) {
         pw.println("KeyguardClockSwitch:");
-        pw.println("  mClockFrame: " + mSmallClockFrame);
+        pw.println("  mSmallClockFrame: " + mSmallClockFrame);
         pw.println("  mLargeClockFrame: " + mLargeClockFrame);
         pw.println("  mStatusArea: " + mStatusArea);
         pw.println("  mDisplayedClockSize: " + mDisplayedClockSize);
diff --git a/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java b/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java
index dd78f1cef78ca..2165099b474e3 100644
--- a/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java
+++ b/packages/SystemUI/src/com/android/keyguard/KeyguardClockSwitchController.java
@@ -72,7 +72,6 @@ public class KeyguardClockSwitchController extends ViewController<KeyguardClockS
     private final DumpManager mDumpManager;
     private final ClockEventController mClockEventController;
 
-    /** Clock frames for both small and large sizes */
     private FrameLayout mSmallClockFrame; // top aligned clock
     private FrameLayout mLargeClockFrame; // centered clock
 
@@ -151,7 +150,7 @@ public class KeyguardClockSwitchController extends ViewController<KeyguardClockS
      * Attach the controller to the view it relates to.
      */
     @Override
-    public void onInit() {
+    protected void onInit() {
         mKeyguardSliceViewController.init();
 
         mSmallClockFrame = mView.findViewById(R.id.lockscreen_clock_view);
@@ -390,8 +389,6 @@ public class KeyguardClockSwitchController extends ViewController<KeyguardClockS
      * bounds during the unlock transition.
      */
     private void setClipChildrenForUnlock(boolean clip) {
-        mView.setClipChildren(clip);
-
         if (mStatusArea != null) {
             mStatusArea.setClipChildren(clip);
         }
diff --git a/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/DefaultClockProviderTest.kt b/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/DefaultClockProviderTest.kt
index 2f22455f8fe05..a978294c98eca 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/DefaultClockProviderTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/shared/clocks/DefaultClockProviderTest.kt
@@ -21,10 +21,12 @@ import android.graphics.drawable.Drawable
 import android.testing.AndroidTestingRunner
 import android.util.TypedValue
 import android.view.LayoutInflater
+import android.widget.FrameLayout
 import androidx.test.filters.SmallTest
 import com.android.systemui.R
 import com.android.systemui.SysuiTestCase
 import com.android.systemui.shared.clocks.DefaultClock.Companion.DOZE_COLOR
+import com.android.systemui.util.mockito.any
 import com.android.systemui.util.mockito.eq
 import com.android.systemui.util.mockito.mock
 import java.util.Locale
@@ -35,6 +37,7 @@ import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.junit.runner.RunWith
+import org.mockito.ArgumentMatchers.anyBoolean
 import org.mockito.ArgumentMatchers.anyFloat
 import org.mockito.ArgumentMatchers.anyInt
 import org.mockito.Mock
@@ -58,14 +61,16 @@ class DefaultClockProviderTest : SysuiTestCase() {
 
     @Before
     fun setUp() {
-        whenever(layoutInflater.inflate(R.layout.clock_default_small, null))
+        whenever(layoutInflater.inflate(eq(R.layout.clock_default_small), any(), anyBoolean()))
             .thenReturn(mockSmallClockView)
-        whenever(layoutInflater.inflate(R.layout.clock_default_large, null))
+        whenever(layoutInflater.inflate(eq(R.layout.clock_default_large), any(), anyBoolean()))
             .thenReturn(mockLargeClockView)
         whenever(resources.getDrawable(R.drawable.clock_default_thumbnail, null))
             .thenReturn(mockClockThumbnail)
+        whenever(mockSmallClockView.getLayoutParams()).thenReturn(FrameLayout.LayoutParams(10, 10))
+        whenever(mockLargeClockView.getLayoutParams()).thenReturn(FrameLayout.LayoutParams(10, 10))
 
-        provider = DefaultClockProvider(layoutInflater, resources)
+        provider = DefaultClockProvider(context, layoutInflater, resources)
     }
 
     @Test
@@ -95,6 +100,7 @@ class DefaultClockProviderTest : SysuiTestCase() {
         verify(mockLargeClockView, times(2)).setColors(eq(DOZE_COLOR), anyInt())
         verify(mockSmallClockView).refreshTime()
         verify(mockLargeClockView).refreshTime()
+        verify(mockLargeClockView).setLayoutParams(any())
     }
 
     @Test
@@ -132,7 +138,7 @@ class DefaultClockProviderTest : SysuiTestCase() {
 
         verify(mockSmallClockView).setTextSize(eq(TypedValue.COMPLEX_UNIT_PX), anyFloat())
         verify(mockLargeClockView).setTextSize(eq(TypedValue.COMPLEX_UNIT_PX), anyFloat())
-        verify(mockLargeClockView).setPadding(eq(0), anyInt(), eq(0), eq(0))
+        verify(mockLargeClockView).setLayoutParams(any())
     }
 
     @Test