Merge "Use sp<> on creating BufferHubBinderService"
diff --git a/libs/ui/Android.bp b/libs/ui/Android.bp
index 7b35409..0a0c8ca 100644
--- a/libs/ui/Android.bp
+++ b/libs/ui/Android.bp
@@ -78,7 +78,7 @@
 
     shared_libs: [
         "android.hardware.graphics.allocator@2.0",
-        "android.hardware.graphics.common@1.1",
+        "android.hardware.graphics.common@1.2",
         "android.hardware.graphics.mapper@2.0",
         "android.hardware.graphics.mapper@2.1",
         "android.hardware.configstore@1.0",
@@ -97,7 +97,7 @@
     ],
 
     export_shared_lib_headers: [
-        "android.hardware.graphics.common@1.1",
+        "android.hardware.graphics.common@1.2",
     ],
 
     static_libs: [
diff --git a/libs/ui/DebugUtils.cpp b/libs/ui/DebugUtils.cpp
index 61df02d..ee06d93 100644
--- a/libs/ui/DebugUtils.cpp
+++ b/libs/ui/DebugUtils.cpp
@@ -234,6 +234,9 @@
         case ColorMode::BT2020:
             return std::string("ColorMode::BT2020");
 
+        case ColorMode::DISPLAY_BT2020:
+            return std::string("ColorMode::DISPLAY_BT2020");
+
         case ColorMode::BT2100_PQ:
             return std::string("ColorMode::BT2100_PQ");
 
diff --git a/libs/ui/include/ui/GraphicTypes.h b/libs/ui/include/ui/GraphicTypes.h
index 0fa819d..1d53ac8 100644
--- a/libs/ui/include/ui/GraphicTypes.h
+++ b/libs/ui/include/ui/GraphicTypes.h
@@ -17,6 +17,7 @@
 #pragma once
 
 #include <android/hardware/graphics/common/1.1/types.h>
+#include <android/hardware/graphics/common/1.2/types.h>
 #include <system/graphics.h>
 
 // android::ui::* in this header file will alias different types as
@@ -25,10 +26,10 @@
 namespace ui {
 
 using android::hardware::graphics::common::V1_0::Hdr;
-using android::hardware::graphics::common::V1_1::ColorMode;
-using android::hardware::graphics::common::V1_1::Dataspace;
 using android::hardware::graphics::common::V1_1::PixelFormat;
 using android::hardware::graphics::common::V1_1::RenderIntent;
+using android::hardware::graphics::common::V1_2::ColorMode;
+using android::hardware::graphics::common::V1_2::Dataspace;
 
 }  // namespace ui
 }  // namespace android
diff --git a/services/surfaceflinger/Android.bp b/services/surfaceflinger/Android.bp
index 339f83b..16003a2 100644
--- a/services/surfaceflinger/Android.bp
+++ b/services/surfaceflinger/Android.bp
@@ -30,6 +30,7 @@
         "android.hardware.configstore@1.1",
         "android.hardware.configstore@1.2",
         "android.hardware.graphics.allocator@2.0",
+        "android.hardware.graphics.common@1.2",
         "android.hardware.graphics.composer@2.1",
         "android.hardware.graphics.composer@2.2",
         "android.hardware.graphics.composer@2.3",
