Title: [231987] branches/safari-606.1.17-branch/Source
Revision
231987
Author
kocsen_ch...@apple.com
Date
2018-05-18 15:57:17 -0700 (Fri, 18 May 2018)

Log Message

Cherry-pick r231963. rdar://problem/40004666

    Avoid keeping the frame alive when ref'ing a WindowProxy
    https://bugs.webkit.org/show_bug.cgi?id=185737
    <rdar://problem/40004666>

    Reviewed by Sam Weinig.

    Source/WebCore:

    Avoid keeping the frame alive when ref'ing a WindowProxy by making WindowProxy
    manage its own refcount (instead of proxying refcounting to the Frame). As a
    result, a WindowProxy can now be detached from its Frame. When detached, it
    return null when asked for a JSWindowProxy.

    It is important to not extend the lifetime of the Frame because we want script
    to stop running when the Page gets destroyed.

    * bindings/js/JSWindowProxy.cpp:
    (WebCore::toJS):
    (WebCore::toJSWindowProxy):
    * bindings/js/JSWindowProxy.h:
    (WebCore::toJSWindowProxy):
    * bindings/js/ScriptController.cpp:
    (WebCore::ScriptController::evaluateInWorld):
    (WebCore::ScriptController::loadModuleScriptInWorld):
    (WebCore::ScriptController::linkAndEvaluateModuleScriptInWorld):
    (WebCore::ScriptController::evaluateModule):
    (WebCore::ScriptController::setupModuleScriptHandlers):
    (WebCore::ScriptController::jsWindowProxy):
    (WebCore::ScriptController::windowScriptNPObject):
    (WebCore::ScriptController::executeIfJavaScriptURL):
    * bindings/js/ScriptController.h:
    (WebCore::ScriptController::globalObject):
    * bindings/js/ScriptControllerMac.mm:
    (WebCore::ScriptController::windowScriptObject):
    * bindings/js/ScriptState.cpp:
    (WebCore::mainWorldExecState):
    * bindings/js/WindowProxy.cpp:
    (WebCore::WindowProxy::WindowProxy):
    (WebCore::WindowProxy::~WindowProxy):
    (WebCore::WindowProxy::detachFromFrame):
    (WebCore::WindowProxy::createJSWindowProxy):
    (WebCore::WindowProxy::globalObject):
    (WebCore::WindowProxy::createJSWindowProxyWithInitializedScript):
    (WebCore::WindowProxy::setDOMWindow):
    (WebCore::WindowProxy::window const):
    (WebCore::WindowProxy::ref): Deleted.
    (WebCore::WindowProxy::deref): Deleted.
    * bindings/js/WindowProxy.h:
    (WebCore::WindowProxy::create):
    (WebCore::WindowProxy::frame const):
    (WebCore::WindowProxy::jsWindowProxy):
    * dom/DocumentTouch.cpp:
    (WebCore::DocumentTouch::createTouch):
    * page/AbstractFrame.cpp:
    (WebCore::AbstractFrame::AbstractFrame):
    (WebCore::AbstractFrame::~AbstractFrame):
    * page/AbstractFrame.h:

    Source/WebKit:

    * WebProcess/Plugins/PluginView.cpp:
    (WebKit::PluginView::windowScriptNPObject):

    Source/WebKitLegacy/mac:

    * Plugins/Hosted/NetscapePluginInstanceProxy.mm:
    (WebKit::NetscapePluginInstanceProxy::getWindowNPObject):

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

Modified Paths

Diff

