Simplify DOM wrapper destruction, don't deref() in finalizers.
authorakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Apr 2015 02:12:31 +0000 (02:12 +0000)
committerakling@apple.com <akling@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Apr 2015 02:12:31 +0000 (02:12 +0000)
<https://webkit.org/b/144183>

Reviewed by Darin Adler.

DOM JS bindings had two mechanisms to call deref() on the WebCore object,
once through a weak finalizer, and once through the JSCell's regular destructor.

That was once believed to be an optimization, but these days the finalizer will
run just moments before the destructor anyway, all in the same call stack.
And more importantly, the finalizer is not guaranteed to run, for instance in the
case where a Weak is assigned to after going dead, but before the WeakBlock
has been swept by the incremental sweeper.

Simplify this by just removing the deref() from the generated finalizers.
This makes it easier to reason about DOM wrapper destruction, and eliminates
the awkward time window  where a DOM wrapper could have a null impl().

We could spend more time on figuring out a way to have finalizers manage the
destruction of these wrappers, but that would require fundamental changes to
our implementation of JSC::Weak pointers. It would allow us to make JSDOMWrapper
destructor-less, and shrink each wrapper object by 1 pointer (the ClassInfo*.)
However the risk:reward ratio does not seem justified at this point in time.

* bindings/scripts/CodeGeneratorJS.pm:
(GenerateHeader):
(GenerateImplementation):
* bindings/js/JSCSSValueCustom.cpp:
* bindings/scripts/test/JS/JSTestActiveDOMObject.cpp:
(WebCore::JSTestActiveDOMObject::~JSTestActiveDOMObject):
(WebCore::JSTestActiveDOMObjectOwner::finalize):
* bindings/scripts/test/JS/JSTestActiveDOMObject.h:
* bindings/scripts/test/JS/JSTestCustomNamedGetter.cpp:
(WebCore::JSTestCustomNamedGetter::~JSTestCustomNamedGetter):
(WebCore::JSTestCustomNamedGetterOwner::finalize):
* bindings/scripts/test/JS/JSTestCustomNamedGetter.h:
* bindings/scripts/test/JS/JSTestEventConstructor.cpp:
(WebCore::JSTestEventConstructor::~JSTestEventConstructor):
(WebCore::JSTestEventConstructorOwner::finalize):
* bindings/scripts/test/JS/JSTestEventConstructor.h:
* bindings/scripts/test/JS/JSTestEventTarget.cpp:
(WebCore::JSTestEventTarget::~JSTestEventTarget):
(WebCore::JSTestEventTargetOwner::finalize):
* bindings/scripts/test/JS/JSTestEventTarget.h:
* bindings/scripts/test/JS/JSTestException.cpp:
(WebCore::JSTestException::~JSTestException):
(WebCore::JSTestExceptionOwner::finalize):
* bindings/scripts/test/JS/JSTestException.h:
* bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp:
(WebCore::JSTestGenerateIsReachable::~JSTestGenerateIsReachable):
(WebCore::JSTestGenerateIsReachableOwner::finalize):
* bindings/scripts/test/JS/JSTestGenerateIsReachable.h:
* bindings/scripts/test/JS/JSTestInterface.cpp:
(WebCore::JSTestInterface::~JSTestInterface):
(WebCore::JSTestInterfaceOwner::finalize):
* bindings/scripts/test/JS/JSTestInterface.h:
* bindings/scripts/test/JS/JSTestMediaQueryListListener.cpp:
(WebCore::JSTestMediaQueryListListener::~JSTestMediaQueryListListener):
(WebCore::JSTestMediaQueryListListenerOwner::finalize):
* bindings/scripts/test/JS/JSTestMediaQueryListListener.h:
* bindings/scripts/test/JS/JSTestNamedConstructor.cpp:
(WebCore::JSTestNamedConstructor::~JSTestNamedConstructor):
(WebCore::JSTestNamedConstructorOwner::finalize):
* bindings/scripts/test/JS/JSTestNamedConstructor.h:
* bindings/scripts/test/JS/JSTestNondeterministic.cpp:
(WebCore::JSTestNondeterministic::~JSTestNondeterministic):
(WebCore::JSTestNondeterministicOwner::finalize):
* bindings/scripts/test/JS/JSTestNondeterministic.h:
* bindings/scripts/test/JS/JSTestObj.cpp:
(WebCore::JSTestObj::~JSTestObj):
(WebCore::JSTestObjOwner::finalize):
* bindings/scripts/test/JS/JSTestObj.h:
* bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp:
(WebCore::JSTestOverloadedConstructors::~JSTestOverloadedConstructors):
(WebCore::JSTestOverloadedConstructorsOwner::finalize):
* bindings/scripts/test/JS/JSTestOverloadedConstructors.h:
* bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp:
(WebCore::JSTestSerializedScriptValueInterface::~JSTestSerializedScriptValueInterface):
(WebCore::JSTestSerializedScriptValueInterfaceOwner::finalize):
* bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.h:
* bindings/scripts/test/JS/JSTestTypedefs.cpp:
(WebCore::JSTestTypedefs::~JSTestTypedefs):
(WebCore::JSTestTypedefsOwner::finalize):
* bindings/scripts/test/JS/JSTestTypedefs.h:
* bindings/scripts/test/JS/JSattribute.cpp:
(WebCore::JSattribute::~JSattribute):
(WebCore::JSattributeOwner::finalize):
* bindings/scripts/test/JS/JSattribute.h:
* bindings/scripts/test/JS/JSreadonly.cpp:
(WebCore::JSreadonly::~JSreadonly):
(WebCore::JSreadonlyOwner::finalize):
* bindings/scripts/test/JS/JSreadonly.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@183523 268f45cc-cd09-0410-ab3c-d52691b4dbfc

