Changeset 234578 in webkit

Timestamp:

Aug 4, 2018 2:02:44 AM (6 years ago)

Author:

rniwa@webkit.org

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/custom-elements/custom-element-registry-wrapper-should-stay-alive-expected.txt (added)
• LayoutTests/fast/custom-elements/custom-element-registry-wrapper-should-stay-alive.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/dom/CustomElementRegistry.cpp (modified) (1 diff)
• Source/WebCore/dom/CustomElementRegistry.h (modified) (3 diffs)
• Source/WebCore/dom/CustomElementRegistry.idl (modified) (1 diff)
• Source/WebCore/page/DOMWindow.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-08-03  Ryosuke Niwa  <rniwa@webkit.org>
+
+        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  <justin_fan@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-08-04  Ryosuke Niwa  <rniwa@webkit.org>
+
+        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  <rniwa@webkit.org>
 

trunk/Source/WebCore/dom/CustomElementRegistry.cpp

@@ -42 +42 @@
 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)
 {
 }

trunk/Source/WebCore/dom/CustomElementRegistry.h

@@ -26 +26 @@
 #pragma once
 
+#include "ContextDestructionObserver.h"
 #include "QualifiedName.h"
 #include <wtf/HashMap.h>
@@ -48 +49 @@
 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();
 
@@ -69 +70 @@
 
 private:
-    CustomElementRegistry(DOMWindow&);
+    CustomElementRegistry(DOMWindow&, ScriptExecutionContext*);
 
     DOMWindow& m_window;

trunk/Source/WebCore/dom/CustomElementRegistry.idl

@@ -26 +26 @@
 [
     EnabledAtRuntime=CustomElements,
-    ImplementationLacksVTable,
     JSGenerateToNativeObject,
+    GenerateIsReachable=ImplScriptExecutionContext
 ] interface CustomElementRegistry {
     [CEReactions, Custom] void define(DOMString name, Function constructor);

trunk/Source/WebCore/page/DOMWindow.cpp

@@ -611 +611 @@
 {
     if (!m_customElementRegistry)
-        m_customElementRegistry = CustomElementRegistry::create(*this);
+        m_customElementRegistry = CustomElementRegistry::create(*this, scriptExecutionContext());
     return *m_customElementRegistry;
 }