Modified: branches/safari-606.1.17-branch/Source/WebCore/ChangeLog (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/ChangeLog	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/ChangeLog	2018-05-18 22:57:17 UTC (rev 231987)
@@ -1,3 +1,136 @@
+2018-05-18  Kocsen Chung  <kocsen_ch...@apple.com>
+
+        Cherry-pick r231963. rdar://problem/40004666
+
+    Avoid keeping the frame alive when ref'ing a WindowProxy
+    https://bugs.webkit.org/show_bug.cgi?id=185737
+    <rdar://problem/40004666>
+    
+    Reviewed by Sam Weinig.
+    
+    Source/WebCore:
+    
+    Avoid keeping the frame alive when ref'ing a WindowProxy by making WindowProxy
+    manage its own refcount (instead of proxying refcounting to the Frame). As a
+    result, a WindowProxy can now be detached from its Frame. When detached, it
+    return null when asked for a JSWindowProxy.
+    
+    It is important to not extend the lifetime of the Frame because we want script
+    to stop running when the Page gets destroyed.
+    
+    * bindings/js/JSWindowProxy.cpp:
+    (WebCore::toJS):
+    (WebCore::toJSWindowProxy):
+    * bindings/js/JSWindowProxy.h:
+    (WebCore::toJSWindowProxy):
+    * bindings/js/ScriptController.cpp:
+    (WebCore::ScriptController::evaluateInWorld):
+    (WebCore::ScriptController::loadModuleScriptInWorld):
+    (WebCore::ScriptController::linkAndEvaluateModuleScriptInWorld):
+    (WebCore::ScriptController::evaluateModule):
+    (WebCore::ScriptController::setupModuleScriptHandlers):
+    (WebCore::ScriptController::jsWindowProxy):
+    (WebCore::ScriptController::windowScriptNPObject):
+    (WebCore::ScriptController::executeIfJavaScriptURL):
+    * bindings/js/ScriptController.h:
+    (WebCore::ScriptController::globalObject):
+    * bindings/js/ScriptControllerMac.mm:
+    (WebCore::ScriptController::windowScriptObject):
+    * bindings/js/ScriptState.cpp:
+    (WebCore::mainWorldExecState):
+    * bindings/js/WindowProxy.cpp:
+    (WebCore::WindowProxy::WindowProxy):
+    (WebCore::WindowProxy::~WindowProxy):
+    (WebCore::WindowProxy::detachFromFrame):
+    (WebCore::WindowProxy::createJSWindowProxy):
+    (WebCore::WindowProxy::globalObject):
+    (WebCore::WindowProxy::createJSWindowProxyWithInitializedScript):
+    (WebCore::WindowProxy::setDOMWindow):
+    (WebCore::WindowProxy::window const):
+    (WebCore::WindowProxy::ref): Deleted.
+    (WebCore::WindowProxy::deref): Deleted.
+    * bindings/js/WindowProxy.h:
+    (WebCore::WindowProxy::create):
+    (WebCore::WindowProxy::frame const):
+    (WebCore::WindowProxy::jsWindowProxy):
+    * dom/DocumentTouch.cpp:
+    (WebCore::DocumentTouch::createTouch):
+    * page/AbstractFrame.cpp:
+    (WebCore::AbstractFrame::AbstractFrame):
+    (WebCore::AbstractFrame::~AbstractFrame):
+    * page/AbstractFrame.h:
+    
+    Source/WebKit:
+    
+    * WebProcess/Plugins/PluginView.cpp:
+    (WebKit::PluginView::windowScriptNPObject):
+    
+    Source/WebKitLegacy/mac:
+    
+    * Plugins/Hosted/NetscapePluginInstanceProxy.mm:
+    (WebKit::NetscapePluginInstanceProxy::getWindowNPObject):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231963 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-05-18  Chris Dumez  <cdu...@apple.com>
+
+            Avoid keeping the frame alive when ref'ing a WindowProxy
+            https://bugs.webkit.org/show_bug.cgi?id=185737
+            <rdar://problem/40004666>
+
+            Reviewed by Sam Weinig.
+
+            Avoid keeping the frame alive when ref'ing a WindowProxy by making WindowProxy
+            manage its own refcount (instead of proxying refcounting to the Frame). As a
+            result, a WindowProxy can now be detached from its Frame. When detached, it
+            return null when asked for a JSWindowProxy.
+
+            It is important to not extend the lifetime of the Frame because we want script
+            to stop running when the Page gets destroyed.
+
+            * bindings/js/JSWindowProxy.cpp:
+            (WebCore::toJS):
+            (WebCore::toJSWindowProxy):
+            * bindings/js/JSWindowProxy.h:
+            (WebCore::toJSWindowProxy):
+            * bindings/js/ScriptController.cpp:
+            (WebCore::ScriptController::evaluateInWorld):
+            (WebCore::ScriptController::loadModuleScriptInWorld):
+            (WebCore::ScriptController::linkAndEvaluateModuleScriptInWorld):
+            (WebCore::ScriptController::evaluateModule):
+            (WebCore::ScriptController::setupModuleScriptHandlers):
+            (WebCore::ScriptController::jsWindowProxy):
+            (WebCore::ScriptController::windowScriptNPObject):
+            (WebCore::ScriptController::executeIfJavaScriptURL):
+            * bindings/js/ScriptController.h:
+            (WebCore::ScriptController::globalObject):
+            * bindings/js/ScriptControllerMac.mm:
+            (WebCore::ScriptController::windowScriptObject):
+            * bindings/js/ScriptState.cpp:
+            (WebCore::mainWorldExecState):
+            * bindings/js/WindowProxy.cpp:
+            (WebCore::WindowProxy::WindowProxy):
+            (WebCore::WindowProxy::~WindowProxy):
+            (WebCore::WindowProxy::detachFromFrame):
+            (WebCore::WindowProxy::createJSWindowProxy):
+            (WebCore::WindowProxy::globalObject):
+            (WebCore::WindowProxy::createJSWindowProxyWithInitializedScript):
+            (WebCore::WindowProxy::setDOMWindow):
+            (WebCore::WindowProxy::window const):
+            (WebCore::WindowProxy::ref): Deleted.
+            (WebCore::WindowProxy::deref): Deleted.
+            * bindings/js/WindowProxy.h:
+            (WebCore::WindowProxy::create):
+            (WebCore::WindowProxy::frame const):
+            (WebCore::WindowProxy::jsWindowProxy):
+            * dom/DocumentTouch.cpp:
+            (WebCore::DocumentTouch::createTouch):
+            * page/AbstractFrame.cpp:
+            (WebCore::AbstractFrame::AbstractFrame):
+            (WebCore::AbstractFrame::~AbstractFrame):
+            * page/AbstractFrame.h:
+
 2018-05-17  Babak Shafiei  <bshaf...@apple.com>
 
         Cherry-pick r231910. rdar://problem/39105791

Modified: branches/safari-606.1.17-branch/Source/WebCore/bindings/js/JSWindowProxy.cpp (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/bindings/js/JSWindowProxy.cpp	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/bindings/js/JSWindowProxy.cpp	2018-05-18 22:57:17 UTC (rev 231987)
@@ -146,10 +146,11 @@
 
 JSValue toJS(ExecState* state, WindowProxy& windowProxy)
 {
-    return &windowProxy.jsWindowProxy(currentWorld(*state));
+    auto* jsWindowProxy = windowProxy.jsWindowProxy(currentWorld(*state));
+    return jsWindowProxy ? JSValue(jsWindowProxy) : jsNull();
 }
 
-JSWindowProxy& toJSWindowProxy(WindowProxy& windowProxy, DOMWrapperWorld& world)
+JSWindowProxy* toJSWindowProxy(WindowProxy& windowProxy, DOMWrapperWorld& world)
 {
     return windowProxy.jsWindowProxy(world);
 }

Modified: branches/safari-606.1.17-branch/Source/WebCore/bindings/js/JSWindowProxy.h (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/bindings/js/JSWindowProxy.h	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/bindings/js/JSWindowProxy.h	2018-05-18 22:57:17 UTC (rev 231987)
@@ -77,8 +77,8 @@
 inline JSC::JSValue toJS(JSC::ExecState* state, JSDOMGlobalObject*, WindowProxy& windowProxy) { return toJS(state, windowProxy); }
 inline JSC::JSValue toJS(JSC::ExecState* state, JSDOMGlobalObject* globalObject, WindowProxy* windowProxy) { return windowProxy ? toJS(state, globalObject, *windowProxy) : JSC::jsNull(); }
 
-JSWindowProxy& toJSWindowProxy(WindowProxy&, DOMWrapperWorld&);
-inline JSWindowProxy* toJSWindowProxy(WindowProxy* windowProxy, DOMWrapperWorld& world) { return windowProxy ? &toJSWindowProxy(*windowProxy, world) : nullptr; }
+JSWindowProxy* toJSWindowProxy(WindowProxy&, DOMWrapperWorld&);
+inline JSWindowProxy* toJSWindowProxy(WindowProxy* windowProxy, DOMWrapperWorld& world) { return windowProxy ? toJSWindowProxy(*windowProxy, world) : nullptr; }
 
 
 template<> struct JSDOMWrapperConverterTraits<WindowProxy> {

Modified: branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptController.cpp (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptController.cpp	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptController.cpp	2018-05-18 22:57:17 UTC (rev 231987)
@@ -117,7 +117,7 @@
     // and false for <script>doSomething()</script>. Check if it has the
     // expected value in all cases.
     // See smart window.open policy for where this is used.
-    auto& proxy = windowProxy().jsWindowProxy(world);
+    auto& proxy = jsWindowProxy(world);
     auto& exec = *proxy.window()->globalExec();
     const String* savedSourceURL = m_sourceURL;
     m_sourceURL = &sourceURL;
@@ -150,7 +150,7 @@
 {
     JSLockHolder lock(world.vm());
 
-    auto& proxy = windowProxy().jsWindowProxy(world);
+    auto& proxy = jsWindowProxy(world);
     auto& state = *proxy.window()->globalExec();
 
     auto& promise = JSMainThreadExecState::loadModule(state, moduleName, JSC::JSScriptFetchParameters::create(state.vm(), WTFMove(topLevelFetchParameters)), JSC::JSScriptFetcher::create(state.vm(), { &moduleScript }));
@@ -166,7 +166,7 @@
 {
     JSLockHolder lock(world.vm());
 
-    auto& proxy = windowProxy().jsWindowProxy(world);
+    auto& proxy = jsWindowProxy(world);
     auto& state = *proxy.window()->globalExec();
 
     auto& promise = JSMainThreadExecState::loadModule(state, sourceCode.jsSourceCode(), JSC::JSScriptFetcher::create(state.vm(), { &moduleScript }));
@@ -182,7 +182,7 @@
 {
     JSLockHolder lock(world.vm());
 
-    auto& proxy = windowProxy().jsWindowProxy(world);
+    auto& proxy = jsWindowProxy(world);
     auto& state = *proxy.window()->globalExec();
 
     // FIXME: Preventing Frame from being destroyed is essentially unnecessary.
@@ -211,7 +211,7 @@
 
     const auto& jsSourceCode = moduleRecord.sourceCode();
 
-    auto& proxy = windowProxy().jsWindowProxy(world);
+    auto& proxy = jsWindowProxy(world);
     auto& state = *proxy.window()->globalExec();
     SetForScope<const String*> sourceURLScope(m_sourceURL, &sourceURL.string());
 
@@ -268,7 +268,7 @@
 
 void ScriptController::setupModuleScriptHandlers(LoadableModuleScript& moduleScriptRef, JSInternalPromise& promise, DOMWrapperWorld& world)
 {
-    auto& proxy = windowProxy().jsWindowProxy(world);
+    auto& proxy = jsWindowProxy(world);
     auto& state = *proxy.window()->globalExec();
 
     // It is not guaranteed that either fulfillHandler or rejectHandler is eventually called.
@@ -325,6 +325,13 @@
     return m_frame.windowProxy();
 }
 
+JSWindowProxy& ScriptController::jsWindowProxy(DOMWrapperWorld& world)
+{
+    auto* jsWindowProxy = m_frame.windowProxy().jsWindowProxy(world);
+    ASSERT_WITH_MESSAGE(jsWindowProxy, "The JSWindowProxy can only be null if the frame has been destroyed");
+    return *jsWindowProxy;
+}
+
 TextPosition ScriptController::eventHandlerPosition() const
 {
     // FIXME: If we are not currently parsing, we should use our current location
@@ -442,7 +449,7 @@
         if (canExecuteScripts(NotAboutToExecuteScript)) {
             // _javascript_ is enabled, so there is a _javascript_ window object.
             // Return an NPObject bound to the window object.
-            auto* window = windowProxy().jsWindowProxy(pluginWorld()).window();
+            auto* window = jsWindowProxy(pluginWorld()).window();
             ASSERT(window);
             Bindings::RootObject* root = bindingRootObject();
             m_windowScriptNPObject = _NPN_CreateScriptObject(0, window, root);
@@ -603,7 +610,7 @@
         return true;
 
     String scriptResult;
-    if (!result || !result.getString(windowProxy().jsWindowProxy(mainThreadNormalWorld()).window()->globalExec(), scriptResult))
+    if (!result || !result.getString(jsWindowProxy(mainThreadNormalWorld()).window()->globalExec(), scriptResult))
         return true;
 
     // FIXME: We should always replace the document, but doing so

Modified: branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptController.h (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptController.h	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptController.h	2018-05-18 22:57:17 UTC (rev 231987)
@@ -83,7 +83,7 @@
 
     JSDOMWindow* globalObject(DOMWrapperWorld& world)
     {
-        return JSC::jsCast<JSDOMWindow*>(windowProxy().jsWindowProxy(world).window());
+        return JSC::jsCast<JSDOMWindow*>(jsWindowProxy(world).window());
     }
 
     static void getAllWorlds(Vector<Ref<DOMWrapperWorld>>&);
@@ -166,6 +166,7 @@
     void disconnectPlatformScriptObjects();
 
     WEBCORE_EXPORT WindowProxy& windowProxy();
+    WEBCORE_EXPORT JSWindowProxy& jsWindowProxy(DOMWrapperWorld&);
 
     Frame& m_frame;
     const String* m_sourceURL;

Modified: branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptControllerMac.mm (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptControllerMac.mm	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptControllerMac.mm	2018-05-18 22:57:17 UTC (rev 231987)
@@ -103,7 +103,7 @@
     if (!m_windowScriptObject) {
         JSC::JSLockHolder lock(commonVM());
         JSC::Bindings::RootObject* root = bindingRootObject();
-        m_windowScriptObject = [WebScriptObject scriptObjectForJSObject:toRef(&windowProxy().jsWindowProxy(pluginWorld())) originRootObject:root rootObject:root];
+        m_windowScriptObject = [WebScriptObject scriptObjectForJSObject:toRef(&jsWindowProxy(pluginWorld())) originRootObject:root rootObject:root];
     }
 
     return m_windowScriptObject.get();

Modified: branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptState.cpp (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptState.cpp	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/bindings/js/ScriptState.cpp	2018-05-18 22:57:17 UTC (rev 231987)
@@ -75,7 +75,7 @@
 {
     if (!frame)
         return nullptr;
-    return frame->windowProxy().jsWindowProxy(mainThreadNormalWorld()).window()->globalExec();
+    return frame->windowProxy().jsWindowProxy(mainThreadNormalWorld())->window()->globalExec();
 }
 
 JSC::ExecState* execStateFromNode(DOMWrapperWorld& world, Node* node)

Modified: branches/safari-606.1.17-branch/Source/WebCore/bindings/js/WindowProxy.cpp (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/bindings/js/WindowProxy.cpp	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/bindings/js/WindowProxy.cpp	2018-05-18 22:57:17 UTC (rev 231987)
@@ -49,12 +49,22 @@
 }
 
 WindowProxy::WindowProxy(AbstractFrame& frame)
-    : m_frame(frame)
+    : m_frame(&frame)
 {
 }
 
 WindowProxy::~WindowProxy()
 {
+    ASSERT(!m_frame);
+    ASSERT(m_jsWindowProxies.isEmpty());
+}
+
+void WindowProxy::detachFromFrame()
+{
+    ASSERT(m_frame);
+
+    m_frame = nullptr;
+
     // It's likely that destroying windowProxies will create a lot of garbage.
     if (!m_jsWindowProxies.isEmpty()) {
         while (!m_jsWindowProxies.isEmpty()) {
@@ -75,12 +85,14 @@
 
 JSWindowProxy& WindowProxy::createJSWindowProxy(DOMWrapperWorld& world)
 {
+    ASSERT(m_frame);
+
     ASSERT(!m_jsWindowProxies.contains(&world));
-    ASSERT(m_frame.window());
+    ASSERT(m_frame->window());
 
     VM& vm = world.vm();
 
-    Strong<JSWindowProxy> jsWindowProxy(vm, &JSWindowProxy::create(vm, *m_frame.window(), world));
+    Strong<JSWindowProxy> jsWindowProxy(vm, &JSWindowProxy::create(vm, *m_frame->window(), world));
     Strong<JSWindowProxy> jsWindowProxy2(jsWindowProxy);
     m_jsWindowProxies.add(&world, jsWindowProxy);
     world.didCreateWindowProxy(this);
@@ -94,15 +106,19 @@
 
 JSDOMGlobalObject* WindowProxy::globalObject(DOMWrapperWorld& world)
 {
-    return jsWindowProxy(world).window();
+    if (auto* windowProxy = jsWindowProxy(world))
+        return windowProxy->window();
+    return nullptr;
 }
 
 JSWindowProxy& WindowProxy::createJSWindowProxyWithInitializedScript(DOMWrapperWorld& world)
 {
+    ASSERT(m_frame);
+
     JSLockHolder lock(world.vm());
     auto& windowProxy = createJSWindowProxy(world);
-    if (is<Frame>(m_frame))
-        downcast<Frame>(m_frame).script().initScriptForWindowProxy(windowProxy);
+    if (is<Frame>(*m_frame))
+        downcast<Frame>(*m_frame).script().initScriptForWindowProxy(windowProxy);
     return windowProxy;
 }
 
@@ -137,6 +153,8 @@
     if (m_jsWindowProxies.isEmpty())
         return;
 
+    ASSERT(m_frame);
+
     JSLockHolder lock(commonVM());
 
     for (auto& windowProxy : jsWindowProxiesAsVector()) {
@@ -147,8 +165,8 @@
 
         ScriptController* scriptController = nullptr;
         Page* page = nullptr;
-        if (is<Frame>(m_frame)) {
-            auto& frame = downcast<Frame>(m_frame);
+        if (is<Frame>(*m_frame)) {
+            auto& frame = downcast<Frame>(*m_frame);
             scriptController = &frame.script();
             page = frame.page();
         }
@@ -173,17 +191,7 @@
 
 AbstractDOMWindow* WindowProxy::window() const
 {
-    return m_frame.window();
+    return m_frame ? m_frame->window() : nullptr;
 }
 
-void WindowProxy::ref()
-{
-    m_frame.ref();
-}
-
-void WindowProxy::deref()
-{
-    m_frame.deref();
-}
-
 } // namespace WebCore

Modified: branches/safari-606.1.17-branch/Source/WebCore/bindings/js/WindowProxy.h (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/bindings/js/WindowProxy.h	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/bindings/js/WindowProxy.h	2018-05-18 22:57:17 UTC (rev 231987)
@@ -23,6 +23,7 @@
 #include "DOMWrapperWorld.h"
 #include <_javascript_Core/Strong.h>
 #include <wtf/HashMap.h>
+#include <wtf/RefCounted.h>
 
 namespace JSC {
 class Debugger;
@@ -35,16 +36,21 @@
 class JSDOMGlobalObject;
 class JSWindowProxy;
 
-class WindowProxy {
+class WindowProxy : public RefCounted<WindowProxy> {
     WTF_MAKE_FAST_ALLOCATED;
 public:
     using ProxyMap = HashMap<RefPtr<DOMWrapperWorld>, JSC::Strong<JSWindowProxy>>;
 
-    explicit WindowProxy(AbstractFrame&);
-    ~WindowProxy();
+    static Ref<WindowProxy> create(AbstractFrame& frame)
+    {
+        return adoptRef(*new WindowProxy(frame));
+    }
 
-    AbstractFrame& frame() const { return m_frame; }
+    WEBCORE_EXPORT ~WindowProxy();
 
+    AbstractFrame* frame() const { return m_frame; }
+    void detachFromFrame();
+
     void destroyJSWindowProxy(DOMWrapperWorld&);
 
     ProxyMap::ValuesConstIteratorRange jsWindowProxies() const { return m_jsWindowProxies.values(); }
@@ -53,13 +59,15 @@
     ProxyMap releaseJSWindowProxies() { return std::exchange(m_jsWindowProxies, ProxyMap()); }
     void setJSWindowProxies(ProxyMap&& windowProxies) { m_jsWindowProxies = WTFMove(windowProxies); }
 
-    JSWindowProxy& jsWindowProxy(DOMWrapperWorld& world)
+    JSWindowProxy* jsWindowProxy(DOMWrapperWorld& world)
     {
-        auto it = m_jsWindowProxies.find(&world);
-        if (it != m_jsWindowProxies.end())
-            return *it->value.get();
+        if (!m_frame)
+            return nullptr;
 
-        return createJSWindowProxyWithInitializedScript(world);
+        if (auto* existingProxy = existingJSWindowProxy(world))
+            return existingProxy;
+
+        return &createJSWindowProxyWithInitializedScript(world);
     }
 
     JSWindowProxy* existingJSWindowProxy(DOMWrapperWorld& world) const
@@ -79,14 +87,13 @@
 
     WEBCORE_EXPORT AbstractDOMWindow* window() const;
 
-    WEBCORE_EXPORT void ref();
-    WEBCORE_EXPORT void deref();
+private:
+    explicit WindowProxy(AbstractFrame&);
 
-private:
     JSWindowProxy& createJSWindowProxy(DOMWrapperWorld&);
     WEBCORE_EXPORT JSWindowProxy& createJSWindowProxyWithInitializedScript(DOMWrapperWorld&);
 
-    AbstractFrame& m_frame;
+    AbstractFrame* m_frame;
     ProxyMap m_jsWindowProxies;
 };
 

Modified: branches/safari-606.1.17-branch/Source/WebCore/dom/DocumentTouch.cpp (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/dom/DocumentTouch.cpp	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/dom/DocumentTouch.cpp	2018-05-18 22:57:17 UTC (rev 231987)
@@ -40,7 +40,7 @@
 {
     Frame* frame;
     if (window && is<Frame>(window->frame()))
-        frame = &downcast<Frame>(window->frame());
+        frame = downcast<Frame>(window->frame());
     else
         frame = document.frame();
 

Modified: branches/safari-606.1.17-branch/Source/WebCore/page/AbstractFrame.cpp (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/page/AbstractFrame.cpp	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/page/AbstractFrame.cpp	2018-05-18 22:57:17 UTC (rev 231987)
@@ -31,12 +31,13 @@
 namespace WebCore {
 
 AbstractFrame::AbstractFrame()
-    : m_windowProxy(makeUniqueRef<WindowProxy>(*this))
+    : m_windowProxy(WindowProxy::create(*this))
 {
 }
 
 AbstractFrame::~AbstractFrame()
 {
+    m_windowProxy->detachFromFrame();
 }
 
 } // namespace WebCore

Modified: branches/safari-606.1.17-branch/Source/WebCore/page/AbstractFrame.h (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebCore/page/AbstractFrame.h	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebCore/page/AbstractFrame.h	2018-05-18 22:57:17 UTC (rev 231987)
@@ -25,8 +25,8 @@
 
 #pragma once
 
+#include <wtf/Ref.h>
 #include <wtf/ThreadSafeRefCounted.h>
-#include <wtf/UniqueRef.h>
 
 namespace WebCore {
 
@@ -52,7 +52,7 @@
 private:
     virtual AbstractDOMWindow* virtualWindow() const = 0;
 
-    UniqueRef<WindowProxy> m_windowProxy;
+    Ref<WindowProxy> m_windowProxy;
 };
 
 } // namespace WebCore

Modified: branches/safari-606.1.17-branch/Source/WebKit/ChangeLog (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebKit/ChangeLog	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebKit/ChangeLog	2018-05-18 22:57:17 UTC (rev 231987)
@@ -1,3 +1,89 @@
+2018-05-18  Kocsen Chung  <kocsen_ch...@apple.com>
+
+        Cherry-pick r231963. rdar://problem/40004666
+
+    Avoid keeping the frame alive when ref'ing a WindowProxy
+    https://bugs.webkit.org/show_bug.cgi?id=185737
+    <rdar://problem/40004666>
+    
+    Reviewed by Sam Weinig.
+    
+    Source/WebCore:
+    
+    Avoid keeping the frame alive when ref'ing a WindowProxy by making WindowProxy
+    manage its own refcount (instead of proxying refcounting to the Frame). As a
+    result, a WindowProxy can now be detached from its Frame. When detached, it
+    return null when asked for a JSWindowProxy.
+    
+    It is important to not extend the lifetime of the Frame because we want script
+    to stop running when the Page gets destroyed.
+    
+    * bindings/js/JSWindowProxy.cpp:
+    (WebCore::toJS):
+    (WebCore::toJSWindowProxy):
+    * bindings/js/JSWindowProxy.h:
+    (WebCore::toJSWindowProxy):
+    * bindings/js/ScriptController.cpp:
+    (WebCore::ScriptController::evaluateInWorld):
+    (WebCore::ScriptController::loadModuleScriptInWorld):
+    (WebCore::ScriptController::linkAndEvaluateModuleScriptInWorld):
+    (WebCore::ScriptController::evaluateModule):
+    (WebCore::ScriptController::setupModuleScriptHandlers):
+    (WebCore::ScriptController::jsWindowProxy):
+    (WebCore::ScriptController::windowScriptNPObject):
+    (WebCore::ScriptController::executeIfJavaScriptURL):
+    * bindings/js/ScriptController.h:
+    (WebCore::ScriptController::globalObject):
+    * bindings/js/ScriptControllerMac.mm:
+    (WebCore::ScriptController::windowScriptObject):
+    * bindings/js/ScriptState.cpp:
+    (WebCore::mainWorldExecState):
+    * bindings/js/WindowProxy.cpp:
+    (WebCore::WindowProxy::WindowProxy):
+    (WebCore::WindowProxy::~WindowProxy):
+    (WebCore::WindowProxy::detachFromFrame):
+    (WebCore::WindowProxy::createJSWindowProxy):
+    (WebCore::WindowProxy::globalObject):
+    (WebCore::WindowProxy::createJSWindowProxyWithInitializedScript):
+    (WebCore::WindowProxy::setDOMWindow):
+    (WebCore::WindowProxy::window const):
+    (WebCore::WindowProxy::ref): Deleted.
+    (WebCore::WindowProxy::deref): Deleted.
+    * bindings/js/WindowProxy.h:
+    (WebCore::WindowProxy::create):
+    (WebCore::WindowProxy::frame const):
+    (WebCore::WindowProxy::jsWindowProxy):
+    * dom/DocumentTouch.cpp:
+    (WebCore::DocumentTouch::createTouch):
+    * page/AbstractFrame.cpp:
+    (WebCore::AbstractFrame::AbstractFrame):
+    (WebCore::AbstractFrame::~AbstractFrame):
+    * page/AbstractFrame.h:
+    
+    Source/WebKit:
+    
+    * WebProcess/Plugins/PluginView.cpp:
+    (WebKit::PluginView::windowScriptNPObject):
+    
+    Source/WebKitLegacy/mac:
+    
+    * Plugins/Hosted/NetscapePluginInstanceProxy.mm:
+    (WebKit::NetscapePluginInstanceProxy::getWindowNPObject):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231963 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-05-18  Chris Dumez  <cdu...@apple.com>
+
+            Avoid keeping the frame alive when ref'ing a WindowProxy
+            https://bugs.webkit.org/show_bug.cgi?id=185737
+            <rdar://problem/40004666>
+
+            Reviewed by Sam Weinig.
+
+            * WebProcess/Plugins/PluginView.cpp:
+            (WebKit::PluginView::windowScriptNPObject):
+
 2018-05-17  Babak Shafiei  <bshaf...@apple.com>
 
         Cherry-pick r231922. rdar://problem/40220659

Modified: branches/safari-606.1.17-branch/Source/WebKit/WebProcess/Plugins/PluginView.cpp (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebKit/WebProcess/Plugins/PluginView.cpp	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebKit/WebProcess/Plugins/PluginView.cpp	2018-05-18 22:57:17 UTC (rev 231987)
@@ -1448,7 +1448,7 @@
         return nullptr;
     }
 
-    return m_npRuntimeObjectMap.getOrCreateNPObject(pluginWorld().vm(), frame()->windowProxy().jsWindowProxy(pluginWorld()).window());
+    return m_npRuntimeObjectMap.getOrCreateNPObject(pluginWorld().vm(), frame()->windowProxy().jsWindowProxy(pluginWorld())->window());
 }
 
 NPObject* PluginView::pluginElementNPObject()

Modified: branches/safari-606.1.17-branch/Source/WebKitLegacy/mac/ChangeLog (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebKitLegacy/mac/ChangeLog	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebKitLegacy/mac/ChangeLog	2018-05-18 22:57:17 UTC (rev 231987)
@@ -1,3 +1,89 @@
+2018-05-18  Kocsen Chung  <kocsen_ch...@apple.com>
+
+        Cherry-pick r231963. rdar://problem/40004666
+
+    Avoid keeping the frame alive when ref'ing a WindowProxy
+    https://bugs.webkit.org/show_bug.cgi?id=185737
+    <rdar://problem/40004666>
+    
+    Reviewed by Sam Weinig.
+    
+    Source/WebCore:
+    
+    Avoid keeping the frame alive when ref'ing a WindowProxy by making WindowProxy
+    manage its own refcount (instead of proxying refcounting to the Frame). As a
+    result, a WindowProxy can now be detached from its Frame. When detached, it
+    return null when asked for a JSWindowProxy.
+    
+    It is important to not extend the lifetime of the Frame because we want script
+    to stop running when the Page gets destroyed.
+    
+    * bindings/js/JSWindowProxy.cpp:
+    (WebCore::toJS):
+    (WebCore::toJSWindowProxy):
+    * bindings/js/JSWindowProxy.h:
+    (WebCore::toJSWindowProxy):
+    * bindings/js/ScriptController.cpp:
+    (WebCore::ScriptController::evaluateInWorld):
+    (WebCore::ScriptController::loadModuleScriptInWorld):
+    (WebCore::ScriptController::linkAndEvaluateModuleScriptInWorld):
+    (WebCore::ScriptController::evaluateModule):
+    (WebCore::ScriptController::setupModuleScriptHandlers):
+    (WebCore::ScriptController::jsWindowProxy):
+    (WebCore::ScriptController::windowScriptNPObject):
+    (WebCore::ScriptController::executeIfJavaScriptURL):
+    * bindings/js/ScriptController.h:
+    (WebCore::ScriptController::globalObject):
+    * bindings/js/ScriptControllerMac.mm:
+    (WebCore::ScriptController::windowScriptObject):
+    * bindings/js/ScriptState.cpp:
+    (WebCore::mainWorldExecState):
+    * bindings/js/WindowProxy.cpp:
+    (WebCore::WindowProxy::WindowProxy):
+    (WebCore::WindowProxy::~WindowProxy):
+    (WebCore::WindowProxy::detachFromFrame):
+    (WebCore::WindowProxy::createJSWindowProxy):
+    (WebCore::WindowProxy::globalObject):
+    (WebCore::WindowProxy::createJSWindowProxyWithInitializedScript):
+    (WebCore::WindowProxy::setDOMWindow):
+    (WebCore::WindowProxy::window const):
+    (WebCore::WindowProxy::ref): Deleted.
+    (WebCore::WindowProxy::deref): Deleted.
+    * bindings/js/WindowProxy.h:
+    (WebCore::WindowProxy::create):
+    (WebCore::WindowProxy::frame const):
+    (WebCore::WindowProxy::jsWindowProxy):
+    * dom/DocumentTouch.cpp:
+    (WebCore::DocumentTouch::createTouch):
+    * page/AbstractFrame.cpp:
+    (WebCore::AbstractFrame::AbstractFrame):
+    (WebCore::AbstractFrame::~AbstractFrame):
+    * page/AbstractFrame.h:
+    
+    Source/WebKit:
+    
+    * WebProcess/Plugins/PluginView.cpp:
+    (WebKit::PluginView::windowScriptNPObject):
+    
+    Source/WebKitLegacy/mac:
+    
+    * Plugins/Hosted/NetscapePluginInstanceProxy.mm:
+    (WebKit::NetscapePluginInstanceProxy::getWindowNPObject):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231963 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-05-18  Chris Dumez  <cdu...@apple.com>
+
+            Avoid keeping the frame alive when ref'ing a WindowProxy
+            https://bugs.webkit.org/show_bug.cgi?id=185737
+            <rdar://problem/40004666>
+
+            Reviewed by Sam Weinig.
+
+            * Plugins/Hosted/NetscapePluginInstanceProxy.mm:
+            (WebKit::NetscapePluginInstanceProxy::getWindowNPObject):
+
 2018-05-07  Daniel Bates  <daba...@apple.com>
 
         Substitute CrossOriginPreflightResultCache::clear() for CrossOriginPreflightResultCache::empty()

Modified: branches/safari-606.1.17-branch/Source/WebKitLegacy/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm (231986 => 231987)


--- branches/safari-606.1.17-branch/Source/WebKitLegacy/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm	2018-05-18 22:57:11 UTC (rev 231986)
+++ branches/safari-606.1.17-branch/Source/WebKitLegacy/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm	2018-05-18 22:57:17 UTC (rev 231987)
@@ -839,7 +839,7 @@
     if (!frame->script().canExecuteScripts(NotAboutToExecuteScript))
         objectID = 0;
     else
-        objectID = m_localObjects.idForObject(pluginWorld().vm(), frame->windowProxy().jsWindowProxy(pluginWorld()).window());
+        objectID = m_localObjects.idForObject(pluginWorld().vm(), frame->windowProxy().jsWindowProxy(pluginWorld())->window());
         
     return true;
 }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to