Bug 1635928 [wpt PR 23442] - Improve webxr-test-api polyfill logic, a=testonly
authorAlexander Cooper <alcooper@chromium.org>
Wed, 13 May 2020 09:48:15 +0000
changeset 531156 1dbd10db1c1054c896e5e5cd24caae68ebe40f8d
parent 531155 e8d82466ca218e703d2109edfa57f593021eba95
child 531157 e7567a2f9fbcfc2e982388bb2b84dbe94ac45762
push id37435
push userapavel@mozilla.com
push dateWed, 20 May 2020 15:28:23 +0000
treeherdermozilla-central@5415da14ec9a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstestonly
bugs1635928, 23442, 1074452, 2185398, 766908
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1635928 [wpt PR 23442] - Improve webxr-test-api polyfill logic, a=testonly Automatic update from web-platform-tests Improve webxr-test-api polyfill logic Now that Chromium no longer needs the polyfill to be created before the xr object is utilized, we can convert the loose promises into promise functions. This helps by allowing us to remove a duplicate check on the engine types in the relevant promises, and helps to make the loading behavior a bit more consistent. In addition, now that navigator.xr.test can be queried before creating the polyfill object, simplify the logic in webxr-test.js; this includes removing a no-longer-used guard for the old WebVR tests. Fixed: 1074452 Change-Id: Ieb52c412a0a4825387ef0b136cc4a85bbdb33191 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2185398 Reviewed-by: Brandon Jones <bajones@chromium.org> Commit-Queue: Alexander Cooper <alcooper@chromium.org> Cr-Commit-Position: refs/heads/master@{#766908} -- wpt-commits: 699e916586620aeb96eb0554b3e5dc09035bf8ae wpt-pr: 23442
testing/web-platform/tests/resources/chromium/webxr-test.js
testing/web-platform/tests/webxr/resources/webxr_util.js
--- a/testing/web-platform/tests/resources/chromium/webxr-test.js
+++ b/testing/web-platform/tests/resources/chromium/webxr-test.js
@@ -1522,26 +1522,9 @@ class MockXRPresentationProvider {
   }
 
   // Utility methods
   Close() {
     this.binding_.close();
   }
 }
 
-// This is a temporary workaround for the fact that spinning up webxr before
-// the mojo interceptors are created will cause the interceptors to not get
-// registered, so we have to create this before we query xr;
-const XRTest = new ChromeXRTest();
-
-// This test API is also used to run Chrome's internal legacy VR tests; however,
-// those fail if navigator.xr has been used. Those tests will set a bool telling
-// us not to try to check navigator.xr
-if ((typeof legacy_vr_test === 'undefined') || !legacy_vr_test) {
-  // Some tests may run in the http context where navigator.xr isn't exposed
-  // This should just be to test that it isn't exposed, but don't try to set up
-  // the test framework in this case.
-  if (navigator.xr) {
-    navigator.xr.test = XRTest;
-  }
-} else {
-  navigator.vr = { test: XRTest };
-}
+navigator.xr.test = new ChromeXRTest();
--- a/testing/web-platform/tests/webxr/resources/webxr_util.js
+++ b/testing/web-platform/tests/webxr/resources/webxr_util.js
@@ -13,25 +13,35 @@ var xr_debug = function(name, msg) {}
 var isChromiumBased = 'MojoInterfaceInterceptor' in self;
 var isWebKitBased = 'internals' in self && 'xrTest' in internals;
 
 function xr_promise_test(name, func, properties) {
   promise_test(async (t) => {
     // Perform any required test setup:
     xr_debug(name, 'setup');
 
-    if (isChromiumBased) {
-      // Chrome setup
-      await loadChromiumResources;
-      xr_debug = navigator.xr.test.Debug;
+    if (!navigator.xr.test) {
+      if (isChromiumBased) {
+        // Chrome setup
+        await loadChromiumResources();
+      } else if (isWebKitBased) {
+        // WebKit setup
+        await setupWebKitWebXRTestAPI();
+      }
     }
 
-    if (isWebKitBased) {
-      // WebKit setup
-      await setupWebKitWebXRTestAPI;
+    // Either the test api needs to be polyfilled and it's not set up above, or
+    // something happened to one of the known polyfills and it failed to be
+    // setup properly. Either way, the fact that xr_promise_test is being used
+    // means that the tests expect navigator.xr.test to be set. By rejecting now
+    // we can hopefully provide a clearer indication of what went wrong.
+    if (!navigator.xr.test) {
+      // We can't use assert_true here because it causes the wpt testharness
+      // to treat this as a test page and not as a test.
+      return Promise.reject("No navigator.xr.test object found, even after attempted load");
     }
 
     // Ensure that any devices are disconnected when done. If this were done in
     // a .then() for the success case, a test that expected failure would
     // already be marked done at the time that runs and the shutdown would
     // interfere with the next test.
     t.add_cleanup(async () => {
       // Ensure system state is cleaned up.
@@ -170,23 +180,17 @@ function forEachWebxrObject(callback) {
   callback(window.XRCoordinateSystem, 'XRCoordinateSystem');
   callback(window.XRFrameOfReference, 'XRFrameOfReference');
   callback(window.XRStageBounds, 'XRStageBounds');
   callback(window.XRSessionEvent, 'XRSessionEvent');
   callback(window.XRCoordinateSystemEvent, 'XRCoordinateSystemEvent');
 }
 
 // Code for loading test API in Chromium.
-let loadChromiumResources = Promise.resolve().then(() => {
-  if (!isChromiumBased) {
-    // Do nothing on non-Chromium-based browsers or when the Mojo bindings are
-    // not present in the global namespace.
-    return;
-  }
-
+function loadChromiumResources() {
   let chromiumResources = [
     '/gen/layout_test_data/mojo/public/js/mojo_bindings.js',
     '/gen/mojo/public/mojom/base/time.mojom.js',
     '/gen/gpu/ipc/common/mailbox_holder.mojom.js',
     '/gen/gpu/ipc/common/sync_token.mojom.js',
     '/gen/ui/display/mojom/display.mojom.js',
     '/gen/ui/gfx/geometry/mojom/geometry.mojom.js',
     '/gen/ui/gfx/mojom/gpu_fence_handle.mojom.js',
@@ -212,23 +216,22 @@ let loadChromiumResources = Promise.reso
       script.src = path;
       script.async = false;
       chain = chain.then(() => new Promise(resolve => {
                            script.onload = () => resolve();
                          }));
       document.head.appendChild(script);
   });
 
-  return chain;
-});
+  chain = chain.then(() => {
+    xr_debug = navigator.xr.test.Debug;
+  });
 
-let setupWebKitWebXRTestAPI = Promise.resolve().then(() => {
-  if (!isWebKitBased) {
-    // Do nothing on non-WebKit-based browsers.
-    return;
-  }
+  return chain;
+}
 
+function setupWebKitWebXRTestAPI() {
   // WebKit setup. The internals object is used by the WebKit test runner
   // to provide JS access to internal APIs. In this case it's used to
   // ensure that XRTest is only exposed to wpt tests.
   navigator.xr.test = internals.xrTest;
   return Promise.resolve();
-});
+}