From 4a474e8ed9f5714bd540a7714a4f062fc76f6226 Mon Sep 17 00:00:00 2001
From: Ady Abraham <adyabr@google.com>
Date: Fri, 2 Oct 2020 15:47:55 -0700
Subject: [PATCH] SurfaceFlinger: fix a crash with RefreshRateOverlay

 - Fix a nullptr deference in createLayer
 - Add a test for RefreshRateOverlay

Test: adb shell /data/nativetest64/SurfaceFlinger_test/SurfaceFlinger_test
Bug: 169867983
Change-Id: I496aaeceaa86b55f806ddc921fc5b050417725b7
---
 TEST_MAPPING                                  |  3 +
 services/surfaceflinger/SurfaceFlinger.cpp    |  4 +-
 services/surfaceflinger/tests/Android.bp      |  1 +
 .../tests/RefreshRateOverlay_test.cpp         | 95 +++++++++++++++++++
 4 files changed, 102 insertions(+), 1 deletion(-)
 create mode 100644 services/surfaceflinger/tests/RefreshRateOverlay_test.cpp

diff --git a/TEST_MAPPING b/TEST_MAPPING
index 5db44c7d85..307e21cc44 100644
--- a/TEST_MAPPING
+++ b/TEST_MAPPING
@@ -53,6 +53,9 @@
         },
         {
           "include-filter": "*RelativeZTest.*"
+        },
+        {
+          "include-filter": "*RefreshRateOverlayTest.*"
         }
       ]
     },
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index df87e3b1cf..d127ef4578 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3942,7 +3942,9 @@ status_t SurfaceFlinger::createLayer(const String8& name, const sp<Client>& clie
     mInterceptor->saveSurfaceCreation(layer);
 
     setTransactionFlags(eTransactionNeeded);
-    *outId = layer->sequence;
+    if (outId) {
+        *outId = layer->sequence;
+    }
     return result;
 }
 
diff --git a/services/surfaceflinger/tests/Android.bp b/services/surfaceflinger/tests/Android.bp
index 719c46ed52..b7edb8444a 100644
--- a/services/surfaceflinger/tests/Android.bp
+++ b/services/surfaceflinger/tests/Android.bp
@@ -34,6 +34,7 @@ cc_test {
         "LayerUpdate_test.cpp",
         "MirrorLayer_test.cpp",
         "MultiDisplayLayerBounds_test.cpp",
+        "RefreshRateOverlay_test.cpp",
         "RelativeZ_test.cpp",
         "ScreenCapture_test.cpp",
         "SetFrameRate_test.cpp",
diff --git a/services/surfaceflinger/tests/RefreshRateOverlay_test.cpp b/services/surfaceflinger/tests/RefreshRateOverlay_test.cpp
new file mode 100644
index 0000000000..05858bf839
--- /dev/null
+++ b/services/surfaceflinger/tests/RefreshRateOverlay_test.cpp
@@ -0,0 +1,95 @@
+/*
+ * Copyright 2020 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <thread>
+
+#include <gtest/gtest.h>
+
+#include <gui/SurfaceComposerClient.h>
+#include <private/gui/ComposerService.h>
+
+static constexpr int kRefreshRateOverlayCode = 1034;
+static constexpr int kRefreshRateOverlayEnable = 1;
+static constexpr int kRefreshRateOverlayDisable = 0;
+static constexpr int kRefreshRateOverlayQuery = 2;
+
+// These values must match the ones we used for developer options in
+// com.android.settings.development.ShowRefreshRatePreferenceController
+static_assert(kRefreshRateOverlayCode == 1034);
+static_assert(kRefreshRateOverlayEnable == 1);
+static_assert(kRefreshRateOverlayDisable == 0);
+static_assert(kRefreshRateOverlayQuery == 2);
+
+namespace android {
+
+namespace {
+void sendCommandToSf(int command, Parcel& reply) {
+    sp<ISurfaceComposer> sf(ComposerService::getComposerService());
+    Parcel request;
+    request.writeInterfaceToken(String16("android.ui.ISurfaceComposer"));
+    request.writeInt32(command);
+    ASSERT_EQ(NO_ERROR,
+              IInterface::asBinder(sf)->transact(kRefreshRateOverlayCode, request, &reply));
+}
+
+bool isOverlayEnabled() {
+    Parcel reply;
+    sendCommandToSf(kRefreshRateOverlayQuery, reply);
+    return reply.readBool();
+}
+
+void waitForOverlay(bool enabled) {
+    static constexpr auto kTimeout = std::chrono::nanoseconds(1s);
+    static constexpr auto kIterations = 10;
+    for (int i = 0; i < kIterations; i++) {
+        if (enabled == isOverlayEnabled()) {
+            return;
+        }
+        std::this_thread::sleep_for(kTimeout / kIterations);
+    }
+}
+
+void toggleOverlay(bool enabled) {
+    if (enabled == isOverlayEnabled()) {
+        return;
+    }
+
+    Parcel reply;
+    const auto command = enabled ? kRefreshRateOverlayEnable : kRefreshRateOverlayDisable;
+    sendCommandToSf(command, reply);
+    waitForOverlay(enabled);
+    ASSERT_EQ(enabled, isOverlayEnabled());
+}
+
+} // namespace
+
+TEST(RefreshRateOverlayTest, enableOverlay) {
+    toggleOverlay(true);
+}
+
+TEST(RefreshRateOverlayTest, disableOverlay) {
+    toggleOverlay(false);
+}
+
+TEST(RefreshRateOverlayTest, enableAndDisableOverlay) {
+    toggleOverlay(true);
+    toggleOverlay(false);
+
+    toggleOverlay(true);
+    toggleOverlay(false);
+}
+
+} // namespace android
\ No newline at end of file