@@ -79,6 +80,7 @@
     ],
     export_shared_lib_headers: [
         "android.hardware.graphics.allocator@2.0",
+        "android.hardware.graphics.common@1.2",
         "android.hardware.graphics.composer@2.1",
         "android.hardware.graphics.composer@2.2",
         "android.hardware.graphics.composer@2.3",
diff --git a/services/surfaceflinger/DisplayDevice.cpp b/services/surfaceflinger/DisplayDevice.cpp
index 60cad02..6f645df 100644
--- a/services/surfaceflinger/DisplayDevice.cpp
+++ b/services/surfaceflinger/DisplayDevice.cpp
@@ -37,8 +37,8 @@
 #include <ui/DebugUtils.h>
 #include <ui/DisplayInfo.h>
 #include <ui/PixelFormat.h>
-#include <utils/RefBase.h>
 #include <utils/Log.h>
+#include <utils/RefBase.h>
 
 #include "DisplayHardware/DisplaySurface.h"
 #include "DisplayHardware/HWComposer.h"
@@ -66,7 +66,8 @@
 namespace {
 
 // ordered list of known SDR color modes
-const std::array<ColorMode, 2> sSdrColorModes = {
+const std::array<ColorMode, 3> sSdrColorModes = {
+        ColorMode::DISPLAY_BT2020,
         ColorMode::DISPLAY_P3,
         ColorMode::SRGB,
 };
@@ -96,6 +97,8 @@
             return Dataspace::SRGB;
         case ColorMode::DISPLAY_P3:
             return Dataspace::DISPLAY_P3;
+        case ColorMode::DISPLAY_BT2020:
+            return Dataspace::DISPLAY_BT2020;
         case ColorMode::BT2100_HLG:
             return Dataspace::BT2020_HLG;
         case ColorMode::BT2100_PQ:
diff --git a/services/surfaceflinger/DisplayHardware/ComposerHal.cpp b/services/surfaceflinger/DisplayHardware/ComposerHal.cpp
index 163b26c..f0bccaa 100644
--- a/services/surfaceflinger/DisplayHardware/ComposerHal.cpp
+++ b/services/surfaceflinger/DisplayHardware/ComposerHal.cpp
@@ -354,16 +354,26 @@
 {
     Error error = kDefaultError;
 
-    if (mClient_2_2) {
-        mClient_2_2->getColorModes_2_2(display,
-                [&](const auto& tmpError, const auto& tmpModes) {
-                    error = tmpError;
-                    if (error != Error::NONE) {
-                        return;
-                    }
+    if (mClient_2_3) {
+        mClient_2_3->getColorModes_2_3(display, [&](const auto& tmpError, const auto& tmpModes) {
+            error = tmpError;
+            if (error != Error::NONE) {
+                return;
+            }
 
-                    *outModes = tmpModes;
-                });
+            *outModes = tmpModes;
+        });
+    } else if (mClient_2_2) {
+        mClient_2_2->getColorModes_2_2(display, [&](const auto& tmpError, const auto& tmpModes) {
+            error = tmpError;
+            if (error != Error::NONE) {
+                return;
+            }
+
+            for (types::V1_1::ColorMode colorMode : tmpModes) {
+                outModes->push_back(static_cast<ColorMode>(colorMode));
+            }
+        });
     } else {
         mClient->getColorModes(display,
                 [&](const auto& tmpError, const auto& tmpModes) {
@@ -555,8 +565,11 @@
         RenderIntent renderIntent)
 {
     hardware::Return<Error> ret(kDefaultError);
-    if (mClient_2_2) {
-        ret = mClient_2_2->setColorMode_2_2(display, mode, renderIntent);
+    if (mClient_2_3) {
+        ret = mClient_2_3->setColorMode_2_3(display, mode, renderIntent);
+    } else if (mClient_2_2) {
+        ret = mClient_2_2->setColorMode_2_2(display, static_cast<types::V1_1::ColorMode>(mode),
+                                            renderIntent);
     } else {
         ret = mClient->setColorMode(display,
                 static_cast<types::V1_0::ColorMode>(mode));
@@ -934,15 +947,22 @@
     }
 
     Error error = kDefaultError;
-    mClient_2_2->getRenderIntents(display, colorMode,
-            [&](const auto& tmpError, const auto& tmpKeys) {
+
+    auto getRenderIntentsLambda = [&](const auto& tmpError, const auto& tmpKeys) {
         error = tmpError;
         if (error != Error::NONE) {
             return;
         }
 
         *outRenderIntents = tmpKeys;
-    });
+    };
+
+    if (mClient_2_3) {
+        mClient_2_3->getRenderIntents_2_3(display, colorMode, getRenderIntentsLambda);
+    } else {
+        mClient_2_2->getRenderIntents(display, static_cast<types::V1_1::ColorMode>(colorMode),
+                                      getRenderIntentsLambda);
+    }
 
     return error;
 }
@@ -955,14 +975,14 @@
     }
 
     Error error = kDefaultError;
-    mClient_2_2->getDataspaceSaturationMatrix(dataspace, [&](const auto& tmpError, const auto& tmpMatrix) {
-        error = tmpError;
-        if (error != Error::NONE) {
-            return;
-        }
-
-        *outMatrix = mat4(tmpMatrix.data());
-    });
+    mClient_2_2->getDataspaceSaturationMatrix(static_cast<types::V1_1::Dataspace>(dataspace),
+                                              [&](const auto& tmpError, const auto& tmpMatrix) {
+                                                  error = tmpError;
+                                                  if (error != Error::NONE) {
+                                                      return;
+                                                  }
+                                                  *outMatrix = mat4(tmpMatrix.data());
+                                              });
 
     return error;
 }
diff --git a/services/surfaceflinger/DisplayHardware/ComposerHal.h b/services/surfaceflinger/DisplayHardware/ComposerHal.h
index 94be6e9..4188352 100644
--- a/services/surfaceflinger/DisplayHardware/ComposerHal.h
+++ b/services/surfaceflinger/DisplayHardware/ComposerHal.h
@@ -49,10 +49,10 @@
 using types::V1_0::Hdr;
 using types::V1_0::Transform;
 
-using types::V1_1::ColorMode;
-using types::V1_1::Dataspace;
 using types::V1_1::PixelFormat;
 using types::V1_1::RenderIntent;
+using types::V1_2::ColorMode;
+using types::V1_2::Dataspace;
 
 using V2_1::Config;
 using V2_1::Display;
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 3a3f5eb..4b22dbb 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -119,13 +119,20 @@
         c->detachLayer(this);
     }
 
-    for (auto& point : mRemoteSyncPoints) {
-        point->setTransactionApplied();
-    }
+    mFrameTracker.logAndResetStats(mName);
+
+    // The remote sync points are cleared out when we are
+    // removed from current state.
+    Mutex::Autolock lock(mLocalSyncPointMutex);
     for (auto& point : mLocalSyncPoints) {
         point->setFrameAvailable();
     }
-    mFrameTracker.logAndResetStats(mName);
+
+    abandon();
+
+    destroyAllHwcLayers();
+
+    mFlinger->onLayerDestroyed();
 }
 
 // ---------------------------------------------------------------------------
@@ -140,10 +147,9 @@
 void Layer::onLayerDisplayed(const sp<Fence>& /*releaseFence*/) {}
 
 void Layer::onRemovedFromCurrentState() {
+    mRemovedFromCurrentState = true;
+
     // the layer is removed from SF mCurrentState to mLayersPendingRemoval
-
-    mPendingRemoval = true;
-
     if (mCurrentState.zOrderRelativeOf != nullptr) {
         sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();
         if (strongRelative != nullptr) {
@@ -153,22 +159,21 @@
         mCurrentState.zOrderRelativeOf = nullptr;
     }
 
+    // Since we are no longer reachable from CurrentState SurfaceFlinger
+    // will no longer invoke doTransaction for us, and so we will
+    // never finish applying transactions. We signal the sync point
+    // now so that another layer will not become indefinitely
+    // blocked.
+    for (auto& point: mRemoteSyncPoints) {
+        point->setTransactionApplied();
+    }
+    mRemoteSyncPoints.clear();
+
     for (const auto& child : mCurrentChildren) {
         child->onRemovedFromCurrentState();
     }
 }
 
-void Layer::onRemoved() {
-    // the layer is removed from SF mLayersPendingRemoval
-    abandon();
-
-    destroyAllHwcLayers();
-
-    for (const auto& child : mCurrentChildren) {
-        child->onRemoved();
-    }
-}
-
 // ---------------------------------------------------------------------------
 // set-up
 // ---------------------------------------------------------------------------
@@ -228,6 +233,10 @@
     }
     LOG_ALWAYS_FATAL_IF(!getBE().mHwcLayers.empty(),
                         "All hardware composer layers should have been destroyed");
+
+    for (const sp<Layer>& child : mDrawingChildren) {
+        child->destroyAllHwcLayers();
+    }
 }
 
 Rect Layer::getContentCrop() const {
@@ -825,7 +834,9 @@
 
     // If this transaction is waiting on the receipt of a frame, generate a sync
     // point and send it to the remote layer.
-    if (mCurrentState.barrierLayer_legacy != nullptr) {
+    // We don't allow installing sync points after we are removed from the current state
+    // as we won't be able to signal our end.
+    if (mCurrentState.barrierLayer_legacy != nullptr && !mRemovedFromCurrentState) {
         sp<Layer> barrierLayer = mCurrentState.barrierLayer_legacy.promote();
         if (barrierLayer == nullptr) {
             ALOGE("[%s] Unable to promote barrier Layer.", mName.string());
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 5d05f05..5999893 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -346,7 +346,7 @@
     virtual bool isCreatedFromMainThread() const { return false; }
 
 
-    bool isPendingRemoval() const { return mPendingRemoval; }
+    bool isRemovedFromCurrentState() const { return mRemovedFromCurrentState; }
 
     void writeToProto(LayerProto* layerInfo,
                       LayerVector::StateSet stateSet = LayerVector::StateSet::Drawing);
@@ -475,12 +475,6 @@
      */
     void onRemovedFromCurrentState();
 
-    /*
-     * called with the state lock from the main thread when the layer is
-     * removed from the pending removal list
-     */
-    void onRemoved();
-
     // Updates the transform hint in our SurfaceFlingerConsumer to match
     // the current orientation of the display device.
     void updateTransformHint(const sp<const DisplayDevice>& display) const;
@@ -595,12 +589,12 @@
      */
     class LayerCleaner {
         sp<SurfaceFlinger> mFlinger;
-        wp<Layer> mLayer;
+        sp<Layer> mLayer;
 
     protected:
         ~LayerCleaner() {
             // destroy client resources
-            mFlinger->onLayerDestroyed(mLayer);
+            mFlinger->onHandleDestroyed(mLayer);
         }
 
     public:
@@ -702,6 +696,8 @@
     virtual PixelFormat getPixelFormat() const { return PIXEL_FORMAT_NONE; }
     bool getPremultipledAlpha() const;
 
+    bool mPendingHWCDestroy{false};
+
 protected:
     // -----------------------------------------------------------------------
     bool usingRelativeZ(LayerVector::StateSet stateSet);
@@ -745,7 +741,7 @@
     // Whether filtering is needed b/c of the drawingstate
     bool mNeedsFiltering{false};
 
-    bool mPendingRemoval{false};
+    bool mRemovedFromCurrentState{false};
 
     // page-flip thread (currently main thread)
     bool mProtectedByApp{false}; // application requires protected path to external sink
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 87d8db7..bbce274 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -124,6 +124,7 @@
         case ColorMode::ADOBE_RGB:
         case ColorMode::DCI_P3:
         case ColorMode::BT2020:
+        case ColorMode::DISPLAY_BT2020:
         case ColorMode::BT2100_PQ:
         case ColorMode::BT2100_HLG:
             return true;
@@ -2003,6 +2004,7 @@
 // can only be one of
 //  - Dataspace::SRGB (use legacy dataspace and let HWC saturate when colors are enhanced)
 //  - Dataspace::DISPLAY_P3
+//  - Dataspace::DISPLAY_BT2020
 // The returned HDR data space is one of
 //  - Dataspace::UNKNOWN
 //  - Dataspace::BT2020_HLG
@@ -2016,6 +2018,12 @@
         switch (layer->getDataSpace()) {
             case Dataspace::V0_SCRGB:
             case Dataspace::V0_SCRGB_LINEAR:
+            case Dataspace::BT2020:
+            case Dataspace::BT2020_ITU:
+            case Dataspace::BT2020_LINEAR:
+            case Dataspace::DISPLAY_BT2020:
+                bestDataSpace = Dataspace::DISPLAY_BT2020;
+                break;
             case Dataspace::DISPLAY_P3:
                 bestDataSpace = Dataspace::DISPLAY_P3;
                 break;
@@ -2715,7 +2723,14 @@
         for (const auto& l : mLayersPendingRemoval) {
             recordBufferingStats(l->getName().string(),
                     l->getOccupancyHistory(true));
-            l->onRemoved();
+
+            // We need to release the HWC layers when the Layer is removed
+            // from the current state otherwise the HWC layer just continues
+            // showing at it's last configured state until we eventually
+            // abandon the buffer queue.
+            if (l->isRemovedFromCurrentState()) {
+                l->destroyAllHwcLayers();
+            }
         }
         mLayersPendingRemoval.clear();
     }
@@ -3148,10 +3163,6 @@
         if (parent == nullptr) {
             mCurrentState.layersSortedByZ.add(lbc);
         } else {
-            if (parent->isPendingRemoval()) {
-                ALOGE("addClientLayer called with a removed parent");
-                return NAME_NOT_FOUND;
-            }
             parent->addChild(lbc);
         }
 
@@ -3178,52 +3189,22 @@
     return removeLayerLocked(mStateLock, layer, topLevelOnly);
 }
 
-status_t SurfaceFlinger::removeLayerLocked(const Mutex&, const sp<Layer>& layer,
+status_t SurfaceFlinger::removeLayerLocked(const Mutex& lock, const sp<Layer>& layer,
                                            bool topLevelOnly) {
-    if (layer->isPendingRemoval()) {
-        return NO_ERROR;
-    }
-
     const auto& p = layer->getParent();
     ssize_t index;
     if (p != nullptr) {
         if (topLevelOnly) {
             return NO_ERROR;
         }
-
-        sp<Layer> ancestor = p;
-        while (ancestor->getParent() != nullptr) {
-            ancestor = ancestor->getParent();
-        }
-        if (mCurrentState.layersSortedByZ.indexOf(ancestor) < 0) {
-            ALOGE("removeLayer called with a layer whose parent has been removed");
-            return NAME_NOT_FOUND;
-        }
-
         index = p->removeChild(layer);
     } else {
         index = mCurrentState.layersSortedByZ.remove(layer);
     }
 
-    // As a matter of normal operation, the LayerCleaner will produce a second
-    // attempt to remove the surface. The Layer will be kept alive in mDrawingState
-    // so we will succeed in promoting it, but it's already been removed
-    // from mCurrentState. As long as we can find it in mDrawingState we have no problem
-    // otherwise something has gone wrong and we are leaking the layer.
-    if (index < 0 && mDrawingState.layersSortedByZ.indexOf(layer) < 0) {
-        ALOGE("Failed to find layer (%s) in layer parent (%s).",
-                layer->getName().string(),
-                (p != nullptr) ? p->getName().string() : "no-parent");
-        return BAD_VALUE;
-    } else if (index < 0) {
-        return NO_ERROR;
-    }
-
     layer->onRemovedFromCurrentState();
-    mLayersPendingRemoval.add(layer);
-    mLayersRemoved = true;
-    mNumLayers -= 1 + layer->getChildrenCount();
-    setTransactionFlags(eTransactionNeeded);
+
+    markLayerPendingRemovalLocked(lock, layer);
     return NO_ERROR;
 }
 
@@ -3424,11 +3405,6 @@
         return 0;
     }
 
-    if (layer->isPendingRemoval()) {
-        ALOGW("Attempting to set client state on removed layer: %s", layer->getName().string());
-        return 0;
-    }
-
     uint32_t flags = 0;
 
     const uint32_t what = s.what;
@@ -3632,11 +3608,6 @@
         return;
     }
 
-    if (layer->isPendingRemoval()) {
-        ALOGW("Attempting to destroy on removed layer: %s", layer->getName().string());
-        return;
-    }
-
     if (state.what & layer_state_t::eDestroySurface) {
         removeLayerLocked(mStateLock, layer);
     }
@@ -3809,17 +3780,16 @@
     return err;
 }
 
-status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)
+void SurfaceFlinger::markLayerPendingRemovalLocked(const Mutex&, const sp<Layer>& layer) {
+    mLayersPendingRemoval.add(layer);
+    mLayersRemoved = true;
+    setTransactionFlags(eTransactionNeeded);
+}
+
+void SurfaceFlinger::onHandleDestroyed(const sp<Layer>& layer)
 {
-    // called by ~LayerCleaner() when all references to the IBinder (handle)
-    // are gone
-    sp<Layer> l = layer.promote();
-    if (l == nullptr) {
-        // The layer has already been removed, carry on
-        return NO_ERROR;
-    }
-    // If we have a parent, then we can continue to live as long as it does.
-    return removeLayer(l, true);
+    Mutex::Autolock lock(mStateLock);
+    markLayerPendingRemovalLocked(mStateLock, layer);
 }
 
 // ---------------------------------------------------------------------------
@@ -5212,7 +5182,7 @@
     auto layerHandle = reinterpret_cast<Layer::Handle*>(layerHandleBinder.get());
     auto parent = layerHandle->owner.promote();
 
-    if (parent == nullptr || parent->isPendingRemoval()) {
+    if (parent == nullptr || parent->isRemovedFromCurrentState()) {
         ALOGE("captureLayers called with a removed parent");
         return NAME_NOT_FOUND;
     }
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 658f04e..dd6d4fb 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -356,6 +356,8 @@
     bool authenticateSurfaceTextureLocked(
         const sp<IGraphicBufferProducer>& bufferProducer) const;
 
+    inline void onLayerDestroyed() { mNumLayers--; }
+
 private:
     friend class Client;
     friend class DisplayEventConnection;
@@ -561,10 +563,12 @@
     // ISurfaceComposerClient::destroySurface()
     status_t onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle);
 
+    void markLayerPendingRemovalLocked(const Mutex& /* mStateLock */, const sp<Layer>& layer);
+
     // called when all clients have released all their references to
     // this layer meaning it is entirely safe to destroy all
     // resources associated to this layer.
-    status_t onLayerDestroyed(const wp<Layer>& layer);
+    void onHandleDestroyed(const sp<Layer>& layer);
 
     // remove a layer from SurfaceFlinger immediately
     status_t removeLayer(const sp<Layer>& layer, bool topLevelOnly = false);
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index 3166a8c..df1fee2 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -2550,6 +2550,37 @@
     }
 }
 
+TEST_F(ChildLayerTest, ChildrenSurviveParentDestruction) {
+    sp<SurfaceControl> mGrandChild =
+        mClient->createSurface(String8("Grand Child"), 10, 10,
+                PIXEL_FORMAT_RGBA_8888, 0, mChild.get());
+    fillSurfaceRGBA8(mGrandChild, 111, 111, 111);
+
+    {
+        SCOPED_TRACE("Grandchild visible");
+        ScreenCapture::captureScreen(&mCapture);
+        mCapture->checkPixel(64, 64, 111, 111, 111);
+    }
+
+    mChild->clear();
+
+    {
+        SCOPED_TRACE("After destroying child");
+        ScreenCapture::captureScreen(&mCapture);
+        mCapture->expectFGColor(64, 64);
+    }
+
+    asTransaction([&](Transaction& t) {
+         t.reparent(mGrandChild, mFGSurfaceControl->getHandle());
+    });
+
+    {
+        SCOPED_TRACE("After reparenting grandchild");
+        ScreenCapture::captureScreen(&mCapture);
+        mCapture->checkPixel(64, 64, 111, 111, 111);
+    }
+}
+
 TEST_F(ChildLayerTest, DetachChildrenSameClient) {
     asTransaction([&](Transaction& t) {
         t.show(mChild);
diff --git a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
index b474e42..d32627a 100644
--- a/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
+++ b/services/surfaceflinger/tests/unittests/DisplayTransactionTest.cpp
@@ -22,6 +22,7 @@
 
 #include <log/log.h>
 
+#include <ui/DebugUtils.h>
 #include "TestableSurfaceFlinger.h"
 #include "mock/DisplayHardware/MockComposer.h"
 #include "mock/DisplayHardware/MockDisplaySurface.h"
@@ -997,6 +998,93 @@
 }
 
 /* ------------------------------------------------------------------------
+ * DisplayDevice::GetBestColorMode
+ */
+class GetBestColorModeTest : public DisplayTransactionTest {
+public:
+    GetBestColorModeTest()
+          : DisplayTransactionTest(),
+            mInjector(FakeDisplayDeviceInjector(mFlinger, DisplayDevice::DISPLAY_PRIMARY, 0)) {}
+
+    void setHasWideColorGamut(bool hasWideColorGamut) { mHasWideColorGamut = hasWideColorGamut; }
+
+    void addHwcColorModesMapping(ui::ColorMode colorMode,
+                                 std::vector<ui::RenderIntent> renderIntents) {
+        mHwcColorModes[colorMode] = renderIntents;
+    }
+
+    void setInputDataspace(ui::Dataspace dataspace) { mInputDataspace = dataspace; }
+
+    void setInputRenderIntent(ui::RenderIntent renderIntent) { mInputRenderIntent = renderIntent; }
+
+    void getBestColorMode() {
+        mInjector.setHwcColorModes(mHwcColorModes);
+        mInjector.setHasWideColorGamut(mHasWideColorGamut);
+        auto displayDevice = mInjector.inject();
+
+        displayDevice->getBestColorMode(mInputDataspace, mInputRenderIntent, &mOutDataspace,
+                                        &mOutColorMode, &mOutRenderIntent);
+    }
+
+    ui::Dataspace mOutDataspace;
+    ui::ColorMode mOutColorMode;
+    ui::RenderIntent mOutRenderIntent;
+
+private:
+    ui::Dataspace mInputDataspace;
+    ui::RenderIntent mInputRenderIntent;
+    bool mHasWideColorGamut = false;
+    std::unordered_map<ui::ColorMode, std::vector<ui::RenderIntent>> mHwcColorModes;
+    FakeDisplayDeviceInjector mInjector;
+};
+
+TEST_F(GetBestColorModeTest, DataspaceDisplayP3_ColorModeSRGB) {
+    addHwcColorModesMapping(ui::ColorMode::SRGB,
+                            std::vector<ui::RenderIntent>(1, RenderIntent::COLORIMETRIC));
+    setInputDataspace(ui::Dataspace::DISPLAY_P3);
+    setInputRenderIntent(ui::RenderIntent::COLORIMETRIC);
+    setHasWideColorGamut(true);
+
+    getBestColorMode();
+
+    ASSERT_EQ(ui::Dataspace::SRGB, mOutDataspace);
+    ASSERT_EQ(ui::ColorMode::SRGB, mOutColorMode);
+    ASSERT_EQ(ui::RenderIntent::COLORIMETRIC, mOutRenderIntent);
+}
+
+TEST_F(GetBestColorModeTest, DataspaceDisplayP3_ColorModeDisplayP3) {
+    addHwcColorModesMapping(ui::ColorMode::DISPLAY_P3,
+                            std::vector<ui::RenderIntent>(1, RenderIntent::COLORIMETRIC));
+    addHwcColorModesMapping(ui::ColorMode::SRGB,
+                            std::vector<ui::RenderIntent>(1, RenderIntent::COLORIMETRIC));
+    addHwcColorModesMapping(ui::ColorMode::DISPLAY_BT2020,
+                            std::vector<ui::RenderIntent>(1, RenderIntent::COLORIMETRIC));
+    setInputDataspace(ui::Dataspace::DISPLAY_P3);
+    setInputRenderIntent(ui::RenderIntent::COLORIMETRIC);
+    setHasWideColorGamut(true);
+
+    getBestColorMode();
+
+    ASSERT_EQ(ui::Dataspace::DISPLAY_P3, mOutDataspace);
+    ASSERT_EQ(ui::ColorMode::DISPLAY_P3, mOutColorMode);
+    ASSERT_EQ(ui::RenderIntent::COLORIMETRIC, mOutRenderIntent);
+}
+
+TEST_F(GetBestColorModeTest, DataspaceDisplayP3_ColorModeDISPLAY_BT2020) {
+    addHwcColorModesMapping(ui::ColorMode::DISPLAY_BT2020,
+                            std::vector<ui::RenderIntent>(1, RenderIntent::COLORIMETRIC));
+    setInputDataspace(ui::Dataspace::DISPLAY_P3);
+    setInputRenderIntent(ui::RenderIntent::COLORIMETRIC);
+    setHasWideColorGamut(true);
+
+    getBestColorMode();
+
+    ASSERT_EQ(ui::Dataspace::DISPLAY_BT2020, mOutDataspace);
+    ASSERT_EQ(ui::ColorMode::DISPLAY_BT2020, mOutColorMode);
+    ASSERT_EQ(ui::RenderIntent::COLORIMETRIC, mOutRenderIntent);
+}
+
+/* ------------------------------------------------------------------------
  * SurfaceFlinger::setupNewDisplayDeviceInternal
  */
 
diff --git a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
index 2046439..62afde9 100644
--- a/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
+++ b/services/surfaceflinger/tests/unittests/TestableSurfaceFlinger.h
@@ -369,6 +369,18 @@
             return *this;
         }
 
+        auto& setHwcColorModes(
+                const std::unordered_map<ui::ColorMode, std::vector<ui::RenderIntent>>
+                        hwcColorModes) {
+            mCreationArgs.hwcColorModes = hwcColorModes;
+            return *this;
+        }
+
+        auto& setHasWideColorGamut(bool hasWideColorGamut) {
+            mCreationArgs.hasWideColorGamut = hasWideColorGamut;
+            return *this;
+        }
+
         sp<DisplayDevice> inject() {
             DisplayDeviceState state;
             state.type = mCreationArgs.type;
diff --git a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
index ecf3181..c0395c0 100644
--- a/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
+++ b/services/surfaceflinger/tests/unittests/mock/DisplayHardware/MockComposer.h
@@ -30,10 +30,10 @@
 using android::hardware::graphics::common::V1_0::ColorTransform;
 using android::hardware::graphics::common::V1_0::Hdr;
 using android::hardware::graphics::common::V1_0::Transform;
-using android::hardware::graphics::common::V1_1::ColorMode;
-using android::hardware::graphics::common::V1_1::Dataspace;
 using android::hardware::graphics::common::V1_1::PixelFormat;
 using android::hardware::graphics::common::V1_1::RenderIntent;
+using android::hardware::graphics::common::V1_2::ColorMode;
+using android::hardware::graphics::common::V1_2::Dataspace;
 
 using android::hardware::graphics::composer::V2_1::Config;
 using android::hardware::graphics::composer::V2_1::Display;