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
diff --git a/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSourceImpl.java b/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSourceImpl.java
index d8bf2dc..b8070ab 100644
--- a/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSourceImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSourceImpl.java
@@ -38,6 +38,7 @@
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 @@
}
}
+ 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 @@
* 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) {
diff --git a/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java b/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java
index d554b5e..0de5029 100644
--- a/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java
+++ b/packages/SystemUI/src/com/android/systemui/communal/service/CommunalSurfaceViewController.java
@@ -37,6 +37,7 @@
import com.google.common.util.concurrent.ListenableFuture;
+import java.util.Optional;
import java.util.concurrent.Executor;
/**
@@ -64,6 +65,8 @@
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 @@
mSurfaceViewTouchableRegion.set(left, top + topMargin, right, bottom - bottomMargin);
updateTouchExclusion();
+
+ // Trigger showing (or hiding) surface based on new dimensions.
+ showSurface();
}
};
@@ -149,7 +155,7 @@
mCurrentState = newState;
- showSurface(newState == STATE_CAN_SHOW_SURFACE);
+ showSurface();
updateTouchExclusion();
}
@@ -167,25 +173,34 @@
}
}
- 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
diff --git a/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java b/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java
index c2d6b9a..cf2e029 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/communal/service/CommunalSurfaceViewControllerTest.java
@@ -18,9 +18,9 @@
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 @@
mPackageFuture = SettableFuture.create();
- when(mCommunalSource.requestCommunalSurface(any(), anyInt(), anyInt(), anyInt()))
+ when(mCommunalSource.requestCommunalSurface(any()))
.thenReturn(mPackageFuture);
}
@@ -144,19 +144,20 @@
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 @@
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 @@
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());
+ }
}
