Memory Leak in CompositePanel.onDetach()

[GoogleCodeExporter]

There is a memory leak in your CompositePanel.onDetach() method. If you
notice in the code that the very last thing you do is reset the
EventListener. This makes it so that the widget is never properly disposed
of by the browser. IE becomes very unstable in a short amount of time in
situations where you are attaching/detaching widgets that extend from
CompositePanel many times.

I am not sure why you are making this call in many of your onDetach() methods:

if (0 != this.getSunkEventsBitMask())
{
    DOM.setEventListener(this.getSunkEventsTarget(), this);
}

but it is problematic when you do it after GWT onDetach mechanism. I would
either remove the code entirely (as I said I don't understand why it is
even there) or simply call it before GWT onDetach(). I looked at other
classes where you so this (CompositeWidget) and it doesn't hurt anything
because you make sure to call the GWT onDetach() code afterwards, which
cleans it up for you.

Workaround to fix memory leak (at least in the usage of Menus):

Simply override the onDetach() method, call super.onDetach() first, and
then call DOM.setEventListener(this.getElement(), null).

Example:

private HorizontalMenuBar menu = new HorizontalMenuBar()
{
    // HACK: to fix the memory leak problem in the rocket library
    @Override
    protected void onDetach()
    {
        super.onDetach();
    DOM.setEventListener(getElement(), null);
    }
};

Original issue reported on code.google.com by xsegr...@gmail.com on 26 May 2008 at 9:00

[GoogleCodeExporter]

Thanx for catching this problem - changes made.

Original comment by miroslav...@gmail.com on 28 May 2008 at 8:46

• Changed state: Fixed

[GoogleCodeExporter]

THe fix has been made and looks like this. It is possible to change the actual 
having
the element having bits sunk. The code caters for this. Please confirm.

protected void onDetach() {
        Element element = this.getSunkEventsTarget();
        if (0 != this.getSunkEventsBitMask()) {
            element = this.getSunkEventsTarget();
        } else {
            elesment = this.getElement(); // prolly dont need but to be sure...
        }
        DOM.setEventListener(element, null );
        super.onDetach();
    }

Original comment by miroslav...@gmail.com on 28 May 2008 at 8:52

[GoogleCodeExporter]

Actually the bottom code is better please patch CompositeWidget and confirm...

    /**
     * The complement of onAttach. This method removes the event listener for
     * the sunk event target.
     */
    protected void onDetach() {
        this.clearSinkEvents();
        super.onDetach();
    }

    /**
     * This method is called when a widget is deatached from the dom, cleaning up any
event listener references to avoid
     * memory leaks in certain browsers.
     */
    protected void clearSinkEvents(){
        Element element = this.getSunkEventsTarget();
        if (0 != this.getSunkEventsBitMask()) {
            element = this.getSunkEventsTarget();
        } else {
            element = this.getElement(); // prolly dont need but to be sure...
        }
        DOM.setEventListener(element, null );
    }

Original comment by miroslav...@gmail.com on 28 May 2008 at 8:54