Only create InsetsSourceConsumer for InsetsSourceControl

Previously, the insets source consumer is only created if the insets
source exists. However, if a control target is not below an insets
source, the target won't receive the source, and the consumer won't be
created, and the insets cannot be controlled.

Also, InsetsSourceConsumer only works when the control exists. So we
don't have to create redundant consumers for types not controllable.

This CL only creates the insets source consumer for the control, not for
the source. This fixes the z-order problem.

Fix: 290884763
Bug: 234093736
Test: atest InsetsControllerTest InsetsSourceConsumerTest
      ImeInsetsSourceConsumerTest
Test: Watch a video in Google TV. Long press an icon on taskbar before
      taskbar fades out. See if taskbar is visible.
(cherry picked from commit aa27825aff00b69d0a77db6143c52acfa22a41a6)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:28fc69cb89c8090ea8dabbeb2ebf3b5baeedd1f1)
Merged-In: I16a854d52f46d36899c826426afc5ebbcf47b586
Change-Id: I16a854d52f46d36899c826426afc5ebbcf47b586

4 files changed, 58 insertions, 57 deletions

diff --git a/core/java/android/view/InsetsController.java b/core/java/android/view/InsetsController.java
index 5019b85ca503..a3d1be07c51c 100644
--- a/core/java/android/view/InsetsController.java
+++ b/core/java/android/view/InsetsController.java
@@ -69,6 +69,7 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.graphics.SfVsyncFrameCallbackProvider;
 import com.android.internal.inputmethod.ImeTracing;
 import com.android.internal.inputmethod.SoftInputShowHideReason;
+import com.android.internal.util.function.TriFunction;
 
 import java.io.PrintWriter;
 import java.lang.annotation.Retention;
