From a0447977791f9cf2bb24bd6b54ea0c58f557d8ab Mon Sep 17 00:00:00 2001
From: Selim Cinek <cinek@google.com>
Date: Mon, 29 Jun 2020 19:41:20 -0700
Subject: [PATCH] Fixed that texts of players would often remain empty

Because we were binding the view too late, at the time
we were taking our measurements, they were not bound
yet and hence the measurements were completely wrong.
In the past this worked accidentally, since they sizes
of the views didn't really depend on the content, but
various changes have been using wrap_content recently,
which could result in empty titles and songs.
Similarly, buttons could not have the right state
as a result and would be positioned wrongly.

This also could lead to the players having the wrong
order since isPlaying is depending on the view being
bound.

Fixes: 160197584
Test: add resumable player, observe no empty texts
Change-Id: I8fe4fb683efdabea4f1e985515f4c31d610170ea
---
 .../systemui/media/MediaCarouselController.kt | 19 +++++++++++--------
 .../systemui/media/MediaViewController.kt     | 16 +++++++++-------
 2 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt
index 1a730c39dcd3e..7c30d6424b6ff 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaCarouselController.kt
@@ -249,6 +249,7 @@ class MediaCarouselController @Inject constructor(
             val lp = LinearLayout.LayoutParams(ViewGroup.LayoutParams.MATCH_PARENT,
                     ViewGroup.LayoutParams.WRAP_CONTENT)
             existingPlayer.view?.player?.setLayoutParams(lp)
+            existingPlayer.bind(data)
             existingPlayer.setListening(currentlyExpanded)
             updatePlayerToState(existingPlayer, noAnimation = true)
             if (existingPlayer.isPlaying) {
@@ -256,16 +257,18 @@ class MediaCarouselController @Inject constructor(
             } else {
                 mediaContent.addView(existingPlayer.view?.player)
             }
-        } else if (existingPlayer.isPlaying &&
-                mediaContent.indexOfChild(existingPlayer.view?.player) != 0) {
-            if (visualStabilityManager.isReorderingAllowed) {
-                mediaContent.removeView(existingPlayer.view?.player)
-                mediaContent.addView(existingPlayer.view?.player, 0)
-            } else {
-                needsReordering = true
+        } else {
+            existingPlayer.bind(data)
+            if (existingPlayer.isPlaying &&
+                    mediaContent.indexOfChild(existingPlayer.view?.player) != 0) {
+                if (visualStabilityManager.isReorderingAllowed) {
+                    mediaContent.removeView(existingPlayer.view?.player)
+                    mediaContent.addView(existingPlayer.view?.player, 0)
+                } else {
+                    needsReordering = true
+                }
             }
         }
-        existingPlayer?.bind(data)
         updatePageIndicator()
         mediaCarouselScrollHandler.onPlayersChanged()
         mediaCarousel.requiresRemeasuring = true
diff --git a/packages/SystemUI/src/com/android/systemui/media/MediaViewController.kt b/packages/SystemUI/src/com/android/systemui/media/MediaViewController.kt
index 83cfdd5f2699d..38817d7b579e2 100644
--- a/packages/SystemUI/src/com/android/systemui/media/MediaViewController.kt
+++ b/packages/SystemUI/src/com/android/systemui/media/MediaViewController.kt
@@ -268,7 +268,6 @@ class MediaViewController @Inject constructor(
     fun attach(transitionLayout: TransitionLayout) {
         this.transitionLayout = transitionLayout
         layoutController.attach(transitionLayout)
-        ensureAllMeasurements()
         if (currentEndLocation == -1) {
             return
         }
@@ -414,13 +413,16 @@ class MediaViewController @Inject constructor(
      * Clear all existing measurements and refresh the state to match the view.
      */
     fun refreshState() {
-        if (!firstRefresh) {
-            // Let's clear all of our measurements and recreate them!
-            viewStates.clear()
-            setCurrentState(currentStartLocation, currentEndLocation, currentTransitionProgress,
-                    applyImmediately = true)
+        // Let's clear all of our measurements and recreate them!
+        viewStates.clear()
+        if (firstRefresh) {
+            // This is the first bind, let's ensure we pre-cache all measurements. Otherwise
+            // We'll just load these on demand.
+            ensureAllMeasurements()
+            firstRefresh = false
         }
-        firstRefresh = false
+        setCurrentState(currentStartLocation, currentEndLocation, currentTransitionProgress,
+                applyImmediately = true)
     }
 }