35 files changed:
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSCSSValueCustom.cpp
Source/WebCore/bindings/scripts/CodeGeneratorJS.pm
Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.h
Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.h
Source/WebCore/bindings/scripts/test/JS/JSTestEventConstructor.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestEventConstructor.h
Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.h
Source/WebCore/bindings/scripts/test/JS/JSTestException.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestException.h
Source/WebCore/bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestGenerateIsReachable.h
Source/WebCore/bindings/scripts/test/JS/JSTestInterface.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestInterface.h
Source/WebCore/bindings/scripts/test/JS/JSTestMediaQueryListListener.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestMediaQueryListListener.h
Source/WebCore/bindings/scripts/test/JS/JSTestNamedConstructor.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestNamedConstructor.h
Source/WebCore/bindings/scripts/test/JS/JSTestNondeterministic.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestNondeterministic.h
Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestObj.h
Source/WebCore/bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestOverloadedConstructors.h
Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.h
Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.cpp
Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.h
Source/WebCore/bindings/scripts/test/JS/JSattribute.cpp
Source/WebCore/bindings/scripts/test/JS/JSattribute.h
Source/WebCore/bindings/scripts/test/JS/JSreadonly.cpp
Source/WebCore/bindings/scripts/test/JS/JSreadonly.h

index c7e7ca0..944cdbf 100644 (file)
@@ -1,3 +1,98 @@
+2015-04-28  Andreas Kling  <akling@apple.com>
+
+        Simplify DOM wrapper destruction, don't deref() in finalizers.
+        <https://webkit.org/b/144183>
+
+        Reviewed by Darin Adler.
+
+        DOM JS bindings had two mechanisms to call deref() on the WebCore object,
+        once through a weak finalizer, and once through the JSCell's regular destructor.
+
+        That was once believed to be an optimization, but these days the finalizer will
+        run just moments before the destructor anyway, all in the same call stack.
+        And more importantly, the finalizer is not guaranteed to run, for instance in the
+        case where a Weak is assigned to after going dead, but before the WeakBlock
+        has been swept by the incremental sweeper.
+
+        Simplify this by just removing the deref() from the generated finalizers.
+        This makes it easier to reason about DOM wrapper destruction, and eliminates
+        the awkward time window  where a DOM wrapper could have a null impl().
+
+        We could spend more time on figuring out a way to have finalizers manage the
+        destruction of these wrappers, but that would require fundamental changes to
+        our implementation of JSC::Weak pointers. It would allow us to make JSDOMWrapper
+        destructor-less, and shrink each wrapper object by 1 pointer (the ClassInfo*.)
+        However the risk:reward ratio does not seem justified at this point in time.
+
+        * bindings/scripts/CodeGeneratorJS.pm:
+        (GenerateHeader):
+        (GenerateImplementation):
+        * bindings/js/JSCSSValueCustom.cpp:
+        * bindings/scripts/test/JS/JSTestActiveDOMObject.cpp:
+        (WebCore::JSTestActiveDOMObject::~JSTestActiveDOMObject):
+        (WebCore::JSTestActiveDOMObjectOwner::finalize):
+        * bindings/scripts/test/JS/JSTestActiveDOMObject.h:
+        * bindings/scripts/test/JS/JSTestCustomNamedGetter.cpp:
+        (WebCore::JSTestCustomNamedGetter::~JSTestCustomNamedGetter):
+        (WebCore::JSTestCustomNamedGetterOwner::finalize):
+        * bindings/scripts/test/JS/JSTestCustomNamedGetter.h:
+        * bindings/scripts/test/JS/JSTestEventConstructor.cpp:
+        (WebCore::JSTestEventConstructor::~JSTestEventConstructor):
+        (WebCore::JSTestEventConstructorOwner::finalize):
+        * bindings/scripts/test/JS/JSTestEventConstructor.h:
+        * bindings/scripts/test/JS/JSTestEventTarget.cpp:
+        (WebCore::JSTestEventTarget::~JSTestEventTarget):
+        (WebCore::JSTestEventTargetOwner::finalize):
+        * bindings/scripts/test/JS/JSTestEventTarget.h:
+        * bindings/scripts/test/JS/JSTestException.cpp:
+        (WebCore::JSTestException::~JSTestException):
+        (WebCore::JSTestExceptionOwner::finalize):
+        * bindings/scripts/test/JS/JSTestException.h:
+        * bindings/scripts/test/JS/JSTestGenerateIsReachable.cpp:
+        (WebCore::JSTestGenerateIsReachable::~JSTestGenerateIsReachable):
+        (WebCore::JSTestGenerateIsReachableOwner::finalize):
+        * bindings/scripts/test/JS/JSTestGenerateIsReachable.h:
+        * bindings/scripts/test/JS/JSTestInterface.cpp:
+        (WebCore::JSTestInterface::~JSTestInterface):
+        (WebCore::JSTestInterfaceOwner::finalize):
+        * bindings/scripts/test/JS/JSTestInterface.h:
+        * bindings/scripts/test/JS/JSTestMediaQueryListListener.cpp:
+        (WebCore::JSTestMediaQueryListListener::~JSTestMediaQueryListListener):
+        (WebCore::JSTestMediaQueryListListenerOwner::finalize):
+        * bindings/scripts/test/JS/JSTestMediaQueryListListener.h:
+        * bindings/scripts/test/JS/JSTestNamedConstructor.cpp:
+        (WebCore::JSTestNamedConstructor::~JSTestNamedConstructor):
+        (WebCore::JSTestNamedConstructorOwner::finalize):
+        * bindings/scripts/test/JS/JSTestNamedConstructor.h:
+        * bindings/scripts/test/JS/JSTestNondeterministic.cpp:
+        (WebCore::JSTestNondeterministic::~JSTestNondeterministic):
+        (WebCore::JSTestNondeterministicOwner::finalize):
+        * bindings/scripts/test/JS/JSTestNondeterministic.h:
+        * bindings/scripts/test/JS/JSTestObj.cpp:
+        (WebCore::JSTestObj::~JSTestObj):
+        (WebCore::JSTestObjOwner::finalize):
+        * bindings/scripts/test/JS/JSTestObj.h:
+        * bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp:
+        (WebCore::JSTestOverloadedConstructors::~JSTestOverloadedConstructors):
+        (WebCore::JSTestOverloadedConstructorsOwner::finalize):
+        * bindings/scripts/test/JS/JSTestOverloadedConstructors.h:
+        * bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp:
+        (WebCore::JSTestSerializedScriptValueInterface::~JSTestSerializedScriptValueInterface):
+        (WebCore::JSTestSerializedScriptValueInterfaceOwner::finalize):
+        * bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.h:
+        * bindings/scripts/test/JS/JSTestTypedefs.cpp:
+        (WebCore::JSTestTypedefs::~JSTestTypedefs):
+        (WebCore::JSTestTypedefsOwner::finalize):
+        * bindings/scripts/test/JS/JSTestTypedefs.h:
+        * bindings/scripts/test/JS/JSattribute.cpp:
+        (WebCore::JSattribute::~JSattribute):
+        (WebCore::JSattributeOwner::finalize):
+        * bindings/scripts/test/JS/JSattribute.h:
+        * bindings/scripts/test/JS/JSreadonly.cpp:
+        (WebCore::JSreadonly::~JSreadonly):
+        (WebCore::JSreadonlyOwner::finalize):
+        * bindings/scripts/test/JS/JSreadonly.h:
+
 2015-04-28  Alex Christensen  <achristensen@webkit.org>
 
         Build WinCairo without cygwin.