@@ -77,7 +78,6 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
-import java.util.function.BiFunction;
 
 /**
  * Implements {@link WindowInsetsController} on the client.
@@ -621,7 +621,8 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
     private final InsetsState mLastDispatchedState = new InsetsState();
 
     private final Rect mFrame = new Rect();
-    private final BiFunction<InsetsController, InsetsSource, InsetsSourceConsumer> mConsumerCreator;
+    private final TriFunction<InsetsController, Integer, Integer, InsetsSourceConsumer>
+            mConsumerCreator;
     private final SparseArray<InsetsSourceConsumer> mSourceConsumers = new SparseArray<>();
     private final InsetsSourceConsumer mImeSourceConsumer;
     private final Host mHost;
@@ -689,13 +690,6 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
 
                     // Don't change the indexes of the sources while traversing. Remove it later.
                     mPendingRemoveIndexes.add(index1);
-
-                    // Remove the consumer as well except the IME one. IME consumer should always
-                    // be there since we need to communicate with InputMethodManager no matter we
-                    // have the source or not.
-                    if (source1.getType() != ime()) {
-                        mSourceConsumers.remove(source1.getId());
-                    }
                 }
 
                 @Override
@@ -750,12 +744,12 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
             };
 
     public InsetsController(Host host) {
-        this(host, (controller, source) -> {
-            if (source.getType() == ime()) {
-                return new ImeInsetsSourceConsumer(source.getId(), controller.mState,
+        this(host, (controller, id, type) -> {
+            if (type == ime()) {
+                return new ImeInsetsSourceConsumer(id, controller.mState,
                         Transaction::new, controller);
             } else {
-                return new InsetsSourceConsumer(source.getId(), source.getType(), controller.mState,
+                return new InsetsSourceConsumer(id, type, controller.mState,
                         Transaction::new, controller);
             }
         }, host.getHandler());
@@ -763,7 +757,7 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
 
     @VisibleForTesting
     public InsetsController(Host host,
-            BiFunction<InsetsController, InsetsSource, InsetsSourceConsumer> consumerCreator,
+            TriFunction<InsetsController, Integer, Integer, InsetsSourceConsumer> consumerCreator,
             Handler handler) {
         mHost = host;
         mConsumerCreator = consumerCreator;
@@ -815,7 +809,7 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
         };
 
         // Make mImeSourceConsumer always non-null.
-        mImeSourceConsumer = getSourceConsumer(new InsetsSource(ID_IME, ime()));
+        mImeSourceConsumer = getSourceConsumer(ID_IME, ime());
     }
 
     @VisibleForTesting
@@ -893,7 +887,12 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
                     cancelledUserAnimationTypes[0] |= type;
                 }
             }
-            getSourceConsumer(source).updateSource(source, animationType);
+            final InsetsSourceConsumer consumer = mSourceConsumers.get(source.getId());
+            if (consumer != null) {
+                consumer.updateSource(source, animationType);
+            } else {
+                mState.addSource(source);
+            }
             existingTypes |= type;
             if (source.isVisible()) {
                 visibleTypes |= type;
@@ -997,8 +996,8 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
 
         @InsetsType int controllableTypes = 0;
         int consumedControlCount = 0;
-        final int[] showTypes = new int[1];
-        final int[] hideTypes = new int[1];
+        final @InsetsType int[] showTypes = new int[1];
+        final @InsetsType int[] hideTypes = new int[1];
 
         // Ensure to update all existing source consumers
         for (int i = mSourceConsumers.size() - 1; i >= 0; i--) {
@@ -1014,15 +1013,12 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
             consumer.setControl(control, showTypes, hideTypes);
         }
 
+        // Ensure to create source consumers if not available yet.
         if (consumedControlCount != mTmpControlArray.size()) {
-            // Whoops! The server sent us some controls without sending corresponding sources.
             for (int i = mTmpControlArray.size() - 1; i >= 0; i--) {
                 final InsetsSourceControl control = mTmpControlArray.valueAt(i);
-                final InsetsSourceConsumer consumer = mSourceConsumers.get(control.getId());
-                if (consumer == null) {
-                    control.release(SurfaceControl::release);
-                    Log.e(TAG, control + " has no consumer.");
-                }
+                getSourceConsumer(control.getId(), control.getType())
+                        .setControl(control, showTypes, hideTypes);
             }
         }
 
@@ -1587,6 +1583,11 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
         if (type == ime()) {
             abortPendingImeControlRequest();
         }
+        if (consumer.getType() != ime()) {
+            // IME consumer should always be there since we need to communicate with
+            // InputMethodManager no matter we have the control or not.
+            mSourceConsumers.remove(consumer.getId());
+        }
     }
 
     private void cancelAnimation(InsetsAnimationControlRunner control, boolean invokeCallback) {
@@ -1640,21 +1641,20 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
     }
 
     @VisibleForTesting
-    public @NonNull InsetsSourceConsumer getSourceConsumer(InsetsSource source) {
-        final int sourceId = source.getId();
-        InsetsSourceConsumer consumer = mSourceConsumers.get(sourceId);
+    public @NonNull InsetsSourceConsumer getSourceConsumer(int id, int type) {
+        InsetsSourceConsumer consumer = mSourceConsumers.get(id);
         if (consumer != null) {
             return consumer;
         }
-        if (source.getType() == ime() && mImeSourceConsumer != null) {
+        if (type == ime() && mImeSourceConsumer != null) {
             // WindowInsets.Type.ime() should be only provided by one source.
             mSourceConsumers.remove(mImeSourceConsumer.getId());
             consumer = mImeSourceConsumer;
-            consumer.setId(sourceId);
+            consumer.setId(id);
         } else {
-            consumer = mConsumerCreator.apply(this, source);
+            consumer = mConsumerCreator.apply(this, id, type);
         }
-        mSourceConsumers.put(sourceId, consumer);
+        mSourceConsumers.put(id, consumer);
         return consumer;
     }
 
@@ -1663,8 +1663,7 @@ public class InsetsController implements WindowInsetsController, InsetsAnimation
         return mImeSourceConsumer;
     }
 
-    @VisibleForTesting
-    public void notifyVisibilityChanged() {
+    void notifyVisibilityChanged() {
         mHost.notifyInsetsChanged();
     }
 
diff --git a/core/java/android/view/InsetsSourceConsumer.java b/core/java/android/view/InsetsSourceConsumer.java
index 467d720196b1..34b288460a67 100644
--- a/core/java/android/view/InsetsSourceConsumer.java
+++ b/core/java/android/view/InsetsSourceConsumer.java
@@ -149,9 +149,12 @@ public class InsetsSourceConsumer {
             // Check if we need to restore server visibility.
             final InsetsSource localSource = mState.peekSource(mId);
             final InsetsSource serverSource = mController.getLastDispatchedState().peekSource(mId);
-            if (localSource != null && serverSource != null
-                    && localSource.isVisible() != serverSource.isVisible()) {
-                localSource.setVisible(serverSource.isVisible());
+            final boolean localVisible = localSource != null && localSource.isVisible();
+            final boolean serverVisible = serverSource != null && serverSource.isVisible();
+            if (localSource != null) {
+                localSource.setVisible(serverVisible);
+            }
+            if (localVisible != serverVisible) {
                 mController.notifyVisibilityChanged();
             }
         } else {
diff --git a/core/tests/coretests/src/android/view/InsetsControllerTest.java b/core/tests/coretests/src/android/view/InsetsControllerTest.java
index 06920524acfc..b8f0d5c82eac 100644
--- a/core/tests/coretests/src/android/view/InsetsControllerTest.java
+++ b/core/tests/coretests/src/android/view/InsetsControllerTest.java
@@ -131,10 +131,10 @@ public class InsetsControllerTest {
             mTestClock = new OffsettableClock();
             mTestHandler = new TestHandler(null, mTestClock);
             mTestHost = spy(new TestHost(mViewRoot));
-            mController = new InsetsController(mTestHost, (controller, source) -> {
-                if (source.getType() == ime()) {
-                    return new InsetsSourceConsumer(source.getId(), source.getType(),
-                            controller.getState(), Transaction::new, controller) {
+            mController = new InsetsController(mTestHost, (controller, id, type) -> {
+                if (type == ime()) {
+                    return new InsetsSourceConsumer(id, type, controller.getState(),
+                            Transaction::new, controller) {
 
                         private boolean mImeRequestedShow;
 
@@ -150,8 +150,8 @@ public class InsetsControllerTest {
                         }
                     };
                 } else {
-                    return new InsetsSourceConsumer(source.getId(), source.getType(),
-                            controller.getState(), Transaction::new, controller);
+                    return new InsetsSourceConsumer(id, type, controller.getState(),
+                            Transaction::new, controller);
                 }
             }, mTestHandler);
             final Rect rect = new Rect(5, 5, 5, 5);
@@ -182,7 +182,8 @@ public class InsetsControllerTest {
     @Test
     public void testControlsChanged() {
         mController.onControlsChanged(createSingletonControl(ID_STATUS_BAR, statusBars()));
-        assertNotNull(mController.getSourceConsumer(mStatusSource).getControl().getLeash());
+        assertNotNull(
+                mController.getSourceConsumer(ID_STATUS_BAR, statusBars()).getControl().getLeash());
         mController.addOnControllableInsetsChangedListener(
                 ((controller, typeMask) -> assertEquals(statusBars(), typeMask)));
     }
@@ -194,7 +195,7 @@ public class InsetsControllerTest {
         mController.addOnControllableInsetsChangedListener(listener);
         mController.onControlsChanged(createSingletonControl(ID_STATUS_BAR, statusBars()));
         mController.onControlsChanged(new InsetsSourceControl[0]);
-        assertNull(mController.getSourceConsumer(mStatusSource).getControl());
+        assertNull(mController.getSourceConsumer(ID_STATUS_BAR, statusBars()).getControl());
         InOrder inOrder = Mockito.inOrder(listener);
         inOrder.verify(listener).onControllableInsetsChanged(eq(mController), eq(0));
         inOrder.verify(listener).onControllableInsetsChanged(eq(mController), eq(statusBars()));
@@ -254,7 +255,7 @@ public class InsetsControllerTest {
         // only the original thread that created view hierarchy can touch its views
         InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
             mController.setSystemDrivenInsetsAnimationLoggingListener(loggingListener);
-            mController.getSourceConsumer(mImeSource).onWindowFocusGained(true);
+            mController.getSourceConsumer(ID_IME, ime()).onWindowFocusGained(true);
             // since there is no focused view, forcefully make IME visible.
             mController.show(WindowInsets.Type.ime(), true /* fromIme */, null /* statsToken */);
             // When using the animation thread, this must not invoke onReady()
