Resetting the scroll position of the media players when allowed

Previously the scroll position would never be reset anymore
unless the user actively did so. We're now resetting the position
whenever the visual stability manager tells us we're allowed to
do so.

Bug: 154137987
Test: atest SystemUITests
Change-Id: Ifd5858f47827610a64a9a12cff777bd2440334a9
This commit is contained in:
Selim Cinek
2020-05-11 16:03:52 -07:00
parent 8ae0cfe36b
commit 0260c7563b
6 changed files with 70 additions and 22 deletions

View File

@@ -42,10 +42,10 @@ class MediaViewManager @Inject constructor(
val mediaCarousel: HorizontalScrollView
private val mediaContent: ViewGroup
private val mediaPlayers: MutableMap<String, MediaControlPanel> = mutableMapOf()
private val visualStabilityCallback = ::reorderAllPlayers
private val visualStabilityCallback : VisualStabilityManager.Callback
private var activeMediaIndex: Int = 0
private var needsReordering: Boolean = false
private var scrollIntoCurrentMedia: Int = 0
private var currentlyExpanded = true
set(value) {
if (field != value) {
@@ -75,6 +75,16 @@ class MediaViewManager @Inject constructor(
mediaCarousel = inflateMediaCarousel()
mediaCarousel.setOnScrollChangeListener(scrollChangedListener)
mediaContent = mediaCarousel.requireViewById(R.id.media_carousel)
visualStabilityCallback = VisualStabilityManager.Callback {
if (needsReordering) {
needsReordering = false
reorderAllPlayers()
}
// Let's reset our scroll position
mediaCarousel.scrollX = 0
}
visualStabilityManager.addReorderingAllowedCallback(visualStabilityCallback,
true /* persistent */)
mediaManager.addListener(object : MediaDataManager.Listener {
override fun onMediaDataLoaded(key: String, data: MediaData) {
updateView(key, data)
@@ -169,7 +179,7 @@ class MediaViewManager @Inject constructor(
mediaContent.removeView(existingPlayer.view?.player)
mediaContent.addView(existingPlayer.view?.player, 0)
} else {
visualStabilityManager.addReorderingAllowedCallback(visualStabilityCallback)
needsReordering = true
}
}
existingPlayer.bind(data)

View File

@@ -195,13 +195,15 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
boolean wasChildInGroup = ent.isChildInGroup();
if (isChildInGroup && !wasChildInGroup) {
isChildInGroup = wasChildInGroup;
mVisualStabilityManager.addGroupChangesAllowedCallback(mEntryManager);
mVisualStabilityManager.addGroupChangesAllowedCallback(mEntryManager,
false /* persistent */);
} else if (!isChildInGroup && wasChildInGroup) {
// We allow grouping changes if the group was collapsed
if (mGroupManager.isLogicalGroupExpanded(ent.getSbn())) {
isChildInGroup = wasChildInGroup;
parent = ent.getRow().getNotificationParent().getEntry();
mVisualStabilityManager.addGroupChangesAllowedCallback(mEntryManager);
mVisualStabilityManager.addGroupChangesAllowedCallback(mEntryManager,
false /* persistent */);
}
}
}
@@ -286,7 +288,8 @@ public class NotificationViewHierarchyManager implements DynamicPrivacyControlle
if (mVisualStabilityManager.canReorderNotification(targetChild)) {
mListContainer.changeViewPosition(targetChild, i);
} else {
mVisualStabilityManager.addReorderingAllowedCallback(mEntryManager);
mVisualStabilityManager.addReorderingAllowedCallback(mEntryManager,
false /* persistent */);
}
}
j++;

View File

@@ -43,7 +43,9 @@ public class VisualStabilityManager implements OnHeadsUpChangedListener, Dumpabl
private static final long TEMPORARY_REORDERING_ALLOWED_DURATION = 1000;
private final ArrayList<Callback> mReorderingAllowedCallbacks = new ArrayList<>();
private final ArraySet<Callback> mPersistentReorderingCallbacks = new ArraySet<>();
private final ArrayList<Callback> mGroupChangesAllowedCallbacks = new ArrayList<>();
private final ArraySet<Callback> mPersistentGroupCallbacks = new ArraySet<>();
private final Handler mHandler;
private boolean mPanelExpanded;
@@ -85,8 +87,15 @@ public class VisualStabilityManager implements OnHeadsUpChangedListener, Dumpabl
/**
* Add a callback to invoke when reordering is allowed again.
*
* @param callback the callback to add
* @param persistent {@code true} if this callback should this callback be persisted, otherwise
* it will be removed after a single invocation
*/
public void addReorderingAllowedCallback(Callback callback) {
public void addReorderingAllowedCallback(Callback callback, boolean persistent) {
if (persistent) {
mPersistentReorderingCallbacks.add(callback);
}
if (mReorderingAllowedCallbacks.contains(callback)) {
return;
}
@@ -95,8 +104,15 @@ public class VisualStabilityManager implements OnHeadsUpChangedListener, Dumpabl
/**
* Add a callback to invoke when group changes are allowed again.
*
* @param callback the callback to add
* @param persistent {@code true} if this callback should this callback be persisted, otherwise
* it will be removed after a single invocation
*/
public void addGroupChangesAllowedCallback(Callback callback) {
public void addGroupChangesAllowedCallback(Callback callback, boolean persistent) {
if (persistent) {
mPersistentGroupCallbacks.add(callback);
}
if (mGroupChangesAllowedCallbacks.contains(callback)) {
return;
}
@@ -136,21 +152,26 @@ public class VisualStabilityManager implements OnHeadsUpChangedListener, Dumpabl
boolean changedToTrue = reorderingAllowed && !mReorderingAllowed;
mReorderingAllowed = reorderingAllowed;
if (changedToTrue) {
notifyChangeAllowed(mReorderingAllowedCallbacks);
notifyChangeAllowed(mReorderingAllowedCallbacks, mPersistentReorderingCallbacks);
}
boolean groupChangesAllowed = (!mScreenOn || !mPanelExpanded) && !mPulsing;
changedToTrue = groupChangesAllowed && !mGroupChangedAllowed;
mGroupChangedAllowed = groupChangesAllowed;
if (changedToTrue) {
notifyChangeAllowed(mGroupChangesAllowedCallbacks);
notifyChangeAllowed(mGroupChangesAllowedCallbacks, mPersistentGroupCallbacks);
}
}
private void notifyChangeAllowed(ArrayList<Callback> callbacks) {
private void notifyChangeAllowed(ArrayList<Callback> callbacks,
ArraySet<Callback> persistentCallbacks) {
for (int i = 0; i < callbacks.size(); i++) {
callbacks.get(i).onChangeAllowed();
Callback callback = callbacks.get(i);
callback.onChangeAllowed();
if (!persistentCallbacks.contains(callback)) {
callbacks.remove(callback);
i--;
}
}
callbacks.clear();
}
/**

View File

@@ -463,7 +463,8 @@ public class NotificationChildrenContainer extends ViewGroup {
mAttachedChildren.add(i, desiredChild);
result = true;
} else {
visualStabilityManager.addReorderingAllowedCallback(callback);
visualStabilityManager.addReorderingAllowedCallback(callback,
false /* persistent */);
}
}
}

View File

@@ -439,7 +439,8 @@ public class HeadsUpManagerPhone extends HeadsUpManager implements Dumpable,
// time out anyway
&& !entry.showingPulsing()) {
mEntriesToRemoveWhenReorderingAllowed.add(entry);
mVisualStabilityManager.addReorderingAllowedCallback(HeadsUpManagerPhone.this);
mVisualStabilityManager.addReorderingAllowedCallback(HeadsUpManagerPhone.this,
false /* persistent */);
} else if (mTrackingHeadsUp) {
mEntriesToRemoveAfterExpand.add(entry);
} else if (mIsAutoHeadsUp && mStatusBarState == StatusBarState.KEYGUARD) {

View File

@@ -21,6 +21,7 @@ import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -108,7 +109,7 @@ public class VisualStabilityManagerTest extends SysuiTestCase {
public void testCallBackCalledScreenOn() {
mVisualStabilityManager.setPanelExpanded(true);
mVisualStabilityManager.setScreenOn(true);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback, false /* persistent */);
mVisualStabilityManager.setScreenOn(false);
verify(mCallback).onChangeAllowed();
}
@@ -117,7 +118,7 @@ public class VisualStabilityManagerTest extends SysuiTestCase {
public void testCallBackCalledPanelExpanded() {
mVisualStabilityManager.setPanelExpanded(true);
mVisualStabilityManager.setScreenOn(true);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback, false /* persistent */);
mVisualStabilityManager.setPanelExpanded(false);
verify(mCallback).onChangeAllowed();
}
@@ -126,13 +127,24 @@ public class VisualStabilityManagerTest extends SysuiTestCase {
public void testCallBackExactlyOnce() {
mVisualStabilityManager.setPanelExpanded(true);
mVisualStabilityManager.setScreenOn(true);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback, false /* persistent */);
mVisualStabilityManager.setScreenOn(false);
mVisualStabilityManager.setScreenOn(true);
mVisualStabilityManager.setScreenOn(false);
verify(mCallback).onChangeAllowed();
}
@Test
public void testCallBackCalledContinuouslyWhenRequested() {
mVisualStabilityManager.setPanelExpanded(true);
mVisualStabilityManager.setScreenOn(true);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback, true /* persistent */);
mVisualStabilityManager.setScreenOn(false);
mVisualStabilityManager.setScreenOn(true);
mVisualStabilityManager.setScreenOn(false);
verify(mCallback, times(2)).onChangeAllowed();
}
@Test
public void testAddedCanReorder() {
mVisualStabilityManager.setPanelExpanded(true);
@@ -188,7 +200,7 @@ public class VisualStabilityManagerTest extends SysuiTestCase {
@Test
public void testCallBackCalled_Pulsing() {
mVisualStabilityManager.setPulsing(true);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback, false /* persistent */);
mVisualStabilityManager.setPulsing(false);
verify(mCallback).onChangeAllowed();
}
@@ -198,7 +210,7 @@ public class VisualStabilityManagerTest extends SysuiTestCase {
// GIVEN having the panel open (which would block reordering)
mVisualStabilityManager.setScreenOn(true);
mVisualStabilityManager.setPanelExpanded(true);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback, false /* persistent */);
// WHEN we temprarily allow reordering
mVisualStabilityManager.temporarilyAllowReordering();
@@ -212,7 +224,7 @@ public class VisualStabilityManagerTest extends SysuiTestCase {
public void testTemporarilyAllowReorderingDoesntOverridePulsing() {
// GIVEN we are in a pulsing state
mVisualStabilityManager.setPulsing(true);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback, false /* persistent */);
// WHEN we temprarily allow reordering
mVisualStabilityManager.temporarilyAllowReordering();
@@ -227,7 +239,7 @@ public class VisualStabilityManagerTest extends SysuiTestCase {
// GIVEN having the panel open (which would block reordering)
mVisualStabilityManager.setScreenOn(true);
mVisualStabilityManager.setPanelExpanded(true);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback);
mVisualStabilityManager.addReorderingAllowedCallback(mCallback, false /* persistent */);
// WHEN we temprarily allow reordering and then wait until the window expires
mVisualStabilityManager.temporarilyAllowReordering();