summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--services/surfaceflinger/FlagManager.cpp114
-rw-r--r--services/surfaceflinger/FlagManager.h24
-rw-r--r--services/surfaceflinger/SurfaceFlinger.cpp1
-rw-r--r--services/surfaceflinger/tests/unittests/FlagManagerTest.cpp129
4 files changed, 100 insertions, 168 deletions
diff --git a/services/surfaceflinger/FlagManager.cpp b/services/surfaceflinger/FlagManager.cpp
index 303714ce2f..919ea4244e 100644
--- a/services/surfaceflinger/FlagManager.cpp
+++ b/services/surfaceflinger/FlagManager.cpp
@@ -28,7 +28,6 @@
namespace android {
static constexpr const char* kExperimentNamespace = "surface_flinger_native_boot";
-static constexpr const int64_t kDemoFlag = -1;
std::unique_ptr<FlagManager> FlagManager::mInstance;
std::once_flag FlagManager::mOnce;
@@ -36,40 +35,8 @@ std::once_flag FlagManager::mOnce;
FlagManager::FlagManager(ConstructorTag) {}
FlagManager::~FlagManager() = default;
-FlagManager& FlagManager::getInstance() {
- std::call_once(mOnce, [&] {
- LOG_ALWAYS_FATAL_IF(mInstance, "Instance already created");
- mInstance = std::make_unique<FlagManager>(ConstructorTag{});
- });
-
- return *mInstance;
-}
-
-void FlagManager::dump(std::string& result) const {
- base::StringAppendF(&result, "FlagManager values: \n");
- base::StringAppendF(&result, "demo_flag: %" PRId64 "\n", demo_flag());
- base::StringAppendF(&result, "use_adpf_cpu_hint: %s\n", use_adpf_cpu_hint() ? "true" : "false");
- base::StringAppendF(&result, "use_skia_tracing: %s\n", use_skia_tracing() ? "true" : "false");
-}
-
namespace {
-template <typename T>
-std::optional<T> doParse(const char* str);
-
-template <>
-[[maybe_unused]] std::optional<int32_t> doParse(const char* str) {
- int32_t ret;
- return base::ParseInt(str, &ret) ? std::make_optional(ret) : std::nullopt;
-}
-
-template <>
-[[maybe_unused]] std::optional<int64_t> doParse(const char* str) {
- int64_t ret;
- return base::ParseInt(str, &ret) ? std::make_optional(ret) : std::nullopt;
-}
-
-template <>
-[[maybe_unused]] std::optional<bool> doParse(const char* str) {
+std::optional<bool> parseBool(const char* str) {
base::ParseBoolResult parseResult = base::ParseBool(str);
switch (parseResult) {
case base::ParseBoolResult::kTrue:
@@ -80,44 +47,67 @@ template <>
return std::nullopt;
}
}
+
+void dumpFlag(std::string& result, const char* name, std::function<bool()> getter) {
+ base::StringAppendF(&result, "%s: %s\n", name, getter() ? "true" : "false");
+}
+
} // namespace
-std::string FlagManager::getServerConfigurableFlag(const std::string& experimentFlagName) const {
- return server_configurable_flags::GetServerConfigurableFlag(kExperimentNamespace,
- experimentFlagName, "");
+const FlagManager& FlagManager::getInstance() {
+ return getMutableInstance();
}
-template int32_t FlagManager::getValue<int32_t>(const std::string&, std::optional<int32_t>,
- int32_t) const;
-template int64_t FlagManager::getValue<int64_t>(const std::string&, std::optional<int64_t>,
- int64_t) const;
-template bool FlagManager::getValue<bool>(const std::string&, std::optional<bool>, bool) const;
-template <typename T>
-T FlagManager::getValue(const std::string& experimentFlagName, std::optional<T> systemPropertyOpt,
- T defaultValue) const {
- // System property takes precedence over the experiment config server value.
- if (systemPropertyOpt.has_value()) {
- return *systemPropertyOpt;
- }
- std::string str = getServerConfigurableFlag(experimentFlagName);
- return str.empty() ? defaultValue : doParse<T>(str.c_str()).value_or(defaultValue);
+FlagManager& FlagManager::getMutableInstance() {
+ std::call_once(mOnce, [&] {
+ LOG_ALWAYS_FATAL_IF(mInstance, "Instance already created");
+ mInstance = std::make_unique<FlagManager>(ConstructorTag{});
+ });
+
+ return *mInstance;
+}
+
+void FlagManager::markBootCompleted() {
+ mBootCompleted = true;
}
-int64_t FlagManager::demo_flag() const {
- std::optional<int64_t> sysPropVal = std::nullopt;
- return getValue("DemoFeature__demo_flag", sysPropVal, kDemoFlag);
+void FlagManager::dump(std::string& result) const {
+#define DUMP_FLAG(name) dumpFlag(result, #name, std::bind(&FlagManager::name, this));
+
+ base::StringAppendF(&result, "FlagManager values: \n");
+ DUMP_FLAG(use_adpf_cpu_hint);
+ DUMP_FLAG(use_skia_tracing);
+
+#undef DUMP_FLAG
}
-bool FlagManager::use_adpf_cpu_hint() const {
- std::optional<bool> sysPropVal =
- doParse<bool>(base::GetProperty("debug.sf.enable_adpf_cpu_hint", "").c_str());
- return getValue("AdpfFeature__adpf_cpu_hint", sysPropVal, false);
+std::optional<bool> FlagManager::getBoolProperty(const char* property) const {
+ return parseBool(base::GetProperty(property, "").c_str());
}
-bool FlagManager::use_skia_tracing() const {
- std::optional<bool> sysPropVal =
- doParse<bool>(base::GetProperty(PROPERTY_SKIA_ATRACE_ENABLED, "").c_str());
- return getValue("SkiaTracingFeature__use_skia_tracing", sysPropVal, false);
+bool FlagManager::getServerConfigurableFlag(const char* experimentFlagName) const {
+ const auto value = server_configurable_flags::GetServerConfigurableFlag(kExperimentNamespace,
+ experimentFlagName, "");
+ const auto res = parseBool(value.c_str());
+ return res.has_value() && res.value();
}
+#define FLAG_MANAGER_SERVER_FLAG(name, syspropOverride, serverFlagName) \
+ bool FlagManager::name() const { \
+ LOG_ALWAYS_FATAL_IF(!mBootCompleted, \
+ "Can't read %s before boot completed as it is server writable", \
+ __func__); \
+ const auto debugOverride = getBoolProperty(syspropOverride); \
+ if (debugOverride.has_value()) return debugOverride.value(); \
+ return getServerConfigurableFlag(serverFlagName); \
+ }
+
+FLAG_MANAGER_SERVER_FLAG(test_flag, "", "")
+FLAG_MANAGER_SERVER_FLAG(use_adpf_cpu_hint, "debug.sf.enable_adpf_cpu_hint",
+ "AdpfFeature__adpf_cpu_hint")
+FLAG_MANAGER_SERVER_FLAG(use_skia_tracing, PROPERTY_SKIA_ATRACE_ENABLED,
+ "SkiaTracingFeature__use_skia_tracing")
+
+#undef FLAG_MANAGER_SERVER_FLAG
+
} // namespace android
diff --git a/services/surfaceflinger/FlagManager.h b/services/surfaceflinger/FlagManager.h
index 8af0b4fc5a..facf6c35d8 100644
--- a/services/surfaceflinger/FlagManager.h
+++ b/services/surfaceflinger/FlagManager.h
@@ -30,30 +30,30 @@ private:
struct ConstructorTag {};
public:
- static FlagManager& getInstance();
+ static const FlagManager& getInstance();
+ static FlagManager& getMutableInstance();
FlagManager(ConstructorTag);
-
virtual ~FlagManager();
- void dump(std::string& result) const;
- int64_t demo_flag() const;
+ void markBootCompleted();
+ void dump(std::string& result) const;
+ bool test_flag() const;
bool use_adpf_cpu_hint() const;
-
bool use_skia_tracing() const;
+protected:
+ // overridden for unit tests
+ virtual std::optional<bool> getBoolProperty(const char*) const;
+ virtual bool getServerConfigurableFlag(const char*) const;
+
private:
- friend class FlagManagerTest;
+ friend class TestableFlagManager;
FlagManager(const FlagManager&) = delete;
- // Wrapper for mocking in test.
- virtual std::string getServerConfigurableFlag(const std::string& experimentFlagName) const;
-
- template <typename T>
- T getValue(const std::string& experimentFlagName, std::optional<T> systemPropertyOpt,
- T defaultValue) const;
+ std::atomic_bool mBootCompleted;
static std::unique_ptr<FlagManager> mInstance;
static std::once_flag mOnce;
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index a36def7c25..3050520d79 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -689,6 +689,7 @@ void SurfaceFlinger::bootFinished() {
return;
}
mBootFinished = true;
+ FlagManager::getMutableInstance().markBootCompleted();
if (mStartPropertySetThread->join() != NO_ERROR) {
ALOGE("Join StartPropertySetThread failed!");
}
diff --git a/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp b/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp
index 0905cd14b1..32c4d05919 100644
--- a/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/FlagManagerTest.cpp
@@ -14,130 +14,71 @@
* limitations under the License.
*/
-#include <cstdint>
#undef LOG_TAG
#define LOG_TAG "FlagManagerTest"
#include "FlagManager.h"
-#include <android-base/properties.h>
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <log/log.h>
-#include <server_configurable_flags/get_flags.h>
-#include <optional>
namespace android {
using testing::Return;
-class MockFlagManager : public FlagManager {
+class TestableFlagManager : public FlagManager {
public:
- MockFlagManager() = default;
- ~MockFlagManager() = default;
+ TestableFlagManager() : FlagManager(ConstructorTag{}) { markBootCompleted(); }
+ ~TestableFlagManager() = default;
- MOCK_METHOD(std::string, getServerConfigurableFlag, (const std::string& experimentFlagName),
- (const, override));
+ MOCK_METHOD(std::optional<bool>, getBoolProperty, (const char*), (const, override));
+ MOCK_METHOD(bool, getServerConfigurableFlag, (const char*), (const, override));
+
+ void markBootIncomplete() { mBootCompleted = false; }
};
class FlagManagerTest : public testing::Test {
public:
- FlagManagerTest();
- ~FlagManagerTest() override;
- std::unique_ptr<MockFlagManager> mFlagManager;
-
- template <typename T>
- T getValue(const std::string& experimentFlagName, std::optional<T> systemPropertyOpt,
- T defaultValue);
+ FlagManagerTest() {
+ const ::testing::TestInfo* const test_info =
+ ::testing::UnitTest::GetInstance()->current_test_info();
+ ALOGD("**** Setting up for %s.%s\n", test_info->test_case_name(), test_info->name());
+ }
+ ~FlagManagerTest() override {
+ const ::testing::TestInfo* const test_info =
+ ::testing::UnitTest::GetInstance()->current_test_info();
+ ALOGD("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name());
+ }
+
+ TestableFlagManager mFlagManager;
};
-FlagManagerTest::FlagManagerTest() {
- const ::testing::TestInfo* const test_info =
- ::testing::UnitTest::GetInstance()->current_test_info();
- ALOGD("**** Setting up for %s.%s\n", test_info->test_case_name(), test_info->name());
- mFlagManager = std::make_unique<MockFlagManager>();
-}
-
-FlagManagerTest::~FlagManagerTest() {
- const ::testing::TestInfo* const test_info =
- ::testing::UnitTest::GetInstance()->current_test_info();
- ALOGD("**** Tearing down after %s.%s\n", test_info->test_case_name(), test_info->name());
-}
-
-template <typename T>
-T FlagManagerTest::getValue(const std::string& experimentFlagName,
- std::optional<T> systemPropertyOpt, T defaultValue) {
- return mFlagManager->getValue(experimentFlagName, systemPropertyOpt, defaultValue);
-}
-
-namespace {
-TEST_F(FlagManagerTest, getValue_bool_default) {
- EXPECT_CALL(*mFlagManager, getServerConfigurableFlag).Times(1).WillOnce(Return(""));
- const bool defaultValue = false;
- std::optional<bool> systemPropertyValue = std::nullopt;
- const bool result = FlagManagerTest::getValue("test_flag", systemPropertyValue, defaultValue);
- ASSERT_EQ(result, defaultValue);
+TEST_F(FlagManagerTest, isSingleton) {
+ EXPECT_EQ(&FlagManager::getInstance(), &FlagManager::getInstance());
}
-TEST_F(FlagManagerTest, getValue_bool_sysprop) {
- const bool defaultValue = false;
- std::optional<bool> systemPropertyValue = std::make_optional(true);
- const bool result = FlagManagerTest::getValue("test_flag", systemPropertyValue, defaultValue);
- ASSERT_EQ(result, true);
+TEST_F(FlagManagerTest, creashesIfQueriedBeforeBoot) {
+ mFlagManager.markBootIncomplete();
+ EXPECT_DEATH(FlagManager::getInstance().use_adpf_cpu_hint(), "");
}
-TEST_F(FlagManagerTest, getValue_bool_experiment) {
- EXPECT_CALL(*mFlagManager, getServerConfigurableFlag).Times(1).WillOnce(Return("1"));
- const bool defaultValue = false;
- std::optional<bool> systemPropertyValue = std::nullopt;
- const bool result = FlagManagerTest::getValue("test_flag", systemPropertyValue, defaultValue);
- ASSERT_EQ(result, true);
-}
+TEST_F(FlagManagerTest, returnsOverride) {
+ EXPECT_CALL(mFlagManager, getBoolProperty).WillOnce(Return(true));
+ EXPECT_EQ(true, mFlagManager.test_flag());
-TEST_F(FlagManagerTest, getValue_int32_default) {
- EXPECT_CALL(*mFlagManager, getServerConfigurableFlag).Times(1).WillOnce(Return(""));
- int32_t defaultValue = 30;
- std::optional<int32_t> systemPropertyValue = std::nullopt;
- int32_t result = FlagManagerTest::getValue("test_flag", systemPropertyValue, defaultValue);
- ASSERT_EQ(result, defaultValue);
+ EXPECT_CALL(mFlagManager, getBoolProperty).WillOnce(Return(false));
+ EXPECT_EQ(false, mFlagManager.test_flag());
}
-TEST_F(FlagManagerTest, getValue_int32_sysprop) {
- int32_t defaultValue = 30;
- std::optional<int32_t> systemPropertyValue = std::make_optional(10);
- int32_t result = FlagManagerTest::getValue("test_flag", systemPropertyValue, defaultValue);
- ASSERT_EQ(result, 10);
-}
+TEST_F(FlagManagerTest, returnsValue) {
+ EXPECT_CALL(mFlagManager, getBoolProperty).WillRepeatedly(Return(std::nullopt));
-TEST_F(FlagManagerTest, getValue_int32_experiment) {
- EXPECT_CALL(*mFlagManager, getServerConfigurableFlag).Times(1).WillOnce(Return("50"));
- std::int32_t defaultValue = 30;
- std::optional<std::int32_t> systemPropertyValue = std::nullopt;
- std::int32_t result = FlagManagerTest::getValue("test_flag", systemPropertyValue, defaultValue);
- ASSERT_EQ(result, 50);
-}
+ EXPECT_CALL(mFlagManager, getServerConfigurableFlag).WillOnce(Return(true));
+ EXPECT_EQ(true, mFlagManager.test_flag());
-TEST_F(FlagManagerTest, getValue_int64_default) {
- EXPECT_CALL(*mFlagManager, getServerConfigurableFlag).Times(1).WillOnce(Return(""));
- int64_t defaultValue = 30;
- std::optional<int64_t> systemPropertyValue = std::nullopt;
- int64_t result = getValue("flag_name", systemPropertyValue, defaultValue);
- ASSERT_EQ(result, defaultValue);
+ EXPECT_CALL(mFlagManager, getServerConfigurableFlag).WillOnce(Return(false));
+ EXPECT_EQ(false, mFlagManager.test_flag());
}
-TEST_F(FlagManagerTest, getValue_int64_sysprop) {
- int64_t defaultValue = 30;
- std::optional<int64_t> systemPropertyValue = std::make_optional(10);
- int64_t result = getValue("flag_name", systemPropertyValue, defaultValue);
- ASSERT_EQ(result, 10);
-}
-
-TEST_F(FlagManagerTest, getValue_int64_experiment) {
- EXPECT_CALL(*mFlagManager, getServerConfigurableFlag).Times(1).WillOnce(Return("50"));
- int64_t defaultValue = 30;
- std::optional<int64_t> systemPropertyValue = std::nullopt;
- int64_t result = getValue("flag_name", systemPropertyValue, defaultValue);
- ASSERT_EQ(result, 50);
-}
-} // namespace
} // namespace android
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2025-10-21 22:13:48 +0000