Changeset 19820 in webkit


Ignore:
Timestamp:
Feb 22, 2007 7:48:56 PM (17 years 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:
Location:
trunk
Files:
2 added
7 edited

Legend:

Unmodified
Added
Removed

  • 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;
Note: See TracChangeset for help on using the changeset viewer.