index 4ad1f60..ed51253 100644 (file)
@@ -62,7 +62,6 @@ void JSCSSValueOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
     DOMWrapperWorld& world = *static_cast<DOMWrapperWorld*>(context);
     world.m_cssValueRoots.remove(&jsCSSValue->impl());
     uncacheWrapper(world, &jsCSSValue->impl(), jsCSSValue);
-    jsCSSValue->releaseImpl();
 }
 
 JSValue toJS(ExecState*, JSDOMGlobalObject* globalObject, CSSValue* value)
index 625c139..515da75 100644 (file)
@@ -1086,14 +1086,7 @@ sub GenerateHeader
 
     if (!$hasParent) {
         push(@headerContent, "    $implType& impl() const { return *m_impl; }\n");
-        push(@headerContent, "    void releaseImpl() { m_impl->deref(); m_impl = 0; }\n\n");
-        push(@headerContent, "    void releaseImplIfNotNull()\n");
-        push(@headerContent, "    {\n");
-        push(@headerContent, "        if (m_impl) {\n");
-        push(@headerContent, "            m_impl->deref();\n");
-        push(@headerContent, "            m_impl = 0;\n");
-        push(@headerContent, "        }\n");
-        push(@headerContent, "    }\n\n");
+        push(@headerContent, "    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }\n\n");
         push(@headerContent, "private:\n");
         push(@headerContent, "    $implType* m_impl;\n");
     } else {
@@ -2077,23 +2070,15 @@ sub GenerateImplementation
     }
 
     if (!$hasParent) {
-        # FIXME: This destroy function should not be necessary, as 
-        # a finalizer should be called for each DOM object wrapper.
-        # However, that seems not to be the case, so this has been
-        # added back to avoid leaking while we figure out why the
-        # finalizers are not always getting called. The work tracking
-        # the finalizer issue is being tracked in http://webkit.org/b/75451
         push(@implContent, "void ${className}::destroy(JSC::JSCell* cell)\n");
         push(@implContent, "{\n");
         push(@implContent, "    ${className}* thisObject = static_cast<${className}*>(cell);\n");
         push(@implContent, "    thisObject->${className}::~${className}();\n");
         push(@implContent, "}\n\n");
 
-        # We also need a destructor for the allocateCell to work properly with the destructor-free part of the heap.
-        # Otherwise, these destroy functions/destructors won't get called.
         push(@implContent, "${className}::~${className}()\n");
         push(@implContent, "{\n");
-        push(@implContent, "    releaseImplIfNotNull();\n");
+        push(@implContent, "    releaseImpl();\n");
         push(@implContent, "}\n\n");
     }
 
