Title: [234578] trunk
Revision
234578
Author
[email protected]
Date
2018-08-04 02:02:44 -0700 (Sat, 04 Aug 2018)

Log Message

Properties set on window.customElements can disappear due to GC
https://bugs.webkit.org/show_bug.cgi?id=172575
<rdar://problem/32440668>

Reviewed by Saam Barati.

Source/WebCore:

Fixed the bug that JS wrapper of CustomElementsRegistry can erroneously get collected during GC
by keeping it alive as long as the global object is alive.

Test: fast/custom-elements/custom-element-registry-wrapper-should-stay-alive.html

* dom/CustomElementRegistry.cpp:
(WebCore::CustomElementRegistry::create):
(WebCore::CustomElementRegistry::CustomElementRegistry):
* dom/CustomElementRegistry.h:
(WebCore::CustomElementRegistry): Make this inherited from ContextDestructionObserver.
* dom/CustomElementRegistry.idl: Set GenerateIsReachable=ImplScriptExecutionContext in IDL. This will
make CustomElementRegistry reachable from the global object.
* page/DOMWindow.cpp:
(WebCore::DOMWindow::ensureCustomElementRegistry):

LayoutTests:

Added a regression test.

* fast/custom-elements/custom-element-registry-wrapper-should-stay-alive-expected.txt: Added.
* fast/custom-elements/custom-element-registry-wrapper-should-stay-alive.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (234577 => 234578)


--- trunk/LayoutTests/ChangeLog	2018-08-04 06:35:37 UTC (rev 234577)
+++ trunk/LayoutTests/ChangeLog	2018-08-04 09:02:44 UTC (rev 234578)
@@ -1,3 +1,16 @@
+2018-08-03  Ryosuke Niwa  <[email protected]>
+
+        Properties set on window.customElements can disappear due to GC
+        https://bugs.webkit.org/show_bug.cgi?id=172575
+        <rdar://problem/32440668>
+
+        Reviewed by Saam Barati.
+
+        Added a regression test.
+
+        * fast/custom-elements/custom-element-registry-wrapper-should-stay-alive-expected.txt: Added.
+        * fast/custom-elements/custom-element-registry-wrapper-should-stay-alive.html: Added.
+
 2018-08-03  Justin Fan  <[email protected]>
 
         WebGL 2 conformance: vertex_arrays/vertex_array_object.html

Added: trunk/LayoutTests/fast/custom-elements/custom-element-registry-wrapper-should-stay-alive-expected.txt (0 => 234578)


--- trunk/LayoutTests/fast/custom-elements/custom-element-registry-wrapper-should-stay-alive-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/custom-elements/custom-element-registry-wrapper-should-stay-alive-expected.txt	2018-08-04 09:02:44 UTC (rev 234578)
@@ -0,0 +1,19 @@
+This tests that the property added on window.customElements persist after a lot of memory allocation
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS The property was present - the JS wrapper of customElements was not collected
+PASS The property was present - the JS wrapper of customElements was not collected
+PASS The property was present - the JS wrapper of customElements was not collected
+PASS The property was present - the JS wrapper of customElements was not collected
+PASS The property was present - the JS wrapper of customElements was not collected
+PASS The property was present - the JS wrapper of customElements was not collected
+PASS The property was present - the JS wrapper of customElements was not collected
+PASS The property was present - the JS wrapper of customElements was not collected
+PASS The property was present - the JS wrapper of customElements was not collected
+PASS The property was present - the JS wrapper of customElements was not collected
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/custom-elements/custom-element-registry-wrapper-should-stay-alive.html (0 => 234578)


--- trunk/LayoutTests/fast/custom-elements/custom-element-registry-wrapper-should-stay-alive.html	                        (rev 0)
+++ trunk/LayoutTests/fast/custom-elements/custom-element-registry-wrapper-should-stay-alive.html	2018-08-04 09:02:44 UTC (rev 234578)
@@ -0,0 +1,33 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src=""
+<script>
+
+description('This tests that the property added on window.customElements persist after a lot of memory allocation');
+
+for (let i = 0; i < 10; i++) {
+    // Using iframe makes this test more reliable.
+    const iframe = document.createElement('iframe');
+    document.body.appendChild(iframe);
+    iframe.contentWindow.eval(`
+        window.customElements.someProperty = 'storedValue';
+        const a = [];
+        for (let i = 0; i < 1000000; i++)
+            a.push({});
+        if (window.GCController)
+            GCController.collect();
+        top.check(window.customElements.someProperty);`);
+    iframe.remove();
+}
+
+function check(value) {
+    if (value == 'storedValue')
+        testPassed('The property was present - the JS wrapper of customElements was not collected');
+    else
+        testFailed('The property was not present - the JS wrapper of customElements was erroneously collected');
+}
+
+</script>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebCore/ChangeLog (234577 => 234578)