@@ -271,7 +272,7 @@ public class InsetsControllerTest {
         prepareControls();
 
         InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
-            mController.getSourceConsumer(mImeSource).onWindowFocusGained(true);
+            mController.getSourceConsumer(ID_IME, ime()).onWindowFocusGained(true);
             // since there is no focused view, forcefully make IME visible.
             mController.show(WindowInsets.Type.ime(), true /* fromIme */, null /* statsToken */);
             mController.show(all());
@@ -284,7 +285,7 @@ public class InsetsControllerTest {
             mController.hide(all());
             mController.cancelExistingAnimations();
             assertEquals(0, mController.getRequestedVisibleTypes() & types);
-            mController.getSourceConsumer(mImeSource).onWindowFocusLost();
+            mController.getSourceConsumer(ID_IME, ime()).onWindowFocusLost();
         });
         InstrumentationRegistry.getInstrumentation().waitForIdleSync();
     }
@@ -294,14 +295,14 @@ public class InsetsControllerTest {
         InsetsSourceControl ime = createControl(ID_IME, ime());
         mController.onControlsChanged(new InsetsSourceControl[] { ime });
         InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
-            mController.getSourceConsumer(mImeSource).onWindowFocusGained(true);
+            mController.getSourceConsumer(ID_IME, ime()).onWindowFocusGained(true);
             mController.show(WindowInsets.Type.ime(), true /* fromIme */, null /* statsToken */);
             mController.cancelExistingAnimations();
             assertTrue(isRequestedVisible(mController, ime()));
             mController.hide(ime(), true /* fromIme */, null /* statsToken */);
             mController.cancelExistingAnimations();
             assertFalse(isRequestedVisible(mController, ime()));
-            mController.getSourceConsumer(mImeSource).onWindowFocusLost();
+            mController.getSourceConsumer(ID_IME, ime()).onWindowFocusLost();
         });
         InstrumentationRegistry.getInstrumentation().waitForIdleSync();
     }