@@ -3004,7 +2989,6 @@ sub GenerateImplementation
         push(@implContent, "    auto* js${interfaceName} = jsCast<JS${interfaceName}*>(handle.slot()->asCell());\n");
         push(@implContent, "    auto& world = *static_cast<DOMWrapperWorld*>(context);\n");
         push(@implContent, "    uncacheWrapper(world, &js${interfaceName}->impl(), js${interfaceName});\n");
-        push(@implContent, "    js${interfaceName}->releaseImpl();\n");
         push(@implContent, "}\n\n");
     }
 
index a7e0aee..74e94f8 100644 (file)
@@ -163,7 +163,7 @@ void JSTestActiveDOMObject::destroy(JSC::JSCell* cell)
 
 JSTestActiveDOMObject::~JSTestActiveDOMObject()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 bool JSTestActiveDOMObject::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
@@ -250,7 +250,6 @@ void JSTestActiveDOMObjectOwner::finalize(JSC::Handle<JSC::Unknown> handle, void
     auto* jsTestActiveDOMObject = jsCast<JSTestActiveDOMObject*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestActiveDOMObject->impl(), jsTestActiveDOMObject);
-    jsTestActiveDOMObject->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index 5c0ec3f..c7151e1 100644 (file)
@@ -53,15 +53,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     TestActiveDOMObject& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestActiveDOMObject* m_impl;
index 973ab03..a437ba5 100644 (file)
@@ -156,7 +156,7 @@ void JSTestCustomNamedGetter::destroy(JSC::JSCell* cell)
 
 JSTestCustomNamedGetter::~JSTestCustomNamedGetter()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 bool JSTestCustomNamedGetter::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
