From bed7ef2e396726af0782d5c5dbcb6c5062e0d0b4 Mon Sep 17 00:00:00 2001 From: Tiger Date: Wed, 26 Apr 2023 16:33:19 +0800 Subject: [PATCH] Store the source ID in InsetsFrameProvider This CL stores the source ID but not the owner, the index, and the type in InsetsFrameProvider. In this way, we write less info to the parcel. Also, we won't use the BinderProxy to generate the hash code for the ID, but will use the Binder object directly. Because the hash code obatined from the BinderProxy might change. Fix: 272230840 Fix: 279476217 Test: atest InsetsSourceTest WindowContainerTests Change-Id: I0c6434ff19c33e4bb939dee76f49ccb3322735d2 --- .../android/view/InsetsFrameProvider.java | 59 ++++++++----------- core/java/android/view/InsetsSource.java | 22 ++++++- .../src/android/view/InsetsSourceTest.java | 20 +++++++ .../com/android/server/wm/DisplayPolicy.java | 8 +-- .../com/android/server/wm/InsetsPolicy.java | 7 +-- .../android/server/wm/WindowContainer.java | 6 +- .../server/wm/WindowContainerTests.java | 18 ++---- 7 files changed, 79 insertions(+), 61 deletions(-) diff --git a/core/java/android/view/InsetsFrameProvider.java b/core/java/android/view/InsetsFrameProvider.java index a69af24756a02..470c2801d838e 100644 --- a/core/java/android/view/InsetsFrameProvider.java +++ b/core/java/android/view/InsetsFrameProvider.java @@ -62,9 +62,7 @@ public class InsetsFrameProvider implements Parcelable { */ public static final int SOURCE_ARBITRARY_RECTANGLE = 3; - private final IBinder mOwner; - private final int mIndex; - private final @InsetsType int mType; + private final int mId; /** * The selection of the starting rectangle to be converted into source frame. @@ -122,30 +120,30 @@ public class InsetsFrameProvider implements Parcelable { * @param type the {@link InsetsType}. * @see InsetsSource#createId(Object, int, int) */ - public InsetsFrameProvider(IBinder owner, @IntRange(from = 0, to = 2047) int index, + public InsetsFrameProvider(Object owner, @IntRange(from = 0, to = 2047) int index, @InsetsType int type) { - if (index < 0 || index >= 2048) { - throw new IllegalArgumentException(); - } - - // This throws IllegalArgumentException if the type is not valid. - WindowInsets.Type.indexOf(type); - - mOwner = owner; - mIndex = index; - mType = type; + mId = InsetsSource.createId(owner, index, type); } - public IBinder getOwner() { - return mOwner; + /** + * Returns an unique integer which identifies the insets source. + */ + public int getId() { + return mId; } + /** + * Returns the index specified in {@link #InsetsFrameProvider(IBinder, int, int)}. + */ public int getIndex() { - return mIndex; + return InsetsSource.getIndex(mId); } + /** + * Returns the {@link InsetsType} specified in {@link #InsetsFrameProvider(IBinder, int, int)}. + */ public int getType() { - return mType; + return InsetsSource.getType(mId); } public InsetsFrameProvider setSource(int source) { @@ -211,9 +209,9 @@ public class InsetsFrameProvider implements Parcelable { @Override public String toString() { final StringBuilder sb = new StringBuilder("InsetsFrameProvider: {"); - sb.append("owner=").append(mOwner); - sb.append(", index=").append(mIndex); - sb.append(", type=").append(WindowInsets.Type.toString(mType)); + sb.append("id=#").append(Integer.toHexString(mId)); + sb.append(", index=").append(getIndex()); + sb.append(", type=").append(WindowInsets.Type.toString(getType())); sb.append(", source=").append(sourceToString(mSource)); sb.append(", flags=[").append(InsetsSource.flagsToString(mFlags)).append("]"); if (mInsetsSize != null) { @@ -244,9 +242,7 @@ public class InsetsFrameProvider implements Parcelable { } public InsetsFrameProvider(Parcel in) { - mOwner = in.readStrongBinder(); - mIndex = in.readInt(); - mType = in.readInt(); + mId = in.readInt(); mSource = in.readInt(); mFlags = in.readInt(); mInsetsSize = in.readTypedObject(Insets.CREATOR); @@ -256,9 +252,7 @@ public class InsetsFrameProvider implements Parcelable { @Override public void writeToParcel(Parcel out, int flags) { - out.writeStrongBinder(mOwner); - out.writeInt(mIndex); - out.writeInt(mType); + out.writeInt(mId); out.writeInt(mSource); out.writeInt(mFlags); out.writeTypedObject(mInsetsSize, flags); @@ -267,7 +261,7 @@ public class InsetsFrameProvider implements Parcelable { } public boolean idEquals(InsetsFrameProvider o) { - return Objects.equals(mOwner, o.mOwner) && mIndex == o.mIndex && mType == o.mType; + return mId == o.mId; } @Override @@ -279,8 +273,7 @@ public class InsetsFrameProvider implements Parcelable { return false; } final InsetsFrameProvider other = (InsetsFrameProvider) o; - return Objects.equals(mOwner, other.mOwner) && mIndex == other.mIndex - && mType == other.mType && mSource == other.mSource && mFlags == other.mFlags + return mId == other.mId && mSource == other.mSource && mFlags == other.mFlags && Objects.equals(mInsetsSize, other.mInsetsSize) && Arrays.equals(mInsetsSizeOverrides, other.mInsetsSizeOverrides) && Objects.equals(mArbitraryRectangle, other.mArbitraryRectangle); @@ -288,7 +281,7 @@ public class InsetsFrameProvider implements Parcelable { @Override public int hashCode() { - return Objects.hash(mOwner, mIndex, mType, mSource, mFlags, mInsetsSize, + return Objects.hash(mId, mSource, mFlags, mInsetsSize, Arrays.hashCode(mInsetsSizeOverrides), mArbitraryRectangle); } @@ -319,7 +312,7 @@ public class InsetsFrameProvider implements Parcelable { protected InsetsSizeOverride(Parcel in) { mWindowType = in.readInt(); - mInsetsSize = in.readParcelable(null, Insets.class); + mInsetsSize = in.readTypedObject(Insets.CREATOR); } public InsetsSizeOverride(int windowType, Insets insetsSize) { @@ -354,7 +347,7 @@ public class InsetsFrameProvider implements Parcelable { @Override public void writeToParcel(Parcel out, int flags) { out.writeInt(mWindowType); - out.writeParcelable(mInsetsSize, flags); + out.writeTypedObject(mInsetsSize, flags); } @Override diff --git a/core/java/android/view/InsetsSource.java b/core/java/android/view/InsetsSource.java index bd48771ec4b31..114f4ed04ee65 100644 --- a/core/java/android/view/InsetsSource.java +++ b/core/java/android/view/InsetsSource.java @@ -271,7 +271,7 @@ public class InsetsSource implements Parcelable { * @param index An owner may have multiple sources with the same type. For example, the system * server might have multiple display cutout sources. This is used to identify * which one is which. The value must be in a range of [0, 2047]. - * @param type The {@link WindowInsets.Type.InsetsType type} of the source. + * @param type The {@link InsetsType type} of the source. * @return a unique integer as the identifier. */ public static int createId(Object owner, @IntRange(from = 0, to = 2047) int index, @@ -282,11 +282,29 @@ public class InsetsSource implements Parcelable { // owner takes top 16 bits; // index takes 11 bits since the 6th bit; // type takes bottom 5 bits. - return (((owner != null ? owner.hashCode() : 1) % (1 << 16)) << 16) + return ((System.identityHashCode(owner) % (1 << 16)) << 16) + (index << 5) + WindowInsets.Type.indexOf(type); } + /** + * Gets the index from the ID. + * + * @see #createId(Object, int, int) + */ + public static int getIndex(int id) { + return (id % (1 << 16)) >> 5; + } + + /** + * Gets the {@link InsetsType} from the ID. + * + * @see #createId(Object, int, int) + */ + public static int getType(int id) { + return 1 << (id % 32); + } + public static String flagsToString(@Flags int flags) { final StringJoiner joiner = new StringJoiner(" "); if ((flags & FLAG_SUPPRESS_SCRIM) != 0) { diff --git a/core/tests/coretests/src/android/view/InsetsSourceTest.java b/core/tests/coretests/src/android/view/InsetsSourceTest.java index 6fa8f11173438..55680abb159d7 100644 --- a/core/tests/coretests/src/android/view/InsetsSourceTest.java +++ b/core/tests/coretests/src/android/view/InsetsSourceTest.java @@ -227,5 +227,25 @@ public class InsetsSourceTest { assertEquals(numTotalSources, sources.size()); } + @Test + public void testGetIndex() { + for (int index = 0; index < 2048; index++) { + for (int type = FIRST; type <= LAST; type = type << 1) { + final int id = InsetsSource.createId(null, index, type); + assertEquals(index, InsetsSource.getIndex(id)); + } + } + } + + @Test + public void testGetType() { + for (int index = 0; index < 2048; index++) { + for (int type = FIRST; type <= LAST; type = type << 1) { + final int id = InsetsSource.createId(null, index, type); + assertEquals(type, InsetsSource.getType(id)); + } + } + } + // Parcel and equals already tested via InsetsStateTest } diff --git a/services/core/java/com/android/server/wm/DisplayPolicy.java b/services/core/java/com/android/server/wm/DisplayPolicy.java index ce4362853b23b..be52e5a4566b5 100644 --- a/services/core/java/com/android/server/wm/DisplayPolicy.java +++ b/services/core/java/com/android/server/wm/DisplayPolicy.java @@ -1095,11 +1095,9 @@ public class DisplayPolicy { } else { overrideProviders = null; } - final @InsetsType int type = provider.getType(); - final int id = InsetsSource.createId( - provider.getOwner(), provider.getIndex(), type); - mDisplayContent.getInsetsStateController().getOrCreateSourceProvider(id, type) - .setWindowContainer(win, frameProvider, overrideProviders); + mDisplayContent.getInsetsStateController().getOrCreateSourceProvider( + provider.getId(), provider.getType()).setWindowContainer( + win, frameProvider, overrideProviders); mInsetsSourceWindowsExceptIme.add(win); } } diff --git a/services/core/java/com/android/server/wm/InsetsPolicy.java b/services/core/java/com/android/server/wm/InsetsPolicy.java index fe13b87a079a7..ddf96c53323de 100644 --- a/services/core/java/com/android/server/wm/InsetsPolicy.java +++ b/services/core/java/com/android/server/wm/InsetsPolicy.java @@ -311,16 +311,13 @@ class InsetsPolicy { state.removeSource(ID_IME); } else if (attrs.providedInsets != null) { for (InsetsFrameProvider provider : attrs.providedInsets) { - final int id = InsetsSource.createId( - provider.getOwner(), provider.getIndex(), provider.getType()); - final @InsetsType int type = provider.getType(); - if ((type & WindowInsets.Type.systemBars()) == 0) { + if ((provider.getType() & WindowInsets.Type.systemBars()) == 0) { continue; } if (state == originalState) { state = new InsetsState(state); } - state.removeSource(id); + state.removeSource(provider.getId()); } } diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java index 510e6756b8ef0..f1b77acc72ddd 100644 --- a/services/core/java/com/android/server/wm/WindowContainer.java +++ b/services/core/java/com/android/server/wm/WindowContainer.java @@ -435,8 +435,7 @@ class WindowContainer extends ConfigurationContainer< if (mLocalInsetsSources == null) { mLocalInsetsSources = new SparseArray<>(); } - final int id = InsetsSource.createId( - provider.getOwner(), provider.getIndex(), provider.getType()); + final int id = provider.getId(); if (mLocalInsetsSources.get(id) != null) { if (DEBUG) { Slog.d(TAG, "The local insets source for this " + provider @@ -457,8 +456,7 @@ class WindowContainer extends ConfigurationContainer< return; } - final int id = InsetsSource.createId( - provider.getOwner(), provider.getIndex(), provider.getType()); + final int id = provider.getId(); if (mLocalInsetsSources.get(id) == null) { if (DEBUG) { Slog.d(TAG, "Given " + provider + " doesn't have a local insets source."); diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java index ad606cb908413..2d8ddfa47300c 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowContainerTests.java @@ -1443,10 +1443,8 @@ public class WindowContainerTests extends WindowTestsBase { final InsetsFrameProvider provider2 = new InsetsFrameProvider(null, 2, WindowInsets.Type.systemOverlays()) .setArbitraryRectangle(genericOverlayInsetsRect2); - final int sourceId1 = InsetsSource.createId( - provider1.getOwner(), provider1.getIndex(), provider1.getType()); - final int sourceId2 = InsetsSource.createId( - provider2.getOwner(), provider2.getIndex(), provider2.getType()); + final int sourceId1 = provider1.getId(); + final int sourceId2 = provider2.getId(); rootTask.addLocalInsetsFrameProvider(provider1); container.addLocalInsetsFrameProvider(provider2); @@ -1504,10 +1502,8 @@ public class WindowContainerTests extends WindowTestsBase { final InsetsFrameProvider provider2 = new InsetsFrameProvider(null, 1, WindowInsets.Type.systemOverlays()) .setArbitraryRectangle(genericOverlayInsetsRect2); - final int sourceId1 = InsetsSource.createId( - provider1.getOwner(), provider1.getIndex(), provider1.getType()); - final int sourceId2 = InsetsSource.createId( - provider2.getOwner(), provider2.getIndex(), provider2.getType()); + final int sourceId1 = provider1.getId(); + final int sourceId2 = provider2.getId(); rootTask.addLocalInsetsFrameProvider(provider1); activity0.forAllWindows(window -> { @@ -1566,10 +1562,8 @@ public class WindowContainerTests extends WindowTestsBase { final InsetsFrameProvider provider2 = new InsetsFrameProvider(null, 2, WindowInsets.Type.systemOverlays()) .setArbitraryRectangle(navigationBarInsetsRect2); - final int sourceId1 = InsetsSource.createId( - provider1.getOwner(), provider1.getIndex(), provider1.getType()); - final int sourceId2 = InsetsSource.createId( - provider2.getOwner(), provider2.getIndex(), provider2.getType()); + final int sourceId1 = provider1.getId(); + final int sourceId2 = provider2.getId(); rootTask.addLocalInsetsFrameProvider(provider1); container.addLocalInsetsFrameProvider(provider2);