@@ -914,7 +915,7 @@ public class InsetsControllerTest {
             // Simulate IME insets is not controllable
             mController.onControlsChanged(new InsetsSourceControl[0]);
             final InsetsSourceConsumer imeInsetsConsumer =
-                    mController.getSourceConsumer(mImeSource);
+                    mController.getSourceConsumer(ID_IME, ime());
             assertNull(imeInsetsConsumer.getControl());
 
             // Verify IME requested visibility should be updated to IME consumer from controller.
diff --git a/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java b/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java
index 988e69008008..655cb4519d3c 100644
--- a/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java
+++ b/core/tests/coretests/src/android/view/InsetsSourceConsumerTest.java
@@ -219,10 +219,10 @@ public class InsetsSourceConsumerTest {
         InstrumentationRegistry.getInstrumentation().runOnMainSync(() -> {
             InsetsState state = new InsetsState();
             ViewRootInsetsControllerHost host = new ViewRootInsetsControllerHost(mViewRoot);
-            InsetsController insetsController = new InsetsController(host, (controller, source) -> {
-                if (source.getType() == ime()) {
+            InsetsController insetsController = new InsetsController(host, (ic, id, type) -> {
+                if (type == ime()) {
                     return new InsetsSourceConsumer(ID_IME, ime(), state,
-                            () -> mMockTransaction, controller) {
+                            () -> mMockTransaction, ic) {
                         @Override
                         public int requestShow(boolean fromController,
                                 ImeTracker.Token statsToken) {
@@ -230,11 +230,9 @@ public class InsetsSourceConsumerTest {
                         }
                     };
                 }
-                return new InsetsSourceConsumer(source.getId(), source.getType(),
-                        controller.getState(), Transaction::new, controller);
+                return new InsetsSourceConsumer(id, type, ic.getState(), Transaction::new, ic);
             }, host.getHandler());
-            InsetsSource imeSource = new InsetsSource(ID_IME, ime());
-            InsetsSourceConsumer imeConsumer = insetsController.getSourceConsumer(imeSource);
+            InsetsSourceConsumer imeConsumer = insetsController.getSourceConsumer(ID_IME, ime());
 
             // Initial IME insets source control with its leash.
             imeConsumer.setControl(new InsetsSourceControl(ID_IME, ime(), mLeash,