@@ -224,7 +224,6 @@ void JSTestCustomNamedGetterOwner::finalize(JSC::Handle<JSC::Unknown> handle, vo
     auto* jsTestCustomNamedGetter = jsCast<JSTestCustomNamedGetter*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestCustomNamedGetter->impl(), jsTestCustomNamedGetter);
-    jsTestCustomNamedGetter->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index a768cea..be1825c 100644 (file)
@@ -54,15 +54,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     TestCustomNamedGetter& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestCustomNamedGetter* m_impl;
index 807b398..2780bfb 100644 (file)
@@ -191,7 +191,7 @@ void JSTestEventConstructor::destroy(JSC::JSCell* cell)
 
 JSTestEventConstructor::~JSTestEventConstructor()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 EncodedJSValue jsTestEventConstructorAttr1(ExecState* exec, JSObject* slotBase, EncodedJSValue thisValue, PropertyName)
@@ -253,7 +253,6 @@ void JSTestEventConstructorOwner::finalize(JSC::Handle<JSC::Unknown> handle, voi
     auto* jsTestEventConstructor = jsCast<JSTestEventConstructor*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestEventConstructor->impl(), jsTestEventConstructor);
-    jsTestEventConstructor->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index a568560..624a60a 100644 (file)
@@ -54,15 +54,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     TestEventConstructor& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestEventConstructor* m_impl;
index 6bbcdc3..b31aff0 100644 (file)
@@ -168,7 +168,7 @@ void JSTestEventTarget::destroy(JSC::JSCell* cell)
 
 JSTestEventTarget::~JSTestEventTarget()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 bool JSTestEventTarget::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
@@ -326,7 +326,6 @@ void JSTestEventTargetOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* co
     auto* jsTestEventTarget = jsCast<JSTestEventTarget*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestEventTarget->impl(), jsTestEventTarget);
-    jsTestEventTarget->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index 0826f54..ebe2db1 100644 (file)
@@ -58,15 +58,7 @@ public:
     static void visitChildren(JSCell*, JSC::SlotVisitor&);
 
     TestEventTarget& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestEventTarget* m_impl;
index 1793959..a261d8d 100644 (file)
@@ -153,7 +153,7 @@ void JSTestException::destroy(JSC::JSCell* cell)
 
 JSTestException::~JSTestException()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 bool JSTestException::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
@@ -200,7 +200,6 @@ void JSTestExceptionOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* cont
     auto* jsTestException = jsCast<JSTestException*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestException->impl(), jsTestException);
-    jsTestException->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index 53b5e17..71c1431 100644 (file)
@@ -54,15 +54,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     TestException& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestException* m_impl;
index e4f88ff..0bde72a 100644 (file)
@@ -135,7 +135,7 @@ void JSTestGenerateIsReachable::destroy(JSC::JSCell* cell)
 
 JSTestGenerateIsReachable::~JSTestGenerateIsReachable()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 EncodedJSValue jsTestGenerateIsReachableConstructor(ExecState* exec, JSObject* baseValue, EncodedJSValue, PropertyName)
@@ -172,7 +172,6 @@ void JSTestGenerateIsReachableOwner::finalize(JSC::Handle<JSC::Unknown> handle,
     auto* jsTestGenerateIsReachable = jsCast<JSTestGenerateIsReachable*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestGenerateIsReachable->impl(), jsTestGenerateIsReachable);
-    jsTestGenerateIsReachable->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index aacd3b6..e451916 100644 (file)
@@ -52,15 +52,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     TestGenerateIsReachable& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestGenerateIsReachable* m_impl;
index 51f8fee..592e86e 100644 (file)
@@ -434,7 +434,7 @@ void JSTestInterface::destroy(JSC::JSCell* cell)
 
 JSTestInterface::~JSTestInterface()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 bool JSTestInterface::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
@@ -951,7 +951,6 @@ void JSTestInterfaceOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* cont
     auto* jsTestInterface = jsCast<JSTestInterface*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestInterface->impl(), jsTestInterface);
-    jsTestInterface->releaseImpl();
 }
 
 JSC::JSValue toJS(JSC::ExecState*, JSDOMGlobalObject* globalObject, TestInterface* impl)
