Fixing Flaky OpenXR Input Test By Creating Static Wrapper Class

Currently OpenXR tests are flaky because XrInstance is created and
destroyed multiple times. There is also a performance issue because
every time XrCreateInstance called, it will load dlls. The solution is
create a OpenXrStatic class to host the xr instance and static methods.
And have one copy of OpenXrStatic in render process another in where
static functions getting called.


Bug: 1011871
Change-Id: I69843cebaeee9e4b7f30806f431b4ea93182ebca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1865727
Commit-Queue: Zheng Qin <zheqi@microsoft.com>
Reviewed-by: Alexander Cooper <alcooper@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708505}

chrome/services/isolated_xr_device/xr_runtime_provider.cc

@@ -25,6 +25,7 @@
 
 #if BUILDFLAG(ENABLE_OPENXR)
 #include "device/vr/openxr/openxr_device.h"
+#include "device/vr/openxr/openxr_statics.h"
 #endif
 
 enum class IsolatedXRRuntimeProvider::RuntimeStatus {
@@ -158,7 +159,8 @@ void IsolatedXRRuntimeProvider::SetupPollingForDeviceChanges() {
 
 #if BUILDFLAG(ENABLE_OPENXR)
   if (base::FeatureList::IsEnabled(features::kOpenXR)) {
-    should_check_openxr_ = device::OpenXrDevice::IsApiAvailable();
+    openxr_statics_ = std::make_unique<device::OpenXrStatics>();
+    should_check_openxr_ = openxr_statics_->IsApiAvailable();
     any_runtimes_available |= should_check_openxr_;
   }
 #endif
@@ -214,7 +216,7 @@ void IsolatedXRRuntimeProvider::SetWMRRuntimeStatus(RuntimeStatus status) {
 
 #if BUILDFLAG(ENABLE_OPENXR)
 bool IsolatedXRRuntimeProvider::IsOpenXrHardwareAvailable() {
-  return should_check_openxr_ && device::OpenXrDevice::IsHardwareAvailable();
+  return should_check_openxr_ && openxr_statics_->IsHardwareAvailable();
 }
 
 void IsolatedXRRuntimeProvider::SetOpenXrRuntimeStatus(RuntimeStatus status) {

chrome/services/isolated_xr_device/xr_runtime_provider.h

@@ -18,6 +18,7 @@ class OpenVRDevice;
 class MixedRealityDevice;
 class MixedRealityDeviceStatics;
 class OpenXrDevice;
+class OpenXrStatics;
 }  // namespace device
 
 class IsolatedXRRuntimeProvider
@@ -63,6 +64,7 @@ class IsolatedXRRuntimeProvider
   void SetOpenXrRuntimeStatus(RuntimeStatus status);
   bool should_check_openxr_ = false;
   std::unique_ptr<device::OpenXrDevice> openxr_device_;
+  std::unique_ptr<device::OpenXrStatics> openxr_statics_;
 #endif
 
   mojo::Remote<device::mojom::IsolatedXRRuntimeProviderClient> client_;

device/vr/BUILD.gn

@@ -245,6 +245,8 @@ if (enable_vr) {
         "openxr/openxr_input_helper.h",
         "openxr/openxr_render_loop.cc",
         "openxr/openxr_render_loop.h",
+        "openxr/openxr_statics.cc",
+        "openxr/openxr_statics.h",
         "openxr/openxr_util.cc",
         "openxr/openxr_util.h",
       ]

device/vr/openxr/openxr_api_wrapper.cc

@@ -29,62 +29,8 @@ constexpr XrViewConfigurationType kSupportedViewConfiguration =
     XR_VIEW_CONFIGURATION_TYPE_PRIMARY_STEREO;
 constexpr uint32_t kNumViews = 2;
 
-XrResult CreateInstance(XrInstance* instance) {
-  XrInstanceCreateInfo instance_create_info = {XR_TYPE_INSTANCE_CREATE_INFO};
-  strcpy_s(instance_create_info.applicationInfo.applicationName, "Chromium");
-  instance_create_info.applicationInfo.apiVersion = XR_CURRENT_API_VERSION;
-
-  // xrCreateInstance validates the list of extensions and returns
-  // XR_ERROR_EXTENSION_NOT_PRESENT if an extension is not supported,
-  // so we don't need to call xrEnumerateInstanceExtensionProperties
-  // to validate these extensions.
-  const char* extensions[] = {
-      XR_KHR_D3D11_ENABLE_EXTENSION_NAME,
-  };
-
-  instance_create_info.enabledExtensionCount =
-      sizeof(extensions) / sizeof(extensions[0]);
-  instance_create_info.enabledExtensionNames = extensions;
-
-  return xrCreateInstance(&instance_create_info, instance);
-}
-
-XrResult GetSystem(XrInstance instance, XrSystemId* system) {
-  XrSystemGetInfo system_info = {XR_TYPE_SYSTEM_GET_INFO};
-  system_info.formFactor = XR_FORM_FACTOR_HEAD_MOUNTED_DISPLAY;
-  return xrGetSystem(instance, &system_info, system);
-}
-
 }  // namespace
 
-bool OpenXrApiWrapper::IsHardwareAvailable() {
-  bool is_available = false;
-  XrInstance instance = XR_NULL_HANDLE;
-  XrSystemId system;
-
-  if (XR_SUCCEEDED(CreateInstance(&instance)) &&
-      XR_SUCCEEDED(GetSystem(instance, &system))) {
-    is_available = true;
-  }
-
-  if (instance != XR_NULL_HANDLE)
-    xrDestroyInstance(instance);
-
-  // System is not allocated so does not need to be destroyed
-
-  return is_available;
-}
-
-bool OpenXrApiWrapper::IsApiAvailable() {
-  XrInstance instance;
-  if (XR_SUCCEEDED(CreateInstance(&instance))) {
-    xrDestroyInstance(instance);
-    return true;
-  }
-
-  return false;
-}
-
 std::unique_ptr<OpenXrApiWrapper> OpenXrApiWrapper::Create() {
   std::unique_ptr<OpenXrApiWrapper> openxr =
       std::make_unique<OpenXrApiWrapper>();

device/vr/openxr/openxr_api_wrapper.h

@@ -38,9 +38,6 @@ class OpenXrApiWrapper {
 
   static std::unique_ptr<OpenXrApiWrapper> Create();
 
-  static bool IsHardwareAvailable();
-  static bool IsApiAvailable();
-
   static VRTestHook* GetTestHook();
 
   bool session_ended() const { return session_ended_; }

device/vr/openxr/openxr_device.cc

@@ -76,14 +76,6 @@ mojom::VRDisplayInfoPtr CreateFakeVRDisplayInfo(device::mojom::XRDeviceId id) {
 
 }  // namespace
 
-bool OpenXrDevice::IsHardwareAvailable() {
-  return OpenXrApiWrapper::IsHardwareAvailable();
-}
-
-bool OpenXrDevice::IsApiAvailable() {
-  return OpenXrApiWrapper::IsApiAvailable();
-}
-
 OpenXrDevice::OpenXrDevice()
     : VRDeviceBase(device::mojom::XRDeviceId::OPENXR_DEVICE_ID),
       weak_ptr_factory_(this) {

device/vr/openxr/openxr_device.h

@@ -27,9 +27,6 @@ class DEVICE_VR_EXPORT OpenXrDevice
   OpenXrDevice();
   ~OpenXrDevice() override;
 
-  static bool IsHardwareAvailable();
-  static bool IsApiAvailable();
-
   // VRDeviceBase
   void RequestSession(
       mojom::XRRuntimeSessionOptionsPtr options,

device/vr/openxr/openxr_statics.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "device/vr/openxr/openxr_statics.h"
+
+#include "device/vr/openxr/openxr_util.h"
+
+namespace device {
+
+OpenXrStatics::OpenXrStatics() : instance_(XR_NULL_HANDLE) {}
+
+OpenXrStatics::~OpenXrStatics() {
+  if (instance_ != XR_NULL_HANDLE) {
+    xrDestroyInstance(instance_);
+    instance_ = XR_NULL_HANDLE;
+  }
+}
+
+bool OpenXrStatics::IsHardwareAvailable() {
+  if (instance_ == XR_NULL_HANDLE && XR_FAILED(CreateInstance(&instance_))) {
+    return false;
+  }
+
+  XrSystemId system;
+  return XR_SUCCEEDED(GetSystem(instance_, &system));
+}
+
+bool OpenXrStatics::IsApiAvailable() {
+  return instance_ != XR_NULL_HANDLE ||
+         XR_SUCCEEDED(CreateInstance(&instance_));
+}
+
+}  // namespace device
\ No newline at end of file

device/vr/openxr/openxr_statics.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef DEVICE_VR_OPENXR_OPENXR_STATICS_H_
+#define DEVICE_VR_OPENXR_OPENXR_STATICS_H_
+
+#include <d3d11.h>
+#include <memory>
+
+#include "device/vr/vr_export.h"
+#include "third_party/openxr/src/include/openxr/openxr.h"
+#include "third_party/openxr/src/include/openxr/openxr_platform.h"
+
+namespace device {
+
+class DEVICE_VR_EXPORT OpenXrStatics {
+ public:
+  OpenXrStatics();
+  ~OpenXrStatics();
+
+  bool IsHardwareAvailable();
+  bool IsApiAvailable();
+
+ private:
+  XrInstance instance_;
+};
+
+}  // namespace device
+
+#endif  // DEVICE_VR_WINDOWS_MIXED_REALITY_MIXED_REALITY_STATICS_H_
\ No newline at end of file

device/vr/openxr/openxr_util.cc

@@ -4,6 +4,11 @@
 
 #include "device/vr/openxr/openxr_util.h"
 
+#include <d3d11.h>
+#include <string>
+
+#include "third_party/openxr/src/include/openxr/openxr_platform.h"
+
 namespace device {
 
 XrPosef PoseIdentity() {
@@ -12,4 +17,30 @@ XrPosef PoseIdentity() {
   return pose;
 }
 
+XrResult GetSystem(XrInstance instance, XrSystemId* system) {
+  XrSystemGetInfo system_info = {XR_TYPE_SYSTEM_GET_INFO};
+  system_info.formFactor = XR_FORM_FACTOR_HEAD_MOUNTED_DISPLAY;
+  return xrGetSystem(instance, &system_info, system);
+}
+
+XrResult CreateInstance(XrInstance* instance) {
+  XrInstanceCreateInfo instance_create_info = {XR_TYPE_INSTANCE_CREATE_INFO};
+  strcpy_s(instance_create_info.applicationInfo.applicationName, "Chromium");
+  instance_create_info.applicationInfo.apiVersion = XR_CURRENT_API_VERSION;
+
+  // xrCreateInstance validates the list of extensions and returns
+  // XR_ERROR_EXTENSION_NOT_PRESENT if an extension is not supported,
+  // so we don't need to call xrEnumerateInstanceExtensionProperties
+  // to validate these extensions.
+  const char* extensions[] = {
+      XR_KHR_D3D11_ENABLE_EXTENSION_NAME,
+  };
+
+  instance_create_info.enabledExtensionCount =
+      sizeof(extensions) / sizeof(extensions[0]);
+  instance_create_info.enabledExtensionNames = extensions;
+
+  return xrCreateInstance(&instance_create_info, instance);
+}
+
 }  // namespace device

device/vr/openxr/openxr_util.h

@@ -42,6 +42,10 @@ namespace device {
 // orientation is {0, 0, 0, 1}.
 XrPosef PoseIdentity();
 
+XrResult GetSystem(XrInstance instance, XrSystemId* system);
+
+XrResult CreateInstance(XrInstance* instance);
+
 }  // namespace device
 
 #endif  // DEVICE_VR_OPENXR_OPENXR_UTIL_H_

device/vr/openxr/test/fake_openxr_impl_api.cc

@@ -146,9 +146,7 @@ XrResult xrCreateInstance(const XrInstanceCreateInfo* create_info,
                     "enabledExtensionNames contains invalid extensions");
   }
 
-  // Return the test helper object back to the OpenXrAPIWrapper so it can use
-  // it as the TestHookRegistration.
-  *instance = reinterpret_cast<XrInstance>(&g_test_helper);
+  *instance = g_test_helper.CreateInstance();
 
   return XR_SUCCESS;
 }
@@ -250,7 +248,7 @@ XrResult xrDestroyInstance(XrInstance instance) {
   XrResult xr_result;
 
   RETURN_IF_XR_FAILED(g_test_helper.ValidateInstance(instance));
-
+  g_test_helper.Reset();
   return XR_SUCCESS;
 }
 

device/vr/openxr/test/openxr_test_helper.cc

@@ -48,7 +48,11 @@ uint32_t OpenXrTestHelper::NumViews() {
 }
 
 OpenXrTestHelper::OpenXrTestHelper()
-    : system_id_(0),
+    // since openxr_statics is created first, so the first instance returned
+    // should be a fake one since openxr_statics does not need to use
+    // test_hook_;
+    : create_fake_instance_(true),
+      system_id_(0),
       session_(XR_NULL_HANDLE),
       session_state_(XR_SESSION_STATE_UNKNOWN),
       swapchain_(XR_NULL_HANDLE),
@@ -58,6 +62,39 @@ OpenXrTestHelper::OpenXrTestHelper()
 
 OpenXrTestHelper::~OpenXrTestHelper() = default;
 
+void OpenXrTestHelper::Reset() {
+  session_ = XR_NULL_HANDLE;
+  session_state_ = XR_SESSION_STATE_UNKNOWN;
+  swapchain_ = XR_NULL_HANDLE;
+
+  create_fake_instance_ = true;
+  system_id_ = 0;
+  d3d_device_ = nullptr;
+  acquired_swapchain_texture_ = 0;
+  next_action_space_ = 0;
+  next_predicted_display_time_ = 0;
+
+  // vectors
+  textures_arr_.clear();
+  paths_.clear();
+
+  // unordered_maps
+  actions_.clear();
+  action_spaces_.clear();
+  reference_spaces_.clear();
+  action_sets_.clear();
+  float_action_states_.clear();
+  boolean_action_states_.clear();
+  v2f_action_states_.clear();
+  pose_action_state_.clear();
+
+  // unordered_sets
+  action_names_.clear();
+  action_localized_names_.clear();
+  action_set_names_.clear();
+  action_set_localized_names_.clear();
+}
+
 void OpenXrTestHelper::TestFailure() {
   NOTREACHED();
 }
@@ -125,6 +162,20 @@ XrSwapchain OpenXrTestHelper::GetSwapchain() {
   return swapchain_;
 }
 
+XrInstance OpenXrTestHelper::CreateInstance() {
+  // Return the test helper object back to the OpenXrAPIWrapper so it can use
+  // it as the TestHookRegistration.However we have to return different instance
+  // for openxr_statics since openxr loader records instance created and
+  // detroyed. The first instance is used by openxr_statics which does not need
+  // to use test_hook_, So we can give it an arbitrary instance as long as
+  // ValidateInstance method remember it's an valid option.
+  if (create_fake_instance_) {
+    create_fake_instance_ = false;
+    return reinterpret_cast<XrInstance>(this + 1);
+  }
+  return reinterpret_cast<XrInstance>(this);
+}
+
 XrResult OpenXrTestHelper::GetActionStateFloat(XrAction action,
                                                XrActionStateFloat* data) const {
   XrResult xr_result;
@@ -627,7 +678,9 @@ XrResult OpenXrTestHelper::ValidateActionSpaceCreateInfo(
 XrResult OpenXrTestHelper::ValidateInstance(XrInstance instance) const {
   // The Fake OpenXr Runtime returns this global OpenXrTestHelper object as the
   // instance value on xrCreateInstance.
-  RETURN_IF(reinterpret_cast<OpenXrTestHelper*>(instance) != this,
+
+  RETURN_IF(reinterpret_cast<OpenXrTestHelper*>(instance) != this &&
+                reinterpret_cast<OpenXrTestHelper*>(instance) != (this + 1),
             XR_ERROR_VALIDATION_FAILURE, "XrInstance invalid");
 
   return XR_SUCCESS;

device/vr/openxr/test/openxr_test_helper.h

@@ -27,7 +27,7 @@ class OpenXrTestHelper : public device::ServiceTestHook {
  public:
   OpenXrTestHelper();
   ~OpenXrTestHelper();
-
+  void Reset();
   void TestFailure();
 
   // TestHookRegistration
@@ -43,6 +43,7 @@ class OpenXrTestHelper : public device::ServiceTestHook {
 
   XrSystemId GetSystemId();
   XrSwapchain GetSwapchain();
+  XrInstance CreateInstance();
   XrResult GetActionStateFloat(XrAction action, XrActionStateFloat* data) const;
   XrResult GetActionStateBoolean(XrAction action,
                                  XrActionStateBoolean* data) const;
@@ -123,6 +124,8 @@ class OpenXrTestHelper : public device::ServiceTestHook {
   device::ControllerFrameData GetControllerDataFromPath(
       std::string path_string) const;
 
+  bool create_fake_instance_;
+
   // Properties of the mock OpenXR runtime that doesn't change throughout the
   // lifetime of the instance. However, these aren't static because they are
   // initialized to an invalid value and set to their actual value in their