Changeset 231963 in webkit


Ignore:
Timestamp:
May 18, 2018 10:34:19 AM (6 years ago)
Author:
Chris Dumez
Message:

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):

Location:
trunk/Source
Files:
16 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r231962 r231963  
     12018-05-18  Chris Dumez  <cdumez@apple.com>
     2
     3        Avoid keeping the frame alive when ref'ing a WindowProxy
     4        https://bugs.webkit.org/show_bug.cgi?id=185737
     5        <rdar://problem/40004666>
     6
     7        Reviewed by Sam Weinig.
     8
     9        Avoid keeping the frame alive when ref'ing a WindowProxy by making WindowProxy
     10        manage its own refcount (instead of proxying refcounting to the Frame). As a
     11        result, a WindowProxy can now be detached from its Frame. When detached, it
     12        return null when asked for a JSWindowProxy.
     13
     14        It is important to not extend the lifetime of the Frame because we want script
     15        to stop running when the Page gets destroyed.
     16
     17        * bindings/js/JSWindowProxy.cpp:
     18        (WebCore::toJS):
     19        (WebCore::toJSWindowProxy):
     20        * bindings/js/JSWindowProxy.h:
     21        (WebCore::toJSWindowProxy):
     22        * bindings/js/ScriptController.cpp:
     23        (WebCore::ScriptController::evaluateInWorld):
     24        (WebCore::ScriptController::loadModuleScriptInWorld):
     25        (WebCore::ScriptController::linkAndEvaluateModuleScriptInWorld):
     26        (WebCore::ScriptController::evaluateModule):
     27        (WebCore::ScriptController::setupModuleScriptHandlers):
     28        (WebCore::ScriptController::jsWindowProxy):
     29        (WebCore::ScriptController::windowScriptNPObject):
     30        (WebCore::ScriptController::executeIfJavaScriptURL):
     31        * bindings/js/ScriptController.h:
     32        (WebCore::ScriptController::globalObject):
     33        * bindings/js/ScriptControllerMac.mm:
     34        (WebCore::ScriptController::windowScriptObject):
     35        * bindings/js/ScriptState.cpp:
     36        (WebCore::mainWorldExecState):
     37        * bindings/js/WindowProxy.cpp:
     38        (WebCore::WindowProxy::WindowProxy):
     39        (WebCore::WindowProxy::~WindowProxy):
     40        (WebCore::WindowProxy::detachFromFrame):
     41        (WebCore::WindowProxy::createJSWindowProxy):
     42        (WebCore::WindowProxy::globalObject):
     43        (WebCore::WindowProxy::createJSWindowProxyWithInitializedScript):
     44        (WebCore::WindowProxy::setDOMWindow):
     45        (WebCore::WindowProxy::window const):
     46        (WebCore::WindowProxy::ref): Deleted.
     47        (WebCore::WindowProxy::deref): Deleted.
     48        * bindings/js/WindowProxy.h:
     49        (WebCore::WindowProxy::create):
     50        (WebCore::WindowProxy::frame const):
     51        (WebCore::WindowProxy::jsWindowProxy):
     52        * dom/DocumentTouch.cpp:
     53        (WebCore::DocumentTouch::createTouch):
     54        * page/AbstractFrame.cpp:
     55        (WebCore::AbstractFrame::AbstractFrame):
     56        (WebCore::AbstractFrame::~AbstractFrame):
     57        * page/AbstractFrame.h:
     58
    1592018-05-18  Myles C. Maxfield  <mmaxfield@apple.com>
    260
  • trunk/Source/WebCore/bindings/js/JSWindowProxy.cpp

    r231114 r231963  
    147147JSValue toJS(ExecState* state, WindowProxy& windowProxy)
    148148{
    149     return &windowProxy.jsWindowProxy(currentWorld(*state));
     149    auto* jsWindowProxy = windowProxy.jsWindowProxy(currentWorld(*state));
     150    return jsWindowProxy ? JSValue(jsWindowProxy) : jsNull();
    150151}
    151152
    152 JSWindowProxy& toJSWindowProxy(WindowProxy& windowProxy, DOMWrapperWorld& world)
     153JSWindowProxy* toJSWindowProxy(WindowProxy& windowProxy, DOMWrapperWorld& world)
    153154{
    154155    return windowProxy.jsWindowProxy(world);
  • trunk/Source/WebCore/bindings/js/JSWindowProxy.h

    r231114 r231963  
    7878inline JSC::JSValue toJS(JSC::ExecState* state, JSDOMGlobalObject* globalObject, WindowProxy* windowProxy) { return windowProxy ? toJS(state, globalObject, *windowProxy) : JSC::jsNull(); }
    7979
    80 JSWindowProxy& toJSWindowProxy(WindowProxy&, DOMWrapperWorld&);
    81 inline JSWindowProxy* toJSWindowProxy(WindowProxy* windowProxy, DOMWrapperWorld& world) { return windowProxy ? &toJSWindowProxy(*windowProxy, world) : nullptr; }
     80JSWindowProxy* toJSWindowProxy(WindowProxy&, DOMWrapperWorld&);
     81inline JSWindowProxy* toJSWindowProxy(WindowProxy* windowProxy, DOMWrapperWorld& world) { return windowProxy ? toJSWindowProxy(*windowProxy, world) : nullptr; }
    8282
    8383
  • trunk/Source/WebCore/bindings/js/ScriptController.cpp

    r230831 r231963  
    118118    // expected value in all cases.
    119119    // See smart window.open policy for where this is used.
    120     auto& proxy = windowProxy().jsWindowProxy(world);
     120    auto& proxy = jsWindowProxy(world);
    121121    auto& exec = *proxy.window()->globalExec();
    122122    const String* savedSourceURL = m_sourceURL;
     
    151151    JSLockHolder lock(world.vm());
    152152
    153     auto& proxy = windowProxy().jsWindowProxy(world);
     153    auto& proxy = jsWindowProxy(world);
    154154    auto& state = *proxy.window()->globalExec();
    155155
     
    167167    JSLockHolder lock(world.vm());
    168168
    169     auto& proxy = windowProxy().jsWindowProxy(world);
     169    auto& proxy = jsWindowProxy(world);
    170170    auto& state = *proxy.window()->globalExec();
    171171
     
    183183    JSLockHolder lock(world.vm());
    184184
    185     auto& proxy = windowProxy().jsWindowProxy(world);
     185    auto& proxy = jsWindowProxy(world);
    186186    auto& state = *proxy.window()->globalExec();
    187187
     
    212212    const auto& jsSourceCode = moduleRecord.sourceCode();
    213213
    214     auto& proxy = windowProxy().jsWindowProxy(world);
     214    auto& proxy = jsWindowProxy(world);
    215215    auto& state = *proxy.window()->globalExec();
    216216    SetForScope<const String*> sourceURLScope(m_sourceURL, &sourceURL.string());
     
    269269void ScriptController::setupModuleScriptHandlers(LoadableModuleScript& moduleScriptRef, JSInternalPromise& promise, DOMWrapperWorld& world)
    270270{
    271     auto& proxy = windowProxy().jsWindowProxy(world);
     271    auto& proxy = jsWindowProxy(world);
    272272    auto& state = *proxy.window()->globalExec();
    273273
     
    326326}
    327327
     328JSWindowProxy& ScriptController::jsWindowProxy(DOMWrapperWorld& world)
     329{
     330    auto* jsWindowProxy = m_frame.windowProxy().jsWindowProxy(world);
     331    ASSERT_WITH_MESSAGE(jsWindowProxy, "The JSWindowProxy can only be null if the frame has been destroyed");
     332    return *jsWindowProxy;
     333}
     334
    328335TextPosition ScriptController::eventHandlerPosition() const
    329336{
     
    443450            // JavaScript is enabled, so there is a JavaScript window object.
    444451            // Return an NPObject bound to the window object.
    445             auto* window = windowProxy().jsWindowProxy(pluginWorld()).window();
     452            auto* window = jsWindowProxy(pluginWorld()).window();
    446453            ASSERT(window);
    447454            Bindings::RootObject* root = bindingRootObject();
     
    604611
    605612    String scriptResult;
    606     if (!result || !result.getString(windowProxy().jsWindowProxy(mainThreadNormalWorld()).window()->globalExec(), scriptResult))
     613    if (!result || !result.getString(jsWindowProxy(mainThreadNormalWorld()).window()->globalExec(), scriptResult))
    607614        return true;
    608615
  • trunk/Source/WebCore/bindings/js/ScriptController.h

    r230831 r231963  
    8484    JSDOMWindow* globalObject(DOMWrapperWorld& world)
    8585    {
    86         return JSC::jsCast<JSDOMWindow*>(windowProxy().jsWindowProxy(world).window());
     86        return JSC::jsCast<JSDOMWindow*>(jsWindowProxy(world).window());
    8787    }
    8888
     
    167167
    168168    WEBCORE_EXPORT WindowProxy& windowProxy();
     169    WEBCORE_EXPORT JSWindowProxy& jsWindowProxy(DOMWrapperWorld&);
    169170
    170171    Frame& m_frame;
  • trunk/Source/WebCore/bindings/js/ScriptControllerMac.mm

    r230794 r231963  
    104104        JSC::JSLockHolder lock(commonVM());
    105105        JSC::Bindings::RootObject* root = bindingRootObject();
    106         m_windowScriptObject = [WebScriptObject scriptObjectForJSObject:toRef(&windowProxy().jsWindowProxy(pluginWorld())) originRootObject:root rootObject:root];
     106        m_windowScriptObject = [WebScriptObject scriptObjectForJSObject:toRef(&jsWindowProxy(pluginWorld())) originRootObject:root rootObject:root];
    107107    }
    108108
  • trunk/Source/WebCore/bindings/js/ScriptState.cpp

    r230794 r231963  
    7676    if (!frame)
    7777        return nullptr;
    78     return frame->windowProxy().jsWindowProxy(mainThreadNormalWorld()).window()->globalExec();
     78    return frame->windowProxy().jsWindowProxy(mainThreadNormalWorld())->window()->globalExec();
    7979}
    8080
  • trunk/Source/WebCore/bindings/js/WindowProxy.cpp

    r231114 r231963  
    5050
    5151WindowProxy::WindowProxy(AbstractFrame& frame)
    52     : m_frame(frame)
     52    : m_frame(&frame)
    5353{
    5454}
     
    5656WindowProxy::~WindowProxy()
    5757{
     58    ASSERT(!m_frame);
     59    ASSERT(m_jsWindowProxies.isEmpty());
     60}
     61
     62void WindowProxy::detachFromFrame()
     63{
     64    ASSERT(m_frame);
     65
     66    m_frame = nullptr;
     67
    5868    // It's likely that destroying windowProxies will create a lot of garbage.
    5969    if (!m_jsWindowProxies.isEmpty()) {
     
    7686JSWindowProxy& WindowProxy::createJSWindowProxy(DOMWrapperWorld& world)
    7787{
     88    ASSERT(m_frame);
     89
    7890    ASSERT(!m_jsWindowProxies.contains(&world));
    79     ASSERT(m_frame.window());
     91    ASSERT(m_frame->window());
    8092
    8193    VM& vm = world.vm();
    8294
    83     Strong<JSWindowProxy> jsWindowProxy(vm, &JSWindowProxy::create(vm, *m_frame.window(), world));
     95    Strong<JSWindowProxy> jsWindowProxy(vm, &JSWindowProxy::create(vm, *m_frame->window(), world));
    8496    Strong<JSWindowProxy> jsWindowProxy2(jsWindowProxy);
    8597    m_jsWindowProxies.add(&world, jsWindowProxy);
     
    95107JSDOMGlobalObject* WindowProxy::globalObject(DOMWrapperWorld& world)
    96108{
    97     return jsWindowProxy(world).window();
     109    if (auto* windowProxy = jsWindowProxy(world))
     110        return windowProxy->window();
     111    return nullptr;
    98112}
    99113
    100114JSWindowProxy& WindowProxy::createJSWindowProxyWithInitializedScript(DOMWrapperWorld& world)
    101115{
     116    ASSERT(m_frame);
     117
    102118    JSLockHolder lock(world.vm());
    103119    auto& windowProxy = createJSWindowProxy(world);
    104     if (is<Frame>(m_frame))
    105         downcast<Frame>(m_frame).script().initScriptForWindowProxy(windowProxy);
     120    if (is<Frame>(*m_frame))
     121        downcast<Frame>(*m_frame).script().initScriptForWindowProxy(windowProxy);
    106122    return windowProxy;
    107123}
     
    138154        return;
    139155
     156    ASSERT(m_frame);
     157
    140158    JSLockHolder lock(commonVM());
    141159
     
    148166        ScriptController* scriptController = nullptr;
    149167        Page* page = nullptr;
    150         if (is<Frame>(m_frame)) {
    151             auto& frame = downcast<Frame>(m_frame);
     168        if (is<Frame>(*m_frame)) {
     169            auto& frame = downcast<Frame>(*m_frame);
    152170            scriptController = &frame.script();
    153171            page = frame.page();
     
    174192AbstractDOMWindow* WindowProxy::window() const
    175193{
    176     return m_frame.window();
    177 }
    178 
    179 void WindowProxy::ref()
    180 {
    181     m_frame.ref();
    182 }
    183 
    184 void WindowProxy::deref()
    185 {
    186     m_frame.deref();
     194    return m_frame ? m_frame->window() : nullptr;
    187195}
    188196
  • trunk/Source/WebCore/bindings/js/WindowProxy.h

    r231114 r231963  
    2424#include <JavaScriptCore/Strong.h>
    2525#include <wtf/HashMap.h>
     26#include <wtf/RefCounted.h>
    2627
    2728namespace JSC {
     
    3637class JSWindowProxy;
    3738
    38 class WindowProxy {
     39class WindowProxy : public RefCounted<WindowProxy> {
    3940    WTF_MAKE_FAST_ALLOCATED;
    4041public:
    4142    using ProxyMap = HashMap<RefPtr<DOMWrapperWorld>, JSC::Strong<JSWindowProxy>>;
    4243
    43     explicit WindowProxy(AbstractFrame&);
    44     ~WindowProxy();
     44    static Ref<WindowProxy> create(AbstractFrame& frame)
     45    {
     46        return adoptRef(*new WindowProxy(frame));
     47    }
    4548
    46     AbstractFrame& frame() const { return m_frame; }
     49    WEBCORE_EXPORT ~WindowProxy();
     50
     51    AbstractFrame* frame() const { return m_frame; }
     52    void detachFromFrame();
    4753
    4854    void destroyJSWindowProxy(DOMWrapperWorld&);
     
    5460    void setJSWindowProxies(ProxyMap&& windowProxies) { m_jsWindowProxies = WTFMove(windowProxies); }
    5561
    56     JSWindowProxy& jsWindowProxy(DOMWrapperWorld& world)
     62    JSWindowProxy* jsWindowProxy(DOMWrapperWorld& world)
    5763    {
    58         auto it = m_jsWindowProxies.find(&world);
    59         if (it != m_jsWindowProxies.end())
    60             return *it->value.get();
     64        if (!m_frame)
     65            return nullptr;
    6166
    62         return createJSWindowProxyWithInitializedScript(world);
     67        if (auto* existingProxy = existingJSWindowProxy(world))
     68            return existingProxy;
     69
     70        return &createJSWindowProxyWithInitializedScript(world);
    6371    }
    6472
     
    8088    WEBCORE_EXPORT AbstractDOMWindow* window() const;
    8189
    82     WEBCORE_EXPORT void ref();
    83     WEBCORE_EXPORT void deref();
     90private:
     91    explicit WindowProxy(AbstractFrame&);
    8492
    85 private:
    8693    JSWindowProxy& createJSWindowProxy(DOMWrapperWorld&);
    8794    WEBCORE_EXPORT JSWindowProxy& createJSWindowProxyWithInitializedScript(DOMWrapperWorld&);
    8895
    89     AbstractFrame& m_frame;
     96    AbstractFrame* m_frame;
    9097    ProxyMap m_jsWindowProxies;
    9198};
  • trunk/Source/WebCore/dom/DocumentTouch.cpp

    r231114 r231963  
    4141    Frame* frame;
    4242    if (window && is<Frame>(window->frame()))
    43         frame = &downcast<Frame>(window->frame());
     43        frame = downcast<Frame>(window->frame());
    4444    else
    4545        frame = document.frame();
  • trunk/Source/WebCore/page/AbstractFrame.cpp

    r230794 r231963  
    3232
    3333AbstractFrame::AbstractFrame()
    34     : m_windowProxy(makeUniqueRef<WindowProxy>(*this))
     34    : m_windowProxy(WindowProxy::create(*this))
    3535{
    3636}
     
    3838AbstractFrame::~AbstractFrame()
    3939{
     40    m_windowProxy->detachFromFrame();
    4041}
    4142
  • trunk/Source/WebCore/page/AbstractFrame.h

    r230794 r231963  
    2626#pragma once
    2727
     28#include <wtf/Ref.h>
    2829#include <wtf/ThreadSafeRefCounted.h>
    29 #include <wtf/UniqueRef.h>
    3030
    3131namespace WebCore {
     
    5353    virtual AbstractDOMWindow* virtualWindow() const = 0;
    5454
    55     UniqueRef<WindowProxy> m_windowProxy;
     55    Ref<WindowProxy> m_windowProxy;
    5656};
    5757
  • trunk/Source/WebKit/ChangeLog

    r231956 r231963  
     12018-05-18  Chris Dumez  <cdumez@apple.com>
     2
     3        Avoid keeping the frame alive when ref'ing a WindowProxy
     4        https://bugs.webkit.org/show_bug.cgi?id=185737
     5        <rdar://problem/40004666>
     6
     7        Reviewed by Sam Weinig.
     8
     9        * WebProcess/Plugins/PluginView.cpp:
     10        (WebKit::PluginView::windowScriptNPObject):
     11
    1122018-05-18  Youenn Fablet  <youenn@apple.com>
    213
  • trunk/Source/WebKit/WebProcess/Plugins/PluginView.cpp

    r230794 r231963  
    14491449    }
    14501450
    1451     return m_npRuntimeObjectMap.getOrCreateNPObject(pluginWorld().vm(), frame()->windowProxy().jsWindowProxy(pluginWorld()).window());
     1451    return m_npRuntimeObjectMap.getOrCreateNPObject(pluginWorld().vm(), frame()->windowProxy().jsWindowProxy(pluginWorld())->window());
    14521452}
    14531453
  • trunk/Source/WebKitLegacy/mac/ChangeLog

    r231867 r231963  
     12018-05-18  Chris Dumez  <cdumez@apple.com>
     2
     3        Avoid keeping the frame alive when ref'ing a WindowProxy
     4        https://bugs.webkit.org/show_bug.cgi?id=185737
     5        <rdar://problem/40004666>
     6
     7        Reviewed by Sam Weinig.
     8
     9        * Plugins/Hosted/NetscapePluginInstanceProxy.mm:
     10        (WebKit::NetscapePluginInstanceProxy::getWindowNPObject):
     11
    1122018-05-16  Andy VanWagoner  <andy@vanwagoner.family>
    213
  • trunk/Source/WebKitLegacy/mac/Plugins/Hosted/NetscapePluginInstanceProxy.mm

    r230794 r231963  
    840840        objectID = 0;
    841841    else
    842         objectID = m_localObjects.idForObject(pluginWorld().vm(), frame->windowProxy().jsWindowProxy(pluginWorld()).window());
     842        objectID = m_localObjects.idForObject(pluginWorld().vm(), frame->windowProxy().jsWindowProxy(pluginWorld())->window());
    843843       
    844844    return true;
Note: See TracChangeset for help on using the changeset viewer.