index 7ae69c2..e2603dc 100644 (file)
@@ -80,15 +80,7 @@ public:
     JSC::JSValue supplementalMethod3(JSC::ExecState*);
 #endif
     TestInterface& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestInterface* m_impl;
index 5851fcc..bc63fd1 100644 (file)
@@ -143,7 +143,7 @@ void JSTestMediaQueryListListener::destroy(JSC::JSCell* cell)
 
 JSTestMediaQueryListListener::~JSTestMediaQueryListListener()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 EncodedJSValue jsTestMediaQueryListListenerConstructor(ExecState* exec, JSObject* baseValue, EncodedJSValue, PropertyName)
@@ -188,7 +188,6 @@ void JSTestMediaQueryListListenerOwner::finalize(JSC::Handle<JSC::Unknown> handl
     auto* jsTestMediaQueryListListener = jsCast<JSTestMediaQueryListListener*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestMediaQueryListListener->impl(), jsTestMediaQueryListListener);
-    jsTestMediaQueryListListener->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index b11eb08..44b3250 100644 (file)
@@ -52,15 +52,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     TestMediaQueryListListener& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestMediaQueryListListener* m_impl;
index 817db54..aafd153 100644 (file)
@@ -207,7 +207,7 @@ void JSTestNamedConstructor::destroy(JSC::JSCell* cell)
 
 JSTestNamedConstructor::~JSTestNamedConstructor()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 EncodedJSValue jsTestNamedConstructorConstructor(ExecState* exec, JSObject* baseValue, EncodedJSValue, PropertyName)
@@ -242,7 +242,6 @@ void JSTestNamedConstructorOwner::finalize(JSC::Handle<JSC::Unknown> handle, voi
     auto* jsTestNamedConstructor = jsCast<JSTestNamedConstructor*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestNamedConstructor->impl(), jsTestNamedConstructor);
-    jsTestNamedConstructor->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index 861231a..9ffbf3b 100644 (file)
@@ -53,15 +53,7 @@ public:
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     static JSC::JSValue getNamedConstructor(JSC::VM&, JSC::JSGlobalObject*);
     TestNamedConstructor& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestNamedConstructor* m_impl;
index 10766f1..e576145 100644 (file)
@@ -165,7 +165,7 @@ void JSTestNondeterministic::destroy(JSC::JSCell* cell)
 
 JSTestNondeterministic::~JSTestNondeterministic()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 EncodedJSValue jsTestNondeterministicNondeterministicReadonlyAttr(ExecState* exec, JSObject* slotBase, EncodedJSValue thisValue, PropertyName)
@@ -495,7 +495,6 @@ void JSTestNondeterministicOwner::finalize(JSC::Handle<JSC::Unknown> handle, voi
     auto* jsTestNondeterministic = jsCast<JSTestNondeterministic*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestNondeterministic->impl(), jsTestNondeterministic);
-    jsTestNondeterministic->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index a3fca4e..e0b5b65 100644 (file)
@@ -52,15 +52,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     TestNondeterministic& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestNondeterministic* m_impl;
index c2d7d66..f121b72 100644 (file)
@@ -682,7 +682,7 @@ void JSTestObj::destroy(JSC::JSCell* cell)
 
 JSTestObj::~JSTestObj()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 bool JSTestObj::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
@@ -4430,7 +4430,6 @@ void JSTestObjOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
     auto* jsTestObj = jsCast<JSTestObj*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestObj->impl(), jsTestObj);
-    jsTestObj->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index 258a355..d24fd6d 100644 (file)
@@ -66,15 +66,7 @@ public:
     JSC::JSValue customMethodWithArgs(JSC::ExecState*);
     static JSC::JSValue classMethod2(JSC::ExecState*);
     TestObj& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestObj* m_impl;
index 311752a..232cdb2 100644 (file)
@@ -216,7 +216,7 @@ void JSTestOverloadedConstructors::destroy(JSC::JSCell* cell)
 
 JSTestOverloadedConstructors::~JSTestOverloadedConstructors()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 EncodedJSValue jsTestOverloadedConstructorsConstructor(ExecState* exec, JSObject* baseValue, EncodedJSValue, PropertyName)
@@ -244,7 +244,6 @@ void JSTestOverloadedConstructorsOwner::finalize(JSC::Handle<JSC::Unknown> handl
     auto* jsTestOverloadedConstructors = jsCast<JSTestOverloadedConstructors*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestOverloadedConstructors->impl(), jsTestOverloadedConstructors);
