Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 67cc7f81 authored by Dominik Laskowski's avatar Dominik Laskowski
Browse files

Fix signedness of DisplayAddress.Physical ports 

Interpret port as unsigned byte for consistency with uint8_t in SF/IF,
and clarify signedness in doc comments.

Bug: 143713778
Test: adb shell dumpsys display | grep physicalPort
Test: LocalDisplayAdapterTest
Test: DisplayWindowSettingsTests
Change-Id: I56ce0067367372a3a747fde783a1956975e833a0
parent 94f8c28e

core/java/android/hardware/display/DisplayViewport.java

+5 −2
Original line number Diff line number Diff line
@@ -134,7 +134,9 @@ public final class DisplayViewport { 
        result += prime * result + deviceWidth;

        result += prime * result + deviceHeight;

        result += prime * result + uniqueId.hashCode();

        result += prime * result + physicalPort;

        if (physicalPort != null) {

            result += prime * result + physicalPort.hashCode();

        }

        result += prime * result + type;

        return result;

    }
@@ -142,11 +144,12 @@ public final class DisplayViewport { 
    // For debugging purposes.

    @Override

    public String toString() {

        final Integer port = physicalPort == null ? null : Byte.toUnsignedInt(physicalPort);

        return "DisplayViewport{type=" + typeToString(type)

                + ", valid=" + valid

                + ", displayId=" + displayId

                + ", uniqueId='" + uniqueId + "'"

                + ", physicalPort=" + physicalPort

                + ", physicalPort=" + port

                + ", orientation=" + orientation

                + ", logicalFrame=" + logicalFrame

                + ", physicalFrame=" + physicalFrame

core/java/android/view/DisplayAddress.java

+31 −3
Original line number Diff line number Diff line
@@ -40,6 +40,18 @@ public abstract class DisplayAddress implements Parcelable { 
        return new Physical(physicalDisplayId);

    }



    /**

     * Creates an address for a physical display given its port and model.

     *

     * @param port A port in the range [0, 255] interpreted as signed.

     * @param model A positive integer, or {@code null} if the model cannot be identified.

     * @return The {@link Physical} address.

     */

    @NonNull

    public static Physical fromPortAndModel(byte port, Long model) {

        return new Physical(port, model);

    }



    /**

     * Creates an address for a network display given its MAC address.

     *
@@ -64,12 +76,23 @@ public abstract class DisplayAddress implements Parcelable { 
    public static final class Physical extends DisplayAddress {

        private static final long UNKNOWN_MODEL = 0;

        private static final int MODEL_SHIFT = 8;

        private static final int PORT_MASK = 0xFF;



        private final long mPhysicalDisplayId;



        /**

         * Stable display ID combining port and model.

         *

         * @return An ID in the range [0, 2^64) interpreted as signed.

         * @see SurfaceControl#getPhysicalDisplayIds

         */

        public long getPhysicalDisplayId() {

            return mPhysicalDisplayId;

        }



        /**

         * Physical port to which the display is connected.

         *

         * @return A port in the range [0, 255] interpreted as signed.

         */

        public byte getPort() {

            return (byte) mPhysicalDisplayId;
@@ -78,7 +101,7 @@ public abstract class DisplayAddress implements Parcelable { 
        /**

         * Model identifier unique across manufacturers.

         *

         * @return The model ID, or {@code null} if the model cannot be identified.

         * @return A positive integer, or {@code null} if the model cannot be identified.

         */

        @Nullable

        public Long getModel() {
@@ -95,7 +118,7 @@ public abstract class DisplayAddress implements Parcelable { 
        @Override

        public String toString() {

            final StringBuilder builder = new StringBuilder("{")

                    .append("port=").append(getPort() & PORT_MASK);

                    .append("port=").append(Byte.toUnsignedInt(getPort()));



            final Long model = getModel();

            if (model != null) {
@@ -119,6 +142,11 @@ public abstract class DisplayAddress implements Parcelable { 
            mPhysicalDisplayId = physicalDisplayId;

        }



        private Physical(byte port, Long model) {

            mPhysicalDisplayId = Byte.toUnsignedLong(port)

                    | (model == null ? UNKNOWN_MODEL : (model << MODEL_SHIFT));

        }



        public static final @NonNull Parcelable.Creator<Physical> CREATOR =

                new Parcelable.Creator<Physical>() {

                    @Override

services/core/java/com/android/server/display/LocalDisplayAdapter.java

+1 −1
Original line number Diff line number Diff line
@@ -808,7 +808,7 @@ final class LocalDisplayAdapter extends DisplayAdapter { 
            int[] ports = res.getIntArray(

                    com.android.internal.R.array.config_localPrivateDisplayPorts);

            if (ports != null) {

                int port = physicalAddress.getPort();

                int port = Byte.toUnsignedInt(physicalAddress.getPort());

                for (int p : ports) {

                    if (p == port) {

                        return true;

services/core/java/com/android/server/wm/DisplayWindowSettings.java

+2 −1
Original line number Diff line number Diff line
@@ -644,7 +644,8 @@ class DisplayWindowSettings { 
        if (mIdentifier == IDENTIFIER_PORT && displayInfo.address != null) {

            // Config suggests using port as identifier for physical displays.

            if (displayInfo.address instanceof DisplayAddress.Physical) {

                return "port:" + ((DisplayAddress.Physical) displayInfo.address).getPort();

                byte port = ((DisplayAddress.Physical) displayInfo.address).getPort();

                return "port:" + Byte.toUnsignedInt(port);

            }

        }

        return displayInfo.uniqueId;

services/tests/mockingservicestests/src/com/android/server/display/LocalDisplayAdapterTest.java

+38 −41
Original line number Diff line number Diff line
@@ -53,9 +53,12 @@ import java.util.LinkedList; 
@SmallTest

@RunWith(AndroidJUnit4.class)

public class LocalDisplayAdapterTest {

    private static final long HANDLER_WAIT_MS = 100;

    private static final Long DISPLAY_MODEL = Long.valueOf(0xAAAAAAAAL);

    private static final int PORT_A = 0;

    private static final int PORT_B = 0x80;

    private static final int PORT_C = 0xFF;



    private static final int PHYSICAL_DISPLAY_ID_MODEL_SHIFT = 8;

    private static final long HANDLER_WAIT_MS = 100;



    private StaticMockitoSession mMockitoSession;
@@ -74,7 +77,7 @@ public class LocalDisplayAdapterTest { 


    private TestListener mListener = new TestListener();



    private LinkedList<Long> mDisplayIds = new LinkedList<>();

    private LinkedList<DisplayAddress.Physical> mAddresses = new LinkedList<>();



    @Before

    public void setUp() throws Exception {
@@ -106,30 +109,22 @@ public class LocalDisplayAdapterTest { 
     */

    @Test

    public void testPrivateDisplay() throws Exception {

        // needs default one always

        final long displayId0 = 0;

        setUpDisplay(new DisplayConfig(displayId0, createDummyDisplayInfo()));

        final long displayId1 = 1;

        setUpDisplay(new DisplayConfig(displayId1, createDummyDisplayInfo()));

        final long displayId2 = 2;

        setUpDisplay(new DisplayConfig(displayId2, createDummyDisplayInfo()));

        setUpDisplay(new DisplayConfig(createDisplayAddress(PORT_A), createDummyDisplayInfo()));

        setUpDisplay(new DisplayConfig(createDisplayAddress(PORT_B), createDummyDisplayInfo()));

        setUpDisplay(new DisplayConfig(createDisplayAddress(PORT_C), createDummyDisplayInfo()));

        updateAvailableDisplays();

        // display 1 should be marked as private while display 2 is not.

        doReturn(new int[]{(int) displayId1}).when(mMockedResources)

        doReturn(new int[]{ PORT_B }).when(mMockedResources)

                .getIntArray(com.android.internal.R.array.config_localPrivateDisplayPorts);

        mAdapter.registerLocked();



        waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);



        // This should be public

        assertDisplay(mListener.addedDisplays.get(0).getDisplayDeviceInfoLocked(), displayId0,

                false);

        assertDisplay(mListener.addedDisplays.get(0).getDisplayDeviceInfoLocked(), PORT_A, false);

        // This should be private

        assertDisplay(mListener.addedDisplays.get(1).getDisplayDeviceInfoLocked(), displayId1,

                true);

        assertDisplay(mListener.addedDisplays.get(1).getDisplayDeviceInfoLocked(), PORT_B, true);

        // This should be public

        assertDisplay(mListener.addedDisplays.get(2).getDisplayDeviceInfoLocked(), displayId2,

                false);

        assertDisplay(mListener.addedDisplays.get(2).getDisplayDeviceInfoLocked(), PORT_C, false);

    }



    /**
@@ -137,11 +132,8 @@ public class LocalDisplayAdapterTest { 
     */

    @Test

    public void testPublicDisplaysForNoConfigLocalPrivateDisplayPorts() throws Exception {

        // needs default one always

        final long displayId0 = 0;

        setUpDisplay(new DisplayConfig(displayId0, createDummyDisplayInfo()));

        final long displayId1 = 1;

        setUpDisplay(new DisplayConfig(displayId1, createDummyDisplayInfo()));

        setUpDisplay(new DisplayConfig(createDisplayAddress(PORT_A), createDummyDisplayInfo()));

        setUpDisplay(new DisplayConfig(createDisplayAddress(PORT_C), createDummyDisplayInfo()));

        updateAvailableDisplays();

        // config_localPrivateDisplayPorts is null

        mAdapter.registerLocked();
@@ -149,35 +141,36 @@ public class LocalDisplayAdapterTest { 
        waitForHandlerToComplete(mHandler, HANDLER_WAIT_MS);



        // This should be public

        assertDisplay(mListener.addedDisplays.get(0).getDisplayDeviceInfoLocked(), displayId0,

                false);

        assertDisplay(mListener.addedDisplays.get(0).getDisplayDeviceInfoLocked(), PORT_A, false);

        // This should be public

        assertDisplay(mListener.addedDisplays.get(1).getDisplayDeviceInfoLocked(), displayId1,

                false);

        assertDisplay(mListener.addedDisplays.get(1).getDisplayDeviceInfoLocked(), PORT_C, false);

    }



    private void assertDisplay(DisplayDeviceInfo info, long expectedPort, boolean shouldBePrivate) {

        DisplayAddress.Physical physical = (DisplayAddress.Physical) info.address;

        assertNotNull(physical);

        assertEquals(expectedPort, physical.getPort());

    private static void assertDisplay(

            DisplayDeviceInfo info, int expectedPort, boolean shouldBePrivate) {

        final DisplayAddress.Physical address = (DisplayAddress.Physical) info.address;

        assertNotNull(address);

        assertEquals((byte) expectedPort, address.getPort());

        assertEquals(DISPLAY_MODEL, address.getModel());

        assertEquals(shouldBePrivate, (info.flags & DisplayDeviceInfo.FLAG_PRIVATE) != 0);

    }



    private class DisplayConfig {

        public final long displayId;

        public final DisplayAddress.Physical address;

        public final IBinder displayToken = new Binder();

        public final SurfaceControl.PhysicalDisplayInfo displayInfo;



        private DisplayConfig(long displayId, SurfaceControl.PhysicalDisplayInfo displayInfo) {

            this.displayId = displayId | (0x1 << PHYSICAL_DISPLAY_ID_MODEL_SHIFT);

        private DisplayConfig(

                DisplayAddress.Physical address, SurfaceControl.PhysicalDisplayInfo displayInfo) {

            this.address = address;

            this.displayInfo = displayInfo;

        }

    }



    private void setUpDisplay(DisplayConfig config) {

        mDisplayIds.add(config.displayId);

        doReturn(config.displayToken).when(

                () -> SurfaceControl.getPhysicalDisplayToken(config.displayId));

        mAddresses.add(config.address);

        doReturn(config.displayToken).when(() ->

                SurfaceControl.getPhysicalDisplayToken(config.address.getPhysicalDisplayId()));

        doReturn(new SurfaceControl.PhysicalDisplayInfo[]{

                config.displayInfo

        }).when(() -> SurfaceControl.getDisplayConfigs(config.displayToken));
@@ -192,16 +185,20 @@ public class LocalDisplayAdapterTest { 
    }



    private void updateAvailableDisplays() {

        long[] ids = new long[mDisplayIds.size()];

        long[] ids = new long[mAddresses.size()];

        int i = 0;

        for (long id : mDisplayIds) {

            ids[i] = id;

        for (DisplayAddress.Physical address : mAddresses) {

            ids[i] = address.getPhysicalDisplayId();

            i++;

        }

        doReturn(ids).when(() -> SurfaceControl.getPhysicalDisplayIds());

    }



    private SurfaceControl.PhysicalDisplayInfo createDummyDisplayInfo() {

    private static DisplayAddress.Physical createDisplayAddress(int port) {

        return DisplayAddress.fromPortAndModel((byte) port, DISPLAY_MODEL);

    }



    private static SurfaceControl.PhysicalDisplayInfo createDummyDisplayInfo() {

        SurfaceControl.PhysicalDisplayInfo info = new SurfaceControl.PhysicalDisplayInfo();

        info.density = 100;

        info.xDpi = 100;