Add ScopedEnvironmentVar class for setting env variables in BackendVk

In the Vulkan backend, we only need to set VK_LAYER_PATH and
VK_ICD_FILENAMES while we're gathering the device information. After
this, we should unset the environment variables or they will persist
and affect loading Vulkan again (from Dawn or another client) in the
future.

Bug: dawn:406
Change-Id: I30c38b0980e181126fcd7fa911bbf9e8aa35f3b8
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22021
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>

src/common/SystemUtils.cpp

@@ -14,6 +14,8 @@
 
 #include "common/SystemUtils.h"
 
+#include "common/Assert.h"
+
 #if defined(DAWN_PLATFORM_WINDOWS)
 #    include <Windows.h>
 #    include <vector>
@@ -115,3 +117,27 @@ std::string GetExecutableDirectory() {
     size_t lastPathSepLoc = exePath.find_last_of(GetPathSeparator());
     return lastPathSepLoc != std::string::npos ? exePath.substr(0, lastPathSepLoc + 1) : "";
 }
+
+// ScopedEnvironmentVar
+
+ScopedEnvironmentVar::ScopedEnvironmentVar(const char* variableName, const char* value)
+    : mName(variableName),
+      mOriginalValue(GetEnvironmentVar(variableName)),
+      mIsSet(SetEnvironmentVar(variableName, value)) {
+}
+
+ScopedEnvironmentVar::~ScopedEnvironmentVar() {
+    if (mIsSet) {
+        bool success = SetEnvironmentVar(mName.c_str(), mOriginalValue.c_str());
+        // If we set the environment variable in the constructor, we should never fail restoring it.
+        ASSERT(success);
+    }
+}
+
+bool ScopedEnvironmentVar::Set(const char* variableName, const char* value) {
+    ASSERT(!mIsSet);
+    mName = variableName;
+    mOriginalValue = GetEnvironmentVar(variableName);
+    mIsSet = SetEnvironmentVar(variableName, value);
+    return mIsSet;
+}

src/common/SystemUtils.h

@@ -24,4 +24,21 @@ std::string GetEnvironmentVar(const char* variableName);
 bool SetEnvironmentVar(const char* variableName, const char* value);
 std::string GetExecutableDirectory();
 
+class ScopedEnvironmentVar {
+  public:
+    ScopedEnvironmentVar() = default;
+    ScopedEnvironmentVar(const char* variableName, const char* value);
+    ~ScopedEnvironmentVar();
+
+    ScopedEnvironmentVar(const ScopedEnvironmentVar& rhs) = delete;
+    ScopedEnvironmentVar& operator=(const ScopedEnvironmentVar& rhs) = delete;
+
+    bool Set(const char* variableName, const char* value);
+
+  private:
+    std::string mName;
+    std::string mOriginalValue;
+    bool mIsSet = false;
+};
+
 #endif  // COMMON_SYSTEMUTILS_H_

src/dawn_native/vulkan/BackendVk.cpp

@@ -109,14 +109,16 @@ namespace dawn_native { namespace vulkan {
     MaybeError Backend::Initialize(bool useSwiftshader) {
         DAWN_TRY(LoadVulkan(useSwiftshader));
 
-        // TODO(crbug.com/dawn/406): In order to not modify the environment variables of
-        // the rest of an application embedding Dawn, we should set these only
-        // in the scope of this function. See ANGLE's ScopedVkLoaderEnvironment
+        // These environment variables need only be set while loading procs and gathering device
+        // info.
+        ScopedEnvironmentVar vkICDFilenames;
+        ScopedEnvironmentVar vkLayerPath;
+
         if (useSwiftshader) {
 #if defined(DAWN_SWIFTSHADER_VK_ICD_JSON)
             std::string fullSwiftshaderICDPath =
                 GetExecutableDirectory() + DAWN_SWIFTSHADER_VK_ICD_JSON;
-            if (!SetEnvironmentVar("VK_ICD_FILENAMES", fullSwiftshaderICDPath.c_str())) {
+            if (!vkICDFilenames.Set("VK_ICD_FILENAMES", fullSwiftshaderICDPath.c_str())) {
                 return DAWN_INTERNAL_ERROR("Couldn't set VK_ICD_FILENAMES");
             }
 #else
@@ -128,7 +130,7 @@ namespace dawn_native { namespace vulkan {
         if (GetInstance()->IsBackendValidationEnabled()) {
 #if defined(DAWN_ENABLE_VULKAN_VALIDATION_LAYERS)
             std::string vkDataDir = GetExecutableDirectory() + DAWN_VK_DATA_DIR;
-            if (!SetEnvironmentVar("VK_LAYER_PATH", vkDataDir.c_str())) {
+            if (!vkLayerPath.Set("VK_LAYER_PATH", vkDataDir.c_str())) {
                 return DAWN_INTERNAL_ERROR("Couldn't set VK_LAYER_PATH");
             }
 #else

src/tests/unittests/SystemUtilsTests.cpp

@@ -39,3 +39,48 @@ TEST(SystemUtilsTests, GetExecutableDirectory) {
     // Test last charecter in path
     ASSERT_EQ(GetExecutableDirectory().back(), *GetPathSeparator());
 }
+
+// Tests for ScopedEnvironmentVar
+TEST(SystemUtilsTests, ScopedEnvironmentVar) {
+    SetEnvironmentVar("ScopedEnvironmentVarForTest", "original");
+
+    // Test empty environment variable doesn't crash
+    { ScopedEnvironmentVar var; }
+
+    // Test setting empty environment variable
+    {
+        ScopedEnvironmentVar var;
+        var.Set("ScopedEnvironmentVarForTest", "NewEnvironmentVarValue");
+        ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "NewEnvironmentVarValue");
+    }
+    ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
+
+    // Test that the environment variable can be set, and it is unset at the end of the scope.
+    {
+        ScopedEnvironmentVar var("ScopedEnvironmentVarForTest", "NewEnvironmentVarValue");
+        ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "NewEnvironmentVarValue");
+    }
+    ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
+
+    // Test nested scopes
+    {
+        ScopedEnvironmentVar outer("ScopedEnvironmentVarForTest", "outer");
+        ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "outer");
+        {
+            ScopedEnvironmentVar inner("ScopedEnvironmentVarForTest", "inner");
+            ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "inner");
+        }
+        ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "outer");
+    }
+    ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
+
+    // Test redundantly setting scoped variables
+    {
+        ScopedEnvironmentVar var1("ScopedEnvironmentVarForTest", "var1");
+        ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "var1");
+
+        ScopedEnvironmentVar var2("ScopedEnvironmentVarForTest", "var2");
+        ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "var2");
+    }
+    ASSERT_EQ(GetEnvironmentVar("ScopedEnvironmentVarForTest"), "original");
+}