-    jsTestOverloadedConstructors->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index e51cb67..07e915b 100644 (file)
@@ -52,15 +52,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     TestOverloadedConstructors& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestOverloadedConstructors* m_impl;
index 86460a4..c2fe0c0 100644 (file)
@@ -155,7 +155,7 @@ void JSTestSerializedScriptValueInterface::destroy(JSC::JSCell* cell)
 
 JSTestSerializedScriptValueInterface::~JSTestSerializedScriptValueInterface()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 EncodedJSValue jsTestSerializedScriptValueInterfaceValue(ExecState* exec, JSObject* slotBase, EncodedJSValue thisValue, PropertyName)
@@ -323,7 +323,6 @@ void JSTestSerializedScriptValueInterfaceOwner::finalize(JSC::Handle<JSC::Unknow
     auto* jsTestSerializedScriptValueInterface = jsCast<JSTestSerializedScriptValueInterface*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestSerializedScriptValueInterface->impl(), jsTestSerializedScriptValueInterface);
-    jsTestSerializedScriptValueInterface->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index af17b98..52fe9a9 100644 (file)
@@ -58,15 +58,7 @@ public:
     static void visitChildren(JSCell*, JSC::SlotVisitor&);
 
     TestSerializedScriptValueInterface& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestSerializedScriptValueInterface* m_impl;
index 9a7b5dc..1b54489 100644 (file)
@@ -238,7 +238,7 @@ void JSTestTypedefs::destroy(JSC::JSCell* cell)
 
 JSTestTypedefs::~JSTestTypedefs()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 bool JSTestTypedefs::getOwnPropertySlot(JSObject* object, ExecState* exec, PropertyName propertyName, PropertySlot& slot)
@@ -730,7 +730,6 @@ void JSTestTypedefsOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* conte
     auto* jsTestTypedefs = jsCast<JSTestTypedefs*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsTestTypedefs->impl(), jsTestTypedefs);
-    jsTestTypedefs->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index f85eea5..8f90c09 100644 (file)
@@ -53,15 +53,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     TestTypedefs& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     TestTypedefs* m_impl;
index c89cf7e..6137c4e 100644 (file)
@@ -140,7 +140,7 @@ void JSattribute::destroy(JSC::JSCell* cell)
 
 JSattribute::~JSattribute()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 EncodedJSValue jsattributeReadonly(ExecState* exec, JSObject* slotBase, EncodedJSValue thisValue, PropertyName)
@@ -185,7 +185,6 @@ void JSattributeOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
     auto* jsattribute = jsCast<JSattribute*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsattribute->impl(), jsattribute);
-    jsattribute->releaseImpl();
 }
 
 #if ENABLE(BINDING_INTEGRITY)
index 3aa7c56..c8df35b 100644 (file)
@@ -53,15 +53,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     attribute& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     attribute* m_impl;
index c861305..4723eac 100644 (file)
@@ -135,7 +135,7 @@ void JSreadonly::destroy(JSC::JSCell* cell)
 
 JSreadonly::~JSreadonly()
 {
-    releaseImplIfNotNull();
+    releaseImpl();
 }
 
 EncodedJSValue jsreadonlyConstructor(ExecState* exec, JSObject* baseValue, EncodedJSValue, PropertyName)
@@ -163,7 +163,6 @@ void JSreadonlyOwner::finalize(JSC::Handle<JSC::Unknown> handle, void* context)
     auto* jsreadonly = jsCast<JSreadonly*>(handle.slot()->asCell());
     auto& world = *static_cast<DOMWrapperWorld*>(context);
     uncacheWrapper(world, &jsreadonly->impl(), jsreadonly);
-    jsreadonly->releaseImpl();
 }
 
 JSC::JSValue toJS(JSC::ExecState*, JSDOMGlobalObject* globalObject, readonly* impl)
index 36c430f..9abad72 100644 (file)
@@ -52,15 +52,7 @@ public:
 
     static JSC::JSValue getConstructor(JSC::VM&, JSC::JSGlobalObject*);
     readonly& impl() const { return *m_impl; }
-    void releaseImpl() { m_impl->deref(); m_impl = 0; }
-
-    void releaseImplIfNotNull()
-    {
-        if (m_impl) {
-            m_impl->deref();
-            m_impl = 0;
-        }
-    }
+    void releaseImpl() { std::exchange(m_impl, nullptr)->deref(); }
 
 private:
     readonly* m_impl;