Title: [278750] trunk
Revision
278750
Author
wei...@apple.com
Date
2021-06-10 20:00:58 -0700 (Thu, 10 Jun 2021)

Log Message

Nothing is keeping navigator.xr alive during GC
https://bugs.webkit.org/show_bug.cgi?id=226898

Reviewed by Chris Dumez.

Source/WebCore:

Ensure the navigator.xr wrapper is kept alive when it has custom properties
by annotating it with `GenerateIsReachable=ReachableFromNavigator`.

Test: webxr/gc.html

* Modules/webxr/NavigatorWebXR.cpp:
(WebCore::NavigatorWebXR::xr):
* Modules/webxr/WebXRSystem.cpp:
(WebCore::WebXRSystem::create):
(WebCore::WebXRSystem::WebXRSystem):
(WebCore::WebXRSystem::navigator):
* Modules/webxr/WebXRSystem.h:
* Modules/webxr/WebXRSystem.idl:

LayoutTests:

Add test that ensure the navigator.xr wrapper is kept alive when it has
custom properties.

* webxr: Added.
* webxr/gc-expected.txt: Added.
* webxr/gc.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (278749 => 278750)


--- trunk/LayoutTests/ChangeLog	2021-06-11 02:56:28 UTC (rev 278749)
+++ trunk/LayoutTests/ChangeLog	2021-06-11 03:00:58 UTC (rev 278750)
@@ -1,3 +1,17 @@
+2021-06-10  Sam Weinig  <wei...@apple.com>
+
+        Nothing is keeping navigator.xr alive during GC
+        https://bugs.webkit.org/show_bug.cgi?id=226898
+
+        Reviewed by Chris Dumez.
+
+        Add test that ensure the navigator.xr wrapper is kept alive when it has
+        custom properties. 
+
+        * webxr: Added.
+        * webxr/gc-expected.txt: Added.
+        * webxr/gc.html: Added.
+
 2021-06-10  Wenson Hsieh  <wenson_hs...@apple.com>
 
         [Live Text] Add a mechanism to regenerate text in an image element when it changes dimensions

Added: trunk/LayoutTests/webxr/gc-expected.txt (0 => 278750)


--- trunk/LayoutTests/webxr/gc-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/webxr/gc-expected.txt	2021-06-11 03:00:58 UTC (rev 278750)
@@ -0,0 +1,15 @@
+Test that the window.navigator.xr wrapper preserves custom properties.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS window.navigator.xr.customProperty is undefined.
+window.navigator.xr.customProperty = 1
+PASS window.navigator.xr.customProperty is 1
+PASS window.navigator.xr.customProperty is 1
+PASS window.navigator.xr.customProperty is 1
+PASS window.navigator.xr.customProperty is 1
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/webxr/gc.html (0 => 278750)


--- trunk/LayoutTests/webxr/gc.html	                        (rev 0)
+++ trunk/LayoutTests/webxr/gc.html	2021-06-11 03:00:58 UTC (rev 278750)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src=""
+</head>
+<body>
+<script>
+
+description("Test that the window.navigator.xr wrapper preserves custom properties.");
+jsTestIsAsync = true;
+
+shouldBeUndefined("window.navigator.xr.customProperty");
+evalAndLog("window.navigator.xr.customProperty = 1");
+shouldBe("window.navigator.xr.customProperty", "1");
+gc();
+shouldBe("window.navigator.xr.customProperty", "1");
+
+_onload_ = function() {
+    gc();
+    shouldBe("window.navigator.xr.customProperty", "1");
+    setTimeout(checkAndFinish, 0);
+}
+
+function checkAndFinish() {
+    gc();
+    shouldBe("window.navigator.xr.customProperty", "1");
+    finishJSTest();
+}
+
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (278749 => 278750)


--- trunk/Source/WebCore/ChangeLog	2021-06-11 02:56:28 UTC (rev 278749)
+++ trunk/Source/WebCore/ChangeLog	2021-06-11 03:00:58 UTC (rev 278750)
@@ -1,3 +1,24 @@
+2021-06-10  Sam Weinig  <wei...@apple.com>
+
+        Nothing is keeping navigator.xr alive during GC
+        https://bugs.webkit.org/show_bug.cgi?id=226898
+
+        Reviewed by Chris Dumez.
+
+        Ensure the navigator.xr wrapper is kept alive when it has custom properties
+        by annotating it with `GenerateIsReachable=ReachableFromNavigator`. 
+
+        Test: webxr/gc.html
+
+        * Modules/webxr/NavigatorWebXR.cpp:
+        (WebCore::NavigatorWebXR::xr):
+        * Modules/webxr/WebXRSystem.cpp:
+        (WebCore::WebXRSystem::create):
+        (WebCore::WebXRSystem::WebXRSystem):
+        (WebCore::WebXRSystem::navigator):
+        * Modules/webxr/WebXRSystem.h:
+        * Modules/webxr/WebXRSystem.idl:
+
 2021-06-10  Chris Dumez  <cdu...@apple.com>
 
         Drop unnecessary call to StringView::toStringWithoutCopying() in shouldTreatAsPotentiallyTrustworthy()

Modified: trunk/Source/WebCore/Modules/webxr/NavigatorWebXR.cpp (278749 => 278750)


--- trunk/Source/WebCore/Modules/webxr/NavigatorWebXR.cpp	2021-06-11 02:56:28 UTC (rev 278749)
+++ trunk/Source/WebCore/Modules/webxr/NavigatorWebXR.cpp	2021-06-11 03:00:58 UTC (rev 278750)
@@ -37,7 +37,7 @@
 {
     auto& navigator = NavigatorWebXR::from(navigatorObject);
     if (!navigator.m_xr)
-        navigator.m_xr = WebXRSystem::create(*navigatorObject.scriptExecutionContext());
+        navigator.m_xr = WebXRSystem::create(navigatorObject);
     return *navigator.m_xr;
 }
 

