Use EnsureStillAliveScope to keep JSValues alive
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Mar 2020 21:25:38 +0000 (21:25 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Mar 2020 21:25:38 +0000 (21:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=209577

Reviewed by Geoffrey Garen.

Some of WebCore code is using JSC::Strong<> to ensure JSC value alive while doing some operations.
But JSC::EnsureStillAliveScope is sufficient for this use case. This patch replaces these Strong<> use
with JSC::EnsureStillAliveScope.

* bindings/js/JSEventListener.h:
(WebCore::JSEventListener::ensureJSFunction const):
* bindings/js/JSWindowProxy.cpp:
(WebCore::JSWindowProxy::setWindow):
* bindings/js/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::initScript):

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

Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSEventListener.h
Source/WebCore/bindings/js/JSWindowProxy.cpp
Source/WebCore/bindings/js/WorkerScriptController.cpp

index 0061d84..a285873 100644 (file)
@@ -1,3 +1,21 @@
+2020-03-26  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        Use EnsureStillAliveScope to keep JSValues alive
+        https://bugs.webkit.org/show_bug.cgi?id=209577
+
+        Reviewed by Geoffrey Garen.
+
+        Some of WebCore code is using JSC::Strong<> to ensure JSC value alive while doing some operations.
+        But JSC::EnsureStillAliveScope is sufficient for this use case. This patch replaces these Strong<> use
+        with JSC::EnsureStillAliveScope.
+
+        * bindings/js/JSEventListener.h:
+        (WebCore::JSEventListener::ensureJSFunction const):
+        * bindings/js/JSWindowProxy.cpp:
+        (WebCore::JSWindowProxy::setWindow):
+        * bindings/js/WorkerScriptController.cpp:
+        (WebCore::WorkerScriptController::initScript):
+
 2020-03-27  Kenneth Russell  <kbr@chromium.org>
 
         Use ANGLE_robust_client_memory to replace framebuffer/texture validation
index 6bd675d..d5153af 100644 (file)
@@ -100,7 +100,7 @@ inline JSC::JSObject* JSEventListener::ensureJSFunction(ScriptExecutionContext&
     // before we're done. It should always return null in this case.
     JSC::VM& vm = m_isolatedWorld->vm();
     auto protect = makeRef(const_cast<JSEventListener&>(*this));
-    JSC::Strong<JSC::JSObject> wrapper(vm, m_wrapper.get());
+    JSC::EnsureStillAliveScope protectedWrapper(m_wrapper.get());
 
     if (!m_isInitialized) {
         ASSERT(!m_jsFunction);
index d91e87b..6a9f6f4 100644 (file)
@@ -95,16 +95,15 @@ void JSWindowProxy::setWindow(AbstractDOMWindow& domWindow)
 
     // Explicitly protect the prototype so it isn't collected when we allocate the global object.
     // (Once the global object is fully constructed, it will mark its own prototype.)
-    // FIXME: Why do we need to protect this when there's a pointer to it on the stack?
-    // Perhaps the issue is that structure objects aren't seen when scanning the stack?
-    Strong<JSNonFinalObject> prototype(vm, isRemoteDOMWindow ? static_cast<JSNonFinalObject*>(JSRemoteDOMWindowPrototype::create(vm, nullptr, &prototypeStructure)) : static_cast<JSNonFinalObject*>(JSDOMWindowPrototype::create(vm, nullptr, &prototypeStructure)));
+    JSNonFinalObject* prototype = isRemoteDOMWindow ? static_cast<JSNonFinalObject*>(JSRemoteDOMWindowPrototype::create(vm, nullptr, &prototypeStructure)) : static_cast<JSNonFinalObject*>(JSDOMWindowPrototype::create(vm, nullptr, &prototypeStructure));
+    JSC::EnsureStillAliveScope protectedPrototype(prototype);
 
     JSDOMGlobalObject* window = nullptr;
     if (isRemoteDOMWindow) {
-        auto& windowStructure = *JSRemoteDOMWindow::createStructure(vm, nullptr, prototype.get());
+        auto& windowStructure = *JSRemoteDOMWindow::createStructure(vm, nullptr, prototype);
         window = JSRemoteDOMWindow::create(vm, &windowStructure, downcast<RemoteDOMWindow>(domWindow), this);
     } else {
-        auto& windowStructure = *JSDOMWindow::createStructure(vm, nullptr, prototype.get());
+        auto& windowStructure = *JSDOMWindow::createStructure(vm, nullptr, prototype);
         window = JSDOMWindow::create(vm, &windowStructure, downcast<DOMWindow>(domWindow), this);
     }
 
index 2eafb81..931cee7 100644 (file)
@@ -80,8 +80,9 @@ void WorkerScriptController::initScript()
     // constructed, it can mark its own prototype.)
     if (m_workerGlobalScope->isDedicatedWorkerGlobalScope()) {
         Structure* dedicatedContextPrototypeStructure = JSDedicatedWorkerGlobalScopePrototype::createStructure(*m_vm, nullptr, jsNull());
-        Strong<JSDedicatedWorkerGlobalScopePrototype> dedicatedContextPrototype(*m_vm, JSDedicatedWorkerGlobalScopePrototype::create(*m_vm, nullptr, dedicatedContextPrototypeStructure));
-        Structure* structure = JSDedicatedWorkerGlobalScope::createStructure(*m_vm, nullptr, dedicatedContextPrototype.get());
+        JSDedicatedWorkerGlobalScopePrototype* dedicatedContextPrototype = JSDedicatedWorkerGlobalScopePrototype::create(*m_vm, nullptr, dedicatedContextPrototypeStructure);
+        JSC::EnsureStillAliveScope protectedDedicatedContextPrototype(dedicatedContextPrototype);
+        Structure* structure = JSDedicatedWorkerGlobalScope::createStructure(*m_vm, nullptr, dedicatedContextPrototype);
         auto* proxyStructure = JSProxy::createStructure(*m_vm, nullptr, jsNull(), PureForwardingProxyType);
         auto* proxy = JSProxy::create(*m_vm, proxyStructure);
 
@@ -99,8 +100,9 @@ void WorkerScriptController::initScript()
 #if ENABLE(SERVICE_WORKER)
     } else if (m_workerGlobalScope->isServiceWorkerGlobalScope()) {
         Structure* contextPrototypeStructure = JSServiceWorkerGlobalScopePrototype::createStructure(*m_vm, nullptr, jsNull());
-        Strong<JSServiceWorkerGlobalScopePrototype> contextPrototype(*m_vm, JSServiceWorkerGlobalScopePrototype::create(*m_vm, nullptr, contextPrototypeStructure));
-        Structure* structure = JSServiceWorkerGlobalScope::createStructure(*m_vm, nullptr, contextPrototype.get());
+        JSServiceWorkerGlobalScopePrototype* contextPrototype = JSServiceWorkerGlobalScopePrototype::create(*m_vm, nullptr, contextPrototypeStructure);
+        JSC::EnsureStillAliveScope protectedContextPrototype(contextPrototype);
+        Structure* structure = JSServiceWorkerGlobalScope::createStructure(*m_vm, nullptr, contextPrototype);
         auto* proxyStructure = JSProxy::createStructure(*m_vm, nullptr, jsNull(), PureForwardingProxyType);
         auto* proxy = JSProxy::create(*m_vm, proxyStructure);