--- trunk/Source/WebCore/ChangeLog	2018-08-04 06:35:37 UTC (rev 234577)
+++ trunk/Source/WebCore/ChangeLog	2018-08-04 09:02:44 UTC (rev 234578)
@@ -1,3 +1,26 @@
+2018-08-04  Ryosuke Niwa  <[email protected]>
+
+        Properties set on window.customElements can disappear due to GC
+        https://bugs.webkit.org/show_bug.cgi?id=172575
+        <rdar://problem/32440668>
+
+        Reviewed by Saam Barati.
+
+        Fixed the bug that JS wrapper of CustomElementsRegistry can erroneously get collected during GC
+        by keeping it alive as long as the global object is alive.
+
+        Test: fast/custom-elements/custom-element-registry-wrapper-should-stay-alive.html
+
+        * dom/CustomElementRegistry.cpp:
+        (WebCore::CustomElementRegistry::create):
+        (WebCore::CustomElementRegistry::CustomElementRegistry):
+        * dom/CustomElementRegistry.h:
+        (WebCore::CustomElementRegistry): Make this inherited from ContextDestructionObserver.
+        * dom/CustomElementRegistry.idl: Set GenerateIsReachable=ImplScriptExecutionContext in IDL. This will
+        make CustomElementRegistry reachable from the global object.
+        * page/DOMWindow.cpp:
+        (WebCore::DOMWindow::ensureCustomElementRegistry):
+
 2018-08-03  Ryosuke Niwa  <[email protected]>
 
         innerHTML should not synchronously create a custom element

Modified: trunk/Source/WebCore/dom/CustomElementRegistry.cpp (234577 => 234578)


--- trunk/Source/WebCore/dom/CustomElementRegistry.cpp	2018-08-04 06:35:37 UTC (rev 234577)
+++ trunk/Source/WebCore/dom/CustomElementRegistry.cpp	2018-08-04 09:02:44 UTC (rev 234578)
@@ -41,13 +41,14 @@
 
 namespace WebCore {
 
-Ref<CustomElementRegistry> CustomElementRegistry::create(DOMWindow& window)
+Ref<CustomElementRegistry> CustomElementRegistry::create(DOMWindow& window, ScriptExecutionContext* scriptExecutionContext)
 {
-    return adoptRef(*new CustomElementRegistry(window));
+    return adoptRef(*new CustomElementRegistry(window, scriptExecutionContext));
 }
 
-CustomElementRegistry::CustomElementRegistry(DOMWindow& window)
-    : m_window(window)
+CustomElementRegistry::CustomElementRegistry(DOMWindow& window, ScriptExecutionContext* scriptExecutionContext)
+    : ContextDestructionObserver(scriptExecutionContext)
+    , m_window(window)
 {
 }
 

Modified: trunk/Source/WebCore/dom/CustomElementRegistry.h (234577 => 234578)


--- trunk/Source/WebCore/dom/CustomElementRegistry.h	2018-08-04 06:35:37 UTC (rev 234577)
+++ trunk/Source/WebCore/dom/CustomElementRegistry.h	2018-08-04 09:02:44 UTC (rev 234578)
@@ -25,6 +25,7 @@
 
 #pragma once
 
+#include "ContextDestructionObserver.h"
 #include "QualifiedName.h"
 #include <wtf/HashMap.h>
 #include <wtf/text/AtomicString.h>
@@ -47,9 +48,9 @@
 class Node;
 class QualifiedName;
 
-class CustomElementRegistry : public RefCounted<CustomElementRegistry> {
+class CustomElementRegistry : public RefCounted<CustomElementRegistry>, public ContextDestructionObserver {
 public:
-    static Ref<CustomElementRegistry> create(DOMWindow&);
+    static Ref<CustomElementRegistry> create(DOMWindow&, ScriptExecutionContext*);
     ~CustomElementRegistry();
 
     void addElementDefinition(Ref<JSCustomElementInterface>&&);
@@ -68,7 +69,7 @@
     HashMap<AtomicString, Ref<DeferredPromise>>& promiseMap() { return m_promiseMap; }
 
 private:
-    CustomElementRegistry(DOMWindow&);
+    CustomElementRegistry(DOMWindow&, ScriptExecutionContext*);
 
     DOMWindow& m_window;
     HashMap<AtomicString, Ref<JSCustomElementInterface>> m_nameMap;

Modified: trunk/Source/WebCore/dom/CustomElementRegistry.idl (234577 => 234578)


--- trunk/Source/WebCore/dom/CustomElementRegistry.idl	2018-08-04 06:35:37 UTC (rev 234577)
+++ trunk/Source/WebCore/dom/CustomElementRegistry.idl	2018-08-04 09:02:44 UTC (rev 234578)
@@ -25,8 +25,8 @@
 
 [
     EnabledAtRuntime=CustomElements,
-    ImplementationLacksVTable,
     JSGenerateToNativeObject,
+    GenerateIsReachable=ImplScriptExecutionContext
 ] interface CustomElementRegistry {
     [CEReactions, Custom] void define(DOMString name, Function constructor);
     any get(DOMString name);

Modified: trunk/Source/WebCore/page/DOMWindow.cpp (234577 => 234578)


--- trunk/Source/WebCore/page/DOMWindow.cpp	2018-08-04 06:35:37 UTC (rev 234577)
+++ trunk/Source/WebCore/page/DOMWindow.cpp	2018-08-04 09:02:44 UTC (rev 234578)
@@ -610,7 +610,7 @@
 CustomElementRegistry& DOMWindow::ensureCustomElementRegistry()
 {
     if (!m_customElementRegistry)
-        m_customElementRegistry = CustomElementRegistry::create(*this);
+        m_customElementRegistry = CustomElementRegistry::create(*this, scriptExecutionContext());
     return *m_customElementRegistry;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to