Request new communal surface on layout changes.

This changelist causes a request to be made on every
layout change to the communal SurfaceView. This fixes
the issues where the SurfaceView is not resized
properly. This change also ensures that subsequent
identical show requests are ignored.

Bug: 197574964
Test: CommunalSurfaceViewControllerTest#testLayoutChange
Change-Id: I480b6ab5b9728f173cf4fd3e6042aa70ba60db89
This commit is contained in:
Bryce Lee
2021-08-23 17:46:44 -07:00
parent 8379b15dc5
commit 49af78d4b9
3 changed files with 102 additions and 21 deletions

View File

@@ -38,6 +38,7 @@ import com.google.common.util.concurrent.ListenableFuture;
import java.lang.ref.WeakReference;
import java.util.ArrayList;
import java.util.Objects;
import java.util.concurrent.Executor;
import javax.inject.Inject;
@@ -78,6 +79,45 @@ public class CommunalSourceImpl implements CommunalSource {
}
}
static class Request {
private final int mWidth;
private final int mHeight;
private final int mDisplayId;
private final IBinder mHostToken;
Request(int width, int height, int displayId, IBinder hostToken) {
mWidth = width;
mHeight = height;
mDisplayId = displayId;
mHostToken = hostToken;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof Request)) return false;
Request request = (Request) o;
return mWidth == request.mWidth && mHeight == request.mHeight
&& mDisplayId == request.mDisplayId && Objects.equals(mHostToken,
request.mHostToken);
}
@Override
public int hashCode() {
return Objects.hash(mWidth, mHeight, mDisplayId, mHostToken);
}
@Override
public String toString() {
return "Request{"
+ "mWidth=" + mWidth
+ ", mHeight=" + mHeight
+ ", mDisplayId=" + mDisplayId
+ ", mHostToken=" + mHostToken
+ '}';
}
}
// mConnected is initialized to true as it is presumed instances are constructed with valid
// proxies. The source can never be reconnected once the proxy has died. Once this value
// becomes false, the source will always report disconnected to registering callbacks.
@@ -148,18 +188,15 @@ public class CommunalSourceImpl implements CommunalSource {
* Called internally to request a new {@link android.view.SurfaceControlViewHost.SurfacePackage}
* for showing communal content.
*
* @param hostToken The HostToken necessary to generate a {@link SurfaceControlViewHost}.
* @param displayId The id of the display the surface will be shown on.
* @param width The width of the surface.
* @param height The height of the surface.
* @param request A request with the parameters for the new communal surface.
* @return A future that returns the resulting
* {@link android.view.SurfaceControlViewHost.SurfacePackage}.
*/
protected ListenableFuture<SurfaceControlViewHost.SurfacePackage> requestCommunalSurface(
IBinder hostToken, int displayId, int width, int height) {
Request request) {
return CallbackToFutureAdapter.getFuture(completer -> {
mSourceProxy.getCommunalSurface(hostToken, width, height, displayId,
new ICommunalSurfaceCallback.Stub() {
mSourceProxy.getCommunalSurface(request.mHostToken, request.mWidth, request.mHeight,
request.mDisplayId, new ICommunalSurfaceCallback.Stub() {
@Override
public void onSurface(
SurfaceControlViewHost.SurfacePackage surfacePackage) {

View File

@@ -37,6 +37,7 @@ import com.android.systemui.util.ViewController;
import com.google.common.util.concurrent.ListenableFuture;
import java.util.Optional;
import java.util.concurrent.Executor;
/**
@@ -64,6 +65,8 @@ public class CommunalSurfaceViewController extends ViewController<SurfaceView> {
private int mCurrentState;
private Optional<CommunalSourceImpl.Request> mLastRequest = Optional.empty();
// The current in-flight request for a surface package.
private ListenableFuture<SurfaceControlViewHost.SurfacePackage> mCurrentSurfaceFuture;
@@ -100,6 +103,9 @@ public class CommunalSurfaceViewController extends ViewController<SurfaceView> {
mSurfaceViewTouchableRegion.set(left, top + topMargin, right, bottom - bottomMargin);
updateTouchExclusion();
// Trigger showing (or hiding) surface based on new dimensions.
showSurface();
}
};
@@ -149,7 +155,7 @@ public class CommunalSurfaceViewController extends ViewController<SurfaceView> {
mCurrentState = newState;
showSurface(newState == STATE_CAN_SHOW_SURFACE);
showSurface();
updateTouchExclusion();
}
@@ -167,25 +173,34 @@ public class CommunalSurfaceViewController extends ViewController<SurfaceView> {
}
}
private void showSurface(boolean show) {
private void showSurface() {
mView.setWillNotDraw(false);
if (!show) {
if (mCurrentState != STATE_CAN_SHOW_SURFACE) {
// If the surface is no longer showing, cancel any in-flight requests.
if (mCurrentSurfaceFuture != null) {
mCurrentSurfaceFuture.cancel(true);
mCurrentSurfaceFuture = null;
}
mLastRequest = Optional.empty();
mView.setWillNotDraw(true);
return;
}
final CommunalSourceImpl.Request request = new CommunalSourceImpl.Request(
mView.getMeasuredWidth(), mView.getMeasuredHeight(),
mView.getDisplay().getDisplayId(), mView.getHostToken());
if (mLastRequest.isPresent() && mLastRequest.get().equals(request)) {
return;
}
mLastRequest = Optional.of(request);
// Since this method is only called when the state has changed, mCurrentSurfaceFuture should
// be null here.
mCurrentSurfaceFuture = mSource.requestCommunalSurface(mView.getHostToken(),
mView.getDisplay().getDisplayId(), mView.getMeasuredWidth(),
mView.getMeasuredHeight());
mCurrentSurfaceFuture = mSource.requestCommunalSurface(request);
mCurrentSurfaceFuture.addListener(new Runnable() {
@Override

View File

@@ -18,9 +18,9 @@ package com.android.systemui.communal.service;
import static org.junit.Assert.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -136,7 +136,7 @@ public class CommunalSurfaceViewControllerTest extends SysuiTestCase {
mPackageFuture = SettableFuture.create();
when(mCommunalSource.requestCommunalSurface(any(), anyInt(), anyInt(), anyInt()))
when(mCommunalSource.requestCommunalSurface(any()))
.thenReturn(mPackageFuture);
}
@@ -144,19 +144,20 @@ public class CommunalSurfaceViewControllerTest extends SysuiTestCase {
public void testSetSurfacePackage() {
// There should be no requests without the proper state.
verify(mCommunalSource, times(0))
.requestCommunalSurface(any(), anyInt(), anyInt(), anyInt());
.requestCommunalSurface(any());
// The full state must be present to make a request.
mController.onViewAttached();
verify(mCommunalSource, times(0))
.requestCommunalSurface(any(), anyInt(), anyInt(), anyInt());
.requestCommunalSurface(any());
clearInvocations(mSurfaceView);
// Request surface view once all conditions are met.
mCallback.surfaceCreated(mSurfaceHolder);
verify(mCommunalSource)
.requestCommunalSurface(mHostToken, DISPLAY_ID, MEASURED_WIDTH, MEASURED_HEIGHT);
final CommunalSourceImpl.Request expectedRequest = new CommunalSourceImpl.Request(
MEASURED_WIDTH, MEASURED_HEIGHT, DISPLAY_ID, mHostToken);
verify(mCommunalSource).requestCommunalSurface(eq(expectedRequest));
when(mSurfaceView.isAttachedToWindow()).thenReturn(true);
@@ -216,8 +217,9 @@ public class CommunalSurfaceViewControllerTest extends SysuiTestCase {
mFakeExecutor.runAllReady();
clearInvocations(mSurfaceView);
verify(mCommunalSource, times(1))
.requestCommunalSurface(mHostToken, DISPLAY_ID, MEASURED_WIDTH, MEASURED_HEIGHT);
final CommunalSourceImpl.Request expectedRequest = new CommunalSourceImpl.Request(
MEASURED_WIDTH, MEASURED_HEIGHT, DISPLAY_ID, mHostToken);
verify(mCommunalSource, times(1)).requestCommunalSurface(eq(expectedRequest));
mController.onViewDetached();
assertTrue(mPackageFuture.isCancelled());
@@ -270,4 +272,31 @@ public class CommunalSurfaceViewControllerTest extends SysuiTestCase {
verify(mNotificationShadeWindowController)
.setTouchExclusionRegion(eq(new Region()));
}
@Test
public void testLayoutChange() {
final int left = 0;
final int top = 0;
final int right = 200;
final int bottom = 100;
givenSurfacePresent();
// Layout change should trigger a request to get new communal surface.
mLayoutChangeListener.onLayoutChange(mSurfaceView, left, top, right, bottom, 0, 0, 0,
0);
// Note that the measured are preset and different than the layout input.
final CommunalSourceImpl.Request expectedRequest =
new CommunalSourceImpl.Request(MEASURED_WIDTH, MEASURED_HEIGHT, DISPLAY_ID,
mHostToken);
verify(mCommunalSource)
.requestCommunalSurface(eq(expectedRequest));
clearInvocations(mCommunalSource);
// Subsequent matching layout change should not trigger any request.
mLayoutChangeListener.onLayoutChange(mSurfaceView, left, top, right, bottom, 0, 0, 0,
0);
verify(mCommunalSource, never()).requestCommunalSurface(any());
}
}