Changeset 19820

Show
Ignore:
Timestamp:
2007-02-22 19:48:56 (1 year ago)
Author:
weinig
Message:

LayoutTests:

Reviewed by Maciej.

  • fast/events/remove-event-listener-expected.txt: Added.
  • fast/events/remove-event-listener.html: Added.

WebCore:

Reviewed by Maciej.

and

Problem: RemoveEventListener leaks memory if the listener is not
registered.
Fix: Added Window::findJSEventListener function w/o creating a
JSEventListener; Renamed getJSEventListener to findOrCreateJSEventListener;

As an enhancement, added a leak counter for EventListeners.

Added a test case, LayoutTests/fast/events/remove-event-listener.html.

  • WebCore/bindings/js/kjs_dom.cpp:
  • WebCore/bindings/js/kjs_window.h:
  • WebCore/bindings/js/kjs_window.cpp:
  • WebCore/bindings/js/kjs_event.cpp: Add a leak counter.
  • WebCore/bindings/js/JSXMLHttpRequest.cpp:
  • LayoutTests/fast/events/remove-event-listener.html:
Files:

Legend:

Unmodified
Added
Removed
Modified
Copied
Moved

  • trunk/LayoutTests/ChangeLog

    r19811 r19820  
     12007-02-22  Ian Eng  <ian.eng.webkit@gmail.com> 
     2 
     3        Reviewed by Maciej. 
     4 
     5        - Test for http://bugs.webkit.org/show_bug.cgi?id=12850 
     6          Leaks >10k objects 
     7 
     8        * fast/events/remove-event-listener-expected.txt: Added. 
     9        * fast/events/remove-event-listener.html: Added. 
     10 
    1112007-02-22  Justin Garcia  <justin.garcia@apple.com> 
    212 

  • trunk/WebCore/ChangeLog

    r19819 r19820  
     12007-02-22  Ian Eng  <ian.eng.webkit@gmail.com> 
     2 
     3        Reviewed by Maciej. 
     4 
     5        - Patch for http://bugs.webkit.org/show_bug.cgi?id=12850 
     6          Leaks >10k objects 
     7 
     8        and 
     9 
     10        - http://bugs.webkit.org/show_bug.cgi?id=12853 
     11          add a EventListener leak counter 
     12 
     13        Problem: RemoveEventListener leaks memory if the listener is not 
     14        registered. 
     15        Fix: Added Window::findJSEventListener function w/o creating a  
     16        JSEventListener; Renamed getJSEventListener to findOrCreateJSEventListener; 
     17 
     18        As an enhancement, added a leak counter for EventListeners. 
     19 
     20        Added a test case, LayoutTests/fast/events/remove-event-listener.html. 
     21 
     22        * WebCore/bindings/js/kjs_dom.cpp: 
     23        * WebCore/bindings/js/kjs_window.h: 
     24        * WebCore/bindings/js/kjs_window.cpp: 
     25        * WebCore/bindings/js/kjs_event.cpp: Add a leak counter. 
     26        * WebCore/bindings/js/JSXMLHttpRequest.cpp: 
     27        * LayoutTests/fast/events/remove-event-listener.html: 
     28 
    1292007-02-22  Anders Carlsson  <acarlsson@apple.com> 
    230 

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

    r18912 r19820  
    140140    switch (token) { 
    141141        case Onreadystatechange: 
    142             m_impl->setOnReadyStateChangeListener(Window::retrieveActive(exec)->getJSUnprotectedEventListener(value, true)); 
     142            m_impl->setOnReadyStateChangeListener(Window::retrieveActive(exec)->findOrCreateJSUnprotectedEventListener(value, true)); 
    143143            break; 
    144144        case Onload: 
    145             m_impl->setOnLoadListener(Window::retrieveActive(exec)->getJSUnprotectedEventListener(value, true)); 
     145            m_impl->setOnLoadListener(Window::retrieveActive(exec)->findOrCreateJSUnprotectedEventListener(value, true)); 
    146146            break; 
    147147    } 
    …… 
    273273         
    274274        case JSXMLHttpRequest::AddEventListener: { 
    275             JSUnprotectedEventListener* listener = Window::retrieveActive(exec)->getJSUnprotectedEventListener(args[1], true); 
     275            JSUnprotectedEventListener* listener = Window::retrieveActive(exec)->findOrCreateJSUnprotectedEventListener(args[1], true); 
    276276            if (listener) 
    277277                request->m_impl->addEventListener(args[0]->toString(exec), listener, args[2]->toBoolean(exec)); 
    …… 
    279279        } 
    280280        case JSXMLHttpRequest::RemoveEventListener: { 
    281             JSUnprotectedEventListener* listener = Window::retrieveActive(exec)->getJSUnprotectedEventListener(args[1], true); 
     281            JSUnprotectedEventListener* listener = Window::retrieveActive(exec)->findJSUnprotectedEventListener(args[1], true); 
    282282            if (listener) 
    283283                request->m_impl->removeEventListener(args[0]->toString(exec), listener, args[2]->toBoolean(exec)); 

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

    r19617 r19820  
    624624void DOMEventTargetNode::setListener(ExecState* exec, const AtomicString &eventType, JSValue* func) const 
    625625{ 
    626     EventTargetNodeCast(impl())->setHTMLEventListener(eventType, Window::retrieveActive(exec)->getJSEventListener(func, true)); 
     626    EventTargetNodeCast(impl())->setHTMLEventListener(eventType, Window::retrieveActive(exec)->findOrCreateJSEventListener(func, true)); 
    627627} 
    628628 
    …… 
    662662    switch (id) { 
    663663        case DOMEventTargetNode::AddEventListener: { 
    664             JSEventListener *listener = Window::retrieveActive(exec)->getJSEventListener(args[1]); 
     664            JSEventListener *listener = Window::retrieveActive(exec)->findOrCreateJSEventListener(args[1]); 
    665665            if (listener) 
    666666                node->addEventListener(args[0]->toString(exec), listener,args[2]->toBoolean(exec)); 
    …… 
    668668        } 
    669669        case DOMEventTargetNode::RemoveEventListener: { 
    670             JSEventListener *listener = Window::retrieveActive(exec)->getJSEventListener(args[1]); 
    671             if (listener) 
     670            JSEventListener *listener = Window::retrieveActive(exec)->findJSEventListener(args[1]); 
     671            if (listener)  
    672672                node->removeEventListener(args[0]->toString(exec), listener,args[2]->toBoolean(exec)); 
    673673            return jsUndefined(); 

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

    r19538 r19820  
    202202} 
    203203 
     204#ifndef NDEBUG 
     205#ifndef LOG_CHANNEL_PREFIX 
     206#define LOG_CHANNEL_PREFIX Log 
     207#endif 
     208WTFLogChannel LogWebCoreEventListenerLeaks = { 0x00000000, "", WTFLogChannelOn }; 
     209 
     210struct EventListenerCounter { 
     211    static unsigned count; 
     212    ~EventListenerCounter() 
     213    { 
     214        if (count) 
     215            LOG(WebCoreEventListenerLeaks, "LEAK: %u EventListeners\n", count); 
     216    } 
     217}; 
     218unsigned EventListenerCounter::count = 0; 
     219static EventListenerCounter eventListenerCounter; 
     220#endif 
     221 
    204222// ------------------------------------------------------------------------- 
    205223 
    …… 
    214232        listeners.set(_listener, this); 
    215233    } 
     234#ifndef NDEBUG 
     235    ++eventListenerCounter.count; 
     236#endif 
    216237} 
    217238 
    …… 
    223244        listeners.remove(listener); 
    224245    } 
     246#ifndef NDEBUG 
     247    --eventListenerCounter.count; 
     248#endif 
    225249} 
    226250 

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

    r19617 r19820  
    12811281    return; 
    12821282 
    1283   doc->setHTMLWindowEventListener(eventType, getJSEventListener(func,true)); 
     1283  doc->setHTMLWindowEventListener(eventType, findOrCreateJSEventListener(func,true)); 
    12841284} 
    12851285 
    …… 
    12991299} 
    13001300 
    1301 JSEventListener *Window::getJSEventListener(JSValue *val, bool html) 
    1302 { 
     1301JSEventListener* Window::findJSEventListener(JSValue* val, bool html) 
     1302{ 
     1303    if (!val->isObject()) 
     1304        return 0; 
     1305    JSObject* object = static_cast<JSObject*>(val); 
     1306    ListenersMap& listeners = html ? jsHTMLEventListeners : jsEventListeners; 
     1307    return listeners.get(object); 
     1308} 
     1309 
     1310JSEventListener *Window::findOrCreateJSEventListener(JSValue *val, bool html) 
     1311{ 
     1312  JSEventListener *listener = findJSEventListener(val, html); 
     1313  if (listener) 
     1314    return listener; 
     1315 
    13031316  if (!val->isObject()) 
    13041317    return 0; 
    13051318  JSObject *object = static_cast<JSObject *>(val); 
    13061319 
    1307   ListenersMap& listeners = html ? jsHTMLEventListeners : jsEventListeners; 
    1308   if (JSEventListener* listener = listeners.get(object)) 
    1309     return listener; 
    1310  
    13111320  // Note that the JSEventListener constructor adds it to our jsEventListeners list 
    13121321  return new JSEventListener(object, this, html); 
    13131322} 
    13141323 
    1315 JSUnprotectedEventListener *Window::getJSUnprotectedEventListener(JSValue *val, bool html) 
    1316 { 
     1324JSUnprotectedEventListener* Window::findJSUnprotectedEventListener(JSValue* val, bool html) 
     1325{ 
     1326    if (!val->isObject()) 
     1327        return 0; 
     1328    JSObject* object = static_cast<JSObject*>(val); 
     1329    UnprotectedListenersMap& listeners = html ? jsUnprotectedHTMLEventListeners : jsUnprotectedEventListeners; 
     1330    return listeners.get(object); 
     1331} 
     1332 
     1333JSUnprotectedEventListener *Window::findOrCreateJSUnprotectedEventListener(JSValue *val, bool html) 
     1334{ 
     1335  JSUnprotectedEventListener *listener = findJSUnprotectedEventListener(val, html); 
     1336  if (listener) 
     1337    return listener; 
     1338 
    13171339  if (!val->isObject()) 
    13181340    return 0; 
    1319   JSObject* object = static_cast<JSObject *>(val); 
    1320  
    1321   UnprotectedListenersMap& listeners = html ? jsUnprotectedHTMLEventListeners : jsUnprotectedEventListeners; 
    1322   if (JSUnprotectedEventListener* listener = listeners.get(object)) 
    1323     return listener; 
     1341  JSObject *object = static_cast<JSObject *>(val); 
    13241342 
    13251343  // The JSUnprotectedEventListener constructor adds it to our jsUnprotectedEventListeners map. 
    …… 
    17881806        if (!window->isSafeScript(exec)) 
    17891807            return jsUndefined(); 
    1790         if (JSEventListener *listener = Window::retrieveActive(exec)->getJSEventListener(args[1])) 
     1808        if (JSEventListener *listener = Window::retrieveActive(exec)->findOrCreateJSEventListener(args[1])) 
    17911809            if (Document *doc = frame->document()) 
    17921810                doc->addWindowEventListener(AtomicString(args[0]->toString(exec)), listener, args[2]->toBoolean(exec)); 
    …… 
    17951813        if (!window->isSafeScript(exec)) 
    17961814            return jsUndefined(); 
    1797         if (JSEventListener *listener = Window::retrieveActive(exec)->getJSEventListener(args[1])) 
     1815        if (JSEventListener *listener = Window::retrieveActive(exec)->findJSEventListener(args[1])) 
    17981816            if (Document *doc = frame->document()) 
    17991817                doc->removeWindowEventListener(AtomicString(args[0]->toString(exec)), listener, args[2]->toBoolean(exec)); 

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

    r19617 r19820  
    136136    BarInfo *statusbar(ExecState*) const; 
    137137    BarInfo *toolbar(ExecState*) const; 
    138     JSEventListener *getJSEventListener(JSValue*, bool html = false); 
    139     JSUnprotectedEventListener *getJSUnprotectedEventListener(JSValue*, bool html = false); 
     138 
     139    // Finds a wrapper of a JS EventListener, returns 0 if no existing one. 
     140    JSEventListener* findJSEventListener(JSValue*, bool html = false); 
     141 
     142    // Finds or creates a wrapper of a JS EventListener. JS EventListener object is GC-protected. 
     143    JSEventListener *findOrCreateJSEventListener(JSValue*, bool html = false); 
     144 
     145    // Finds a wrapper of a GC-unprotected JS EventListener, returns 0 if no existing one. 
     146    JSUnprotectedEventListener* findJSUnprotectedEventListener(JSValue*, bool html = false); 
     147 
     148    // Finds or creates a wrapper of a JS EventListener. JS EventListener object is *NOT* GC-protected. 
     149    JSUnprotectedEventListener *findOrCreateJSUnprotectedEventListener(JSValue*, bool html = false); 
     150 
    140151    void clear(); 
    141152    virtual UString toString(ExecState *) const;