Changeset 42657 in webkit


Ignore:
Timestamp:
Apr 19, 2009 12:38:26 AM (15 years ago)
Author:
ggaren@apple.com
Message:

2009-04-18 Geoffrey Garen <​ggaren@apple.com>

Reviewed by Alexey Proskuryakov.

More fix for ​https://bugs.webkit.org/show_bug.cgi?id=21260
Unbounded memory growth when churning elements with anonymous event handler functions


Removed a little more complexity from event handler creation and destruction.


Removed the jsProtectedEventListeners, jsProtectedInlineEventListeners,
and jsInlineEventListeners maps, and all the code for managing them.


ProtectedEventListeners don't exist anymore, so they're easy to nix.


Inline EventListeners do still exist, but there's no reason to track
them in a map. The map exists to enable 'removeEventListener' to associate
a unique JSEventListener with a given JavaScript function. But the
'removeEventListener' API only works with non-inline event listeners!


  • bindings/js/JSDOMGlobalObject.cpp: (WebCore::JSDOMGlobalObject::~JSDOMGlobalObject): (WebCore::JSDOMGlobalObject::findJSEventListener): (WebCore::JSDOMGlobalObject::findOrCreateJSEventListener): (WebCore::JSDOMGlobalObject::createJSInlineEventListener):
  • bindings/js/JSDOMGlobalObject.h:
  • bindings/js/JSEventListener.cpp: (WebCore::JSEventListener::JSEventListener): (WebCore::JSEventListener::clearJSFunctionInline):
  • bindings/js/JSLazyEventListener.cpp: (WebCore::JSLazyEventListener::~JSLazyEventListener): (WebCore::JSLazyEventListener::parseCode):
  • bindings/scripts/CodeGeneratorJS.pm:
Location:
trunk/WebCore
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/WebCore/ChangeLog

    r42655 r42657  
     12009-04-18  Geoffrey Garen  <ggaren@apple.com>
     2
     3        Reviewed by Alexey Proskuryakov.
     4
     5        More fix for https://bugs.webkit.org/show_bug.cgi?id=21260
     6        Unbounded memory growth when churning elements with anonymous event handler functions
     7       
     8        Removed a little more complexity from event handler creation and destruction.
     9       
     10        Removed the jsProtectedEventListeners, jsProtectedInlineEventListeners,
     11        and jsInlineEventListeners maps, and all the code for managing them.
     12       
     13        ProtectedEventListeners don't exist anymore, so they're easy to nix.
     14       
     15        Inline EventListeners do still exist, but there's no reason to track
     16        them in a map. The map exists to enable 'removeEventListener' to associate
     17        a unique JSEventListener with a given JavaScript function. But the
     18        'removeEventListener' API only works with non-inline event listeners!
     19       
     20        * bindings/js/JSDOMGlobalObject.cpp:
     21        (WebCore::JSDOMGlobalObject::~JSDOMGlobalObject):
     22        (WebCore::JSDOMGlobalObject::findJSEventListener):
     23        (WebCore::JSDOMGlobalObject::findOrCreateJSEventListener):
     24        (WebCore::JSDOMGlobalObject::createJSInlineEventListener):
     25        * bindings/js/JSDOMGlobalObject.h:
     26        * bindings/js/JSEventListener.cpp:
     27        (WebCore::JSEventListener::JSEventListener):
     28        (WebCore::JSEventListener::clearJSFunctionInline):
     29        * bindings/js/JSLazyEventListener.cpp:
     30        (WebCore::JSLazyEventListener::~JSLazyEventListener):
     31        (WebCore::JSLazyEventListener::parseCode):
     32        * bindings/scripts/CodeGeneratorJS.pm:
     33
    1342009-04-18  Dan Bernstein  <mitz@apple.com>
    235

  • trunk/WebCore/bindings/js/JSDOMGlobalObject.cpp

    r42589 r42657  
    5454JSDOMGlobalObject::~JSDOMGlobalObject()
    5555{
    56     // Clear any backpointers to the window
    57     ProtectedListenersMap::iterator i1 = d()->jsProtectedEventListeners.begin();
    58     ProtectedListenersMap::iterator e1 = d()->jsProtectedEventListeners.end();
    59     for (; i1 != e1; ++i1)
    60         i1->second->clearGlobalObject();
    61 
    62     i1 = d()->jsProtectedInlineEventListeners.begin();
    63     e1 = d()->jsProtectedInlineEventListeners.end();
    64     for (; i1 != e1; ++i1)
    65         i1->second->clearGlobalObject();
    66 
    6756    JSListenersMap::iterator i2 = d()->jsEventListeners.begin();
    6857    JSListenersMap::iterator e2 = d()->jsEventListeners.end();
    69     for (; i2 != e2; ++i2)
    70         i2->second->clearGlobalObject();
    71 
    72     i2 = d()->jsInlineEventListeners.begin();
    73     e2 = d()->jsInlineEventListeners.end();
    7458    for (; i2 != e2; ++i2)
    7559        i2->second->clearGlobalObject();
    … …  
    9175}
    9276
    93 JSEventListener* JSDOMGlobalObject::findJSEventListener(JSValuePtr val, bool isInline)
     77JSEventListener* JSDOMGlobalObject::findJSEventListener(JSValuePtr val)
    9478{
    9579    if (!val.isObject())
    9680        return 0;
    9781
    98     JSListenersMap& listeners = isInline ? d()->jsInlineEventListeners : d()->jsEventListeners;
    99     return listeners.get(asObject(val));
     82    return d()->jsEventListeners.get(asObject(val));
    10083}
    10184
    102 PassRefPtr<JSEventListener> JSDOMGlobalObject::findOrCreateJSEventListener(JSValuePtr val, bool isInline)
     85PassRefPtr<JSEventListener> JSDOMGlobalObject::findOrCreateJSEventListener(JSValuePtr val)
    10386{
    104     if (JSEventListener* listener = findJSEventListener(val, isInline))
     87    if (JSEventListener* listener = findJSEventListener(val))
    10588        return listener;
    10689
    … …  
    10992
    11093    // The JSEventListener constructor adds it to our jsEventListeners map.
    111     return JSEventListener::create(asObject(val), this, isInline).get();
     94    return JSEventListener::create(asObject(val), this, false).get();
    11295}
    11396
    114 JSDOMGlobalObject::ProtectedListenersMap& JSDOMGlobalObject::jsProtectedEventListeners()
     97PassRefPtr<JSEventListener> JSDOMGlobalObject::createJSInlineEventListener(JSValuePtr val)
    11598{
    116     return d()->jsProtectedEventListeners;
    117 }
     99    if (!val.isObject())
     100        return 0;
    118101
    119 JSDOMGlobalObject::ProtectedListenersMap& JSDOMGlobalObject::jsProtectedInlineEventListeners()
    120 {
    121     return d()->jsProtectedInlineEventListeners;
     102    return JSEventListener::create(asObject(val), this, true).get();
    122103}
    123104
    … …  
    125106{
    126107    return d()->jsEventListeners;
    127 }
    128 
    129 JSDOMGlobalObject::JSListenersMap& JSDOMGlobalObject::jsInlineEventListeners()
    130 {
    131     return d()->jsInlineEventListeners;
    132108}
    133109

  • trunk/WebCore/bindings/js/JSDOMGlobalObject.h

    r42589 r42657  
    3434    class Event;
    3535    class JSLazyEventListener;
    36     class JSProtectedEventListener;
    3736    class JSEventListener;
    3837    class ScriptExecutionContext;
    … …  
    5655
    5756        // Finds a wrapper of a GC-unprotected JS EventListener, returns 0 if no existing one.
    58         JSEventListener* findJSEventListener(JSC::JSValuePtr, bool isInline = false);
     57        JSEventListener* findJSEventListener(JSC::JSValuePtr);
    5958
    6059        // Finds or creates a wrapper of a JS EventListener. JS EventListener object is *NOT* GC-protected.
    61         PassRefPtr<JSEventListener> findOrCreateJSEventListener(JSC::JSValuePtr, bool isInline = false);
     60        PassRefPtr<JSEventListener> findOrCreateJSEventListener(JSC::JSValuePtr);
    6261
    63         typedef HashMap<JSC::JSObject*, JSLazyEventListener*> ProtectedListenersMap;
     62        // Creates a GC-protected JS EventListener for an "onXXX" event attribute.
     63        // These listeners cannot be removed through the removeEventListener API.
     64        PassRefPtr<JSEventListener> createJSInlineEventListener(JSC::JSValuePtr);
     65
    6466        typedef HashMap<JSC::JSObject*, JSEventListener*> JSListenersMap;
    6567
    66         ProtectedListenersMap& jsProtectedEventListeners();
    67         ProtectedListenersMap& jsProtectedInlineEventListeners();
    6868        JSListenersMap& jsEventListeners();
    69         JSListenersMap& jsInlineEventListeners();
    7069
    7170        void setCurrentEvent(Event*);
    … …  
    8180            JSDOMConstructorMap constructors;
    8281
    83             JSDOMGlobalObject::ProtectedListenersMap jsProtectedEventListeners;
    84             JSDOMGlobalObject::ProtectedListenersMap jsProtectedInlineEventListeners;
    8582            JSDOMGlobalObject::JSListenersMap jsEventListeners;
    86             JSDOMGlobalObject::JSListenersMap jsInlineEventListeners;
    8783
    8884            Event* evt;

  • trunk/WebCore/bindings/js/JSEventListener.cpp

    r42589 r42657  
    138138    , m_globalObject(globalObject)
    139139{
    140     if (m_jsFunction) {
    141         JSDOMWindow::JSListenersMap& listeners = isInline
    142             ? globalObject->jsInlineEventListeners() : globalObject->jsEventListeners();
    143         listeners.set(m_jsFunction, this);
    144     }
     140    if (!isInline && m_jsFunction)
     141        globalObject->jsEventListeners().set(m_jsFunction, this);
    145142}
    146143
    147144inline void JSEventListener::clearJSFunctionInline()
    148145{
    149     if (m_jsFunction && m_globalObject) {
    150         JSDOMWindow::JSListenersMap& listeners = isInline()
    151             ? m_globalObject->jsInlineEventListeners() : m_globalObject->jsEventListeners();
    152         listeners.remove(m_jsFunction);
    153     }
     146    if (!isInline() && m_jsFunction && m_globalObject)
     147        m_globalObject->jsEventListeners().remove(m_jsFunction);
    154148   
    155149    m_jsFunction = 0;

  • trunk/WebCore/bindings/js/JSLazyEventListener.cpp

    r42633 r42657  
    7272JSLazyEventListener::~JSLazyEventListener()
    7373{
    74     if (m_jsFunction && m_globalObject) {
    75         JSDOMWindow::ProtectedListenersMap& listeners = isInline()
    76             ? m_globalObject->jsProtectedInlineEventListeners() : m_globalObject->jsProtectedEventListeners();
    77         listeners.remove(m_jsFunction);
    78     }
    7974#ifndef NDEBUG
    8075    eventListenerCounter.decrement();
    … …  
    140135    m_code = String();
    141136    m_eventParameterName = String();
    142 
    143     if (m_jsFunction) {
    144         ASSERT(isInline());
    145         JSDOMWindow::ProtectedListenersMap& listeners = m_globalObject->jsProtectedInlineEventListeners();
    146         listeners.set(m_jsFunction, const_cast<JSLazyEventListener*>(this));
    147     }
    148137}
    149138

  • trunk/WebCore/bindings/scripts/CodeGeneratorJS.pm

    r42569 r42657  
    12561256                                push(@implContent, "        return;\n");
    12571257                            }
    1258                             push(@implContent, "    imp->set$implSetterFunctionName(globalObject->findOrCreateJSEventListener(value, true));\n");
     1258                            push(@implContent, "    imp->set$implSetterFunctionName(globalObject->createJSInlineEventListener(value));\n");
    12591259                        } elsif ($attribute->signature->type =~ /Constructor$/) {
    12601260                            my $constructorType = $attribute->signature->type;
Note: See TracChangeset for help on using the changeset viewer.