Modified: trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp (278749 => 278750)


--- trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp	2021-06-11 02:56:28 UTC (rev 278749)
+++ trunk/Source/WebCore/Modules/webxr/WebXRSystem.cpp	2021-06-11 03:00:58 UTC (rev 278750)
@@ -36,6 +36,7 @@
 #include "IDLTypes.h"
 #include "JSWebXRSession.h"
 #include "JSXRReferenceSpaceType.h"
+#include "Navigator.h"
 #include "Page.h"
 #include "PlatformXR.h"
 #include "RequestAnimationFrameCallback.h"
@@ -53,14 +54,15 @@
 
 WTF_MAKE_ISO_ALLOCATED_IMPL(WebXRSystem);
 
-Ref<WebXRSystem> WebXRSystem::create(ScriptExecutionContext& scriptExecutionContext)
+Ref<WebXRSystem> WebXRSystem::create(Navigator& navigator)
 {
-    return adoptRef(*new WebXRSystem(scriptExecutionContext));
+    return adoptRef(*new WebXRSystem(navigator));
 }
 
-WebXRSystem::WebXRSystem(ScriptExecutionContext& scriptExecutionContext)
-    : ActiveDOMObject(&scriptExecutionContext)
-    , m_defaultInlineDevice(scriptExecutionContext)
+WebXRSystem::WebXRSystem(Navigator& navigator)
+    : ActiveDOMObject(navigator.scriptExecutionContext())
+    , m_navigator(makeWeakPtr(navigator))
+    , m_defaultInlineDevice(*navigator.scriptExecutionContext())
 {
     m_inlineXRDevice = makeWeakPtr(m_defaultInlineDevice);
     suspendIfNeeded();
@@ -68,6 +70,11 @@
 
 WebXRSystem::~WebXRSystem() = default;
 
+Navigator* WebXRSystem::navigator()
+{
+    return m_navigator.get();
+}
+
 // https://immersive-web.github.io/webxr/#ensures-an-immersive-xr-device-is-selected
 void WebXRSystem::ensureImmersiveXRDeviceIsSelected(CompletionHandler<void()>&& callback)
 {

Modified: trunk/Source/WebCore/Modules/webxr/WebXRSystem.h (278749 => 278750)


--- trunk/Source/WebCore/Modules/webxr/WebXRSystem.h	2021-06-11 02:56:28 UTC (rev 278749)
+++ trunk/Source/WebCore/Modules/webxr/WebXRSystem.h	2021-06-11 03:00:58 UTC (rev 278750)
@@ -48,6 +48,7 @@
 namespace WebCore {
 
 class DOMWindow;
+class Navigator;
 class ScriptExecutionContext;
 class WebXRSession;
 struct XRSessionInit;
@@ -58,7 +59,7 @@
     using IsSessionSupportedPromise = DOMPromiseDeferred<IDLBoolean>;
     using RequestSessionPromise = DOMPromiseDeferred<IDLInterface<WebXRSession>>;
 
-    static Ref<WebXRSystem> create(ScriptExecutionContext&);
+    static Ref<WebXRSystem> create(Navigator&);
     ~WebXRSystem();
 
     using RefCounted<WebXRSystem>::ref;
@@ -77,6 +78,8 @@
     WEBCORE_EXPORT void registerSimulatedXRDeviceForTesting(PlatformXR::Device&);
     WEBCORE_EXPORT void unregisterSimulatedXRDeviceForTesting(PlatformXR::Device&);
 
+    Navigator* navigator();
+
 protected:
     // EventTarget
     EventTargetInterface eventTargetInterface() const override { return WebXRSystemEventTargetInterfaceType; }
@@ -89,7 +92,7 @@
     void stop() override;
 
 private:
-    WebXRSystem(ScriptExecutionContext&);
+    WebXRSystem(Navigator&);
 
     using FeatureList = PlatformXR::Device::FeatureList;
     using JSFeatureList = Vector<JSC::JSValue>;
@@ -117,6 +120,8 @@
         std::optional<PlatformXR::LayerHandle> createLayerProjection(uint32_t, uint32_t, bool) final { return std::nullopt; }
         void deleteLayer(PlatformXR::LayerHandle) final { }
     };
+
+    WeakPtr<Navigator> m_navigator;
     DummyInlineDevice m_defaultInlineDevice;
 
     bool m_immersiveXRDevicesHaveBeenEnumerated { false };

Modified: trunk/Source/WebCore/Modules/webxr/WebXRSystem.idl (278749 => 278750)


--- trunk/Source/WebCore/Modules/webxr/WebXRSystem.idl	2021-06-11 02:56:28 UTC (rev 278749)
+++ trunk/Source/WebCore/Modules/webxr/WebXRSystem.idl	2021-06-11 03:00:58 UTC (rev 278750)
@@ -24,12 +24,13 @@
  */
 
 [
+    ActiveDOMObject,
+    Conditional=WEBXR,
     EnabledBySetting=WebXR,
-    Conditional=WEBXR,
-    ActiveDOMObject,
-    SecureContext,
     Exposed=Window,
-    InterfaceName=XRSystem
+    GenerateIsReachable=ReachableFromNavigator,
+    InterfaceName=XRSystem,
+    SecureContext
 ] interface WebXRSystem : EventTarget {
     // Methods
     Promise<undefined> isSessionSupported(XRSessionMode mode);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to